* Peter Maydell (peter.mayd...@linaro.org) wrote:
> On Wed, 17 Mar 2021 at 20:17, Dr. David Alan Gilbert
> <dgilb...@redhat.com> wrote:
> >
> > Hi Michael,
> > I noticed your AVR code defines:
> >
> > #define TARGET_PAGE_BITS 8
> >
> > and has an explanation of why.
> >
> > Note however that's not going to work with the current live
> > migration/snapshotting code, since you're a couple of bits smaller
> > than the smallest page size we had so far, and for many years
> > the RAM migration code has stolen the bottom few bits of the address
> > as a flag field, and has already used 0x100 up; see migration/ram.c
> > RAM_SAVE_FLAG_* - and it's actually tricky to change it, because if
> > you change it then it'll break migration compatibility with existing
> > qemu's.
>
> If you want to use low bits as flags for other stuff, you
> should have a compile time assert that you have the number
> of bits you expect, or otherwise force a compile error.
> Otherwise you'll end up with unpleasant surprises like this one...
Yes, I think that's been around for a long time.
> I think that for the cpu-all.h uses of low bits we would
> end up with a compile error for excessively small TARGET_PAGE_BITS
> because we define the bits like this:
> #define TLB_DISCARD_WRITE (1 << (TARGET_PAGE_BITS_MIN - 6))
> and I expect the compiler will complain if the RHS of the '<<'
> is a negative constant. But I don't know if that's deliberate
> or a happy accident :-)
Something like this, does fail for AVR:
>From 8a617836955ebba1a4932d238fce600be51b9182 Mon Sep 17 00:00:00 2001
From: "Dr. David Alan Gilbert" <dgilb...@redhat.com>
Date: Thu, 18 Mar 2021 10:17:27 +0000
Subject: [PATCH] migration: Check TARGET_PAGE_SIZE vs RAM flags
migration/ram.c steals the bottom few bits of address for flags;
check that we don't run into TARGET_PAGE_SIZE
Signed-off-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
---
migration/ram.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/migration/ram.c b/migration/ram.c
index 1ee7ff9c6d..f053d45f3c 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -81,6 +81,8 @@
/* 0x80 is reserved in migration.h start with 0x100 next */
#define RAM_SAVE_FLAG_COMPRESS_PAGE 0x100
+#define RAM_SAVE_FLAG__MAX RAM_SAVE_FLAG_COMPRESS_PAGE
+
static inline bool is_zero_range(uint8_t *p, uint64_t size)
{
return buffer_is_zero(p, size);
@@ -4101,5 +4103,10 @@ static SaveVMHandlers savevm_ram_handlers = {
void ram_mig_init(void)
{
qemu_mutex_init(&XBZRLE.lock);
+#ifndef TARGET_PAGE_BITS_VARY
+ QEMU_BUILD_BUG_ON(RAM_SAVE_FLAG__MAX >= TARGET_PAGE_SIZE);
+#else
+ QEMU_BUILD_BUG_ON(RAM_SAVE_FLAG__MAX >= (1 << TARGET_PAGE_BITS_MIN));
+#endif
register_savevm_live("ram", 0, 4, &savevm_ram_handlers, &ram_state);
}
--
2.30.2
> thanks
> -- PMM
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
