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

[John Baldwin]

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 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.
> 
> 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