[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 6/6] acpi: pci: use build_append_foo() API to
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [Qemu-devel] [PATCH v4 6/6] acpi: pci: use build_append_foo() API to construct MCFG |
Date: |
Wed, 15 May 2019 07:29:17 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 |
On 5/15/19 3:10 AM, Michael S. Tsirkin wrote:
> On Fri, Apr 19, 2019 at 08:30:53AM +0800, Wei Yang wrote:
>> build_append_foo() API doesn't need explicit endianness conversions
>> which eliminates a source of errors and it makes build_mcfg() look like
>> declarative definition of MCFG table in ACPI spec, which makes it easy
>> to review.
>>
>> Signed-off-by: Wei Yang <address@hidden>
>> Suggested-by: Igor Mammedov <address@hidden>
>> Reviewed-by: Igor Mammedov <address@hidden>
>> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
>
> Causes a regression with an invalid MCFG produced.
> Dropped.
>
>> ---
>> hw/acpi/pci.c | 33 +++++++++++++++++++++------------
>> include/hw/acpi/acpi-defs.h | 18 ------------------
>> 2 files changed, 21 insertions(+), 30 deletions(-)
>>
>> diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
>> index fa0fa30bb9..341805e786 100644
>> --- a/hw/acpi/pci.c
>> +++ b/hw/acpi/pci.c
>> @@ -30,17 +30,26 @@
>>
>> void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
>> {
>> - AcpiTableMcfg *mcfg;
>> - int len = sizeof(*mcfg) + sizeof(mcfg->allocation[0]);
>> -
>> - mcfg = acpi_data_push(table_data, len);
>> - mcfg->allocation[0].address = cpu_to_le64(info->base);
>> -
>> - /* Only a single allocation so no need to play with segments */
>> - mcfg->allocation[0].pci_segment = cpu_to_le16(0);
>> - mcfg->allocation[0].start_bus_number = 0;
>> - mcfg->allocation[0].end_bus_number = PCIE_MMCFG_BUS(info->size - 1);
>> -
>> - build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1, NULL,
>> NULL);
>> + int mcfg_start = table_data->len;
>> +
>> + acpi_data_push(table_data, sizeof(AcpiTableHeader));
Ah, it seems we missed AcpiTableMcfg.reserved[8]...:
build_append_int_noprefix(table_data, 0, 8);
>> +
>> + /*
>> + * PCI Firmware Specification, Revision 3.0
>> + * 4.1.2 MCFG Table Description.
>> + */
>> + /* Base address, processor-relative */
>> + build_append_int_noprefix(table_data, info->base, 8);
>> + /* PCI segment group number */
>> + build_append_int_noprefix(table_data, 0, 2);
>> + /* Starting PCI Bus number */
>> + build_append_int_noprefix(table_data, 0, 1);
>> + /* Final PCI Bus number */
>> + build_append_int_noprefix(table_data, PCIE_MMCFG_BUS(info->size - 1),
>> 1);
>> + /* Reserved */
>> + build_append_int_noprefix(table_data, 0, 4);
>> +
>> + build_header(linker, table_data, (void *)(table_data->data +
>> mcfg_start),
>> + "MCFG", table_data->len - mcfg_start, 1, NULL, NULL);
>> }
>>
>> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
>> index f9aa4bd398..57a3f58b0c 100644
>> --- a/include/hw/acpi/acpi-defs.h
>> +++ b/include/hw/acpi/acpi-defs.h
>> @@ -449,24 +449,6 @@ struct AcpiSratProcessorGiccAffinity {
>>
>> typedef struct AcpiSratProcessorGiccAffinity AcpiSratProcessorGiccAffinity;
>>
>> -/* PCI fw r3.0 MCFG table. */
>> -/* Subtable */
>> -struct AcpiMcfgAllocation {
>> - uint64_t address; /* Base address, processor-relative */
>> - uint16_t pci_segment; /* PCI segment group number */
>> - uint8_t start_bus_number; /* Starting PCI Bus number */
>> - uint8_t end_bus_number; /* Final PCI Bus number */
>> - uint32_t reserved;
>> -} QEMU_PACKED;
>> -typedef struct AcpiMcfgAllocation AcpiMcfgAllocation;
>> -
>> -struct AcpiTableMcfg {
>> - ACPI_TABLE_HEADER_DEF;
>> - uint8_t reserved[8];
--------^
Thanks Michael for testing...
Wei, can you add a MCFG test in tests/bios-tables-test.c?
Regards,
Phil.
>> - AcpiMcfgAllocation allocation[0];
>> -} QEMU_PACKED;
>> -typedef struct AcpiTableMcfg AcpiTableMcfg;
>> -
>> /*
>> * TCPA Description Table
>> *
>> --
>> 2.19.1
Re: [Qemu-devel] [PATCH v4 6/6] acpi: pci: use build_append_foo() API to construct MCFG, Wei Yang, 2019/05/15