From mboxrd@z Thu Jan  1 00:00:00 1970
From: Avi Kivity <avi@redhat.com>
Subject: Re: [PATCH 2/2] Clean up RSDT Table Creation
Date: Sun, 17 May 2009 21:43:21 +0300
Message-ID: <4A105AC9.40709@redhat.com>
References: <1242443800-22686-1-git-send-email-eak@us.ibm.com> <1242443800-22686-2-git-send-email-eak@us.ibm.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Cc: kvm@vger.kernel.org, mtosatti@redhat.com,
	vincent@vincent-minet.net, gleb@redhat.com, anthony@codemonkey.ws
To: Beth Kon <eak@us.ibm.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx2.redhat.com ([66.187.237.31]:53467 "EHLO mx2.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1753259AbZEQSn1 (ORCPT <rfc822;kvm@vger.kernel.org>);
	Sun, 17 May 2009 14:43:27 -0400
In-Reply-To: <1242443800-22686-2-git-send-email-eak@us.ibm.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Beth Kon wrote:
> This patch is also based on the patch by Vincent Minet. It corrects the size
> calculation of the RSDT, and checks for overflow of MAX_RSDT_ENTRIES, 
> assuming that the external table entry count is contained within
> MAX_RSDT_ENTRIES.
>
> Signed-off-by: Beth Kon <eak@us.ibm.com>
>
> diff --git a/kvm/bios/rombios32.c b/kvm/bios/rombios32.c
> index 7f62e4f..ac8f9c5 100755
> --- a/kvm/bios/rombios32.c
> +++ b/kvm/bios/rombios32.c
> @@ -1626,7 +1626,7 @@ void acpi_bios_init(void)
>      addr = base_addr = ram_size - ACPI_DATA_SIZE;
>      rsdt_addr = addr;
>      rsdt = (void *)(addr);
> -    rsdt_size = sizeof(*rsdt) + external_tables * 4;
> +    rsdt_size = sizeof(*rsdt);
>      addr += rsdt_size;
>  
>      fadt_addr = addr;
> @@ -1873,16 +1873,6 @@ void acpi_bios_init(void)
>                               "HPET", sizeof(*hpet), 1);
>  #endif
>  
> -    acpi_additional_tables(); /* resets cfg to required entry */
> -    for(i = 0; i < external_tables; i++) {
> -        uint16_t len;
> -        if(acpi_load_table(i, addr, &len) < 0)
> -            BX_PANIC("Failed to load ACPI table from QEMU\n");
> -        rsdt->table_offset_entry[nb_rsdt_entries++] = cpu_to_le32(addr);
> -        addr += len;
> -        if(addr >= ram_size)
> -            BX_PANIC("ACPI table overflow\n");
> -    }
>  #endif
>  
>      /* RSDT */
> @@ -1895,6 +1885,19 @@ void acpi_bios_init(void)
>  //  rsdt->table_offset_entry[nb_rsdt_entries++] = cpu_to_le32(hpet_addr);
>      if (nb_numa_nodes > 0)
>          rsdt->table_offset_entry[nb_rsdt_entries++] = cpu_to_le32(srat_addr);
> +    acpi_additional_tables(); /* resets cfg to required entry */
> +    /* external_tables load must occur last to 
> +     * properly check for MAX_RSDT_ENTRIES overflow.
> +     */
> +    for(i = 0; i < external_tables; i++) {
> +        uint16_t len;
> +        if(acpi_load_table(i, addr, &len) < 0)
> +            BX_PANIC("Failed to load ACPI table from QEMU\n");
> +        rsdt->table_offset_entry[nb_rsdt_entries++] = cpu_to_le32(addr);
> +        addr += len;
> +        if((addr >= ram_size) || (nb_rsdt_entries > MAX_RSDT_ENTRIES))     
> +            BX_PANIC("ACPI table overflow\n");
> +    }
>  #endif
>      rsdt_size -= MAX_RSDT_ENTRIES * 4;
>      rsdt_size += nb_rsdt_entries * 4;
>   

Same comment - instead of calculating the size incrementally, set 
rsdt_end = &rsdt->table_offset_entry[nb_rsdt_entries] and calculate the 
size from that.

btw, why did you move the code?

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.