On Thu, 27 Feb 2025, Nicholas Piggin wrote:
On Thu Feb 27, 2025 at 11:48 AM AEST, BALATON Zoltan wrote:
On Thu, 27 Feb 2025, Nicholas Piggin wrote:
On Sun Feb 23, 2025 at 3:52 AM AEST, BALATON Zoltan wrote:
The board has a battery backed NVRAM where U-Boot environment is
stored which is also accessed by AmigaOS and e.g. C:NVGetVar command
crashes without it having at least a valid checksum.
Signed-off-by: BALATON Zoltan <bala...@eik.bme.hu>
---
hw/ppc/amigaone.c | 116 ++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 113 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/amigaone.c b/hw/ppc/amigaone.c
index 4290d58613..5273543460 100644
--- a/hw/ppc/amigaone.c
+++ b/hw/ppc/amigaone.c
@@ -21,10 +21,13 @@
#include "hw/ide/pci.h"
#include "hw/i2c/smbus_eeprom.h"
#include "hw/ppc/ppc.h"
+#include "system/block-backend.h"
#include "system/qtest.h"
#include "system/reset.h"
#include "kvm_ppc.h"
+#include <zlib.h> /* for crc32 */
+
#define BUS_FREQ_HZ 100000000
/*
@@ -46,6 +49,103 @@ static const char dummy_fw[] = {
0x4e, 0x80, 0x00, 0x20, /* blr */
};
+#define NVRAM_ADDR 0xfd0e0000
+#define NVRAM_SIZE (4 * KiB)
+
+#define TYPE_A1_NVRAM "a1-nvram"
+OBJECT_DECLARE_SIMPLE_TYPE(A1NVRAMState, A1_NVRAM)
+
+struct A1NVRAMState {
+ SysBusDevice parent_obj;
+
+ MemoryRegion mr;
+ BlockBackend *blk;
+};
+
+/* read callback not used because of romd mode, only here just in case */
Better make it g_assert_not_reached() then.
There is a memory_region_rom_device_set_romd() function. It's not called
here so a read function should not be needed but it's also trivial and
would work if romd mode is turned off for some reason in the future so
adding it seems safer to me. The comment is just to note it's a romd
region so reads normally don't go through this function unless romd mode
is turned off.
It's trivial to add back if you do turn it off. Adding dead code is no
good. The memory API default won't get changed underneath you without
proper code audit or deprecating the API so that's no problem.
OK, done in v2.
Better make these addresses #defines at the top of the file with
the NVRAM_ADDR?
I don't have defines for these as these are single use constants to set up
memory map and with defines it's less obvious and has to be looked up
where these are while this way I can see it directly without having to
scroll up so I prefer this. I've added defines where the constant is used
more than once where it makes sense to keep consistency.
But now you have to lok in two different places anyway because you have
the NVRAM_ADDR etc defines at the top of the file. Seems like a good
time to move all defines there so you can easily see the memory map in
one place.
I don't think this makes it more readable at all but I've added a patch in
v2 to add defines for these.
Regards,
BALATON Zoltan
