| Message ID | 1242443800-22686-1-git-send-email-eak@us.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. Why not set madt_end = int_override and calculate madt_size = madt_end - madt?
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? > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; #ifdef BX_QEMU @@ -1786,7 +1791,6 @@ void acpi_bios_init(void) continue; } int_override++; - madt_size += sizeof(struct madt_int_override); } acpi_build_table_header((struct acpi_table_header *)madt, "APIC", madt_size, 1);
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> -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html