* [PATCH 0/1] hw/block/nvme: fix assert on invalid irq vector
@ 2020-06-09 9:45 Klaus Jensen
2020-06-09 9:45 ` [PATCH 1/2] hw/block/nvme: add msix_qsize parameter Klaus Jensen
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Klaus Jensen @ 2020-06-09 9:45 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Klaus Jensen, qemu-devel, Max Reitz, Klaus Jensen,
Keith Busch, Javier Gonzalez, Maxim Levitsky,
Philippe Mathieu-Daudé
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] hw/block/nvme: add msix_qsize parameter
2020-06-09 9:45 [PATCH 0/1] hw/block/nvme: fix assert on invalid irq vector Klaus Jensen
@ 2020-06-09 9:45 ` Klaus Jensen
2020-06-09 9:45 ` [PATCH 2/2] hw/block/nvme: verify msix_init_exclusive_bar() return value Klaus Jensen
2020-06-09 11:17 ` [PATCH 0/1] hw/block/nvme: fix assert on invalid irq vector Philippe Mathieu-Daudé
2 siblings, 0 replies; 11+ messages in thread
From: Klaus Jensen @ 2020-06-09 9:45 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Klaus Jensen, qemu-devel, Max Reitz, Klaus Jensen,
Keith Busch, Javier Gonzalez, Maxim Levitsky,
Philippe Mathieu-Daudé
From: Klaus Jensen <k.jensen@samsung.com>
Commit 4eb9f5217a16 ("hw/block/nvme: allow use of any valid msix
vector") erronously allows any valid MSI-X vector to be used when
creating completion queues. This is bad because MSI-X is initialized
with max_ioqpairs + 1 vectors, which defaults to 64 + 1. And since
commit ccd579b2ed03 ("hw/block/nvme: Verify msix_vector_use() returned
value"), this also causes an assert if the host creates a completion
queue with an invalid vector.
Fix this by decoupling the requested maximum number of ioqpairs (param
max_ioqpairs) from the number of MSI-X interrupt vectors by introducing
a new msix_qsize parameter and initialize MSI-X with that. This allows
emulating a device that has fewer vectors than I/O queue pairs and also
allows more than 2048 queue pairs. To get the same default behaviour as
previously, msix_qsize defaults to 65 (max_ioqpairs default + 1).
This decoupling was actually suggested by Maxim some time ago in a
slightly different context, so adding a Suggested-by.
Fixes: 4eb9f5217a16 ("hw/block/nvme: allow use of any valid msix vector")
Suggested-by: Maxim Levitsky <mlevitsk@redhat.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
hw/block/nvme.c | 17 +++++++++++++----
hw/block/nvme.h | 1 +
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 567bce75191a..acc6dbc900e2 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -54,6 +54,7 @@
#include "trace.h"
#include "nvme.h"
+#define NVME_MAX_IOQPAIRS 0xffff
#define NVME_REG_SIZE 0x1000
#define NVME_DB_SIZE 4
#define NVME_CMB_BIR 2
@@ -662,7 +663,7 @@ static uint16_t nvme_create_cq(NvmeCtrl *n, NvmeCmd *cmd)
trace_pci_nvme_err_invalid_create_cq_vector(vector);
return NVME_INVALID_IRQ_VECTOR | NVME_DNR;
}
- if (unlikely(vector > PCI_MSIX_FLAGS_QSIZE)) {
+ if (unlikely(vector >= n->params.msix_qsize)) {
trace_pci_nvme_err_invalid_create_cq_vector(vector);
return NVME_INVALID_IRQ_VECTOR | NVME_DNR;
}
@@ -1371,9 +1372,16 @@ static void nvme_check_constraints(NvmeCtrl *n, Error **errp)
}
if (params->max_ioqpairs < 1 ||
- params->max_ioqpairs > PCI_MSIX_FLAGS_QSIZE) {
+ params->max_ioqpairs > NVME_MAX_IOQPAIRS) {
error_setg(errp, "max_ioqpairs must be between 1 and %d",
- PCI_MSIX_FLAGS_QSIZE);
+ NVME_MAX_IOQPAIRS);
+ return;
+ }
+
+ if (params->msix_qsize < 1 ||
+ params->msix_qsize > PCI_MSIX_FLAGS_QSIZE + 1) {
+ error_setg(errp, "msix_qsize must be between 1 and %d",
+ PCI_MSIX_FLAGS_QSIZE + 1);
return;
}
@@ -1527,7 +1535,7 @@ static void nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev)
n->reg_size);
pci_register_bar(pci_dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY |
PCI_BASE_ADDRESS_MEM_TYPE_64, &n->iomem);
- msix_init_exclusive_bar(pci_dev, n->params.max_ioqpairs + 1, 4, NULL);
+ msix_init_exclusive_bar(pci_dev, n->params.msix_qsize, 4, NULL);
if (n->params.cmb_size_mb) {
nvme_init_cmb(n, pci_dev);
@@ -1634,6 +1642,7 @@ static Property nvme_props[] = {
DEFINE_PROP_UINT32("cmb_size_mb", NvmeCtrl, params.cmb_size_mb, 0),
DEFINE_PROP_UINT32("num_queues", NvmeCtrl, params.num_queues, 0),
DEFINE_PROP_UINT32("max_ioqpairs", NvmeCtrl, params.max_ioqpairs, 64),
+ DEFINE_PROP_UINT16("msix_qsize", NvmeCtrl, params.msix_qsize, 65),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/hw/block/nvme.h b/hw/block/nvme.h
index 61dd9b23b81d..1d30c0bca283 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -7,6 +7,7 @@ typedef struct NvmeParams {
char *serial;
uint32_t num_queues; /* deprecated since 5.1 */
uint32_t max_ioqpairs;
+ uint16_t msix_qsize;
uint32_t cmb_size_mb;
} NvmeParams;
--
2.27.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] hw/block/nvme: verify msix_init_exclusive_bar() return value
2020-06-09 9:45 [PATCH 0/1] hw/block/nvme: fix assert on invalid irq vector Klaus Jensen
2020-06-09 9:45 ` [PATCH 1/2] hw/block/nvme: add msix_qsize parameter Klaus Jensen
@ 2020-06-09 9:45 ` Klaus Jensen
2020-06-09 11:17 ` [PATCH 0/1] hw/block/nvme: fix assert on invalid irq vector Philippe Mathieu-Daudé
2 siblings, 0 replies; 11+ messages in thread
From: Klaus Jensen @ 2020-06-09 9:45 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Klaus Jensen, qemu-devel, Max Reitz, Klaus Jensen,
Keith Busch, Javier Gonzalez, Maxim Levitsky,
Philippe Mathieu-Daudé
From: Klaus Jensen <k.jensen@samsung.com>
Pass an Error to msix_init_exclusive_bar() and check it.
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
hw/block/nvme.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index acc6dbc900e2..2a2e43f681f9 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1522,7 +1522,7 @@ static void nvme_init_pmr(NvmeCtrl *n, PCIDevice *pci_dev)
PCI_BASE_ADDRESS_MEM_PREFETCH, &n->pmrdev->mr);
}
-static void nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev)
+static void nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev, Error **errp)
{
uint8_t *pci_conf = pci_dev->config;
@@ -1535,7 +1535,9 @@ static void nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev)
n->reg_size);
pci_register_bar(pci_dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY |
PCI_BASE_ADDRESS_MEM_TYPE_64, &n->iomem);
- msix_init_exclusive_bar(pci_dev, n->params.msix_qsize, 4, NULL);
+ if (msix_init_exclusive_bar(pci_dev, n->params.msix_qsize, 4, errp)) {
+ return;
+ }
if (n->params.cmb_size_mb) {
nvme_init_cmb(n, pci_dev);
@@ -1603,7 +1605,12 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
return;
}
- nvme_init_pci(n, pci_dev);
+ nvme_init_pci(n, pci_dev, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+
nvme_init_ctrl(n, pci_dev);
for (i = 0; i < n->num_namespaces; i++) {
--
2.27.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 0/1] hw/block/nvme: fix assert on invalid irq vector
2020-06-09 9:45 [PATCH 0/1] hw/block/nvme: fix assert on invalid irq vector Klaus Jensen
2020-06-09 9:45 ` [PATCH 1/2] hw/block/nvme: add msix_qsize parameter Klaus Jensen
2020-06-09 9:45 ` [PATCH 2/2] hw/block/nvme: verify msix_init_exclusive_bar() return value Klaus Jensen
@ 2020-06-09 11:17 ` Philippe Mathieu-Daudé
2020-06-09 11:46 ` Klaus Jensen
2 siblings, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-09 11:17 UTC (permalink / raw)
To: Klaus Jensen, qemu-block, Kevin Wolf
Cc: Klaus Jensen, qemu-devel, Maxim Levitsky, Keith Busch,
Javier Gonzalez, Max Reitz
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(-)
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/1] hw/block/nvme: fix assert on invalid irq vector
2020-06-09 11:17 ` [PATCH 0/1] hw/block/nvme: fix assert on invalid irq vector Philippe Mathieu-Daudé
@ 2020-06-09 11:46 ` Klaus Jensen
2020-06-09 14:14 ` Kevin Wolf
0 siblings, 1 reply; 11+ messages in thread
From: Klaus Jensen @ 2020-06-09 11:46 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Kevin Wolf, qemu-block, Klaus Jensen, qemu-devel, Max Reitz,
Keith Busch, Javier Gonzalez, Maxim Levitsky
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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/1] hw/block/nvme: fix assert on invalid irq vector
2020-06-09 11:46 ` Klaus Jensen
@ 2020-06-09 14:14 ` Kevin Wolf
2020-06-09 14:18 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 11+ messages in thread
From: Kevin Wolf @ 2020-06-09 14:14 UTC (permalink / raw)
To: Klaus Jensen
Cc: qemu-block, Klaus Jensen, qemu-devel, Max Reitz, Keith Busch,
Javier Gonzalez, Maxim Levitsky, Philippe Mathieu-Daudé
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/1] hw/block/nvme: fix assert on invalid irq vector
2020-06-09 14:14 ` Kevin Wolf
@ 2020-06-09 14:18 ` Philippe Mathieu-Daudé
2020-06-09 15:32 ` Kevin Wolf
0 siblings, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-09 14:18 UTC (permalink / raw)
To: Kevin Wolf, Klaus Jensen, Maxim Levitsky
Cc: qemu-block, Klaus Jensen, qemu-devel, Max Reitz, Keith Busch,
Javier Gonzalez
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
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/1] hw/block/nvme: fix assert on invalid irq vector
2020-06-09 14:18 ` Philippe Mathieu-Daudé
@ 2020-06-09 15:32 ` Kevin Wolf
2020-06-09 18:38 ` Klaus Jensen
2020-07-07 9:10 ` Maxim Levitsky
0 siblings, 2 replies; 11+ messages in thread
From: Kevin Wolf @ 2020-06-09 15:32 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-block, Klaus Jensen, qemu-devel, Maxim Levitsky,
Keith Busch, Klaus Jensen, Javier Gonzalez, Max Reitz
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/1] hw/block/nvme: fix assert on invalid irq vector
2020-06-09 15:32 ` Kevin Wolf
@ 2020-06-09 18:38 ` Klaus Jensen
2020-07-07 9:10 ` Maxim Levitsky
1 sibling, 0 replies; 11+ messages in thread
From: Klaus Jensen @ 2020-06-09 18:38 UTC (permalink / raw)
To: Kevin Wolf
Cc: qemu-block, Klaus Jensen, qemu-devel, Max Reitz, Keith Busch,
Javier Gonzalez, Philippe Mathieu-Daudé
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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/1] hw/block/nvme: fix assert on invalid irq vector
2020-06-09 15:32 ` Kevin Wolf
2020-06-09 18:38 ` Klaus Jensen
@ 2020-07-07 9:10 ` Maxim Levitsky
2020-07-07 9:29 ` Klaus Jensen
1 sibling, 1 reply; 11+ messages in thread
From: Maxim Levitsky @ 2020-07-07 9:10 UTC (permalink / raw)
To: Kevin Wolf, Philippe Mathieu-Daudé
Cc: qemu-block, Klaus Jensen, qemu-devel, Max Reitz, Keith Busch,
Klaus Jensen, Javier Gonzalez
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/1] hw/block/nvme: fix assert on invalid irq vector
2020-07-07 9:10 ` Maxim Levitsky
@ 2020-07-07 9:29 ` Klaus Jensen
0 siblings, 0 replies; 11+ messages in thread
From: Klaus Jensen @ 2020-07-07 9:29 UTC (permalink / raw)
To: Maxim Levitsky
Cc: Kevin Wolf, qemu-block, Klaus Jensen, qemu-devel, Max Reitz,
Keith Busch, Javier Gonzalez, Philippe Mathieu-Daudé
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!
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-07-07 9:30 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-09 9:45 [PATCH 0/1] hw/block/nvme: fix assert on invalid irq vector Klaus Jensen
2020-06-09 9:45 ` [PATCH 1/2] hw/block/nvme: add msix_qsize parameter Klaus Jensen
2020-06-09 9:45 ` [PATCH 2/2] hw/block/nvme: verify msix_init_exclusive_bar() return value Klaus Jensen
2020-06-09 11:17 ` [PATCH 0/1] hw/block/nvme: fix assert on invalid irq vector Philippe Mathieu-Daudé
2020-06-09 11:46 ` Klaus Jensen
2020-06-09 14:14 ` Kevin Wolf
2020-06-09 14:18 ` Philippe Mathieu-Daudé
2020-06-09 15:32 ` Kevin Wolf
2020-06-09 18:38 ` Klaus Jensen
2020-07-07 9:10 ` Maxim Levitsky
2020-07-07 9:29 ` Klaus Jensen
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).