* [PATCH for-6.0? 0/1] hw/block/nvme: fix msix uninit
@ 2021-04-22 13:58 Klaus Jensen
2021-04-22 13:58 ` [PATCH for-6.0? 1/1] hw/block/nvme: fix invalid msix exclusive uninit Klaus Jensen
2021-04-22 14:07 ` [PATCH for-6.0? 0/1] hw/block/nvme: fix msix uninit Klaus Jensen
0 siblings, 2 replies; 6+ messages in thread
From: Klaus Jensen @ 2021-04-22 13:58 UTC (permalink / raw)
To: qemu-devel, Peter Maydell
Cc: Kevin Wolf, qemu-block, Klaus Jensen, Max Reitz, Klaus Jensen,
Keith Busch
From: Klaus Jensen <k.jensen@samsung.com>
Hi Peter,
The commit message on the patch describes the issue. This is a QEMU
crashing bug in -rc4 that I introduced early in the cycle and never
found in time. Lack of testing device hotplugging is the cause for that.
I'm not sure what to say other than I'm terribly sorry for introducing
this and if this means an -rc5 needs to be rolled, then I'm even more
sorry.
I think an acceptance test could have caught this, and I am already
working on an acceptance test suite for the nvme device, so I'll add
something that test this as well. But, well, it doesn't help now.
Klaus Jensen (1):
hw/block/nvme: fix invalid msix exclusive uninit
hw/block/nvme.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--
2.31.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH for-6.0? 1/1] hw/block/nvme: fix invalid msix exclusive uninit
2021-04-22 13:58 [PATCH for-6.0? 0/1] hw/block/nvme: fix msix uninit Klaus Jensen
@ 2021-04-22 13:58 ` Klaus Jensen
2021-04-22 15:40 ` Klaus Jensen
2021-04-22 14:07 ` [PATCH for-6.0? 0/1] hw/block/nvme: fix msix uninit Klaus Jensen
1 sibling, 1 reply; 6+ messages in thread
From: Klaus Jensen @ 2021-04-22 13:58 UTC (permalink / raw)
To: qemu-devel, Peter Maydell
Cc: Kevin Wolf, qemu-block, Klaus Jensen, Max Reitz, Klaus Jensen,
Keith Busch
From: Klaus Jensen <k.jensen@samsung.com>
Commit 1901b4967c3f changed the nvme device from using a bar exclusive
for MSI-x to sharing it on bar0.
Unfortunately, the msix_uninit_exclusive_bar() call remains in
nvme_exit() which causes havoc when the device is removed with, say,
device_del.
Fix this.
Fixes: 1901b4967c3f ("hw/block/nvme: move msix table and pba to BAR 0")
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
hw/block/nvme.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 624a1431d072..31a1a59d88c9 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -6235,7 +6235,7 @@ static void nvme_exit(PCIDevice *pci_dev)
if (n->pmr.dev) {
host_memory_backend_set_mapped(n->pmr.dev, false);
}
- msix_uninit_exclusive_bar(pci_dev);
+ msix_uninit(pci_dev, &n->bar0, &n->bar0);
}
static Property nvme_props[] = {
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH for-6.0? 0/1] hw/block/nvme: fix msix uninit
2021-04-22 13:58 [PATCH for-6.0? 0/1] hw/block/nvme: fix msix uninit Klaus Jensen
2021-04-22 13:58 ` [PATCH for-6.0? 1/1] hw/block/nvme: fix invalid msix exclusive uninit Klaus Jensen
@ 2021-04-22 14:07 ` Klaus Jensen
2021-04-22 18:58 ` Peter Maydell
1 sibling, 1 reply; 6+ messages in thread
From: Klaus Jensen @ 2021-04-22 14:07 UTC (permalink / raw)
To: qemu-devel, Peter Maydell
Cc: Kevin Wolf, Klaus Jensen, Keith Busch, qemu-block, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 1069 bytes --]
On Apr 22 15:58, Klaus Jensen wrote:
>From: Klaus Jensen <k.jensen@samsung.com>
>
>Hi Peter,
>
>The commit message on the patch describes the issue. This is a QEMU
>crashing bug in -rc4 that I introduced early in the cycle and never
>found in time. Lack of testing device hotplugging is the cause for that.
>
>I'm not sure what to say other than I'm terribly sorry for introducing
>this and if this means an -rc5 needs to be rolled, then I'm even more
>sorry.
>
>I think an acceptance test could have caught this, and I am already
>working on an acceptance test suite for the nvme device, so I'll add
>something that test this as well. But, well, it doesn't help now.
>
>Klaus Jensen (1):
> hw/block/nvme: fix invalid msix exclusive uninit
>
> hw/block/nvme.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>--
>2.31.1
>
>
As far as I can tell, to cause this crash, monitor access is required,
so I am not sure if we can get away with a note on this in the release
notes and fix this in a potential stable release or next.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH for-6.0? 1/1] hw/block/nvme: fix invalid msix exclusive uninit
2021-04-22 13:58 ` [PATCH for-6.0? 1/1] hw/block/nvme: fix invalid msix exclusive uninit Klaus Jensen
@ 2021-04-22 15:40 ` Klaus Jensen
0 siblings, 0 replies; 6+ messages in thread
From: Klaus Jensen @ 2021-04-22 15:40 UTC (permalink / raw)
To: qemu-devel, Peter Maydell
Cc: Kevin Wolf, Klaus Jensen, Keith Busch, qemu-block, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 1840 bytes --]
On Apr 22 15:58, Klaus Jensen wrote:
>From: Klaus Jensen <k.jensen@samsung.com>
>
>Commit 1901b4967c3f changed the nvme device from using a bar exclusive
>for MSI-x to sharing it on bar0.
>
>Unfortunately, the msix_uninit_exclusive_bar() call remains in
>nvme_exit() which causes havoc when the device is removed with, say,
>device_del.
>
>Fix this.
>
>Fixes: 1901b4967c3f ("hw/block/nvme: move msix table and pba to BAR 0")
>Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
>---
> hw/block/nvme.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/hw/block/nvme.c b/hw/block/nvme.c
>index 624a1431d072..31a1a59d88c9 100644
>--- a/hw/block/nvme.c
>+++ b/hw/block/nvme.c
>@@ -6235,7 +6235,7 @@ static void nvme_exit(PCIDevice *pci_dev)
> if (n->pmr.dev) {
> host_memory_backend_set_mapped(n->pmr.dev, false);
> }
>- msix_uninit_exclusive_bar(pci_dev);
>+ msix_uninit(pci_dev, &n->bar0, &n->bar0);
> }
>
> static Property nvme_props[] = {
>--
>2.31.1
>
Having investigated more, the proper fix must also delete the subregion
that 1901b4967c3f introduced. Otherwise a reference lingers, causing the
attached drive to never be "unlocked".
my_bad_counter++;
I found this screw-up while investigating Bug 1925496, so credit to Oguz
Bektas for reporting this!
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 624a1431d072..5fe082ec34c5 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -6235,7 +6235,8 @@ static void nvme_exit(PCIDevice *pci_dev)
if (n->pmr.dev) {
host_memory_backend_set_mapped(n->pmr.dev, false);
}
- msix_uninit_exclusive_bar(pci_dev);
+ msix_uninit(pci_dev, &n->bar0, &n->bar0);
+ memory_region_del_subregion(&n->bar0, &n->iomem);
}
static Property nvme_props[] = {
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH for-6.0? 0/1] hw/block/nvme: fix msix uninit
2021-04-22 14:07 ` [PATCH for-6.0? 0/1] hw/block/nvme: fix msix uninit Klaus Jensen
@ 2021-04-22 18:58 ` Peter Maydell
2021-04-22 19:17 ` Klaus Jensen
0 siblings, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2021-04-22 18:58 UTC (permalink / raw)
To: Klaus Jensen
Cc: Kevin Wolf, Qemu-block, Klaus Jensen, QEMU Developers, Max Reitz,
Keith Busch
On Thu, 22 Apr 2021 at 15:07, Klaus Jensen <its@irrelevant.dk> wrote:
>
> On Apr 22 15:58, Klaus Jensen wrote:
> >From: Klaus Jensen <k.jensen@samsung.com>
> >
> >Hi Peter,
> >
> >The commit message on the patch describes the issue. This is a QEMU
> >crashing bug in -rc4 that I introduced early in the cycle and never
> >found in time. Lack of testing device hotplugging is the cause for that.
> >
> >I'm not sure what to say other than I'm terribly sorry for introducing
> >this and if this means an -rc5 needs to be rolled, then I'm even more
> >sorry.
> >
> >I think an acceptance test could have caught this, and I am already
> >working on an acceptance test suite for the nvme device, so I'll add
> >something that test this as well. But, well, it doesn't help now.
> As far as I can tell, to cause this crash, monitor access is required,
> so I am not sure if we can get away with a note on this in the release
> notes and fix this in a potential stable release or next.
Is this a regression since 5.2 ?
thanks
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH for-6.0? 0/1] hw/block/nvme: fix msix uninit
2021-04-22 18:58 ` Peter Maydell
@ 2021-04-22 19:17 ` Klaus Jensen
0 siblings, 0 replies; 6+ messages in thread
From: Klaus Jensen @ 2021-04-22 19:17 UTC (permalink / raw)
To: Peter Maydell
Cc: Kevin Wolf, Qemu-block, Klaus Jensen, QEMU Developers, Max Reitz,
Keith Busch
[-- Attachment #1: Type: text/plain, Size: 1083 bytes --]
On Apr 22 19:58, Peter Maydell wrote:
>On Thu, 22 Apr 2021 at 15:07, Klaus Jensen <its@irrelevant.dk> wrote:
>>
>> On Apr 22 15:58, Klaus Jensen wrote:
>> >From: Klaus Jensen <k.jensen@samsung.com>
>> >
>> >Hi Peter,
>> >
>> >The commit message on the patch describes the issue. This is a QEMU
>> >crashing bug in -rc4 that I introduced early in the cycle and never
>> >found in time. Lack of testing device hotplugging is the cause for that.
>> >
>> >I'm not sure what to say other than I'm terribly sorry for introducing
>> >this and if this means an -rc5 needs to be rolled, then I'm even more
>> >sorry.
>> >
>> >I think an acceptance test could have caught this, and I am already
>> >working on an acceptance test suite for the nvme device, so I'll add
>> >something that test this as well. But, well, it doesn't help now.
>
>> As far as I can tell, to cause this crash, monitor access is required,
>> so I am not sure if we can get away with a note on this in the release
>> notes and fix this in a potential stable release or next.
>
>Is this a regression since 5.2 ?
>
Yes.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-04-22 19:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-22 13:58 [PATCH for-6.0? 0/1] hw/block/nvme: fix msix uninit Klaus Jensen
2021-04-22 13:58 ` [PATCH for-6.0? 1/1] hw/block/nvme: fix invalid msix exclusive uninit Klaus Jensen
2021-04-22 15:40 ` Klaus Jensen
2021-04-22 14:07 ` [PATCH for-6.0? 0/1] hw/block/nvme: fix msix uninit Klaus Jensen
2021-04-22 18:58 ` Peter Maydell
2021-04-22 19:17 ` Klaus Jensen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.