* [Qemu-devel] [PATCH] nvme: do not advertise support for unsupported arbitration mechanism
@ 2019-06-06 9:25 Klaus Birkelund Jensen
2019-06-14 20:39 ` Max Reitz
0 siblings, 1 reply; 5+ messages in thread
From: Klaus Birkelund Jensen @ 2019-06-06 9:25 UTC (permalink / raw)
To: qemu-block; +Cc: Keith Busch, Kevin Wolf, qemu-devel, Max Reitz
The device mistakenly reports that the Weighted Round Robin with Urgent
Priority Class arbitration mechanism is supported.
It is not.
Signed-off-by: Klaus Birkelund Jensen <klaus.jensen@cnexlabs.com>
---
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);
NVME_CAP_SET_TO(n->bar.cap, 0xf);
NVME_CAP_SET_CSS(n->bar.cap, 1);
NVME_CAP_SET_MPSMAX(n->bar.cap, 4);
--
2.21.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] nvme: do not advertise support for unsupported arbitration mechanism
2019-06-06 9:25 [Qemu-devel] [PATCH] nvme: do not advertise support for unsupported arbitration mechanism Klaus Birkelund Jensen
@ 2019-06-14 20:39 ` Max Reitz
2019-06-15 18:08 ` [Qemu-devel] [Qemu-block] " Maxim Levitsky
2019-06-17 6:54 ` [Qemu-devel] " Klaus Birkelund
0 siblings, 2 replies; 5+ messages in thread
From: Max Reitz @ 2019-06-14 20:39 UTC (permalink / raw)
To: Klaus Birkelund Jensen, qemu-block; +Cc: Keith Busch, Kevin Wolf, qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 1187 bytes --]
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.
> Signed-off-by: Klaus Birkelund Jensen <klaus.jensen@cnexlabs.com>
> ---
> 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.)
Max
> NVME_CAP_SET_TO(n->bar.cap, 0xf);
> NVME_CAP_SET_CSS(n->bar.cap, 1);
> NVME_CAP_SET_MPSMAX(n->bar.cap, 4);
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH] nvme: do not advertise support for unsupported arbitration mechanism
2019-06-14 20:39 ` Max Reitz
@ 2019-06-15 18:08 ` Maxim Levitsky
2019-06-17 6:54 ` [Qemu-devel] " Klaus Birkelund
1 sibling, 0 replies; 5+ messages in thread
From: Maxim Levitsky @ 2019-06-15 18:08 UTC (permalink / raw)
To: Max Reitz, Klaus Birkelund Jensen, qemu-block
Cc: Keith Busch, Kevin Wolf, qemu-devel
On Fri, 2019-06-14 at 22:39 +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.
>
> > Signed-off-by: Klaus Birkelund Jensen <klaus.jensen@cnexlabs.com>
> > ---
> > 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.)
>
> Max
>
> > NVME_CAP_SET_TO(n->bar.cap, 0xf);
> > NVME_CAP_SET_CSS(n->bar.cap, 1);
> > NVME_CAP_SET_MPSMAX(n->bar.cap, 4);
> >
>
>
Yea. no way that this driver supports WRRU and I haven't noticed it.
Just checked again to be sure.
To be honest, after some quick look,
that driver doesn' even really support the regular Round-Robin as it uses a timer to process a specific queue,
kicked up by a doorbell write :-)
Acked-by: Maxim Levitsky <mlevitsk@redhat.com>
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] nvme: do not advertise support for unsupported arbitration mechanism
2019-06-14 20:39 ` Max Reitz
2019-06-15 18:08 ` [Qemu-devel] [Qemu-block] " Maxim Levitsky
@ 2019-06-17 6:54 ` Klaus Birkelund
2019-06-17 12:25 ` Max Reitz
1 sibling, 1 reply; 5+ messages in thread
From: Klaus Birkelund @ 2019-06-17 6:54 UTC (permalink / raw)
To: Max Reitz; +Cc: Keith Busch, Kevin Wolf, qemu-devel, qemu-block
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 ;)
> > Signed-off-by: Klaus Birkelund Jensen <klaus.jensen@cnexlabs.com>
> > ---
> > 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.).
Klaus
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] nvme: do not advertise support for unsupported arbitration mechanism
2019-06-17 6:54 ` [Qemu-devel] " Klaus Birkelund
@ 2019-06-17 12:25 ` Max Reitz
0 siblings, 0 replies; 5+ messages in thread
From: Max Reitz @ 2019-06-17 12:25 UTC (permalink / raw)
To: qemu-block, Keith Busch, Kevin Wolf, qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 1872 bytes --]
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 <klaus.jensen@cnexlabs.com>
>>> ---
>>> 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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-06-17 12:37 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-06 9:25 [Qemu-devel] [PATCH] nvme: do not advertise support for unsupported arbitration mechanism Klaus Birkelund Jensen
2019-06-14 20:39 ` Max Reitz
2019-06-15 18:08 ` [Qemu-devel] [Qemu-block] " Maxim Levitsky
2019-06-17 6:54 ` [Qemu-devel] " Klaus Birkelund
2019-06-17 12:25 ` Max Reitz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).