[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 09/19] hw: acpi: Export and generalize the PC
From: |
Samuel Ortiz |
Subject: |
Re: [Qemu-devel] [PATCH v3 09/19] hw: acpi: Export and generalize the PCI host AML API |
Date: |
Tue, 30 Oct 2018 15:57:15 +0100 |
User-agent: |
Mutt/1.10.1 (2018-07-13) |
Hi Philippe,
On Mon, Oct 29, 2018 at 08:29:38PM +0100, Philippe Mathieu-Daudé wrote:
> On 29/10/18 20:23, Philippe Mathieu-Daudé wrote:
> > On 29/10/18 18:01, Samuel Ortiz wrote:
> > > From: Yang Zhong <address@hidden>
> > >
> > > The AML build routines for the PCI host bridge and the corresponding
> > > DSDT addition are neither x86 nor PC machine type specific.
> > > We can move them to the architecture agnostic hw/acpi folder, and by
> > > carrying all the needed information through a new AcpiPciBus structure,
> > > we can make them PC machine type independent.
> > >
> > > Signed-off-by: Yang Zhong <address@hidden>
> > > Signed-off-by: Rob Bradford <address@hidden>
> > > ---
> > > hw/acpi/aml-build.c | 208 ++++++++++++++++++++++++++++++++++++
> > > hw/i386/acpi-build.c | 167 ++---------------------------
> > > include/hw/acpi/aml-build.h | 10 ++
> > > 3 files changed, 226 insertions(+), 159 deletions(-)
> > >
> > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> > > index 52ac39acdb..aa72b5459c 100644
> > > --- a/hw/acpi/aml-build.c
> > > +++ b/hw/acpi/aml-build.c
> > > @@ -29,6 +29,25 @@
> > > #include "hw/pci/pci_bus.h"
> > > #include "qemu/range.h"
> > > #include "hw/pci/pci_bridge.h"
> > > +#include "hw/i386/pc.h"
>
> Err I just missed that, this include doesn't belong here, ...
>
> > > +#include "sysemu/tpm.h"
> > > +#include "hw/acpi/tpm.h"
> > > +
> > > +#define PCI_HOST_BRIDGE_CONFIG_ADDR 0xcf8
> > > +#define PCI_HOST_BRIDGE_IO_0_MIN_ADDR 0x0000
> > > +#define PCI_HOST_BRIDGE_IO_0_MAX_ADDR 0x0cf7
> > > +#define PCI_HOST_BRIDGE_IO_1_MIN_ADDR 0x0d00
> > > +#define PCI_HOST_BRIDGE_IO_1_MAX_ADDR 0xffff
> > > +#define PCI_VGA_MEM_BASE_ADDR 0x000a0000
> > > +#define PCI_VGA_MEM_MAX_ADDR 0x000bffff
> > > +#define IO_0_LEN 0xcf8
> > > +#define VGA_MEM_LEN 0x20000
> > > +
> > > +static const char *pci_hosts[] = {
> >
> > This array should stay in hw/i386/acpi-build.c.
> >
> > > + "/machine/i440fx",
> > > + "/machine/q35",
> > > + NULL,
> > > +};
> > > static GArray *build_alloc_array(void)
> > > {
> > > @@ -1601,6 +1620,51 @@ void
> > > acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre)
> > > g_array_free(tables->vmgenid, mfre);
> > > }
> > > +/*
> > > + * Because of the PXB hosts we cannot simply query TYPE_PCI_HOST_BRIDGE.
> > > + */
> > > +Object *acpi_get_pci_host(void)
> >
> > This function should take the machine_paths as argument.
> >
> > > +{
> > > + PCIHostState *host;
> > > + int i = 0;
> > > +
> > > + while (pci_hosts[i]) {
> > > + host = OBJECT_CHECK(PCIHostState,
> > > + object_resolve_path(pci_hosts[i], NULL),
> > > + TYPE_PCI_HOST_BRIDGE);
> > > + if (host) {
> > > + return OBJECT(host);
> > > + }
> > > +
> > > + i++;
> > > + }
> > > +
> > > + return NULL;
> > > +}
> > > +
> > > +void acpi_get_pci_holes(Range *hole, Range *hole64)
>
> ... and this function neither, it should stay in hw/i386/acpi-build.c, thus
> you don't need to modify the prototype and can call
> acpi_get_pci_host(x86_machine_paths) directly.
>
So the idea for those routines is that they're not x86 specific. As a
matter of fact, they will eventually get called from architecture
agnostic code like e.g. hw/acpi/reduced.c. So I don't think they should
live under hw/i386/
Moreover adding an argument to acpi_get_pci_host() means the caller should
know which potential machines it needs to go through. And once both
arm/virt and i386/virt calls into hw/acpi/reduced.c, we need to somehow
pass a relevant set of machines paths to this API.
So I think a more robust way to do so would be for each machine instance to
be able to set its own PCI host pointer instead of having to maintain
a per architecture set of potential PCI machine paths. I'm thinking
about adding that to the AcpiConfiguration structure and let each
machine fills it if/when it builds its PCI host.
Cheers,
Samuel.
- [Qemu-devel] [PATCH v3 04/19] hw: acpi: Implement XSDT support for RSDP, (continued)
- [Qemu-devel] [PATCH v3 04/19] hw: acpi: Implement XSDT support for RSDP, Samuel Ortiz, 2018/10/29
- [Qemu-devel] [PATCH v3 05/19] hw: arm: Switch to the AML build RSDP building routine, Samuel Ortiz, 2018/10/29
- [Qemu-devel] [PATCH v3 06/19] hw: acpi: Generalize AML build routines, Samuel Ortiz, 2018/10/29
- [Qemu-devel] [PATCH v3 08/19] hw: i386: Refactor PCI host getter, Samuel Ortiz, 2018/10/29
- [Qemu-devel] [PATCH v3 07/19] hw: acpi: Factorize _OSC AML across architectures, Samuel Ortiz, 2018/10/29
- [Qemu-devel] [PATCH v3 09/19] hw: acpi: Export and generalize the PCI host AML API, Samuel Ortiz, 2018/10/29
[Qemu-devel] [PATCH v3 12/19] hw: i386: Make the hotpluggable memory size property more generic, Samuel Ortiz, 2018/10/29
[Qemu-devel] [PATCH v3 14/19] hw: acpi: Fix memory hotplug AML generation error, Samuel Ortiz, 2018/10/29
[Qemu-devel] [PATCH v3 11/19] hw: acpi: Do not create hotplug method when handler is not defined, Samuel Ortiz, 2018/10/29
[Qemu-devel] [PATCH v3 16/19] hw: acpi: Retrieve the PCI bus from AcpiPciHpState, Samuel Ortiz, 2018/10/29