svn commit: r345171 - head/usr.sbin/bhyve

Chuck Tuffli ctuffli at gmail.com
Fri Mar 15 21:37:13 UTC 2019


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 at 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 at fud.org.nz> wrote:
> >>
> >> On Fri, 15 Mar 2019 at 15:11, Chuck Tuffli <chuck at 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


More information about the svn-src-head mailing list