[PATCH] softmmu: Move dirtylimit.c into the target independent source set

[PATCH] softmmu: Move dirtylimit.c into the target independent source set

[Thomas Huth]

dirtylimit.c just uses one TARGET_PAGE_SIZE macro - change it to
qemu_target_page_size() so we can move thefile into the target
independent source set. Then we only have to compile this file
once during the build instead of multiple times (one time for
each target).

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 softmmu/dirtylimit.c | 3 ++-
 softmmu/meson.build  | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/softmmu/dirtylimit.c b/softmmu/dirtylimit.c
index c56f0f58c8..82986c1499 100644
--- a/softmmu/dirtylimit.c
+++ b/softmmu/dirtylimit.c
@@ -20,6 +20,7 @@
 #include "monitor/hmp.h"
 #include "monitor/monitor.h"
 #include "exec/memory.h"
+#include "exec/target_page.h"
 #include "hw/boards.h"
 #include "sysemu/kvm.h"
 #include "trace.h"
@@ -236,7 +237,7 @@ static inline int64_t dirtylimit_dirty_ring_full_time(uint64_t dirtyrate)
     static uint64_t max_dirtyrate;
     uint32_t dirty_ring_size = kvm_dirty_ring_size();
     uint64_t dirty_ring_size_meory_MB =
-        dirty_ring_size * TARGET_PAGE_SIZE >> 20;
+        dirty_ring_size * qemu_target_page_size() >> 20;
 
     if (max_dirtyrate < dirtyrate) {
         max_dirtyrate = dirtyrate;
diff --git a/softmmu/meson.build b/softmmu/meson.build
index b392f0bd35..974732b0f3 100644
--- a/softmmu/meson.build
+++ b/softmmu/meson.build
@@ -3,7 +3,6 @@ specific_ss.add(when: 'CONFIG_SOFTMMU', if_true: [files(
   'ioport.c',
   'memory.c',
   'physmem.c',
-  'dirtylimit.c',
   'watchpoint.c',
 )])
 
@@ -18,6 +17,7 @@ softmmu_ss.add(files(
   'cpu-throttle.c',
   'cpu-timers.c',
   'datadir.c',
+  'dirtylimit.c',
   'dma-helpers.c',
   'globals.c',
   'memory_mapping.c',
-- 
2.31.1

Re: [PATCH] softmmu: Move dirtylimit.c into the target independent source set

[Richard Henderson]

On 4/13/23 07:45, Thomas Huth wrote:
>       uint32_t dirty_ring_size = kvm_dirty_ring_size();
>       uint64_t dirty_ring_size_meory_MB =
> -        dirty_ring_size * TARGET_PAGE_SIZE >> 20;
> +        dirty_ring_size * qemu_target_page_size() >> 20;

Existing problem, the types here are suspicious: dirty_ring_size is uint32_t, 
dirty_ring_size_meory (typo) is uint64_t.

I wonder if this is better computed as

     uint32_t dirty_ring_size_MB = dirty_ring_size >> (20 - qemu_target_page_bits());


r~

Re: [PATCH] softmmu: Move dirtylimit.c into the target independent source set

[Fabiano Rosas]

Thomas Huth <thuth@redhat.com> writes:

> dirtylimit.c just uses one TARGET_PAGE_SIZE macro - change it to
> qemu_target_page_size() so we can move thefile into the target
> independent source set. Then we only have to compile this file
> once during the build instead of multiple times (one time for
> each target).
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Fabiano Rosas <farosas@suse.de>

Re: [PATCH] softmmu: Move dirtylimit.c into the target independent source set

[Thomas Huth]

On 13/04/2023 12.31, Richard Henderson wrote:
> On 4/13/23 07:45, Thomas Huth wrote:
>>       uint32_t dirty_ring_size = kvm_dirty_ring_size();
>>       uint64_t dirty_ring_size_meory_MB =
>> -        dirty_ring_size * TARGET_PAGE_SIZE >> 20;
>> +        dirty_ring_size * qemu_target_page_size() >> 20;
> 
> Existing problem, the types here are suspicious: dirty_ring_size is 
> uint32_t, dirty_ring_size_meory (typo) is uint64_t.
> 
> I wonder if this is better computed as
> 
>      uint32_t dirty_ring_size_MB = dirty_ring_size >> (20 - 
> qemu_target_page_bits());

qemu_target_page_size() returns a "size_t", so I think it should be fine for 
64-bit hosts. But for 32-bit hosts, this looks error prone, indeed, so I 
think your suggestion is a good idea. Care to send a patch?

  Thomas