On Wed, 18 Jun 2014 12:19:10 +0300
"Michael S. Tsirkin" <m...@redhat.com> wrote:
> On Wed, Jun 18, 2014 at 12:04:31PM +0300, Marcel Apfelbaum wrote:
> > On Tue, 2014-06-17 at 23:19 +0300, Michael S. Tsirkin wrote:
> > > On Tue, Jun 17, 2014 at 10:14:01PM +0200, Igor Mammedov wrote:
> > > > linker and RSDP tables are build only once, so if later
> > > > during rebuild sizes of ACPI tables change pointers will
> > > > be patched incorrectly due to wrong offsets.
> > > >
> > > > To fix it rebuild linker and RSDP tables along with
> > > > the rest of ACPI tables so that they would have correct
> > > > offsets.
> > > >
> > > > Signed-off-by: Igor Mammedov <imamm...@redhat.com>
> > >
> > > If you are adding a new blob you are breaking cross-version migration.
> > > Need to handle that depending on pc version as appropriate.
> > Besides the above comment,
> > Tested-by: Marcel Apfelbaum <marce...@redhat.com>
> > Reviewed-by: Marcel Apfelbaum <marce...@redhat.com>
> >
> > Thanks,
> > Marcel
>
> This will need a bit more motivation to go in than is
> provided here.
I'll address comment above and also will dig up example
why it was needed in a month old dynamic ACPI tables series.
>
> > >
> > > > ---
> > > > hw/i386/acpi-build.c | 21 ++++++++++++++-------
> > > > 1 file changed, 14 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > > index fab25ad..8301bb4 100644
> > > > --- a/hw/i386/acpi-build.c
> > > > +++ b/hw/i386/acpi-build.c
> > > > @@ -1862,6 +1862,10 @@ struct AcpiBuildState {
> > > > /* Copy of table in RAM (for patching). */
> > > > uint8_t *table_ram;
> > > > uint32_t table_size;
> > > > + uint8_t *linker_ram;
> > > > + uint32_t linker_size;
> > > > + uint8_t *rsdp_ram;
> > > > + uint32_t rsdp_size;
> > > > /* Is table patched? */
> > > > uint8_t patched;
> > > > PcGuestInfo *guest_info;
> > > > @@ -1998,6 +2002,10 @@ static void acpi_build_update(void
> > > > *build_opaque, uint32_t offset)
> > > > assert(acpi_data_len(tables.table_data) ==
> > > > build_state->table_size);
> > > > memcpy(build_state->table_ram, tables.table_data->data,
> > > > build_state->table_size);
> > > > + memcpy(build_state->linker_ram, tables.linker->data,
> > > > + build_state->linker_size);
> > > > + memcpy(build_state->rsdp_ram, tables.rsdp->data,
> > > > + build_state->rsdp_size);
> > > >
> > > > acpi_build_tables_cleanup(&tables, true);
> > > > }
> > > > @@ -2060,14 +2068,13 @@ void acpi_setup(PcGuestInfo *guest_info)
> > > > ACPI_BUILD_TABLE_FILE);
> > > > build_state->table_size = acpi_data_len(tables.table_data);
> > > >
> > > > - acpi_add_rom_blob(NULL, tables.linker, "etc/table-loader");
> > > > + build_state->linker_ram = acpi_add_rom_blob(build_state,
> > > > tables.linker,
> > > > + "etc/table-loader");
> > > > + build_state->linker_size = acpi_data_len(tables.linker);
> > > >
> > > > - /*
> > > > - * RSDP is small so it's easy to keep it immutable, no need to
> > > > - * bother with ROM blobs.
> > > > - */
> > > > - fw_cfg_add_file(guest_info->fw_cfg, ACPI_BUILD_RSDP_FILE,
> > > > - tables.rsdp->data, acpi_data_len(tables.rsdp));
> > > > + build_state->rsdp_ram = acpi_add_rom_blob(build_state, tables.rsdp,
> > > > + ACPI_BUILD_RSDP_FILE);
> > > > + build_state->rsdp_size = acpi_data_len(tables.rsdp);
> > > >
> > > > qemu_register_reset(acpi_build_reset, build_state);
> > > > acpi_build_reset(build_state);
> > > > --
> > > > 1.9.3
> >
> >
>
