The NVMe device should not use target specific API.
Use memory_region_do_writeback() (which was introduced
in commit 61c490e25e0, after the NVMe emulated device
was added) to replace qemu_ram_writeback().
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
RFC because I have no clue how dirty_log_mask works.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Beata Michalska <beata.michalska@linaro.org>
---
hw/block/nvme.c | 4 +---
hw/block/Makefile.objs | 2 +-
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 9b453423cf..9b0ac0ea2a 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -46,7 +46,6 @@
#include "qapi/visitor.h"
#include "sysemu/hostmem.h"
#include "sysemu/block-backend.h"
-#include "exec/ram_addr.h"
#include "qemu/log.h"
#include "qemu/module.h"
@@ -1207,8 +1206,7 @@ static uint64_t nvme_mmio_read(void *opaque, hwaddr addr, unsigned size)
*/
if (addr == 0xE08 &&
(NVME_PMRCAP_PMRWBM(n->bar.pmrcap) & 0x02)) {
- qemu_ram_writeback(n->pmrdev->mr.ram_block,
- 0, n->pmrdev->size);
+ memory_region_do_writeback(&n->pmrdev->mr, 0, n->pmrdev->size);
}
memcpy(&val, ptr + addr, size);
} else {
diff --git a/hw/block/Makefile.objs b/hw/block/Makefile.objs
index 47960b5f0d..8855c22656 100644
--- a/hw/block/Makefile.objs
+++ b/hw/block/Makefile.objs
@@ -13,6 +13,6 @@ common-obj-$(CONFIG_SH4) += tc58128.o
obj-$(CONFIG_VIRTIO_BLK) += virtio-blk.o
obj-$(CONFIG_VHOST_USER_BLK) += vhost-user-blk.o
-obj-$(CONFIG_NVME_PCI) += nvme.o
+common-obj-$(CONFIG_NVME_PCI) += nvme.o
obj-y += dataplane/
--
2.21.3
+Keith new email
On 5/4/20 11:46 AM, Philippe Mathieu-Daudé wrote:
> The NVMe device should not use target specific API.
> Use memory_region_do_writeback() (which was introduced
> in commit 61c490e25e0, after the NVMe emulated device
> was added) to replace qemu_ram_writeback().
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> RFC because I have no clue how dirty_log_mask works.
>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Beata Michalska <beata.michalska@linaro.org>
> ---
> hw/block/nvme.c | 4 +---
> hw/block/Makefile.objs | 2 +-
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 9b453423cf..9b0ac0ea2a 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -46,7 +46,6 @@
> #include "qapi/visitor.h"
> #include "sysemu/hostmem.h"
> #include "sysemu/block-backend.h"
> -#include "exec/ram_addr.h"
>
> #include "qemu/log.h"
> #include "qemu/module.h"
> @@ -1207,8 +1206,7 @@ static uint64_t nvme_mmio_read(void *opaque, hwaddr addr, unsigned size)
> */
> if (addr == 0xE08 &&
> (NVME_PMRCAP_PMRWBM(n->bar.pmrcap) & 0x02)) {
> - qemu_ram_writeback(n->pmrdev->mr.ram_block,
> - 0, n->pmrdev->size);
> + memory_region_do_writeback(&n->pmrdev->mr, 0, n->pmrdev->size);
> }
> memcpy(&val, ptr + addr, size);
> } else {
> diff --git a/hw/block/Makefile.objs b/hw/block/Makefile.objs
> index 47960b5f0d..8855c22656 100644
> --- a/hw/block/Makefile.objs
> +++ b/hw/block/Makefile.objs
> @@ -13,6 +13,6 @@ common-obj-$(CONFIG_SH4) += tc58128.o
>
> obj-$(CONFIG_VIRTIO_BLK) += virtio-blk.o
> obj-$(CONFIG_VHOST_USER_BLK) += vhost-user-blk.o
> -obj-$(CONFIG_NVME_PCI) += nvme.o
> +common-obj-$(CONFIG_NVME_PCI) += nvme.o
>
> obj-y += dataplane/
>
On Mon, May 04, 2020 at 05:36:22PM +0200, Philippe Mathieu-Daudé wrote:
> +Keith new email
>
> On 5/4/20 11:46 AM, Philippe Mathieu-Daudé wrote:
> > The NVMe device should not use target specific API.
> > Use memory_region_do_writeback() (which was introduced
> > in commit 61c490e25e0, after the NVMe emulated device
> > was added) to replace qemu_ram_writeback().
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > ---
> > RFC because I have no clue how dirty_log_mask works.
> >
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Beata Michalska <beata.michalska@linaro.org>
> > ---
> > hw/block/nvme.c | 4 +---
> > hw/block/Makefile.objs | 2 +-
> > 2 files changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> > index 9b453423cf..9b0ac0ea2a 100644
> > --- a/hw/block/nvme.c
> > +++ b/hw/block/nvme.c
> > @@ -46,7 +46,6 @@
> > #include "qapi/visitor.h"
> > #include "sysemu/hostmem.h"
> > #include "sysemu/block-backend.h"
> > -#include "exec/ram_addr.h"
> > #include "qemu/log.h"
> > #include "qemu/module.h"
> > @@ -1207,8 +1206,7 @@ static uint64_t nvme_mmio_read(void *opaque, hwaddr addr, unsigned size)
> > */
> > if (addr == 0xE08 &&
> > (NVME_PMRCAP_PMRWBM(n->bar.pmrcap) & 0x02)) {
> > - qemu_ram_writeback(n->pmrdev->mr.ram_block,
> > - 0, n->pmrdev->size);
> > + memory_region_do_writeback(&n->pmrdev->mr, 0, n->pmrdev->size);
qemu_ram_write() is being called because we need to msync or persist
pmem here.
The memory_region_do_writeback() API is not equivalent, its purpose is
for dirty write logging (which we don't care about here because the
writes themselves will already have been logged when the guest performed
them).
I think qemu_ram_writeback() should just be made common so that this
code isn't target-specific. Maybe it should be renamed to
qemu_ram_msync() to avoid confusion with dirty write APIs.
Stefan
On 07/05/20 12:04, Stefan Hajnoczi wrote:
>>> (NVME_PMRCAP_PMRWBM(n->bar.pmrcap) & 0x02)) {
>>> - qemu_ram_writeback(n->pmrdev->mr.ram_block,
>>> - 0, n->pmrdev->size);
>>> + memory_region_do_writeback(&n->pmrdev->mr, 0, n->pmrdev->size);
> qemu_ram_write() is being called because we need to msync or persist
> pmem here.
>
> The memory_region_do_writeback() API is not equivalent, its purpose is
> for dirty write logging (which we don't care about here because the
> writes themselves will already have been logged when the guest performed
> them).
>
> I think qemu_ram_writeback() should just be made common so that this
> code isn't target-specific. Maybe it should be renamed to
> qemu_ram_msync() to avoid confusion with dirty write APIs.
Yes, we can add qemu_ram_msync and memory_region_msync.
Paolo
© 2016 - 2026 Red Hat, Inc.