Re: [Qemu-devel] [PATCH for-2.6] acpi: fix bios linker loadder COMMAND_ALLOCATE on bigendian host

[Marcel Apfelbaum <marcel.apfelbaum@gmail.com>]

On 04/29/2016 03:44 PM, Igor Mammedov wrote:
> 'make check' fails with:
>
> ERROR:tests/bios-tables-test.c:493:load_expected_aml:
>     assertion failed: (g_file_test(aml_file, G_FILE_TEST_EXISTS))
>
> since commit:
> caf50c7166a6ed96c462ab5db4b495e1234e4cc6
> tests: pc: acpi: drop not needed 'expected SSDT' blobs
>
> Assert happens because qemu-system-x86_64 generates
> SSDT table and test looks for a corresponding expected
> table to compare with.
>
> However there is no expected SSDT blob anymore, since
> QEMU souldn't generate one. As it happens BIOS is not
> able to read ACPI tables from QEMU and fallbacks to
> embeded legacy ACPI codepath, which generates SSDT.
> That happens due to wrongly sized endiannes conversion
> which makes
>   uint8_t BiosLinkerLoaderEntry.alloc.zone
> end up with 0 due to truncation of 32 bit integer
> which on host is 1 or 2.
>
> Fix it by dropping invalid cpu_to_le32() as uint8_t
> doesn't require any conversion.
>
> RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1330174
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>   hw/acpi/bios-linker-loader.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/hw/acpi/bios-linker-loader.c b/hw/acpi/bios-linker-loader.c
> index ace9abb..5153ab1 100644
> --- a/hw/acpi/bios-linker-loader.c
> +++ b/hw/acpi/bios-linker-loader.c
> @@ -135,9 +135,8 @@ void bios_linker_loader_alloc(GArray *linker,
>       strncpy(entry.alloc.file, file, sizeof entry.alloc.file - 1);
>       entry.command = cpu_to_le32(BIOS_LINKER_LOADER_COMMAND_ALLOCATE);
>       entry.alloc.align = cpu_to_le32(alloc_align);
> -    entry.alloc.zone = cpu_to_le32(alloc_fseg ?
> -                                    BIOS_LINKER_LOADER_ALLOC_ZONE_FSEG :
> -                                    BIOS_LINKER_LOADER_ALLOC_ZONE_HIGH);
> +    entry.alloc.zone = alloc_fseg ? BIOS_LINKER_LOADER_ALLOC_ZONE_FSEG :
> +                                    BIOS_LINKER_LOADER_ALLOC_ZONE_HIGH;
>
>       /* Alloc entries must come first, so prepend them */
>       g_array_prepend_vals(linker, &entry, sizeof entry);
>

Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>

Thanks,
Marcel