Re: [Qemu-devel] [PATCH 1/2] pc: acpi: use correct RSDT length for checksum

[Michael S. Tsirkin]

On Thu, Nov 22, 2018 at 11:29:33AM +0100, Igor Mammedov wrote:
> AcpiRsdpDescriptor describes revision 2 RSDP table so using sizeof(*rsdp)
> for checksum calculation isn't correct since we are adding extra 16 bytes.
> But acpi_data_push() zeroes out table, so just by luck we are summing up
> exta zeros which still yelds correct checksum.
> 
> Fix it up by explicitly stating table size instead of using
> pointer arithmetics on stucture.
> 
> PS:
> Extra 16 bytes are still wasted, but droping them will break migration
> for machines older than 2.3 due to size mismatch, for 2.3 and older it's
> not an issue since they are using resizable memory regions (a1666142d)
> for ACPI blobs. So keep wasting memory to avoid breaking old machines.

I'd like this explanation in code comments please.

> Fixes: 72c194f7e (i386: ACPI table generation code from seabios)
> Signed-off-by: Igor Mammedov <address@hidden>
> ---
> there is no changes to the current RSDP content caused by this patch
> ---
>  hw/i386/acpi-build.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 236a20e..131c565 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2567,7 +2567,7 @@ build_rsdp(GArray *rsdp_table, BIOSLinker *linker, 
> unsigned rsdt_tbl_offset)
>  
>      /* Checksum to be filled by Guest linker */
>      bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE,
> -        (char *)rsdp - rsdp_table->data, sizeof *rsdp,
> +        (char *)rsdp - rsdp_table->data, 20 /* ACPI rev 1.0 RSDP size */,
>          (char *)&rsdp->checksum - rsdp_table->data);
>  
>      return rsdp_table;

I dislike hard-coded size math like this.
How about a sub-structure for RSDPv1?


> -- 
> 2.7.4