Re: [Qemu-devel] [PATCH v5 10/24] hw: acpi: Export the PCI host and holes getters
On Wed, 21 Nov 2018 16:43:12 +0100
Samuel Ortiz wrote:
> On Tue, Nov 13, 2018 at 04:59:18PM +0100, Igor Mammedov wrote:
> > On Mon, 5 Nov 2018 02:40:33 +0100
> > Samuel Ortiz wrote:
> >
> > > This is going to be needed by the hardware reduced implementation, so
> > > let's export it.
> > > Once the ACPI builder methods and getters will be implemented, the
> > > acpi_get_pci_host() implementation will become hardware agnostic.
> > >
> > > Signed-off-by: Samuel Ortiz
> > > ---
> > > include/hw/acpi/aml-build.h | 2 ++
> > > hw/acpi/aml-build.c | 43 +
> > > hw/i386/acpi-build.c| 47 ++---
> > > 3 files changed, 47 insertions(+), 45 deletions(-)
> > >
> > > diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> > > index c27c0935ae..fde2785b9a 100644
> > > --- a/include/hw/acpi/aml-build.h
> > > +++ b/include/hw/acpi/aml-build.h
> > > @@ -400,6 +400,8 @@ build_header(BIOSLinker *linker, GArray *table_data,
> > > const char *oem_id, const char *oem_table_id);
> > > void *acpi_data_push(GArray *table_data, unsigned size);
> > > unsigned acpi_data_len(GArray *table);
> > > +Object *acpi_get_pci_host(void);
> > > +void acpi_get_pci_holes(Range *hole, Range *hole64);
> > > /* Align AML blob size to a multiple of 'align' */
> > > void acpi_align_size(GArray *blob, unsigned align);
> > > void acpi_add_table(GArray *table_offsets, GArray *table_data);
> > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> > > index 2b9a636e75..b8e32f15f7 100644
> > > --- a/hw/acpi/aml-build.c
> > > +++ b/hw/acpi/aml-build.c
> > > @@ -1601,6 +1601,49 @@ 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)
> > > +{
> > > +PCIHostState *host;
> > > +
> > > +host = OBJECT_CHECK(PCIHostState,
> > > +object_resolve_path("/machine/i440fx", NULL),
> > > +TYPE_PCI_HOST_BRIDGE);
> > > +if (!host) {
> > > +host = OBJECT_CHECK(PCIHostState,
> > > +object_resolve_path("/machine/q35", NULL),
> > > +TYPE_PCI_HOST_BRIDGE);
> > > +}
> > > +
> > > +return OBJECT(host);
> > > +}
> > in general aml-build.c is a place to put ACPI spec primitives,
> > so I'd suggest to move it somewhere else.
> >
> > Considering it's x86 code (so far), maybe move it to something like
> > hw/i386/acpi-pci.c
> >
> > Also it might be good to get rid of acpi_get_pci_host() and pass
> > a pointer to pci_host as acpi_setup() an argument, since it's static
> > for life of boar we can keep it in AcpiBuildState, and reuse for
> > mfg/pci_hole/pci bus accesses.
> That's what I'm trying to do with patches #23 and 24, but through the
> ACPI configuration structure. I could try using the build state instead,
> as it's platform agnostic as well.
May be it will work.
Note:
try not to pass whole build_state to concrete tables builders and use
arguments/dedicated structures to pass data needed for that concrete
table.
>
> Cheers,
> Samuel.
>
Re: [Qemu-devel] [PATCH v5 10/24] hw: acpi: Export the PCI host and holes getters
On Tue, Nov 13, 2018 at 04:59:18PM +0100, Igor Mammedov wrote:
> On Mon, 5 Nov 2018 02:40:33 +0100
> Samuel Ortiz wrote:
>
> > This is going to be needed by the hardware reduced implementation, so
> > let's export it.
> > Once the ACPI builder methods and getters will be implemented, the
> > acpi_get_pci_host() implementation will become hardware agnostic.
> >
> > Signed-off-by: Samuel Ortiz
> > ---
> > include/hw/acpi/aml-build.h | 2 ++
> > hw/acpi/aml-build.c | 43 +
> > hw/i386/acpi-build.c| 47 ++---
> > 3 files changed, 47 insertions(+), 45 deletions(-)
> >
> > diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> > index c27c0935ae..fde2785b9a 100644
> > --- a/include/hw/acpi/aml-build.h
> > +++ b/include/hw/acpi/aml-build.h
> > @@ -400,6 +400,8 @@ build_header(BIOSLinker *linker, GArray *table_data,
> > const char *oem_id, const char *oem_table_id);
> > void *acpi_data_push(GArray *table_data, unsigned size);
> > unsigned acpi_data_len(GArray *table);
> > +Object *acpi_get_pci_host(void);
> > +void acpi_get_pci_holes(Range *hole, Range *hole64);
> > /* Align AML blob size to a multiple of 'align' */
> > void acpi_align_size(GArray *blob, unsigned align);
> > void acpi_add_table(GArray *table_offsets, GArray *table_data);
> > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> > index 2b9a636e75..b8e32f15f7 100644
> > --- a/hw/acpi/aml-build.c
> > +++ b/hw/acpi/aml-build.c
> > @@ -1601,6 +1601,49 @@ 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)
> > +{
> > +PCIHostState *host;
> > +
> > +host = OBJECT_CHECK(PCIHostState,
> > +object_resolve_path("/machine/i440fx", NULL),
> > +TYPE_PCI_HOST_BRIDGE);
> > +if (!host) {
> > +host = OBJECT_CHECK(PCIHostState,
> > +object_resolve_path("/machine/q35", NULL),
> > +TYPE_PCI_HOST_BRIDGE);
> > +}
> > +
> > +return OBJECT(host);
> > +}
> in general aml-build.c is a place to put ACPI spec primitives,
> so I'd suggest to move it somewhere else.
>
> Considering it's x86 code (so far), maybe move it to something like
> hw/i386/acpi-pci.c
>
> Also it might be good to get rid of acpi_get_pci_host() and pass
> a pointer to pci_host as acpi_setup() an argument, since it's static
> for life of boar we can keep it in AcpiBuildState, and reuse for
> mfg/pci_hole/pci bus accesses.
That's what I'm trying to do with patches #23 and 24, but through the
ACPI configuration structure. I could try using the build state instead,
as it's platform agnostic as well.
Cheers,
Samuel.
Re: [Qemu-devel] [PATCH v5 10/24] hw: acpi: Export the PCI host and holes getters
On Mon, 5 Nov 2018 02:40:33 +0100
Samuel Ortiz wrote:
> This is going to be needed by the hardware reduced implementation, so
> let's export it.
> Once the ACPI builder methods and getters will be implemented, the
> acpi_get_pci_host() implementation will become hardware agnostic.
>
> Signed-off-by: Samuel Ortiz
> ---
> include/hw/acpi/aml-build.h | 2 ++
> hw/acpi/aml-build.c | 43 +
> hw/i386/acpi-build.c| 47 ++---
> 3 files changed, 47 insertions(+), 45 deletions(-)
>
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index c27c0935ae..fde2785b9a 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -400,6 +400,8 @@ build_header(BIOSLinker *linker, GArray *table_data,
> const char *oem_id, const char *oem_table_id);
> void *acpi_data_push(GArray *table_data, unsigned size);
> unsigned acpi_data_len(GArray *table);
> +Object *acpi_get_pci_host(void);
> +void acpi_get_pci_holes(Range *hole, Range *hole64);
> /* Align AML blob size to a multiple of 'align' */
> void acpi_align_size(GArray *blob, unsigned align);
> void acpi_add_table(GArray *table_offsets, GArray *table_data);
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 2b9a636e75..b8e32f15f7 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -1601,6 +1601,49 @@ 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)
> +{
> +PCIHostState *host;
> +
> +host = OBJECT_CHECK(PCIHostState,
> +object_resolve_path("/machine/i440fx", NULL),
> +TYPE_PCI_HOST_BRIDGE);
> +if (!host) {
> +host = OBJECT_CHECK(PCIHostState,
> +object_resolve_path("/machine/q35", NULL),
> +TYPE_PCI_HOST_BRIDGE);
> +}
> +
> +return OBJECT(host);
> +}
in general aml-build.c is a place to put ACPI spec primitives,
so I'd suggest to move it somewhere else.
Considering it's x86 code (so far), maybe move it to something like
hw/i386/acpi-pci.c
Also it might be good to get rid of acpi_get_pci_host() and pass
a pointer to pci_host as acpi_setup() an argument, since it's static
for life of boar we can keep it in AcpiBuildState, and reuse for
mfg/pci_hole/pci bus accesses.
That way we can simplify code a bit and avoid lookup cost of
object_resolve_path() that's called several times unnecessarily.
> +void acpi_get_pci_holes(Range *hole, Range *hole64)
> +{
> +Object *pci_host;
> +
> +pci_host = acpi_get_pci_host();
> +g_assert(pci_host);
> +
> +range_set_bounds1(hole,
> + object_property_get_uint(pci_host,
> + PCI_HOST_PROP_PCI_HOLE_START,
> + NULL),
> + object_property_get_uint(pci_host,
> + PCI_HOST_PROP_PCI_HOLE_END,
> + NULL));
> +range_set_bounds1(hole64,
> + object_property_get_uint(pci_host,
> +
> PCI_HOST_PROP_PCI_HOLE64_START,
> + NULL),
> + object_property_get_uint(pci_host,
> + PCI_HOST_PROP_PCI_HOLE64_END,
> + NULL));
> +}
> +
> static void crs_range_insert(GPtrArray *ranges, uint64_t base, uint64_t
> limit)
> {
> CrsRangeEntry *entry;
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index bd147a6bd2..a5f5f83500 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -232,49 +232,6 @@ static void acpi_get_misc_info(AcpiMiscInfo *info)
> info->applesmc_io_base = applesmc_port();
> }
>
> -/*
> - * Because of the PXB hosts we cannot simply query TYPE_PCI_HOST_BRIDGE.
> - * On i386 arch we only have two pci hosts, so we can look only for them.
> - */
> -static Object *acpi_get_i386_pci_host(void)
> -{
> -PCIHostState *host;
> -
> -host = OBJECT_CHECK(PCIHostState,
> -object_resolve_path("/machine/i440fx", NULL),
> -TYPE_PCI_HOST_BRIDGE);
> -if (!host) {
> -host = OBJECT_CHECK(PCIHostState,
> -object_resolve_path("/machine/q35", NULL),
> -TYPE_PCI_HOST_BRIDGE);
> -}
> -
> -return OBJECT(host);
> -}
> -
> -static void acpi_get_pci_holes(Range *hole, Range *hole64)
> -{
> -Object *pci_host;
> -
> -pci_host = acpi_get_i386_pci_host();
> -g_assert(pci_host);
> -
> -range_set_bounds1(hole,
> -
[Qemu-devel] [PATCH v5 10/24] hw: acpi: Export the PCI host and holes getters
This is going to be needed by the hardware reduced implementation, so
let's export it.
Once the ACPI builder methods and getters will be implemented, the
acpi_get_pci_host() implementation will become hardware agnostic.
Signed-off-by: Samuel Ortiz
---
include/hw/acpi/aml-build.h | 2 ++
hw/acpi/aml-build.c | 43 +
hw/i386/acpi-build.c| 47 ++---
3 files changed, 47 insertions(+), 45 deletions(-)
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index c27c0935ae..fde2785b9a 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -400,6 +400,8 @@ build_header(BIOSLinker *linker, GArray *table_data,
const char *oem_id, const char *oem_table_id);
void *acpi_data_push(GArray *table_data, unsigned size);
unsigned acpi_data_len(GArray *table);
+Object *acpi_get_pci_host(void);
+void acpi_get_pci_holes(Range *hole, Range *hole64);
/* Align AML blob size to a multiple of 'align' */
void acpi_align_size(GArray *blob, unsigned align);
void acpi_add_table(GArray *table_offsets, GArray *table_data);
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 2b9a636e75..b8e32f15f7 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1601,6 +1601,49 @@ 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)
+{
+PCIHostState *host;
+
+host = OBJECT_CHECK(PCIHostState,
+object_resolve_path("/machine/i440fx", NULL),
+TYPE_PCI_HOST_BRIDGE);
+if (!host) {
+host = OBJECT_CHECK(PCIHostState,
+object_resolve_path("/machine/q35", NULL),
+TYPE_PCI_HOST_BRIDGE);
+}
+
+return OBJECT(host);
+}
+
+
+void acpi_get_pci_holes(Range *hole, Range *hole64)
+{
+Object *pci_host;
+
+pci_host = acpi_get_pci_host();
+g_assert(pci_host);
+
+range_set_bounds1(hole,
+ object_property_get_uint(pci_host,
+ PCI_HOST_PROP_PCI_HOLE_START,
+ NULL),
+ object_property_get_uint(pci_host,
+ PCI_HOST_PROP_PCI_HOLE_END,
+ NULL));
+range_set_bounds1(hole64,
+ object_property_get_uint(pci_host,
+ PCI_HOST_PROP_PCI_HOLE64_START,
+ NULL),
+ object_property_get_uint(pci_host,
+ PCI_HOST_PROP_PCI_HOLE64_END,
+ NULL));
+}
+
static void crs_range_insert(GPtrArray *ranges, uint64_t base, uint64_t limit)
{
CrsRangeEntry *entry;
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index bd147a6bd2..a5f5f83500 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -232,49 +232,6 @@ static void acpi_get_misc_info(AcpiMiscInfo *info)
info->applesmc_io_base = applesmc_port();
}
-/*
- * Because of the PXB hosts we cannot simply query TYPE_PCI_HOST_BRIDGE.
- * On i386 arch we only have two pci hosts, so we can look only for them.
- */
-static Object *acpi_get_i386_pci_host(void)
-{
-PCIHostState *host;
-
-host = OBJECT_CHECK(PCIHostState,
-object_resolve_path("/machine/i440fx", NULL),
-TYPE_PCI_HOST_BRIDGE);
-if (!host) {
-host = OBJECT_CHECK(PCIHostState,
-object_resolve_path("/machine/q35", NULL),
-TYPE_PCI_HOST_BRIDGE);
-}
-
-return OBJECT(host);
-}
-
-static void acpi_get_pci_holes(Range *hole, Range *hole64)
-{
-Object *pci_host;
-
-pci_host = acpi_get_i386_pci_host();
-g_assert(pci_host);
-
-range_set_bounds1(hole,
- object_property_get_uint(pci_host,
- PCI_HOST_PROP_PCI_HOLE_START,
- NULL),
- object_property_get_uint(pci_host,
- PCI_HOST_PROP_PCI_HOLE_END,
- NULL));
-range_set_bounds1(hole64,
- object_property_get_uint(pci_host,
- PCI_HOST_PROP_PCI_HOLE64_START,
- NULL),
- object_property_get_uint(pci_host,
- PCI_HOST_PROP_PCI_HOLE64_END,
- NULL));
-}
-
/* FACS */
static void
build_facs(GArray *table_data, BIOSLinker *linker)
