Il 28/07/2014 13:45, Michael S. Tsirkin ha scritto:
>> +/* These are used to size the ACPI tables for -M pc-i440fx-1.7 and
>> + * -M pc-i440fx-2.0.
>
> Let's just say 2.0 and earlier?
This would give the idea that 1.6 is broken, but it isn't.
>> Even if the actual amount of AML generated grows
>> + * a little bit, there should be plenty of free space since the DSDT
>> + * shrunk by ~1.5k between QEMU 2.0 and QEMU 2.1.
>> + */
>> +#define ACPI_BUILD_CPU_AML_SIZE 97
>> +#define ACPI_BUILD_BRIDGE_AML_SIZE 1875
>
> Let's put _LEGACY_ somewhere here?
Ok.
>> +
>> +#define ACPI_BUILD_TABLE_SIZE 0x10000
>> +
>> typedef struct AcpiCpuInfo {
>> DECLARE_BITMAP(found_cpus, ACPI_CPU_HOTPLUG_ID_LIMIT);
>> } AcpiCpuInfo;
>> @@ -87,6 +99,8 @@ typedef struct AcpiBuildPciBusHotplugState {
>> struct AcpiBuildPciBusHotplugState *parent;
>> } AcpiBuildPciBusHotplugState;
>>
>> +unsigned bsel_alloc;
>> +
>
> Patch will be better contained if instead of using a global
> bsel_alloc, we actually go and count the devices that
> have ACPI_PCIHP_PROP_BSEL.
> You can just scan all devices, or all pci devices, it
> should not matter.
> This way, this code will be local to the legacy path.
Ok.
>
>> static void acpi_get_dsdt(AcpiMiscInfo *info)
>> {
>> uint16_t *applesmc_sta;
>> @@ -759,8 +773,8 @@ static void *acpi_set_bsel(PCIBus *bus, void *opaque)
>> static void acpi_set_pci_info(void)
>> {
>> PCIBus *bus = find_i440fx(); /* TODO: Q35 support */
>> - unsigned bsel_alloc = 0;
>>
>> + assert(bsel_alloc == 0);
>> if (bus) {
>> /* Scan all PCI buses. Set property to enable acpi based hotplug. */
>> pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, &bsel_alloc);
>> @@ -1440,13 +1454,14 @@ static
>> void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>> {
>> GArray *table_offsets;
>> - unsigned facs, dsdt, rsdt;
>> + unsigned facs, ssdt, dsdt, rsdt;
>> AcpiCpuInfo cpu;
>> AcpiPmInfo pm;
>> AcpiMiscInfo misc;
>> AcpiMcfgInfo mcfg;
>> PcPciInfo pci;
>> uint8_t *u;
>> + size_t aml_len = 0;
>>
>> acpi_get_cpu_info(&cpu);
>> acpi_get_pm_info(&pm);
>> @@ -1474,13 +1489,20 @@ void acpi_build(PcGuestInfo *guest_info,
>> AcpiBuildTables *tables)
>> dsdt = tables->table_data->len;
>> build_dsdt(tables->table_data, tables->linker, &misc);
>>
>> + /* Count the size of the DSDT and SSDT, we will need it for legacy
>> + * sizing of ACPI tables.
>> + */
>> + aml_len += tables->table_data->len - dsdt;
>> +
>> /* ACPI tables pointed to by RSDT */
>> acpi_add_table(table_offsets, tables->table_data);
>> build_fadt(tables->table_data, tables->linker, &pm, facs, dsdt);
>>
>> + ssdt = tables->table_data->len;
>> acpi_add_table(table_offsets, tables->table_data);
>> build_ssdt(tables->table_data, tables->linker, &cpu, &pm, &misc, &pci,
>> guest_info);
>> + aml_len += tables->table_data->len - ssdt;
>>
>> acpi_add_table(table_offsets, tables->table_data);
>> build_madt(tables->table_data, tables->linker, &cpu, guest_info);
>> @@ -1513,12 +1535,53 @@ void acpi_build(PcGuestInfo *guest_info,
>> AcpiBuildTables *tables)
>> /* RSDP is in FSEG memory, so allocate it separately */
>> build_rsdp(tables->rsdp, tables->linker, rsdt);
>>
>> - /* We'll expose it all to Guest so align size to reduce
>> + /* We'll expose it all to Guest so we want to reduce
>> * chance of size changes.
>> * RSDP is small so it's easy to keep it immutable, no need to
>> * bother with alignment.
>> + *
>> + * We used to align the tables to 4k, but of course this would
>> + * too simple to be enough. 4k turned out to be too small an
>> + * alignment very soon, and in fact it is almost impossible to
>> + * keep the table size stable for all (max_cpus, max_memory_slots)
>> + * combinations. So the table size is always 64k for pc-i440fx-2.1
>> + * and we give an error if the table grows beyond that limit.
>> + *
>> + * We still have the problem of migrating from "-M pc-i440fx-2.0". For
>> + * that, we exploit the fact that QEMU 2.1 generates _smaller_ tables
>> + * than 2.0 and we can always pad the smaller tables with zeros. We can
>> + * then use the exact size of the 2.0 tables.
>> + *
>> + * All this is for PIIX4, since QEMU 2.0 didn't support Q35 migration.
>> */
>> - acpi_align_size(tables->table_data, 0x1000);
>> + if (guest_info->legacy_acpi_table_size) {
>> + /* Subtracting aml_len gives the size of fixed tables. Then add the
>> + * size of the PIIX4 DSDT/SSDT in QEMU 2.0.
>> + */
>> + int legacy_aml_len =
>> + guest_info->legacy_acpi_table_size +
>> + ACPI_BUILD_CPU_AML_SIZE * max_cpus +
>> + ACPI_BUILD_BRIDGE_AML_SIZE * (MAX(bsel_alloc, 1) - 1);
>> + int legacy_table_size =
>> + ROUND_UP(tables->table_data->len - aml_len + legacy_aml_len,
>> 0x1000);
>> + if (tables->table_data->len > legacy_table_size) {
>> + /* -M pc-i440fx-2.0 doesn't support memory hotplug, so this
>> should
>> + * never happen.
>> + */
>> + error_report("This configuration is not supported with -M
>> pc-i440fx-2.0.");
>> + error_report("Please report this to qemu-devel@nongnu.org.");
>
> Downstreams would need to patch this line out? Let's just drop the
> second line.
This should never happen really, so...
>> + exit(1);
>
>
> so what happens here is 2.x -> 2.0 migration becomes broken, but
> 2.0 -> 2.x could still work.
Only with Igor's patches.
> I think I would prefer a warning instead.
>
>> + }
>> + g_array_set_size(tables->table_data, legacy_table_size);
>> + } else {
>> + if (tables->table_data->len > ACPI_BUILD_TABLE_SIZE) {
>> + /* As of QEMU 2.1, this fires with 160 VCPUs and 255 memory
>> slots. */
>> + error_report("ACPI tables are larger than 64k. Please remove");
>> + error_report("CPUs, NUMA nodes, memory slots or PCI bridges.");
>> + exit(1);
>> + }
>> + g_array_set_size(tables->table_data, ACPI_BUILD_TABLE_SIZE);
>
>
> Let's split the idea to use 64K always out, to a separate patch, ok?
Ok.
Paolo
