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 <a...@fud.org.nz> wrote: >>> >>> On Fri, 15 Mar 2019 at 15:11, Chuck Tuffli <ch...@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 _______________________________________________ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
