Date: Fri, 15 Mar 2019 14:36:59 -0700 From: Chuck Tuffli <ctuffli@gmail.com> To: John Baldwin <jhb@freebsd.org> Cc: cem@freebsd.org, Andrew Thompson <andy@fud.org.nz>, src-committers <src-committers@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>, svn-src-head <svn-src-head@freebsd.org> Subject: Re: svn commit: r345171 - head/usr.sbin/bhyve Message-ID: <CAKAYmML_2Vsv5k=t9jp5YRPtj%2B-tMUzQ9giobQKssuK9M-amRA@mail.gmail.com> In-Reply-To: <5adee283-2cac-14d2-ec06-bce43bf3bcde@FreeBSD.org> References: <201903150211.x2F2BSai079898@repo.freebsd.org> <CAFAOGNTT_ZKCe1zs2XN6H7r-G2E%2BU77WUOrutfWshDNGOR36bQ@mail.gmail.com> <CAG6CVpVYyWywKFtqtgayghkrEptmDEP3dm%2BwWE0t7MDqyHXvUw@mail.gmail.com> <5adee283-2cac-14d2-ec06-bce43bf3bcde@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Apologies all the way around. I was ignorant about the maintainer for this code and goofed. See inline for other comments. On Fri, Mar 15, 2019 at 9:28 AM John Baldwin <jhb@freebsd.org> wrote: > > On 3/14/19 10:24 PM, Conrad Meyer wrote: > > On Thu, Mar 14, 2019 at 8:06 PM Andrew Thompson <andy@fud.org.nz> wrote: > >> > >> On Fri, 15 Mar 2019 at 15:11, Chuck Tuffli <chuck@freebsd.org> wrote: > >>> bzero(&pciecap, sizeof(pciecap)); > > ... > >>> + pciecap.dev_capabilities = PCIEM_CAP_ROLE_ERR_RPT; > >> > >> If the message you say 'set the bit' but you are overwriting the whole variable, is this intended? > > > > Looks like it was zero before. So yeah, it sets the bit. > > It would probably be cleaner for future changes to make it a |=, but that's a > tiny nit. style(9) wants a blank line before the comment as well. Happy to make those changes > I hadn't approved it yet only because I hadn't gone and dug through my PCIe > books / specs to see what this bit is and confirm it is required. > > OTOH, it's not clear to me that bhyve PCI-e devices don't want to just be 1.0a > devices as a lowest common denominator to be as accommodating to as wide variety > of OS's as possible. > > One thing I didn't see in a review was a reason for why to make this change? > Does some OS reject devices without this bit set or is it just based on reading > the spec? bhyve doesn't assert any PCI-e errors for virtual devices, so > this bit is pretty meaningless. I was contacted by a bhyve user who mentioned that Windows didn't seem to like bhyve's NVMe emulation. This change doesn't fix that, but this is one of a handful of changes inspired by qemu. While trying to reverse engineer why Windows is grumpy, I've run across several comments in the qemu code which make claims about what Windows needs to see before starting a device. This was one of those, and (at the time) seemed innocuous enough to commit. --chuck
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAKAYmML_2Vsv5k=t9jp5YRPtj%2B-tMUzQ9giobQKssuK9M-amRA>