* [Qemu-devel] [PATCH] pc: acpi-build: make linker & RSDP tables dynamic
@ 2014-06-17 20:14 Igor Mammedov
2014-06-17 20:19 ` Michael S. Tsirkin
0 siblings, 1 reply; 5+ messages in thread
From: Igor Mammedov @ 2014-06-17 20:14 UTC (permalink / raw)
To: qemu-devel; +Cc: marcel.a, mst
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 <imammedo@redhat.com>
---
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH] pc: acpi-build: make linker & RSDP tables dynamic
2014-06-17 20:14 [Qemu-devel] [PATCH] pc: acpi-build: make linker & RSDP tables dynamic Igor Mammedov
@ 2014-06-17 20:19 ` Michael S. Tsirkin
2014-06-18 9:04 ` Marcel Apfelbaum
0 siblings, 1 reply; 5+ messages in thread
From: Michael S. Tsirkin @ 2014-06-17 20:19 UTC (permalink / raw)
To: Igor Mammedov; +Cc: qemu-devel, marcel.a
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 <imammedo@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.
> ---
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH] pc: acpi-build: make linker & RSDP tables dynamic
2014-06-17 20:19 ` Michael S. Tsirkin
@ 2014-06-18 9:04 ` Marcel Apfelbaum
2014-06-18 9:19 ` Michael S. Tsirkin
0 siblings, 1 reply; 5+ messages in thread
From: Marcel Apfelbaum @ 2014-06-18 9:04 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Igor Mammedov, qemu-devel
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 <imammedo@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 <marcel.a@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel.a@redhat.com>
Thanks,
Marcel
>
> > ---
> > 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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH] pc: acpi-build: make linker & RSDP tables dynamic
2014-06-18 9:04 ` Marcel Apfelbaum
@ 2014-06-18 9:19 ` Michael S. Tsirkin
2014-06-18 10:00 ` Igor Mammedov
0 siblings, 1 reply; 5+ messages in thread
From: Michael S. Tsirkin @ 2014-06-18 9:19 UTC (permalink / raw)
To: Marcel Apfelbaum; +Cc: Igor Mammedov, qemu-devel
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 <imammedo@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 <marcel.a@redhat.com>
> Reviewed-by: Marcel Apfelbaum <marcel.a@redhat.com>
>
> Thanks,
> Marcel
This will need a bit more motivation to go in than is
provided here.
> >
> > > ---
> > > 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
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH] pc: acpi-build: make linker & RSDP tables dynamic
2014-06-18 9:19 ` Michael S. Tsirkin
@ 2014-06-18 10:00 ` Igor Mammedov
0 siblings, 0 replies; 5+ messages in thread
From: Igor Mammedov @ 2014-06-18 10:00 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel, Marcel Apfelbaum
On Wed, 18 Jun 2014 12:19:10 +0300
"Michael S. Tsirkin" <mst@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 <imammedo@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 <marcel.a@redhat.com>
> > Reviewed-by: Marcel Apfelbaum <marcel.a@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
> >
> >
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-06-18 10:01 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-17 20:14 [Qemu-devel] [PATCH] pc: acpi-build: make linker & RSDP tables dynamic Igor Mammedov
2014-06-17 20:19 ` Michael S. Tsirkin
2014-06-18 9:04 ` Marcel Apfelbaum
2014-06-18 9:19 ` Michael S. Tsirkin
2014-06-18 10:00 ` Igor Mammedov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).