From: Marcel Apfelbaum <marcel@redhat.com>
To: Markus Armbruster <armbru@redhat.com>, qemu-devel@nongnu.org
Cc: mst@redhat.com, imammedo@redhat.com, ehabkost@redhat.com
Subject: Re: [Qemu-devel] [PATCH 2/2] pc: Eliminate PcPciInfo
Date: Thu, 16 Jun 2016 10:30:43 +0300 [thread overview]
Message-ID: <576255A3.9050902@redhat.com> (raw)
In-Reply-To: <1466013391-16028-3-git-send-email-armbru@redhat.com>
On 06/15/2016 08:56 PM, Markus Armbruster wrote:
> PcPciInfo has two (ill-named) members: Range w32 is the PCI hole, and
> w64 is the PCI64 hole.
>
> Three users:
>
> * I440FXState and MCHPCIState have a member PcPciInfo pci_info, but
> only pci_info.w32 is actually used. This is confusing. Replace by
> Range pci_hole.
>
> * acpi_build() uses auto PcPciInfo pci_info to forward both PCI holes
> from acpi_get_pci_info() to build_dsdt(). Replace by two variables
> Range pci_hole, pci_hole64. Rename acpi_get_pci_info() to
> acpi_get_pci_holes().
>
> PcPciInfo is now unused; drop it.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> hw/i386/acpi-build.c | 43 ++++++++++++++++++++++---------------------
> hw/pci-host/piix.c | 10 +++++-----
> hw/pci-host/q35.c | 12 ++++++------
> include/hw/i386/pc.h | 5 -----
> include/hw/pci-host/q35.h | 2 +-
> 5 files changed, 34 insertions(+), 38 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 8ca2032..02fc534 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -225,26 +225,25 @@ static Object *acpi_get_i386_pci_host(void)
> return OBJECT(host);
> }
>
> -static void acpi_get_pci_info(PcPciInfo *info)
> +static void acpi_get_pci_holes(Range *hole, Range *hole64)
> {
> Object *pci_host;
>
> -
> pci_host = acpi_get_i386_pci_host();
> g_assert(pci_host);
>
> - info->w32.begin = object_property_get_int(pci_host,
> - PCI_HOST_PROP_PCI_HOLE_START,
> - NULL);
> - info->w32.end = object_property_get_int(pci_host,
> - PCI_HOST_PROP_PCI_HOLE_END,
> - NULL);
> - info->w64.begin = object_property_get_int(pci_host,
> - PCI_HOST_PROP_PCI_HOLE64_START,
> - NULL);
> - info->w64.end = object_property_get_int(pci_host,
> - PCI_HOST_PROP_PCI_HOLE64_END,
> + hole->begin = object_property_get_int(pci_host,
> + PCI_HOST_PROP_PCI_HOLE_START,
> + NULL);
> + hole->end = object_property_get_int(pci_host,
> + PCI_HOST_PROP_PCI_HOLE_END,
> + NULL);
> + hole64->begin = object_property_get_int(pci_host,
> + PCI_HOST_PROP_PCI_HOLE64_START,
> NULL);
> + hole64->end = object_property_get_int(pci_host,
> + PCI_HOST_PROP_PCI_HOLE64_END,
> + NULL);
> }
>
> #define ACPI_PORT_SMI_CMD 0x00b2 /* TODO: this is APM_CNT_IOPORT */
> @@ -1867,7 +1866,7 @@ static Aml *build_q35_osc_method(void)
> static void
> build_dsdt(GArray *table_data, BIOSLinker *linker,
> AcpiPmInfo *pm, AcpiMiscInfo *misc,
> - PcPciInfo *pci, MachineState *machine)
> + Range *pci_hole, Range *pci_hole64, MachineState *machine)
> {
> CrsRangeEntry *entry;
> Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs;
> @@ -2015,7 +2014,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> AML_CACHEABLE, AML_READ_WRITE,
> 0, 0x000A0000, 0x000BFFFF, 0, 0x00020000));
>
> - crs_replace_with_free_ranges(mem_ranges, pci->w32.begin, pci->w32.end - 1);
> + crs_replace_with_free_ranges(mem_ranges,
> + pci_hole->begin, pci_hole->end - 1);
> for (i = 0; i < mem_ranges->len; i++) {
> entry = g_ptr_array_index(mem_ranges, i);
> aml_append(crs,
> @@ -2025,12 +2025,12 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> 0, entry->limit - entry->base + 1));
> }
>
> - if (pci->w64.begin) {
> + if (pci_hole64->begin) {
> aml_append(crs,
> aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
> AML_CACHEABLE, AML_READ_WRITE,
> - 0, pci->w64.begin, pci->w64.end - 1, 0,
> - pci->w64.end - pci->w64.begin));
> + 0, pci_hole64->begin, pci_hole64->end - 1, 0,
> + pci_hole64->end - pci_hole64->begin));
> }
>
> if (misc->tpm_version != TPM_VERSION_UNSPEC) {
> @@ -2518,7 +2518,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
> AcpiPmInfo pm;
> AcpiMiscInfo misc;
> AcpiMcfgInfo mcfg;
> - PcPciInfo pci;
> + Range pci_hole, pci_hole64;
> uint8_t *u;
> size_t aml_len = 0;
> GArray *tables_blob = tables->table_data;
> @@ -2526,7 +2526,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>
> acpi_get_pm_info(&pm);
> acpi_get_misc_info(&misc);
> - acpi_get_pci_info(&pci);
> + acpi_get_pci_holes(&pci_hole, &pci_hole64);
> acpi_get_slic_oem(&slic_oem);
>
> table_offsets = g_array_new(false, true /* clear */,
> @@ -2548,7 +2548,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>
> /* DSDT is pointed to by FADT */
> dsdt = tables_blob->len;
> - build_dsdt(tables_blob, tables->linker, &pm, &misc, &pci, machine);
> + build_dsdt(tables_blob, tables->linker, &pm, &misc,
> + &pci_hole, &pci_hole64, machine);
>
> /* Count the size of the DSDT and SSDT, we will need it for legacy
> * sizing of ACPI tables.
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index c63c424..8db0f09 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -48,7 +48,7 @@
>
> typedef struct I440FXState {
> PCIHostState parent_obj;
> - PcPciInfo pci_info;
> + Range pci_hole;
> uint64_t pci_hole64_size;
> uint32_t short_root_bus;
> } I440FXState;
> @@ -221,7 +221,7 @@ static void i440fx_pcihost_get_pci_hole_start(Object *obj, Visitor *v,
> Error **errp)
> {
> I440FXState *s = I440FX_PCI_HOST_BRIDGE(obj);
> - uint32_t value = s->pci_info.w32.begin;
> + uint32_t value = s->pci_hole.begin;
>
> visit_type_uint32(v, name, &value, errp);
> }
> @@ -231,7 +231,7 @@ static void i440fx_pcihost_get_pci_hole_end(Object *obj, Visitor *v,
> Error **errp)
> {
> I440FXState *s = I440FX_PCI_HOST_BRIDGE(obj);
> - uint32_t value = s->pci_info.w32.end;
> + uint32_t value = s->pci_hole.end;
>
> visit_type_uint32(v, name, &value, errp);
> }
> @@ -344,8 +344,8 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
> f->ram_memory = ram_memory;
>
> i440fx = I440FX_PCI_HOST_BRIDGE(dev);
> - i440fx->pci_info.w32.begin = below_4g_mem_size;
> - i440fx->pci_info.w32.end = IO_APIC_DEFAULT_ADDRESS;
> + i440fx->pci_hole.begin = below_4g_mem_size;
> + i440fx->pci_hole.end = IO_APIC_DEFAULT_ADDRESS;
>
> /* setup pci memory mapping */
> pc_pci_as_mapping_init(OBJECT(f), f->system_memory,
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index 70f897e..8c2c1db 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -73,7 +73,7 @@ static void q35_host_get_pci_hole_start(Object *obj, Visitor *v,
> Error **errp)
> {
> Q35PCIHost *s = Q35_HOST_DEVICE(obj);
> - uint32_t value = s->mch.pci_info.w32.begin;
> + uint32_t value = s->mch.pci_hole.begin;
>
> visit_type_uint32(v, name, &value, errp);
> }
> @@ -83,7 +83,7 @@ static void q35_host_get_pci_hole_end(Object *obj, Visitor *v,
> Error **errp)
> {
> Q35PCIHost *s = Q35_HOST_DEVICE(obj);
> - uint32_t value = s->mch.pci_info.w32.end;
> + uint32_t value = s->mch.pci_hole.end;
>
> visit_type_uint32(v, name, &value, errp);
> }
> @@ -182,9 +182,9 @@ static void q35_host_initfn(Object *obj)
> * it's not a power of two, which means an MTRR
> * can't cover it exactly.
> */
> - s->mch.pci_info.w32.begin = MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT +
> + s->mch.pci_hole.begin = MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT +
> MCH_HOST_BRIDGE_PCIEXBAR_MAX;
> - s->mch.pci_info.w32.end = IO_APIC_DEFAULT_ADDRESS;
> + s->mch.pci_hole.end = IO_APIC_DEFAULT_ADDRESS;
> }
>
> static const TypeInfo q35_host_info = {
> @@ -265,9 +265,9 @@ static void mch_update_pciexbar(MCHPCIState *mch)
> * which means an MTRR can't cover it exactly.
> */
> if (enable) {
> - mch->pci_info.w32.begin = addr + length;
> + mch->pci_hole.begin = addr + length;
> } else {
> - mch->pci_info.w32.begin = MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT;
> + mch->pci_hole.begin = MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT;
> }
> }
>
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 9ca2309..36ac326 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -148,11 +148,6 @@ struct PCMachineClass {
>
> /* PC-style peripherals (also used by other machines). */
>
> -typedef struct PcPciInfo {
> - Range w32;
> - Range w64;
> -} PcPciInfo;
> -
> #define ACPI_PM_PROP_S3_DISABLED "disable_s3"
> #define ACPI_PM_PROP_S4_DISABLED "disable_s4"
> #define ACPI_PM_PROP_S4_VAL "s4_val"
> diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h
> index c5c073d..949e3a9 100644
> --- a/include/hw/pci-host/q35.h
> +++ b/include/hw/pci-host/q35.h
> @@ -55,7 +55,7 @@ typedef struct MCHPCIState {
> MemoryRegion smram_region, open_high_smram;
> MemoryRegion smram, low_smram, high_smram;
> MemoryRegion tseg_blackhole, tseg_window;
> - PcPciInfo pci_info;
> + Range pci_hole;
> ram_addr_t below_4g_mem_size;
> ram_addr_t above_4g_mem_size;
> uint64_t pci_hole64_size;
>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Thanks,
Marcel
next prev parent reply other threads:[~2016-06-16 7:30 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-15 17:56 [Qemu-devel] [PATCH 0/2] Clean up around the PCI holes Markus Armbruster
2016-06-15 17:56 ` [Qemu-devel] [PATCH 1/2] piix: Set I440FXState member pci_info.w32 in one place Markus Armbruster
2016-06-15 19:16 ` Eric Blake
2016-06-16 7:26 ` Marcel Apfelbaum
2016-06-15 17:56 ` [Qemu-devel] [PATCH 2/2] pc: Eliminate PcPciInfo Markus Armbruster
2016-06-15 19:22 ` Eric Blake
2016-06-16 7:30 ` Marcel Apfelbaum [this message]
2016-06-30 5:18 ` [Qemu-devel] [PATCH 0/2] Clean up around the PCI holes Markus Armbruster
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=576255A3.9050902@redhat.com \
--to=marcel@redhat.com \
--cc=armbru@redhat.com \
--cc=ehabkost@redhat.com \
--cc=imammedo@redhat.com \
--cc=mst@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).