[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/5] hw/arm: Align ACPI blob len to PAGE size
From: |
Igor Mammedov |
Subject: |
Re: [PATCH 1/5] hw/arm: Align ACPI blob len to PAGE size |
Date: |
Fri, 8 Nov 2019 17:17:45 +0100 |
On Fri, 4 Oct 2019 16:52:58 +0100
Shameer Kolothum <address@hidden> wrote:
> If ACPI blob length modifications happens after the initial
> virt_acpi_build() call, and the changed blob length is within
> the PAGE size boundary, then the revised size is not seen by
> the firmware on Guest reboot. The is because in the
> virt_acpi_build_update() -> acpi_ram_update() -> qemu_ram_resize()
> path, qemu_ram_resize() uses ram_block size which is aligned
> to PAGE size and the "resize callback" to update the size seen
> by firmware is not getting invoked. Hence align ACPI blob sizes
> to PAGE boundary.
>
> Signed-off-by: Shameer Kolothum <address@hidden>
> ---
> More details on this issue can be found here,
> https://patchwork.kernel.org/patch/11154757/
re-read it again and it seems to me that this patch is workaround
rather than a solution to the problem.
CCing Michael as an author this code.
on x86 we have crazy history of manually aligning acpi blobs, see code under
comment
/* We'll expose it all to Guest so we want to reduce
so used_length endups with over-sized value which includes table and padding
and it happens that ACPI_BUILD_TABLE_SIZE is much bigger than host page size
so if on reboot we happen to exceed ACPI_BUILD_TABLE_SIZE, the next padded table
size (used_length) would be 2 x ACPI_BUILD_TABLE_SIZE which doesn't trigger
block->used_length == HOST_PAGE_ALIGN(newsize)
condition so fwcfg gets updated value.
> ---
> hw/arm/virt-acpi-build.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 4cd50175e0..074e0c858e 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -790,6 +790,7 @@ void virt_acpi_build(VirtMachineState *vms,
> AcpiBuildTables *tables)
> GArray *table_offsets;
> unsigned dsdt, xsdt;
> GArray *tables_blob = tables->table_data;
> + GArray *cmd_blob = tables->linker->cmd_blob;
> MachineState *ms = MACHINE(vms);
>
> table_offsets = g_array_new(false, true /* clear */,
> @@ -854,6 +855,19 @@ void virt_acpi_build(VirtMachineState *vms,
> AcpiBuildTables *tables)
> build_rsdp(tables->rsdp, tables->linker, &rsdp_data);
> }
>
> + /*
> + * Align the ACPI blob lengths to PAGE size so that on ACPI table
> + * regeneration, the length that firmware sees really gets updated
> + * through 'resize' callback in qemu_ram_resize() in the
> + * virt_acpi_build_update() -> acpi_ram_update() -> qemu_ram_resize()
> + * path.
> + */
> + g_array_set_size(tables_blob,
> + TARGET_PAGE_ALIGN(acpi_data_len(tables_blob)));
here it would depend on TARGET_PAGE_ALIGN vs HOST_PAGE_ALIGN relation
so depending on host it could flip it's behavior to opposite.
one thing we could do is dropping (block->used_length == newsize) condition
another is to use value of block->used_length for s->files->f[index].size.
Michael,
what's your take in this?
> + g_array_set_size(tables->rsdp,
> + TARGET_PAGE_ALIGN(acpi_data_len(tables->rsdp)));
> + g_array_set_size(cmd_blob,
> + TARGET_PAGE_ALIGN(acpi_data_len(cmd_blob)));
> /* Cleanup memory that's no longer used. */
> g_array_free(table_offsets, true);
> }
- Re: [PATCH 1/5] hw/arm: Align ACPI blob len to PAGE size,
Igor Mammedov <=