Date: Fri, 15 Mar 2019 09:34:00 -0700 From: John Baldwin <jhb@FreeBSD.org> To: cem@freebsd.org, Andrew Thompson <andy@fud.org.nz> Cc: 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: <adf48b48-fe70-50b9-5182-929a48d1743a@FreeBSD.org> 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
On 3/15/19 9:27 AM, John Baldwin 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. > > 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. On the topic of a hard lock, the intention of "Review requested" is not to enforce a hard lock, but to request a heads up so that work can be coordinated. I think committing this was ok given other people ok'd the change, though I think I still I want some answers as to the "why" this is needed to think about if we actually want the change or not. -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?adf48b48-fe70-50b9-5182-929a48d1743a>