1. Patchew
  2. QEMU
  3. View series

[PATCH 0/1] hw/block/nvme: fix assert on invalid irq vector

Klaus Jensen posted 1 patch 5 years, 8 months ago
Failed in applying to current master (apply log)
hw/block/nvme.c | 28 ++++++++++++++++++++++------
hw/block/nvme.h |  1 +
2 files changed, 23 insertions(+), 6 deletions(-)
[PATCH 0/1] hw/block/nvme: fix assert on invalid irq vector
Posted by Klaus Jensen 5 years, 8 months ago 
From: Klaus Jensen <k.jensen@samsung.com>

I goofed up with commit c09794fe40e3 ("hw/block/nvme: allow use of any
valid msix vector").

This fixes the goof by adding a new msix_qsize parameter. As a nice
side-effect this allows a device with less interrupt vectors available
than supported queues. Also, improve the error handling in
nvme_init_pci().

Kevin, please consider picking this up for the block branch when
reviewed.

Cc: qemu-devel@nongnu.org
Cc: Keith Busch <kbusch@kernel.org>
Cc: Max Reitz <mreitz@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Klaus Jensen <its@irrelevant.dk>
Cc: Javier Gonzalez <javier.gonz@samsung.com>
Cc: Maxim Levitsky <mlevitsk@redhat.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>

Klaus Jensen (2):
  hw/block/nvme: add msix_qsize parameter
  hw/block/nvme: verify msix_init_exclusive_bar() return value

 hw/block/nvme.c | 28 ++++++++++++++++++++++------
 hw/block/nvme.h |  1 +
 2 files changed, 23 insertions(+), 6 deletions(-)

-- 
2.27.0
Re: [PATCH 0/1] hw/block/nvme: fix assert on invalid irq vector
Posted by Philippe Mathieu-Daudé 5 years, 8 months ago 
On 6/9/20 11:45 AM, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
> 
> I goofed up with commit c09794fe40e3 ("hw/block/nvme: allow use of any
> valid msix vector").

Kevin, since your queue isn't merged, can you directly squash the fix?

> 
> This fixes the goof by adding a new msix_qsize parameter. As a nice
> side-effect this allows a device with less interrupt vectors available
> than supported queues. Also, improve the error handling in
> nvme_init_pci().
> 
> Kevin, please consider picking this up for the block branch when
> reviewed.
> 
> Cc: qemu-devel@nongnu.org
> Cc: Keith Busch <kbusch@kernel.org>
> Cc: Max Reitz <mreitz@redhat.com>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Klaus Jensen <its@irrelevant.dk>
> Cc: Javier Gonzalez <javier.gonz@samsung.com>
> Cc: Maxim Levitsky <mlevitsk@redhat.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> Klaus Jensen (2):
>   hw/block/nvme: add msix_qsize parameter
>   hw/block/nvme: verify msix_init_exclusive_bar() return value
> 
>  hw/block/nvme.c | 28 ++++++++++++++++++++++------
>  hw/block/nvme.h |  1 +
>  2 files changed, 23 insertions(+), 6 deletions(-)
>
Re: [PATCH 0/1] hw/block/nvme: fix assert on invalid irq vector
Posted by Klaus Jensen 5 years, 8 months ago 
On Jun  9 13:17, Philippe Mathieu-Daudé wrote:
> On 6/9/20 11:45 AM, Klaus Jensen wrote:
> > From: Klaus Jensen <k.jensen@samsung.com>
> > 
> > I goofed up with commit c09794fe40e3 ("hw/block/nvme: allow use of any
> > valid msix vector").
> 
> Kevin, since your queue isn't merged, can you directly squash the fix?

The commit (c09794fe40e3) can just be dropped without conflicts, but it
leaves a use of n->params.num_queues in nvme_create_cq() which commit
cde74bfd4b87 ("hw/block/nvme: add max_ioqpairs device parameter") must
fix.
Re: [PATCH 0/1] hw/block/nvme: fix assert on invalid irq vector
Posted by Kevin Wolf 5 years, 8 months ago 
Am 09.06.2020 um 13:46 hat Klaus Jensen geschrieben:
> On Jun  9 13:17, Philippe Mathieu-Daudé wrote:
> > On 6/9/20 11:45 AM, Klaus Jensen wrote:
> > > From: Klaus Jensen <k.jensen@samsung.com>
> > > 
> > > I goofed up with commit c09794fe40e3 ("hw/block/nvme: allow use of any
> > > valid msix vector").
> > 
> > Kevin, since your queue isn't merged, can you directly squash the fix?
> 
> The commit (c09794fe40e3) can just be dropped without conflicts, but it
> leaves a use of n->params.num_queues in nvme_create_cq() which commit
> cde74bfd4b87 ("hw/block/nvme: add max_ioqpairs device parameter") must
> fix.

Hm, so it seems this isn't easy to squash in without conflicts (and I
would have to rewrite the whole commit message), so I think it's better
to just apply the series on top.

One problem with the commit message is that it references commit IDs
which aren't stable yet. Maybe it's best if I apply these patches,
manually fix up the commit ID references and then immediately do a pull
request so that they become stable.

It would be good to have at least one review, though.

Kevin
Re: [PATCH 0/1] hw/block/nvme: fix assert on invalid irq vector
Posted by Philippe Mathieu-Daudé 5 years, 8 months ago 
On 6/9/20 4:14 PM, Kevin Wolf wrote:
> Am 09.06.2020 um 13:46 hat Klaus Jensen geschrieben:
>> On Jun  9 13:17, Philippe Mathieu-Daudé wrote:
>>> On 6/9/20 11:45 AM, Klaus Jensen wrote:
>>>> From: Klaus Jensen <k.jensen@samsung.com>
>>>>
>>>> I goofed up with commit c09794fe40e3 ("hw/block/nvme: allow use of any
>>>> valid msix vector").
>>>
>>> Kevin, since your queue isn't merged, can you directly squash the fix?
>>
>> The commit (c09794fe40e3) can just be dropped without conflicts, but it
>> leaves a use of n->params.num_queues in nvme_create_cq() which commit
>> cde74bfd4b87 ("hw/block/nvme: add max_ioqpairs device parameter") must
>> fix.
> 
> Hm, so it seems this isn't easy to squash in without conflicts (and I
> would have to rewrite the whole commit message), so I think it's better
> to just apply the series on top.
> 
> One problem with the commit message is that it references commit IDs
> which aren't stable yet. Maybe it's best if I apply these patches,
> manually fix up the commit ID references and then immediately do a pull
> request so that they become stable.

This is the friendlier way.

Less friendly way is to drop Klaus's patches and ask him to respin.
While this is a valid outcome, if we can avoid it it will save all of us
review time.

> 
> It would be good to have at least one review, though.

Maxim catched this issue, I'd feel safer if he acks your pre-merge queue.

> 
> Kevin
>
Re: [PATCH 0/1] hw/block/nvme: fix assert on invalid irq vector
Posted by Kevin Wolf 5 years, 8 months ago 
Am 09.06.2020 um 16:18 hat Philippe Mathieu-Daudé geschrieben:
> On 6/9/20 4:14 PM, Kevin Wolf wrote:
> > Am 09.06.2020 um 13:46 hat Klaus Jensen geschrieben:
> >> On Jun  9 13:17, Philippe Mathieu-Daudé wrote:
> >>> On 6/9/20 11:45 AM, Klaus Jensen wrote:
> >>>> From: Klaus Jensen <k.jensen@samsung.com>
> >>>>
> >>>> I goofed up with commit c09794fe40e3 ("hw/block/nvme: allow use of any
> >>>> valid msix vector").
> >>>
> >>> Kevin, since your queue isn't merged, can you directly squash the fix?
> >>
> >> The commit (c09794fe40e3) can just be dropped without conflicts, but it
> >> leaves a use of n->params.num_queues in nvme_create_cq() which commit
> >> cde74bfd4b87 ("hw/block/nvme: add max_ioqpairs device parameter") must
> >> fix.
> > 
> > Hm, so it seems this isn't easy to squash in without conflicts (and I
> > would have to rewrite the whole commit message), so I think it's better
> > to just apply the series on top.
> > 
> > One problem with the commit message is that it references commit IDs
> > which aren't stable yet. Maybe it's best if I apply these patches,
> > manually fix up the commit ID references and then immediately do a pull
> > request so that they become stable.
> 
> This is the friendlier way.
> 
> Less friendly way is to drop Klaus's patches and ask him to respin.
> While this is a valid outcome, if we can avoid it it will save all of us
> review time.

If Klaus wants to do that, fine with me. I'm just trying to find the
easiest solution for all of us.

> > It would be good to have at least one review, though.
> 
> Maxim catched this issue, I'd feel safer if he acks your pre-merge queue.

Ok. Maxim, can you please review this series then?

Kevin
Re: [PATCH 0/1] hw/block/nvme: fix assert on invalid irq vector
Posted by Klaus Jensen 5 years, 8 months ago 
On Jun  9 17:32, Kevin Wolf wrote:
> Am 09.06.2020 um 16:18 hat Philippe Mathieu-Daudé geschrieben:
> > On 6/9/20 4:14 PM, Kevin Wolf wrote:
> > > Am 09.06.2020 um 13:46 hat Klaus Jensen geschrieben:
> > >> On Jun  9 13:17, Philippe Mathieu-Daudé wrote:
> > >>> On 6/9/20 11:45 AM, Klaus Jensen wrote:
> > >>>> From: Klaus Jensen <k.jensen@samsung.com>
> > >>>>
> > >>>> I goofed up with commit c09794fe40e3 ("hw/block/nvme: allow use of any
> > >>>> valid msix vector").
> > >>>
> > >>> Kevin, since your queue isn't merged, can you directly squash the fix?
> > >>
> > >> The commit (c09794fe40e3) can just be dropped without conflicts, but it
> > >> leaves a use of n->params.num_queues in nvme_create_cq() which commit
> > >> cde74bfd4b87 ("hw/block/nvme: add max_ioqpairs device parameter") must
> > >> fix.
> > > 
> > > Hm, so it seems this isn't easy to squash in without conflicts (and I
> > > would have to rewrite the whole commit message), so I think it's better
> > > to just apply the series on top.
> > > 
> > > One problem with the commit message is that it references commit IDs
> > > which aren't stable yet. Maybe it's best if I apply these patches,
> > > manually fix up the commit ID references and then immediately do a pull
> > > request so that they become stable.
> > 
> > This is the friendlier way.
> > 
> > Less friendly way is to drop Klaus's patches and ask him to respin.
> > While this is a valid outcome, if we can avoid it it will save all of us
> > review time.
> 
> If Klaus wants to do that, fine with me. I'm just trying to find the
> easiest solution for all of us.
> 

Sure, I can respin it. I would like to include this series as well
though since I think it's a nice addition.

I'll post a v7 that includes Philippes's return value verification patch
as well as the patches in this series. We should only need a review or
two on those patches then.
Re: [PATCH 0/1] hw/block/nvme: fix assert on invalid irq vector
Posted by Maxim Levitsky 5 years, 7 months ago 
On Tue, 2020-06-09 at 17:32 +0200, Kevin Wolf wrote:
> Am 09.06.2020 um 16:18 hat Philippe Mathieu-Daudé geschrieben:
> > On 6/9/20 4:14 PM, Kevin Wolf wrote:
> > > Am 09.06.2020 um 13:46 hat Klaus Jensen geschrieben:
> > > > On Jun  9 13:17, Philippe Mathieu-Daudé wrote:
> > > > > On 6/9/20 11:45 AM, Klaus Jensen wrote:
> > > > > > From: Klaus Jensen <k.jensen@samsung.com>
> > > > > > 
> > > > > > I goofed up with commit c09794fe40e3 ("hw/block/nvme: allow use of any
> > > > > > valid msix vector").
> > > > > 
> > > > > Kevin, since your queue isn't merged, can you directly squash the fix?
> > > > 
> > > > The commit (c09794fe40e3) can just be dropped without conflicts, but it
> > > > leaves a use of n->params.num_queues in nvme_create_cq() which commit
> > > > cde74bfd4b87 ("hw/block/nvme: add max_ioqpairs device parameter") must
> > > > fix.
> > > 
> > > Hm, so it seems this isn't easy to squash in without conflicts (and I
> > > would have to rewrite the whole commit message), so I think it's better
> > > to just apply the series on top.
> > > 
> > > One problem with the commit message is that it references commit IDs
> > > which aren't stable yet. Maybe it's best if I apply these patches,
> > > manually fix up the commit ID references and then immediately do a pull
> > > request so that they become stable.
> > 
> > This is the friendlier way.
> > 
> > Less friendly way is to drop Klaus's patches and ask him to respin.
> > While this is a valid outcome, if we can avoid it it will save all of us
> > review time.
> 
> If Klaus wants to do that, fine with me. I'm just trying to find the
> easiest solution for all of us.
> 
> > > It would be good to have at least one review, though.
> > 
> > Maxim catched this issue, I'd feel safer if he acks your pre-merge queue.
> 
> Ok. Maxim, can you please review this series then?
> 
> Kevin
I am slowly getting through the heap of the patches trying to understand the current state of things.
I will start reviewing all these patches today.

Best regards,
	Maxim Levitsky
Re: [PATCH 0/1] hw/block/nvme: fix assert on invalid irq vector
Posted by Klaus Jensen 5 years, 7 months ago 
On Jul  7 12:10, Maxim Levitsky wrote:
> On Tue, 2020-06-09 at 17:32 +0200, Kevin Wolf wrote:
> > Am 09.06.2020 um 16:18 hat Philippe Mathieu-Daudé geschrieben:
> > > On 6/9/20 4:14 PM, Kevin Wolf wrote:
> > > > Am 09.06.2020 um 13:46 hat Klaus Jensen geschrieben:
> > > > > On Jun  9 13:17, Philippe Mathieu-Daudé wrote:
> > > > > > On 6/9/20 11:45 AM, Klaus Jensen wrote:
> > > > > > > From: Klaus Jensen <k.jensen@samsung.com>
> > > > > > > 
> > > > > > > I goofed up with commit c09794fe40e3 ("hw/block/nvme: allow use of any
> > > > > > > valid msix vector").
> > > > > > 
> > > > > > Kevin, since your queue isn't merged, can you directly squash the fix?
> > > > > 
> > > > > The commit (c09794fe40e3) can just be dropped without conflicts, but it
> > > > > leaves a use of n->params.num_queues in nvme_create_cq() which commit
> > > > > cde74bfd4b87 ("hw/block/nvme: add max_ioqpairs device parameter") must
> > > > > fix.
> > > > 
> > > > Hm, so it seems this isn't easy to squash in without conflicts (and I
> > > > would have to rewrite the whole commit message), so I think it's better
> > > > to just apply the series on top.
> > > > 
> > > > One problem with the commit message is that it references commit IDs
> > > > which aren't stable yet. Maybe it's best if I apply these patches,
> > > > manually fix up the commit ID references and then immediately do a pull
> > > > request so that they become stable.
> > > 
> > > This is the friendlier way.
> > > 
> > > Less friendly way is to drop Klaus's patches and ask him to respin.
> > > While this is a valid outcome, if we can avoid it it will save all of us
> > > review time.
> > 
> > If Klaus wants to do that, fine with me. I'm just trying to find the
> > easiest solution for all of us.
> > 
> > > > It would be good to have at least one review, though.
> > > 
> > > Maxim catched this issue, I'd feel safer if he acks your pre-merge queue.
> > 
> > Ok. Maxim, can you please review this series then?
> > 
> > Kevin
> I am slowly getting through the heap of the patches trying to understand the current state of things.
> I will start reviewing all these patches today.
> 
 
Hi Maxim,

Yeah, I bombed it again; sorry! ;)

"[PATCH v3 00/18] hw/block/nvme: bump to v1.3" is the series currently
under review.

I also posted:

  [PATCH 00/17] hw/block/nvme: AIO and address mapping refactoring,
  [PATCH 0/2] hw/block/nvme: handle transient dma errors
  [PATCH 0/3] hw/block/nvme: support scatter gather lists
  [PATCH 0/4] hw/block/nvme: support multiple namespaces
  [PATCH] hw/block/nvme: make lba data size configurable
  [PATCH] hw/block/nvme: add support for dulbe
  [PATCH 0/3] hw/block/nvme: bump to v1.4
  [PATCH 00/10] hw/block/nvme: namespace types and zoned namespaces

I really appreciate you reviewing! Your R-b's are on a lot of the
patches already, thanks for that!

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.