* [PATCH v2 1/4] memory: Simplify memory_region_do_writeback()
2020-05-11 8:17 [PATCH v2 0/4] memory: Add memory_region_sync() & make NVMe emulated device generic Philippe Mathieu-Daudé
@ 2020-05-11 8:17 ` Philippe Mathieu-Daudé
2020-05-11 8:17 ` [PATCH v2 2/4] memory: Rename memory_region_do_writeback() -> memory_region_sync() Philippe Mathieu-Daudé
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-05-11 8:17 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Peter Maydell, Beata Michalska, qemu-block, Max Reitz,
qemu-arm, Keith Busch, Paolo Bonzini, Philippe Mathieu-Daudé,
Richard Henderson
mr->dirty_log_mask tells if dirty tracking has been enabled,
not if the page is dirty. It would always be true during live
migration and when running on TCG, but otherwise it would
always be false.
As the value of mr->dirty_log_mask does not matter, remove
the check.
Cc: Beata Michalska <beata.michalska@linaro.org>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
memory.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/memory.c b/memory.c
index 601b749906..fd5c3af535 100644
--- a/memory.c
+++ b/memory.c
@@ -2204,7 +2204,7 @@ void memory_region_do_writeback(MemoryRegion *mr, hwaddr addr, hwaddr size)
* Might be extended case needed to cover
* different types of memory regions
*/
- if (mr->ram_block && mr->dirty_log_mask) {
+ if (mr->ram_block) {
qemu_ram_writeback(mr->ram_block, addr, size);
}
}
--
2.21.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 2/4] memory: Rename memory_region_do_writeback() -> memory_region_sync()
2020-05-11 8:17 [PATCH v2 0/4] memory: Add memory_region_sync() & make NVMe emulated device generic Philippe Mathieu-Daudé
2020-05-11 8:17 ` [PATCH v2 1/4] memory: Simplify memory_region_do_writeback() Philippe Mathieu-Daudé
@ 2020-05-11 8:17 ` Philippe Mathieu-Daudé
2020-05-11 8:17 ` [PATCH v2 3/4] hw/block: Let the NVMe emulated device be target-agnostic Philippe Mathieu-Daudé
2020-05-11 8:17 ` [PATCH v2 4/4] exec: Rename qemu_ram_writeback() as qemu_ram_msync() Philippe Mathieu-Daudé
3 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-05-11 8:17 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Peter Maydell, Beata Michalska, qemu-block,
Richard Henderson, Max Reitz, qemu-arm, Keith Busch,
Paolo Bonzini, Philippe Mathieu-Daudé, Richard Henderson
We usually use '_do_' for internal functions. Rename
memory_region_do_writeback() as memory_region_sync()
to better reflect what it does.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
include/exec/memory.h | 13 +++++++------
memory.c | 2 +-
target/arm/helper.c | 2 +-
3 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index e000bd2f97..4fc1d85b99 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1474,14 +1474,15 @@ void *memory_region_get_ram_ptr(MemoryRegion *mr);
void memory_region_ram_resize(MemoryRegion *mr, ram_addr_t newsize,
Error **errp);
/**
- * memory_region_do_writeback: Trigger cache writeback or msync for
- * selected address range
+ * memory_region_sync: Synchronize selected address range
*
- * @mr: the memory region to be updated
- * @addr: the initial address of the range to be written back
- * @size: the size of the range to be written back
+ * It is only meaningful for RAM regions, otherwise it is no-op.
+ *
+ * @mr: the memory region to be synchronized
+ * @addr: the initial address of the range to be sync
+ * @size: the size of the range to be sync
*/
-void memory_region_do_writeback(MemoryRegion *mr, hwaddr addr, hwaddr size);
+void memory_region_sync(MemoryRegion *mr, hwaddr addr, hwaddr size);
/**
* memory_region_set_log: Turn dirty logging on or off for a region.
diff --git a/memory.c b/memory.c
index fd5c3af535..73534b26f4 100644
--- a/memory.c
+++ b/memory.c
@@ -2198,7 +2198,7 @@ void memory_region_ram_resize(MemoryRegion *mr, ram_addr_t newsize, Error **errp
}
-void memory_region_do_writeback(MemoryRegion *mr, hwaddr addr, hwaddr size)
+void memory_region_sync(MemoryRegion *mr, hwaddr addr, hwaddr size)
{
/*
* Might be extended case needed to cover
diff --git a/target/arm/helper.c b/target/arm/helper.c
index a94f650795..c2697ed7c0 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6829,7 +6829,7 @@ static void dccvap_writefn(CPUARMState *env, const ARMCPRegInfo *opaque,
mr = memory_region_from_host(haddr, &offset);
if (mr) {
- memory_region_do_writeback(mr, offset, dline_size);
+ memory_region_sync(mr, offset, dline_size);
}
}
}
--
2.21.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 3/4] hw/block: Let the NVMe emulated device be target-agnostic
2020-05-11 8:17 [PATCH v2 0/4] memory: Add memory_region_sync() & make NVMe emulated device generic Philippe Mathieu-Daudé
2020-05-11 8:17 ` [PATCH v2 1/4] memory: Simplify memory_region_do_writeback() Philippe Mathieu-Daudé
2020-05-11 8:17 ` [PATCH v2 2/4] memory: Rename memory_region_do_writeback() -> memory_region_sync() Philippe Mathieu-Daudé
@ 2020-05-11 8:17 ` Philippe Mathieu-Daudé
2020-05-11 8:17 ` [PATCH v2 4/4] exec: Rename qemu_ram_writeback() as qemu_ram_msync() Philippe Mathieu-Daudé
3 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-05-11 8:17 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Peter Maydell, Beata Michalska, qemu-block,
Richard Henderson, Max Reitz, qemu-arm, Keith Busch,
Paolo Bonzini, Philippe Mathieu-Daudé, Richard Henderson
Now than the non-target specific memory_region_sync() function
is available, use it to make this device target-agnostic.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/nvme.c | 6 ++----
hw/block/Makefile.objs | 2 +-
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 9b453423cf..d9d0649540 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -46,8 +46,7 @@
#include "qapi/visitor.h"
#include "sysemu/hostmem.h"
#include "sysemu/block-backend.h"
-#include "exec/ram_addr.h"
-
+#include "exec/memory.h"
#include "qemu/log.h"
#include "qemu/module.h"
#include "qemu/cutils.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_sync(&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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 4/4] exec: Rename qemu_ram_writeback() as qemu_ram_msync()
2020-05-11 8:17 [PATCH v2 0/4] memory: Add memory_region_sync() & make NVMe emulated device generic Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2020-05-11 8:17 ` [PATCH v2 3/4] hw/block: Let the NVMe emulated device be target-agnostic Philippe Mathieu-Daudé
@ 2020-05-11 8:17 ` Philippe Mathieu-Daudé
2020-05-11 9:50 ` Stefan Hajnoczi
3 siblings, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-05-11 8:17 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Peter Maydell, Beata Michalska, qemu-block, Max Reitz,
qemu-arm, Stefan Hajnoczi, Keith Busch, Paolo Bonzini,
Philippe Mathieu-Daudé, Richard Henderson
Rename qemu_ram_writeback() as qemu_ram_msync() to better
match what it does.
Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
include/exec/ram_addr.h | 4 ++--
exec.c | 2 +-
memory.c | 3 +--
3 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index a94809f89b..84817e19fa 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -136,12 +136,12 @@ void qemu_ram_free(RAMBlock *block);
int qemu_ram_resize(RAMBlock *block, ram_addr_t newsize, Error **errp);
-void qemu_ram_writeback(RAMBlock *block, ram_addr_t start, ram_addr_t length);
+void qemu_ram_msync(RAMBlock *block, ram_addr_t start, ram_addr_t length);
/* Clear whole block of mem */
static inline void qemu_ram_block_writeback(RAMBlock *block)
{
- qemu_ram_writeback(block, 0, block->used_length);
+ qemu_ram_msync(block, 0, block->used_length);
}
#define DIRTY_CLIENTS_ALL ((1 << DIRTY_MEMORY_NUM) - 1)
diff --git a/exec.c b/exec.c
index 2874bb5088..f5a8cdf370 100644
--- a/exec.c
+++ b/exec.c
@@ -2127,7 +2127,7 @@ int qemu_ram_resize(RAMBlock *block, ram_addr_t newsize, Error **errp)
* Otherwise no-op.
* @Note: this is supposed to be a synchronous op.
*/
-void qemu_ram_writeback(RAMBlock *block, ram_addr_t start, ram_addr_t length)
+void qemu_ram_msync(RAMBlock *block, ram_addr_t start, ram_addr_t length)
{
/* The requested range should fit in within the block range */
g_assert((start + length) <= block->used_length);
diff --git a/memory.c b/memory.c
index 73534b26f4..5bfa1429df 100644
--- a/memory.c
+++ b/memory.c
@@ -2197,7 +2197,6 @@ void memory_region_ram_resize(MemoryRegion *mr, ram_addr_t newsize, Error **errp
qemu_ram_resize(mr->ram_block, newsize, errp);
}
-
void memory_region_sync(MemoryRegion *mr, hwaddr addr, hwaddr size)
{
/*
@@ -2205,7 +2204,7 @@ void memory_region_sync(MemoryRegion *mr, hwaddr addr, hwaddr size)
* different types of memory regions
*/
if (mr->ram_block) {
- qemu_ram_writeback(mr->ram_block, addr, size);
+ qemu_ram_msync(mr->ram_block, addr, size);
}
}
--
2.21.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 4/4] exec: Rename qemu_ram_writeback() as qemu_ram_msync()
2020-05-11 8:17 ` [PATCH v2 4/4] exec: Rename qemu_ram_writeback() as qemu_ram_msync() Philippe Mathieu-Daudé
@ 2020-05-11 9:50 ` Stefan Hajnoczi
0 siblings, 0 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2020-05-11 9:50 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Kevin Wolf, Peter Maydell, Beata Michalska, qemu-block,
Juan Quintela, qemu-devel, Max Reitz, qemu-arm, Keith Busch,
Paolo Bonzini, Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 3074 bytes --]
On Mon, May 11, 2020 at 10:17:19AM +0200, Philippe Mathieu-Daudé wrote:
> Rename qemu_ram_writeback() as qemu_ram_msync() to better
> match what it does.
Based on Juan's comment in the other email thread I think this commit
description could be expanded. Let's explain the rationale for this
change:
qemu_ram_writeback() and memory_region_do_writeback() have similar names
but perform different functions. qemu_ram_writeback() is concerned with
syncing changes to the underlying memory device (NVDIMM or file-backed
RAM). memory_region_do_writeback() is concerned with dirty memory
logging.
Rename qemu_ram_writeback() to prevent confusion with
memory_region_do_writeback().
>
> Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> include/exec/ram_addr.h | 4 ++--
> exec.c | 2 +-
> memory.c | 3 +--
> 3 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> index a94809f89b..84817e19fa 100644
> --- a/include/exec/ram_addr.h
> +++ b/include/exec/ram_addr.h
> @@ -136,12 +136,12 @@ void qemu_ram_free(RAMBlock *block);
>
> int qemu_ram_resize(RAMBlock *block, ram_addr_t newsize, Error **errp);
>
> -void qemu_ram_writeback(RAMBlock *block, ram_addr_t start, ram_addr_t length);
> +void qemu_ram_msync(RAMBlock *block, ram_addr_t start, ram_addr_t length);
>
> /* Clear whole block of mem */
> static inline void qemu_ram_block_writeback(RAMBlock *block)
> {
> - qemu_ram_writeback(block, 0, block->used_length);
> + qemu_ram_msync(block, 0, block->used_length);
> }
>
> #define DIRTY_CLIENTS_ALL ((1 << DIRTY_MEMORY_NUM) - 1)
> diff --git a/exec.c b/exec.c
> index 2874bb5088..f5a8cdf370 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2127,7 +2127,7 @@ int qemu_ram_resize(RAMBlock *block, ram_addr_t newsize, Error **errp)
> * Otherwise no-op.
> * @Note: this is supposed to be a synchronous op.
> */
> -void qemu_ram_writeback(RAMBlock *block, ram_addr_t start, ram_addr_t length)
> +void qemu_ram_msync(RAMBlock *block, ram_addr_t start, ram_addr_t length)
> {
> /* The requested range should fit in within the block range */
> g_assert((start + length) <= block->used_length);
> diff --git a/memory.c b/memory.c
> index 73534b26f4..5bfa1429df 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -2197,7 +2197,6 @@ void memory_region_ram_resize(MemoryRegion *mr, ram_addr_t newsize, Error **errp
> qemu_ram_resize(mr->ram_block, newsize, errp);
> }
>
> -
> void memory_region_sync(MemoryRegion *mr, hwaddr addr, hwaddr size)
> {
> /*
> @@ -2205,7 +2204,7 @@ void memory_region_sync(MemoryRegion *mr, hwaddr addr, hwaddr size)
> * different types of memory regions
> */
> if (mr->ram_block) {
> - qemu_ram_writeback(mr->ram_block, addr, size);
> + qemu_ram_msync(mr->ram_block, addr, size);
> }
> }
>
> --
> 2.21.3
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread