Re: [Qemu-devel] [PATCH] acpi-build: dirty memory on RSDP/linker update

[Michael S. Tsirkin]

On Mon, Feb 16, 2015 at 10:19:35AM +0100, Igor Mammedov wrote:
> On Sun, 15 Feb 2015 20:46:16 +0100
> "Michael S. Tsirkin" <address@hidden> wrote:
> 
> > This is required to make sure they are migrated
> > correctly.
> > 
> > Signed-off-by: Michael S. Tsirkin <address@hidden>
> > ---
> >  hw/i386/acpi-build.c | 28 +++++++++++++++++++++-------
> >  1 file changed, 21 insertions(+), 7 deletions(-)
> > 
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 1dfdf35..fd6ef78 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -1271,6 +1271,8 @@ struct AcpiBuildState {
> >      uint8_t patched;
> >      PcGuestInfo *guest_info;
> >      void *rsdp;
> > +    ram_addr_t rsdp_ram;
> > +    uint32_t rsdp_size;
> >      ram_addr_t linker_ram;
> >      uint32_t linker_size;
> >  } AcpiBuildState;
> > @@ -1478,13 +1480,22 @@ static void acpi_build_update(void *build_opaque, 
> > uint32_t offset)
> >  
> >      memcpy(qemu_get_ram_ptr(build_state->table_ram), 
> > tables.table_data->data,
> >             build_state->table_size);
> > -    memcpy(build_state->rsdp, tables.rsdp->data, 
> > acpi_data_len(tables.rsdp));
> > -    memcpy(qemu_get_ram_ptr(build_state->linker_ram), tables.linker->data,
> > -           build_state->linker_size);
> > -
> >      cpu_physical_memory_set_dirty_range_nocode(build_state->table_ram,
> >                                                 build_state->table_size);
> >  
> > +    if (build_state->rsdp) {
> > +        memcpy(build_state->rsdp, tables.rsdp->data, 
> > acpi_data_len(tables.rsdp));
> > +    } else {
> > +        memcpy(qemu_get_ram_ptr(build_state->rsdp_ram), 
> > tables.linker->data,
> > +               build_state->rsdp_size);
> > +        cpu_physical_memory_set_dirty_range_nocode(build_state->rsdp_ram,
> > +                                                   build_state->rsdp_size);
> > +    }
> > +    memcpy(qemu_get_ram_ptr(build_state->linker_ram), tables.linker->data,
> > +           build_state->linker_size);
> > +    cpu_physical_memory_set_dirty_range_nocode(build_state->linker_ram,
> > +                                               build_state->linker_size);
> > +
> >      acpi_build_tables_cleanup(&tables, true);
> >  }
> >  
> > @@ -1564,10 +1575,13 @@ void acpi_setup(PcGuestInfo *guest_info)
> >                                   acpi_build_update, build_state,
> >                                   tables.rsdp->data, 
> > acpi_data_len(tables.rsdp));
> >          build_state->rsdp = tables.rsdp->data;
> > +        build_state->rsdp_ram = (ram_addr_t)-1;
> > +        build_state->rsdp_size = 0;
> >      } else {
> > -        build_state->rsdp = qemu_get_ram_ptr(
> > -            acpi_add_rom_blob(build_state, tables.rsdp, 
> > ACPI_BUILD_RSDP_FILE, 0)
> > -        );
> > +        build_state->rsdp = NULL;
> > +        build_state->rsdp_ram = acpi_add_rom_blob(build_state, tables.rsdp,
> > +                                                  ACPI_BUILD_RSDP_FILE, 0);
> > +        build_state->rsdp_size = acpi_data_len(tables.rsdp);
> there is no point in carrying rsdp_size state since it's fixed size table so
> just using acpi_data_len(tables.rsdp) or sizeof(AcpiRsdpDescriptor) is 
> sufficient.

I guess I agree if we add assertions checking that size matches
sizeof(AcpiRsdpDescriptor).
I did testing with this one already, so I'll add a patch on top to drop
rsdp_size, ok?

> >      }
> >  
> >      qemu_register_reset(acpi_build_reset, build_state);