On 8/1/24 13:46, Mark Cave-Ayland wrote:
On 08/01/2024 12:01, Philippe Mathieu-Daudé wrote:
On 7/1/24 22:25, Mark Cave-Ayland wrote:
Declaration ROM binary images can be any arbitrary size, however if a
host ROM
memory region is not aligned to qemu_target_page_size() then we fail the
"assert(!(iotlb & ~TARGET_PAGE_MASK))" check in tlb_set_page_full().
IIUC this isn't specific to NuBus but to any ROM used to execute code
in place.
Shouldn't this be handled in memory_region_init_rom()?
There were some previous discussion in the threads here:
https://lore.kernel.org/all/b68ab7d3-d3d3-9f81-569d-454ae9c11...@linaro.org/T/
https://patchew.org/QEMU/20231208020619.117-1-zhiwei._5f...@linux.alibaba.com/
My impression from Richard's last reply in the second thread is that
this should be fixed in nubus-device instead?
Hmm OK. And you need the offset to call load_image_size() at the
proper offset anyway.
Can you add that <...
The Nubus declaration ROMs
are different in that they are aligned to the end of the memory region
rather than the beginning, which is probably quite an unusual use-case.
...> info to the description?
Ensure that the host ROM memory region is aligned to
qemu_target_page_size()
and adjust the offset at which the Declaration ROM image is loaded so
that the
image is still aligned to the end of the Nubus slot.
>> should it be that the ROM memory region must be aligned to
TARGET_PAGE_MASK?
This seems to make sense to me, but I'm out of my comfort zone.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk>
---
hw/nubus/nubus-device.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
index 49008e4938..e4f824d58b 100644
--- a/hw/nubus/nubus-device.c
+++ b/hw/nubus/nubus-device.c
@@ -10,6 +10,7 @@
#include "qemu/osdep.h"
#include "qemu/datadir.h"
+#include "exec/target_page.h"
#include "hw/irq.h"
#include "hw/loader.h"
#include "hw/nubus/nubus.h"
@@ -30,7 +31,7 @@ static void nubus_device_realize(DeviceState *dev,
Error **errp)
NubusDevice *nd = NUBUS_DEVICE(dev);
char *name, *path;
hwaddr slot_offset;
- int64_t size;
+ int64_t size, align_size;
int ret;
/* Super */
@@ -76,16 +77,23 @@ static void nubus_device_realize(DeviceState
*dev, Error **errp)
}
name = g_strdup_printf("nubus-slot-%x-declaration-rom",
nd->slot);
- memory_region_init_rom(&nd->decl_rom, OBJECT(dev), name, size,
+
+ /*
+ * Ensure ROM memory region is aligned to target page size
regardless
+ * of the size of the Declaration ROM image
+ */
+ align_size = ROUND_UP(size, qemu_target_page_size());
+ memory_region_init_rom(&nd->decl_rom, OBJECT(dev), name,
align_size,
&error_abort);
- ret = load_image_mr(path, &nd->decl_rom);
+ ret = load_image_size(path,
memory_region_get_ram_ptr(&nd->decl_rom) +
+ (uintptr_t)align_size - size,
size);
g_free(path);
g_free(name);
if (ret < 0) {
error_setg(errp, "could not load romfile \"%s\"",
nd->romfile);
return;
}
- memory_region_add_subregion(&nd->slot_mem, NUBUS_SLOT_SIZE -
size,
+ memory_region_add_subregion(&nd->slot_mem, NUBUS_SLOT_SIZE -
align_size,
&nd->decl_rom);
}
}
ATB,
Mark.
