On 20.04.21 12:40, Philippe Mathieu-Daudé wrote:
On 4/20/21 12:27 PM, David Hildenbrand wrote:
On 20.04.21 12:20, Philippe Mathieu-Daudé wrote:
Hi David,
On 4/13/21 11:14 AM, David Hildenbrand wrote:
Let's forward ram_flags instead, renaming
memory_region_init_ram_shared_nomigrate() into
memory_region_init_ram_flags_nomigrate(). Forward flags to
qemu_ram_alloc() and qemu_ram_alloc_internal().
Reviewed-by: Peter Xu <pet...@redhat.com>
Signed-off-by: David Hildenbrand <da...@redhat.com>
---
backends/hostmem-ram.c | 6 +++--
hw/m68k/next-cube.c | 4 ++--
include/exec/memory.h | 24 +++++++++----------
include/exec/ram_addr.h | 2 +-
.../memory-region-housekeeping.cocci | 8 +++----
softmmu/memory.c | 20 ++++++++--------
OK up to here, but the qemu_ram_alloc_internal() changes
in softmmu/physmem.c belong to a different patch (except
the line adding "new_block->flags = ram_flags").
Do you mind splitting it?
Can you elaborate? Temporarily passing both "ram_flags" and "bool
resizeable, bool share" to qemu_ram_alloc_internal()?
I don't see a big benefit in doing that besides even more effective
changes in two individual patches. But maybe if you elaborate, i can see
what you would like to see :)
In this patch I see
1/ change a parameter and propagate it
2/ adapt assertions
I'd rather review the assertions modified / cleaned in another patch,
simply because it required me 2 different mental efforts to review the
first part and the second part. But maybe it is not possible, so I'll
review the 2nd part here.
Well, previously you could pass in "bool resizeable, bool share", now
you can pass in ram_flags with RAM_SHARED, RAM_RESIZEABLE. So that
assertion part actually looked fairly straight forward to me.
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index cc59f05593..fdcd38ba61 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -2108,12 +2108,14 @@ RAMBlock *qemu_ram_alloc_internal(ram_addr_t
size, ram_addr_t max_size,
void (*resized)(const char*,
uint64_t length,
void *host),
- void *host, bool resizeable, bool
share,
+ void *host, uint32_t ram_flags,
MemoryRegion *mr, Error **errp)
{
RAMBlock *new_block;
Error *local_err = NULL;
Maybe also:
assert(!host || (ram_flags & RAM_PREALLOC));
It should be even stricter I think
assert(!host ^ (ram_flags & RAM_PREALLOC));
I'd move that change definitely to a separate patch.
Thanks!
--
Thanks,
David / dhildenb
