| Message ID | 20190313044253.31988-2-richardw.yang@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | Extract build_mcfg | expand |
On Wed, 13 Mar 2019 12:42:51 +0800 Wei Yang <richardw.yang@linux.intel.com> wrote: > To build MCFG, two information is necessary: > > * bus number > * base address > > Abstract these two information to AcpiMcfgInfo so that build_mcfg and > build_mcfg_q35 will have the same declaration. > > Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> > --- > hw/arm/virt-acpi-build.c | 28 ++++++++++++++++++++-------- > hw/i386/acpi-build.c | 5 ----- > include/hw/acpi/aml-build.h | 5 +++++ > 3 files changed, 25 insertions(+), 13 deletions(-) > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > index 57679a89bf..7713c2d809 100644 > --- a/hw/arm/virt-acpi-build.c > +++ b/hw/arm/virt-acpi-build.c > @@ -545,23 +545,32 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > "SRAT", table_data->len - srat_start, 3, NULL, NULL); > } > > -static void > -build_mcfg(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > +static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg) > { > - AcpiTableMcfg *mcfg; > + VirtMachineState *vms = VIRT_MACHINE(qdev_get_machine()); > const MemMapEntry *memmap = vms->memmap; > int ecam_id = VIRT_ECAM_ID(vms->highmem_ecam); > + > + mcfg->mcfg_base = memmap[ecam_id].base; > + mcfg->mcfg_size = memmap[ecam_id].size; > + > + return true; > +} > + > +static void > +build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info) > +{ > + AcpiTableMcfg *mcfg; > int len = sizeof(*mcfg) + sizeof(mcfg->allocation[0]); > int mcfg_start = table_data->len; > > mcfg = acpi_data_push(table_data, len); > - mcfg->allocation[0].address = cpu_to_le64(memmap[ecam_id].base); > + mcfg->allocation[0].address = cpu_to_le64(info->mcfg_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(memmap[ecam_id].size - 1); > + mcfg->allocation[0].end_bus_number = PCIE_MMCFG_BUS(info->mcfg_size - 1); > > build_header(linker, table_data, (void *)(table_data->data + mcfg_start), > "MCFG", table_data->len - mcfg_start, 1, NULL, NULL); > @@ -779,6 +788,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables) > VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms); > GArray *table_offsets; > unsigned dsdt, xsdt; > + AcpiMcfgInfo mcfg; > GArray *tables_blob = tables->table_data; > > table_offsets = g_array_new(false, true /* clear */, > @@ -802,8 +812,10 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables) > acpi_add_table(table_offsets, tables_blob); > build_gtdt(tables_blob, tables->linker, vms); > > - acpi_add_table(table_offsets, tables_blob); > - build_mcfg(tables_blob, tables->linker, vms); > + if ( acpi_get_mcfg(&mcfg)) { > + acpi_add_table(table_offsets, tables_blob); > + build_mcfg(tables_blob, tables->linker, &mcfg); > + } acpi_get_mcfg used only once and always true so one doesn't need condition I'd go for simpler: diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index d7e2e4885b..a28e04c249 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -803,8 +803,13 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables) build_gtdt(tables_blob, tables->linker, vms); acpi_add_table(table_offsets, tables_blob); - build_mcfg(tables_blob, tables->linker, vms); - + { + const AcpiTableMcfg mcfg = { + .mcfg_base = vms->memmap[VIRT_ECAM_ID(vms->highmem_ecam)].base; + .mcfg_size = vms->memmap[VIRT_ECAM_ID(vms->highmem_ecam)].size; + }; + build_mcfg(tables_blob, tables->linker, &mcfg); + } acpi_add_table(table_offsets, tables_blob); build_spcr(tables_blob, tables->linker, vms); and get rid of acpi_get_mcfg() helper function > acpi_add_table(table_offsets, tables_blob); > build_spcr(tables_blob, tables->linker, vms); > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 9ecc96dcc7..c5b1c3be99 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -86,11 +86,6 @@ > /* Default IOAPIC ID */ > #define ACPI_BUILD_IOAPIC_ID 0x0 > > -typedef struct AcpiMcfgInfo { > - uint64_t mcfg_base; > - uint32_t mcfg_size; > -} AcpiMcfgInfo; > - > typedef struct AcpiPmInfo { > bool s3_disabled; > bool s4_disabled; > diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h > index 1a563ad756..b63b85d67c 100644 > --- a/include/hw/acpi/aml-build.h > +++ b/include/hw/acpi/aml-build.h > @@ -223,6 +223,11 @@ struct AcpiBuildTables { > BIOSLinker *linker; > } AcpiBuildTables; > > +typedef struct AcpiMcfgInfo { > + uint64_t mcfg_base; > + uint32_t mcfg_size; mcfg_ prefix in fields probably isn't necessary. > +} AcpiMcfgInfo; > + > /** > * init_aml_allocator: > *
On Wed, Mar 13, 2019 at 01:00:22PM +0100, Igor Mammedov wrote: >On Wed, 13 Mar 2019 12:42:51 +0800 >Wei Yang <richardw.yang@linux.intel.com> wrote: > >> To build MCFG, two information is necessary: >> >> * bus number >> * base address >> >> Abstract these two information to AcpiMcfgInfo so that build_mcfg and >> build_mcfg_q35 will have the same declaration. >> >> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> >> --- >> hw/arm/virt-acpi-build.c | 28 ++++++++++++++++++++-------- >> hw/i386/acpi-build.c | 5 ----- >> include/hw/acpi/aml-build.h | 5 +++++ >> 3 files changed, 25 insertions(+), 13 deletions(-) >> >> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c >> index 57679a89bf..7713c2d809 100644 >> --- a/hw/arm/virt-acpi-build.c >> +++ b/hw/arm/virt-acpi-build.c >> @@ -545,23 +545,32 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) >> "SRAT", table_data->len - srat_start, 3, NULL, NULL); >> } >> >> -static void >> -build_mcfg(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) >> +static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg) >> { >> - AcpiTableMcfg *mcfg; >> + VirtMachineState *vms = VIRT_MACHINE(qdev_get_machine()); >> const MemMapEntry *memmap = vms->memmap; >> int ecam_id = VIRT_ECAM_ID(vms->highmem_ecam); >> + >> + mcfg->mcfg_base = memmap[ecam_id].base; >> + mcfg->mcfg_size = memmap[ecam_id].size; >> + >> + return true; >> +} >> + >> +static void >> +build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info) >> +{ >> + AcpiTableMcfg *mcfg; >> int len = sizeof(*mcfg) + sizeof(mcfg->allocation[0]); >> int mcfg_start = table_data->len; >> >> mcfg = acpi_data_push(table_data, len); >> - mcfg->allocation[0].address = cpu_to_le64(memmap[ecam_id].base); >> + mcfg->allocation[0].address = cpu_to_le64(info->mcfg_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(memmap[ecam_id].size - 1); >> + mcfg->allocation[0].end_bus_number = PCIE_MMCFG_BUS(info->mcfg_size - 1); >> >> build_header(linker, table_data, (void *)(table_data->data + mcfg_start), >> "MCFG", table_data->len - mcfg_start, 1, NULL, NULL); >> @@ -779,6 +788,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables) >> VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms); >> GArray *table_offsets; >> unsigned dsdt, xsdt; >> + AcpiMcfgInfo mcfg; >> GArray *tables_blob = tables->table_data; >> >> table_offsets = g_array_new(false, true /* clear */, >> @@ -802,8 +812,10 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables) >> acpi_add_table(table_offsets, tables_blob); >> build_gtdt(tables_blob, tables->linker, vms); >> >> - acpi_add_table(table_offsets, tables_blob); >> - build_mcfg(tables_blob, tables->linker, vms); >> + if ( acpi_get_mcfg(&mcfg)) { >> + acpi_add_table(table_offsets, tables_blob); >> + build_mcfg(tables_blob, tables->linker, &mcfg); >> + } > >acpi_get_mcfg used only once and always true so one doesn't need condition >I'd go for simpler: > >diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c >index d7e2e4885b..a28e04c249 100644 >--- a/hw/arm/virt-acpi-build.c >+++ b/hw/arm/virt-acpi-build.c >@@ -803,8 +803,13 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables) > build_gtdt(tables_blob, tables->linker, vms); > > acpi_add_table(table_offsets, tables_blob); >- build_mcfg(tables_blob, tables->linker, vms); >- >+ { >+ const AcpiTableMcfg mcfg = { >+ .mcfg_base = vms->memmap[VIRT_ECAM_ID(vms->highmem_ecam)].base; >+ .mcfg_size = vms->memmap[VIRT_ECAM_ID(vms->highmem_ecam)].size; >+ }; >+ build_mcfg(tables_blob, tables->linker, &mcfg); >+ } > acpi_add_table(table_offsets, tables_blob); > build_spcr(tables_blob, tables->linker, vms); > >and get rid of acpi_get_mcfg() helper function > That's reasonable. >> acpi_add_table(table_offsets, tables_blob); >> build_spcr(tables_blob, tables->linker, vms); >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c >> index 9ecc96dcc7..c5b1c3be99 100644 >> --- a/hw/i386/acpi-build.c >> +++ b/hw/i386/acpi-build.c >> @@ -86,11 +86,6 @@ >> /* Default IOAPIC ID */ >> #define ACPI_BUILD_IOAPIC_ID 0x0 >> >> -typedef struct AcpiMcfgInfo { >> - uint64_t mcfg_base; >> - uint32_t mcfg_size; >> -} AcpiMcfgInfo; >> - >> typedef struct AcpiPmInfo { >> bool s3_disabled; >> bool s4_disabled; >> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h >> index 1a563ad756..b63b85d67c 100644 >> --- a/include/hw/acpi/aml-build.h >> +++ b/include/hw/acpi/aml-build.h >> @@ -223,6 +223,11 @@ struct AcpiBuildTables { >> BIOSLinker *linker; >> } AcpiBuildTables; >> >> +typedef struct AcpiMcfgInfo { >> + uint64_t mcfg_base; >> + uint32_t mcfg_size; >mcfg_ prefix in fields probably isn't necessary. > Hmm... I may split this to another patch to address. >> +} AcpiMcfgInfo; >> + >> /** >> * init_aml_allocator: >> * >
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 57679a89bf..7713c2d809 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -545,23 +545,32 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) "SRAT", table_data->len - srat_start, 3, NULL, NULL); } -static void -build_mcfg(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) +static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg) { - AcpiTableMcfg *mcfg; + VirtMachineState *vms = VIRT_MACHINE(qdev_get_machine()); const MemMapEntry *memmap = vms->memmap; int ecam_id = VIRT_ECAM_ID(vms->highmem_ecam); + + mcfg->mcfg_base = memmap[ecam_id].base; + mcfg->mcfg_size = memmap[ecam_id].size; + + return true; +} + +static void +build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info) +{ + AcpiTableMcfg *mcfg; int len = sizeof(*mcfg) + sizeof(mcfg->allocation[0]); int mcfg_start = table_data->len; mcfg = acpi_data_push(table_data, len); - mcfg->allocation[0].address = cpu_to_le64(memmap[ecam_id].base); + mcfg->allocation[0].address = cpu_to_le64(info->mcfg_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(memmap[ecam_id].size - 1); + mcfg->allocation[0].end_bus_number = PCIE_MMCFG_BUS(info->mcfg_size - 1); build_header(linker, table_data, (void *)(table_data->data + mcfg_start), "MCFG", table_data->len - mcfg_start, 1, NULL, NULL); @@ -779,6 +788,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables) VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms); GArray *table_offsets; unsigned dsdt, xsdt; + AcpiMcfgInfo mcfg; GArray *tables_blob = tables->table_data; table_offsets = g_array_new(false, true /* clear */, @@ -802,8 +812,10 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables) acpi_add_table(table_offsets, tables_blob); build_gtdt(tables_blob, tables->linker, vms); - acpi_add_table(table_offsets, tables_blob); - build_mcfg(tables_blob, tables->linker, vms); + if (acpi_get_mcfg(&mcfg)) { + acpi_add_table(table_offsets, tables_blob); + build_mcfg(tables_blob, tables->linker, &mcfg); + } acpi_add_table(table_offsets, tables_blob); build_spcr(tables_blob, tables->linker, vms); diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 9ecc96dcc7..c5b1c3be99 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -86,11 +86,6 @@ /* Default IOAPIC ID */ #define ACPI_BUILD_IOAPIC_ID 0x0 -typedef struct AcpiMcfgInfo { - uint64_t mcfg_base; - uint32_t mcfg_size; -} AcpiMcfgInfo; - typedef struct AcpiPmInfo { bool s3_disabled; bool s4_disabled; diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h index 1a563ad756..b63b85d67c 100644 --- a/include/hw/acpi/aml-build.h +++ b/include/hw/acpi/aml-build.h @@ -223,6 +223,11 @@ struct AcpiBuildTables { BIOSLinker *linker; } AcpiBuildTables; +typedef struct AcpiMcfgInfo { + uint64_t mcfg_base; + uint32_t mcfg_size; +} AcpiMcfgInfo; + /** * init_aml_allocator: *
To build MCFG, two information is necessary: * bus number * base address Abstract these two information to AcpiMcfgInfo so that build_mcfg and build_mcfg_q35 will have the same declaration. Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> --- hw/arm/virt-acpi-build.c | 28 ++++++++++++++++++++-------- hw/i386/acpi-build.c | 5 ----- include/hw/acpi/aml-build.h | 5 +++++ 3 files changed, 25 insertions(+), 13 deletions(-)