Re: [Qemu-devel] [PATCH] nvme: do not advertise support for unsupported arbitration mechanism

[Max Reitz]

On 17.06.19 08:54, Klaus Birkelund wrote:
> On Fri, Jun 14, 2019 at 10:39:27PM +0200, Max Reitz wrote:
>> On 06.06.19 11:25, Klaus Birkelund Jensen wrote:
>>> The device mistakenly reports that the Weighted Round Robin with Urgent
>>> Priority Class arbitration mechanism is supported.
>>>
>>> It is not.
>>
>> I believe you based on the fact that there is no “weight” or “priority”
>> anywhere in nvme.c, and that it does not evaluate the Arbitration
>> Mechanism Selected field.
>>
> 
> Not sure if you want me to change the commit message? Feel free to
> change it if you want to ;)

Oh, no, no.  It’s just that I have no idea how the NVMe interface works
(well, I did download the spec then, but...), so this was basically just
my excuse for “I can review this!!”.

>>> Signed-off-by: Klaus Birkelund Jensen <address@hidden>
>>> ---
>>>  hw/block/nvme.c | 1 -
>>>  1 file changed, 1 deletion(-)
>>>
>>> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
>>> index 30e50f7a3853..415b4641d6b4 100644
>>> --- a/hw/block/nvme.c
>>> +++ b/hw/block/nvme.c
>>> @@ -1383,7 +1383,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error 
>>> **errp)
>>>      n->bar.cap = 0;
>>>      NVME_CAP_SET_MQES(n->bar.cap, 0x7ff);
>>>      NVME_CAP_SET_CQR(n->bar.cap, 1);
>>> -    NVME_CAP_SET_AMS(n->bar.cap, 1);
>>
>> I suppose the better way would be to pass 0, so it is more explicit, I
>> think.
>>
>> (Just removing it looks like it may have just been forgotten.)
>>
> 
> Not explicitly setting it to zero aligns with how the other fields in
> CAP are also left out if kept at zero. If we explicitly set it to zero I
> think we should also set all the other fields that way (DSTRD, NSSRS,
> etc.).

OK then.

Thanks, applied to my block branch:

https://git.xanclic.moe/XanClic/qemu/commits/branch/block

Max

signature.asc
Description: OpenPGP digital signature