qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/4] nvme: some small fixes
@ 2018-10-30  5:18 Li Qiang
  2018-10-30  5:18 ` [Qemu-devel] [PATCH 1/4] nvme: use TYPE_NVME instead of constant string Li Qiang
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Li Qiang @ 2018-10-30  5:18 UTC (permalink / raw)
  To: keith.busch, kwolf, mreitz; +Cc: qemu-block, qemu-devel, Li Qiang

Including sanity check and code cleanup.

Li Qiang (4):
  nvme: use TYPE_NVME instead of constant string
  nvme: ensure the num_queues is not zero
  nvme: check msix_init_exclusive_bar return value
  nvme: use pci_dev directly in nvme_realize

 hw/block/nvme.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 1/4] nvme: use TYPE_NVME instead of constant string
  2018-10-30  5:18 [Qemu-devel] [PATCH 0/4] nvme: some small fixes Li Qiang
@ 2018-10-30  5:18 ` Li Qiang
  2019-01-09 14:39   ` Max Reitz
  2018-10-30  5:18 ` [Qemu-devel] [PATCH 2/4] nvme: ensure the num_queues is not zero Li Qiang
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Li Qiang @ 2018-10-30  5:18 UTC (permalink / raw)
  To: keith.busch, kwolf, mreitz; +Cc: qemu-block, qemu-devel, Li Qiang

Signed-off-by: Li Qiang <liq3ea@gmail.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 fc7dacb..676cc48 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1379,7 +1379,7 @@ static void nvme_instance_init(Object *obj)
 }
 
 static const TypeInfo nvme_info = {
-    .name          = "nvme",
+    .name          = TYPE_NVME,
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(NvmeCtrl),
     .class_init    = nvme_class_init,
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 2/4] nvme: ensure the num_queues is not zero
  2018-10-30  5:18 [Qemu-devel] [PATCH 0/4] nvme: some small fixes Li Qiang
  2018-10-30  5:18 ` [Qemu-devel] [PATCH 1/4] nvme: use TYPE_NVME instead of constant string Li Qiang
@ 2018-10-30  5:18 ` Li Qiang
  2019-01-09 14:38   ` Max Reitz
  2018-10-30  5:18 ` [Qemu-devel] [PATCH 3/4] nvme: check msix_init_exclusive_bar return value Li Qiang
  2018-10-30  5:18 ` [Qemu-devel] [PATCH 4/4] nvme: use pci_dev directly in nvme_realize Li Qiang
  3 siblings, 1 reply; 11+ messages in thread
From: Li Qiang @ 2018-10-30  5:18 UTC (permalink / raw)
  To: keith.busch, kwolf, mreitz; +Cc: qemu-block, qemu-devel, Li Qiang

When it is zero, it causes segv. Backtrack:
Thread 5 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffc6c17700 (LWP 51808)]
0x0000555555acbd90 in nvme_start_ctrl (n=0x6240000c8100) at hw/block/nvme.c:820
warning: Source file is more recent than executable.
820	    if (unlikely(n->cq[0])) {
(gdb) bt
0  0x0000555555acbd90 in nvme_start_ctrl (n=0x6240000c8100) at hw/block/nvme.c:820
1  0x0000555555accdbc in nvme_write_bar (n=0x6240000c8100, offset=20, data=4587521, size=4) at hw/block/nvme.c:964
2  0x0000555555acdd2b in nvme_mmio_write (opaque=0x6240000c8100, addr=20, data=4587521, size=4) at hw/block/nvme.c:1158
3  0x00005555558973ed in memory_region_write_accessor (mr=0x6240000c89e0, addr=20, value=0x7fffc6c14428, size=4, shift=0, mask=4294967295, attrs=...) at /home/liqiang02/qemu-upstream/qemu/memory.c:500
4  0x0000555555897600 in access_with_adjusted_size (addr=20, value=0x7fffc6c14428, size=4, access_size_min=2, access_size_max=8, access_fn=0x555555897304 <memory_region_write_accessor>, mr=0x6240000c89e0, attrs=...) at /home/liqiang02/qemu-upstream/qemu/memory.c:566
5  0x000055555589a200 in memory_region_dispatch_write (mr=0x6240000c89e0, addr=20, data=4587521, size=4, attrs=...) at /home/liqiang02/qemu-upstream/qemu/memory.c:1442
6  0x0000555555835151 in flatview_write_continue (fv=0x6060000e6fc0, addr=4273930260, attrs=..., buf=0x7fffc8a18028 "\001", len=4, addr1=20, l=4, mr=0x6240000c89e0) at /home/liqiang02/qemu-upstream/qemu/exec.c:3233
7  0x000055555583529b in flatview_write (fv=0x6060000e6fc0, addr=4273930260, attrs=..., buf=0x7fffc8a18028 "\001", len=4) at /home/liqiang02/qemu-upstream/qemu/exec.c:3272
8  0x00005555558355a1 in address_space_write (as=0x55555683ade0 <address_space_memory>, addr=4273930260, attrs=..., buf=0x7fffc8a18028 "\001", len=4) at /home/liqiang02/qemu-upstream/qemu/exec.c:3362
9  0x00005555558355f2 in address_space_rw (as=0x55555683ade0 <address_space_memory>, addr=4273930260, attrs=..., buf=0x7fffc8a18028 "\001", len=4, is_write=true) at /home/liqiang02/qemu-upstream/qemu/exec.c:3373
10 0x00005555558b66ac in kvm_cpu_exec (cpu=0x631000014800) at /home/liqiang02/qemu-upstream/qemu/accel/kvm/kvm-all.c:2031
11 0x000055555587c3ac in qemu_kvm_cpu_thread_fn (arg=0x631000014800) at /home/liqiang02/qemu-upstream/qemu/cpus.c:1277
12 0x0000555555e54ae6 in qemu_thread_start (args=0x60300002c170) at util/qemu-thread-posix.c:504
13 0x00007fffdadbd494 in start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0
14 0x00007fffdaaffacf in clone () from /lib/x86_64-linux-gnu/libc.so.6
(gdb) q

Signed-off-by: Li Qiang <liq3ea@gmail.com>
---
 hw/block/nvme.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 676cc48..72c9644 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1221,6 +1221,10 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
         error_setg(errp, "serial property not set");
         return;
     }
+
+    if (!n->num_queues) {
+        error_setg(errp, "num_queues can't be zero");
+    }
     blkconf_blocksizes(&n->conf);
     if (!blkconf_apply_backend_options(&n->conf, blk_is_read_only(n->conf.blk),
                                        false, errp)) {
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 3/4] nvme: check msix_init_exclusive_bar return value
  2018-10-30  5:18 [Qemu-devel] [PATCH 0/4] nvme: some small fixes Li Qiang
  2018-10-30  5:18 ` [Qemu-devel] [PATCH 1/4] nvme: use TYPE_NVME instead of constant string Li Qiang
  2018-10-30  5:18 ` [Qemu-devel] [PATCH 2/4] nvme: ensure the num_queues is not zero Li Qiang
@ 2018-10-30  5:18 ` Li Qiang
  2019-01-09 14:51   ` Max Reitz
  2018-10-30  5:18 ` [Qemu-devel] [PATCH 4/4] nvme: use pci_dev directly in nvme_realize Li Qiang
  3 siblings, 1 reply; 11+ messages in thread
From: Li Qiang @ 2018-10-30  5:18 UTC (permalink / raw)
  To: keith.busch, kwolf, mreitz; +Cc: qemu-block, qemu-devel, Li Qiang

As this function can fail.

Signed-off-by: Li Qiang <liq3ea@gmail.com>
---
 hw/block/nvme.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 72c9644..a406c23 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1250,7 +1250,10 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
     pci_register_bar(&n->parent_obj, 0,
         PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64,
         &n->iomem);
-    msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4, NULL);
+    if (msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4, NULL)) {
+        error_setg(errp, "msix_init_exclusive_bar failed");
+        return;
+    }
 
     id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID));
     id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID));
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 4/4] nvme: use pci_dev directly in nvme_realize
  2018-10-30  5:18 [Qemu-devel] [PATCH 0/4] nvme: some small fixes Li Qiang
                   ` (2 preceding siblings ...)
  2018-10-30  5:18 ` [Qemu-devel] [PATCH 3/4] nvme: check msix_init_exclusive_bar return value Li Qiang
@ 2018-10-30  5:18 ` Li Qiang
  2019-01-09 15:05   ` Max Reitz
  3 siblings, 1 reply; 11+ messages in thread
From: Li Qiang @ 2018-10-30  5:18 UTC (permalink / raw)
  To: keith.busch, kwolf, mreitz; +Cc: qemu-block, qemu-devel, Li Qiang

There is no need to make another reference.

Signed-off-by: Li Qiang <liq3ea@gmail.com>
---
 hw/block/nvme.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index a406c23..f63e344 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1235,7 +1235,7 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
     pci_conf[PCI_INTERRUPT_PIN] = 1;
     pci_config_set_prog_interface(pci_dev->config, 0x2);
     pci_config_set_class(pci_dev->config, PCI_CLASS_STORAGE_EXPRESS);
-    pcie_endpoint_cap_init(&n->parent_obj, 0x80);
+    pcie_endpoint_cap_init(pci_dev, 0x80);
 
     n->num_namespaces = 1;
     n->reg_size = pow2ceil(0x1004 + 2 * (n->num_queues + 1) * 4);
@@ -1247,10 +1247,10 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
 
     memory_region_init_io(&n->iomem, OBJECT(n), &nvme_mmio_ops, n,
                           "nvme", n->reg_size);
-    pci_register_bar(&n->parent_obj, 0,
+    pci_register_bar(pci_dev, 0,
         PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64,
         &n->iomem);
-    if (msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4, NULL)) {
+    if (msix_init_exclusive_bar(pci_dev, n->num_queues, 4, NULL)) {
         error_setg(errp, "msix_init_exclusive_bar failed");
         return;
     }
@@ -1308,7 +1308,7 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
         n->cmbuf = g_malloc0(NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
         memory_region_init_io(&n->ctrl_mem, OBJECT(n), &nvme_cmb_ops, n,
                               "nvme-cmb", NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
-        pci_register_bar(&n->parent_obj, NVME_CMBLOC_BIR(n->bar.cmbloc),
+        pci_register_bar(pci_dev, NVME_CMBLOC_BIR(n->bar.cmbloc),
             PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64 |
             PCI_BASE_ADDRESS_MEM_PREFETCH, &n->ctrl_mem);
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 2/4] nvme: ensure the num_queues is not zero
  2018-10-30  5:18 ` [Qemu-devel] [PATCH 2/4] nvme: ensure the num_queues is not zero Li Qiang
@ 2019-01-09 14:38   ` Max Reitz
  2019-01-10  3:02     ` Li Qiang
  0 siblings, 1 reply; 11+ messages in thread
From: Max Reitz @ 2019-01-09 14:38 UTC (permalink / raw)
  To: Li Qiang, keith.busch, kwolf; +Cc: qemu-block, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 3500 bytes --]

On 30.10.18 06:18, Li Qiang wrote:
> When it is zero, it causes segv. Backtrack:
> Thread 5 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 0x7fffc6c17700 (LWP 51808)]
> 0x0000555555acbd90 in nvme_start_ctrl (n=0x6240000c8100) at hw/block/nvme.c:820
> warning: Source file is more recent than executable.
> 820	    if (unlikely(n->cq[0])) {
> (gdb) bt
> 0  0x0000555555acbd90 in nvme_start_ctrl (n=0x6240000c8100) at hw/block/nvme.c:820
> 1  0x0000555555accdbc in nvme_write_bar (n=0x6240000c8100, offset=20, data=4587521, size=4) at hw/block/nvme.c:964
> 2  0x0000555555acdd2b in nvme_mmio_write (opaque=0x6240000c8100, addr=20, data=4587521, size=4) at hw/block/nvme.c:1158
> 3  0x00005555558973ed in memory_region_write_accessor (mr=0x6240000c89e0, addr=20, value=0x7fffc6c14428, size=4, shift=0, mask=4294967295, attrs=...) at /home/liqiang02/qemu-upstream/qemu/memory.c:500
> 4  0x0000555555897600 in access_with_adjusted_size (addr=20, value=0x7fffc6c14428, size=4, access_size_min=2, access_size_max=8, access_fn=0x555555897304 <memory_region_write_accessor>, mr=0x6240000c89e0, attrs=...) at /home/liqiang02/qemu-upstream/qemu/memory.c:566
> 5  0x000055555589a200 in memory_region_dispatch_write (mr=0x6240000c89e0, addr=20, data=4587521, size=4, attrs=...) at /home/liqiang02/qemu-upstream/qemu/memory.c:1442
> 6  0x0000555555835151 in flatview_write_continue (fv=0x6060000e6fc0, addr=4273930260, attrs=..., buf=0x7fffc8a18028 "\001", len=4, addr1=20, l=4, mr=0x6240000c89e0) at /home/liqiang02/qemu-upstream/qemu/exec.c:3233
> 7  0x000055555583529b in flatview_write (fv=0x6060000e6fc0, addr=4273930260, attrs=..., buf=0x7fffc8a18028 "\001", len=4) at /home/liqiang02/qemu-upstream/qemu/exec.c:3272
> 8  0x00005555558355a1 in address_space_write (as=0x55555683ade0 <address_space_memory>, addr=4273930260, attrs=..., buf=0x7fffc8a18028 "\001", len=4) at /home/liqiang02/qemu-upstream/qemu/exec.c:3362
> 9  0x00005555558355f2 in address_space_rw (as=0x55555683ade0 <address_space_memory>, addr=4273930260, attrs=..., buf=0x7fffc8a18028 "\001", len=4, is_write=true) at /home/liqiang02/qemu-upstream/qemu/exec.c:3373
> 10 0x00005555558b66ac in kvm_cpu_exec (cpu=0x631000014800) at /home/liqiang02/qemu-upstream/qemu/accel/kvm/kvm-all.c:2031
> 11 0x000055555587c3ac in qemu_kvm_cpu_thread_fn (arg=0x631000014800) at /home/liqiang02/qemu-upstream/qemu/cpus.c:1277
> 12 0x0000555555e54ae6 in qemu_thread_start (args=0x60300002c170) at util/qemu-thread-posix.c:504
> 13 0x00007fffdadbd494 in start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0
> 14 0x00007fffdaaffacf in clone () from /lib/x86_64-linux-gnu/libc.so.6
> (gdb) q
> 
> Signed-off-by: Li Qiang <liq3ea@gmail.com>
> ---
>  hw/block/nvme.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 676cc48..72c9644 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -1221,6 +1221,10 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
>          error_setg(errp, "serial property not set");
>          return;
>      }
> +
> +    if (!n->num_queues) {
> +        error_setg(errp, "num_queues can't be zero");

I think we should return here.

(Sorry for the late review...)

Max

> +    }
>      blkconf_blocksizes(&n->conf);
>      if (!blkconf_apply_backend_options(&n->conf, blk_is_read_only(n->conf.blk),
>                                         false, errp)) {
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 1/4] nvme: use TYPE_NVME instead of constant string
  2018-10-30  5:18 ` [Qemu-devel] [PATCH 1/4] nvme: use TYPE_NVME instead of constant string Li Qiang
@ 2019-01-09 14:39   ` Max Reitz
  0 siblings, 0 replies; 11+ messages in thread
From: Max Reitz @ 2019-01-09 14:39 UTC (permalink / raw)
  To: Li Qiang, keith.busch, kwolf; +Cc: qemu-block, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 214 bytes --]

On 30.10.18 06:18, Li Qiang wrote:
> Signed-off-by: Li Qiang <liq3ea@gmail.com>
> ---
>  hw/block/nvme.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 3/4] nvme: check msix_init_exclusive_bar return value
  2018-10-30  5:18 ` [Qemu-devel] [PATCH 3/4] nvme: check msix_init_exclusive_bar return value Li Qiang
@ 2019-01-09 14:51   ` Max Reitz
  2019-01-10  3:06     ` Li Qiang
  0 siblings, 1 reply; 11+ messages in thread
From: Max Reitz @ 2019-01-09 14:51 UTC (permalink / raw)
  To: Li Qiang, keith.busch, kwolf; +Cc: qemu-block, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 1455 bytes --]

On 30.10.18 06:18, Li Qiang wrote:
> As this function can fail.
> 
> Signed-off-by: Li Qiang <liq3ea@gmail.com>
> ---
>  hw/block/nvme.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 72c9644..a406c23 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -1250,7 +1250,10 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
>      pci_register_bar(&n->parent_obj, 0,
>          PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64,
>          &n->iomem);
> -    msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4, NULL);
> +    if (msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4, NULL)) {
> +        error_setg(errp, "msix_init_exclusive_bar failed");
> +        return;
> +    }

Commit ee640c625e1 which last touched this function stated that creating
this device won't fail if this function fails, so I suppose it's
intential that the error isn't checked.


But if you think the error should be checked and device creation should
be aborted if this function failed, then I suppose there'd be some
cleaning up to instead of just returning.

Also, we should just pass errp to that function (as its last parameter,
instead of NULL).

Max

>  
>      id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID));
>      id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID));
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 4/4] nvme: use pci_dev directly in nvme_realize
  2018-10-30  5:18 ` [Qemu-devel] [PATCH 4/4] nvme: use pci_dev directly in nvme_realize Li Qiang
@ 2019-01-09 15:05   ` Max Reitz
  0 siblings, 0 replies; 11+ messages in thread
From: Max Reitz @ 2019-01-09 15:05 UTC (permalink / raw)
  To: Li Qiang, keith.busch, kwolf; +Cc: qemu-block, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 273 bytes --]

On 30.10.18 06:18, Li Qiang wrote:
> There is no need to make another reference.
> 
> Signed-off-by: Li Qiang <liq3ea@gmail.com>
> ---
>  hw/block/nvme.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 2/4] nvme: ensure the num_queues is not zero
  2019-01-09 14:38   ` Max Reitz
@ 2019-01-10  3:02     ` Li Qiang
  0 siblings, 0 replies; 11+ messages in thread
From: Li Qiang @ 2019-01-10  3:02 UTC (permalink / raw)
  To: Max Reitz; +Cc: keith.busch, kwolf, qemu-block, Qemu Developers

Max Reitz <mreitz@redhat.com> 于2019年1月9日周三 下午10:38写道:

> On 30.10.18 06:18, Li Qiang wrote:
> > When it is zero, it causes segv. Backtrack:
> > Thread 5 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
> > [Switching to Thread 0x7fffc6c17700 (LWP 51808)]
> > 0x0000555555acbd90 in nvme_start_ctrl (n=0x6240000c8100) at
> hw/block/nvme.c:820
> > warning: Source file is more recent than executable.
> > 820       if (unlikely(n->cq[0])) {
> > (gdb) bt
> > 0  0x0000555555acbd90 in nvme_start_ctrl (n=0x6240000c8100) at
> hw/block/nvme.c:820
> > 1  0x0000555555accdbc in nvme_write_bar (n=0x6240000c8100, offset=20,
> data=4587521, size=4) at hw/block/nvme.c:964
> > 2  0x0000555555acdd2b in nvme_mmio_write (opaque=0x6240000c8100,
> addr=20, data=4587521, size=4) at hw/block/nvme.c:1158
> > 3  0x00005555558973ed in memory_region_write_accessor
> (mr=0x6240000c89e0, addr=20, value=0x7fffc6c14428, size=4, shift=0,
> mask=4294967295, attrs=...) at
> /home/liqiang02/qemu-upstream/qemu/memory.c:500
> > 4  0x0000555555897600 in access_with_adjusted_size (addr=20,
> value=0x7fffc6c14428, size=4, access_size_min=2, access_size_max=8,
> access_fn=0x555555897304 <memory_region_write_accessor>, mr=0x6240000c89e0,
> attrs=...) at /home/liqiang02/qemu-upstream/qemu/memory.c:566
> > 5  0x000055555589a200 in memory_region_dispatch_write
> (mr=0x6240000c89e0, addr=20, data=4587521, size=4, attrs=...) at
> /home/liqiang02/qemu-upstream/qemu/memory.c:1442
> > 6  0x0000555555835151 in flatview_write_continue (fv=0x6060000e6fc0,
> addr=4273930260, attrs=..., buf=0x7fffc8a18028 "\001", len=4, addr1=20,
> l=4, mr=0x6240000c89e0) at /home/liqiang02/qemu-upstream/qemu/exec.c:3233
> > 7  0x000055555583529b in flatview_write (fv=0x6060000e6fc0,
> addr=4273930260, attrs=..., buf=0x7fffc8a18028 "\001", len=4) at
> /home/liqiang02/qemu-upstream/qemu/exec.c:3272
> > 8  0x00005555558355a1 in address_space_write (as=0x55555683ade0
> <address_space_memory>, addr=4273930260, attrs=..., buf=0x7fffc8a18028
> "\001", len=4) at /home/liqiang02/qemu-upstream/qemu/exec.c:3362
> > 9  0x00005555558355f2 in address_space_rw (as=0x55555683ade0
> <address_space_memory>, addr=4273930260, attrs=..., buf=0x7fffc8a18028
> "\001", len=4, is_write=true) at
> /home/liqiang02/qemu-upstream/qemu/exec.c:3373
> > 10 0x00005555558b66ac in kvm_cpu_exec (cpu=0x631000014800) at
> /home/liqiang02/qemu-upstream/qemu/accel/kvm/kvm-all.c:2031
> > 11 0x000055555587c3ac in qemu_kvm_cpu_thread_fn (arg=0x631000014800) at
> /home/liqiang02/qemu-upstream/qemu/cpus.c:1277
> > 12 0x0000555555e54ae6 in qemu_thread_start (args=0x60300002c170) at
> util/qemu-thread-posix.c:504
> > 13 0x00007fffdadbd494 in start_thread () from
> /lib/x86_64-linux-gnu/libpthread.so.0
> > 14 0x00007fffdaaffacf in clone () from /lib/x86_64-linux-gnu/libc.so.6
> > (gdb) q
> >
> > Signed-off-by: Li Qiang <liq3ea@gmail.com>
> > ---
> >  hw/block/nvme.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> > index 676cc48..72c9644 100644
> > --- a/hw/block/nvme.c
> > +++ b/hw/block/nvme.c
> > @@ -1221,6 +1221,10 @@ static void nvme_realize(PCIDevice *pci_dev,
> Error **errp)
> >          error_setg(errp, "serial property not set");
> >          return;
> >      }
> > +
> > +    if (!n->num_queues) {
> > +        error_setg(errp, "num_queues can't be zero");
>
> I think we should return here.
>
>
Ok, forget this. will make a revised version later.

Thanks,
Li Qiang


> (Sorry for the late review...)
>
> Max
>
> > +    }
> >      blkconf_blocksizes(&n->conf);
> >      if (!blkconf_apply_backend_options(&n->conf,
> blk_is_read_only(n->conf.blk),
> >                                         false, errp)) {
> >
>
>
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 3/4] nvme: check msix_init_exclusive_bar return value
  2019-01-09 14:51   ` Max Reitz
@ 2019-01-10  3:06     ` Li Qiang
  0 siblings, 0 replies; 11+ messages in thread
From: Li Qiang @ 2019-01-10  3:06 UTC (permalink / raw)
  To: Max Reitz; +Cc: keith.busch, kwolf, qemu-block, Qemu Developers

Max Reitz <mreitz@redhat.com> 于2019年1月9日周三 下午10:52写道:

> On 30.10.18 06:18, Li Qiang wrote:
> > As this function can fail.
> >
> > Signed-off-by: Li Qiang <liq3ea@gmail.com>
> > ---
> >  hw/block/nvme.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> > index 72c9644..a406c23 100644
> > --- a/hw/block/nvme.c
> > +++ b/hw/block/nvme.c
> > @@ -1250,7 +1250,10 @@ static void nvme_realize(PCIDevice *pci_dev,
> Error **errp)
> >      pci_register_bar(&n->parent_obj, 0,
> >          PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64,
> >          &n->iomem);
> > -    msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4, NULL);
> > +    if (msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4,
> NULL)) {
> > +        error_setg(errp, "msix_init_exclusive_bar failed");
> > +        return;
> > +    }
>
> Commit ee640c625e1 which last touched this function stated that creating
> this device won't fail if this function fails, so I suppose it's
> intential that the error isn't checked.
>

OK, got. I will drop this patch later.

Thanks,
Li Qiang


>
>
> But if you think the error should be checked and device creation should
> be aborted if this function failed, then I suppose there'd be some
> cleaning up to instead of just returning.
>
> Also, we should just pass errp to that function (as its last parameter,
> instead of NULL).
>
> Max
>
> >
> >      id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID));
> >      id->ssvid = cpu_to_le16(pci_get_word(pci_conf +
> PCI_SUBSYSTEM_VENDOR_ID));
> >
>
>
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-01-10  3:06 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-10-30  5:18 [Qemu-devel] [PATCH 0/4] nvme: some small fixes Li Qiang
2018-10-30  5:18 ` [Qemu-devel] [PATCH 1/4] nvme: use TYPE_NVME instead of constant string Li Qiang
2019-01-09 14:39   ` Max Reitz
2018-10-30  5:18 ` [Qemu-devel] [PATCH 2/4] nvme: ensure the num_queues is not zero Li Qiang
2019-01-09 14:38   ` Max Reitz
2019-01-10  3:02     ` Li Qiang
2018-10-30  5:18 ` [Qemu-devel] [PATCH 3/4] nvme: check msix_init_exclusive_bar return value Li Qiang
2019-01-09 14:51   ` Max Reitz
2019-01-10  3:06     ` Li Qiang
2018-10-30  5:18 ` [Qemu-devel] [PATCH 4/4] nvme: use pci_dev directly in nvme_realize Li Qiang
2019-01-09 15:05   ` 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).