Re: [PATCH 1/2] Clean up MADT Table Creation

[Beth Kon <eak@us.ibm.com>]

Avi Kivity wrote:
> Beth Kon wrote:
>> This patch is based on the recent patch from Vincent Minet. I split 
>> Vincent's
>> changes into 2 patches (to separate MADT and RSDT table cleanup, as 
>> suggested by
>> Marcelo) and added a bit to them. And to give credit where it is due, 
>> this
>> cleanup is also related to the patch Marcelo provided when the HPET 
>> addition tripped over the same problem. (Thanks again Marcelo :-)
>> This patch moves all the table layout calculations to the same area of
>> acpi_bios_init. This prevents corruption problems when, in the middle of
>> filling in the tables, the MADT table size grows. The idea is to do 
>> all the layout in one section, then fill things in afterwards. It 
>> also corrects a problem where the madt table was memset to 0 before 
>> the final size of the table had been determined.
>>
>> Signed-off-by: Beth Kon <eak@us.ibm.com>
>>
>> diff --git a/kvm/bios/rombios32.c b/kvm/bios/rombios32.c
>> index cbd5f15..7f62e4f 100755
>> --- a/kvm/bios/rombios32.c
>> +++ b/kvm/bios/rombios32.c
>> @@ -1665,6 +1665,7 @@ void acpi_bios_init(void)
>>  
>>      addr = (addr + 7) & ~7;
>>      madt_addr = addr;
>> +    madt = (void *)(addr);
>>      madt_size = sizeof(*madt) +
>>          sizeof(struct madt_processor_apic) * MAX_CPUS +
>>  #ifdef BX_QEMU
>> @@ -1672,7 +1673,11 @@ void acpi_bios_init(void)
>>  #else
>>          sizeof(struct madt_io_apic);
>>  #endif
>> -    madt = (void *)(addr);
>> +    for ( i = 0; i < 16; i++ ) {
>> +        if ( PCI_ISA_IRQ_MASK & (1U << i) ) {
>> +            madt_size += sizeof(struct madt_int_override);
>> +        }
>> +    }
>>      addr += madt_size;
>>  
>>   
>
> You're just duplicating the override creation loop (with its internal 
> if); if we update it, we'll have to update this too.
Yep, that's a valid complaint. I'll resubmit shortly.
>
> Why not set madt_end = int_override and calculate madt_size = madt_end 
> - madt?
>