* [Qemu-devel] [PATCH v3 0/6] Extract build_mcfg
@ 2019-04-17 1:40 ` Wei Yang
0 siblings, 0 replies; 24+ messages in thread
From: Wei Yang @ 2019-04-17 1:40 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: mst, imammedo, shannon.zhaosl, peter.maydell, marcel.apfelbaum,
yang.zhong, philmd, Wei Yang
This patch set tries to generalize MCFG table build process. And it is
based on one un-merged patch from Igor, which is included in this serials.
v2->v3:
* Includes the un-merged patch from Igor
* use build_append_foo() API to construct MCFG
Igor Mammedov (1):
q35: acpi: do not create dummy MCFG table
Wei Yang (5):
hw/arm/virt-acpi-build: remove unnecessary variable mcfg_start
i386, acpi: remove mcfg_ prefix in AcpiMcfgInfo members
hw/arm/virt-acpi-build: pass AcpiMcfgInfo to build_mcfg()
hw/acpi: Extract build_mcfg to pci.c
acpi: pci: use build_append_foo() API to construct MCFG
default-configs/arm-softmmu.mak | 1 +
default-configs/i386-softmmu.mak | 1 +
hw/acpi/Kconfig | 4 +++
hw/acpi/Makefile.objs | 1 +
hw/acpi/pci.c | 52 ++++++++++++++++++++++++++++++++
hw/arm/virt-acpi-build.c | 31 +++++--------------
hw/i386/acpi-build.c | 44 +++++----------------------
include/hw/acpi/acpi-defs.h | 18 -----------
include/hw/acpi/pci.h | 34 +++++++++++++++++++++
9 files changed, 108 insertions(+), 78 deletions(-)
create mode 100644 hw/acpi/pci.c
create mode 100644 include/hw/acpi/pci.h
--
2.19.1
^ permalink raw reply [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v3 0/6] Extract build_mcfg
@ 2019-04-17 1:40 ` Wei Yang
0 siblings, 0 replies; 24+ messages in thread
From: Wei Yang @ 2019-04-17 1:40 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: yang.zhong, peter.maydell, mst, shannon.zhaosl, Wei Yang,
imammedo, philmd
This patch set tries to generalize MCFG table build process. And it is
based on one un-merged patch from Igor, which is included in this serials.
v2->v3:
* Includes the un-merged patch from Igor
* use build_append_foo() API to construct MCFG
Igor Mammedov (1):
q35: acpi: do not create dummy MCFG table
Wei Yang (5):
hw/arm/virt-acpi-build: remove unnecessary variable mcfg_start
i386, acpi: remove mcfg_ prefix in AcpiMcfgInfo members
hw/arm/virt-acpi-build: pass AcpiMcfgInfo to build_mcfg()
hw/acpi: Extract build_mcfg to pci.c
acpi: pci: use build_append_foo() API to construct MCFG
default-configs/arm-softmmu.mak | 1 +
default-configs/i386-softmmu.mak | 1 +
hw/acpi/Kconfig | 4 +++
hw/acpi/Makefile.objs | 1 +
hw/acpi/pci.c | 52 ++++++++++++++++++++++++++++++++
hw/arm/virt-acpi-build.c | 31 +++++--------------
hw/i386/acpi-build.c | 44 +++++----------------------
include/hw/acpi/acpi-defs.h | 18 -----------
include/hw/acpi/pci.h | 34 +++++++++++++++++++++
9 files changed, 108 insertions(+), 78 deletions(-)
create mode 100644 hw/acpi/pci.c
create mode 100644 include/hw/acpi/pci.h
--
2.19.1
^ permalink raw reply [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v3 1/6] q35: acpi: do not create dummy MCFG table
@ 2019-04-17 1:40 ` Wei Yang
0 siblings, 0 replies; 24+ messages in thread
From: Wei Yang @ 2019-04-17 1:40 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: mst, imammedo, shannon.zhaosl, peter.maydell, marcel.apfelbaum,
yang.zhong, philmd, Wei Yang
From: Igor Mammedov <imammedo@redhat.com>
Dummy table (with signature "QEMU") creation came from original SeaBIOS
codebase. And QEMU would have to keep it around if there were Q35 machine
that depended on keeping ACPI tables blob constant size. Luckily there
were no versioned Q35 machine types before commit:
(since 2.3) a1666142db acpi-build: make ROMs RAM blocks resizeable
which obsoleted need to keep ACPI tables blob the same size on source/destination.
Considering the 1st versioned machine is pc-q35-2.4, the dummy table
is not really necessary and it's safe to drop it without breaking
cross version migration in both directions unconditionally.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
hw/i386/acpi-build.c | 18 ++++--------------
1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b17d4a711d..d009176072 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2401,7 +2401,6 @@ static void
build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
{
AcpiTableMcfg *mcfg;
- const char *sig;
int len = sizeof(*mcfg) + 1 * sizeof(mcfg->allocation[0]);
mcfg = acpi_data_push(table_data, len);
@@ -2411,19 +2410,7 @@ build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
mcfg->allocation[0].start_bus_number = 0;
mcfg->allocation[0].end_bus_number = PCIE_MMCFG_BUS(info->mcfg_size - 1);
- /* MCFG is used for ECAM which can be enabled or disabled by guest.
- * To avoid table size changes (which create migration issues),
- * always create the table even if there are no allocations,
- * but set the signature to a reserved value in this case.
- * ACPI spec requires OSPMs to ignore such tables.
- */
- if (info->mcfg_base == PCIE_BASE_ADDR_UNMAPPED) {
- /* Reserved signature: ignored by OSPM */
- sig = "QEMU";
- } else {
- sig = "MCFG";
- }
- build_header(linker, table_data, (void *)mcfg, sig, len, 1, NULL, NULL);
+ build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1, NULL, NULL);
}
/*
@@ -2592,6 +2579,9 @@ static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg)
}
mcfg->mcfg_base = qnum_get_uint(qobject_to(QNum, o));
qobject_unref(o);
+ if (mcfg->mcfg_base == PCIE_BASE_ADDR_UNMAPPED) {
+ return false;
+ }
o = object_property_get_qobject(pci_host, PCIE_HOST_MCFG_SIZE, NULL);
assert(o);
--
2.19.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v3 1/6] q35: acpi: do not create dummy MCFG table
@ 2019-04-17 1:40 ` Wei Yang
0 siblings, 0 replies; 24+ messages in thread
From: Wei Yang @ 2019-04-17 1:40 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: yang.zhong, peter.maydell, mst, shannon.zhaosl, Wei Yang,
imammedo, philmd
From: Igor Mammedov <imammedo@redhat.com>
Dummy table (with signature "QEMU") creation came from original SeaBIOS
codebase. And QEMU would have to keep it around if there were Q35 machine
that depended on keeping ACPI tables blob constant size. Luckily there
were no versioned Q35 machine types before commit:
(since 2.3) a1666142db acpi-build: make ROMs RAM blocks resizeable
which obsoleted need to keep ACPI tables blob the same size on source/destination.
Considering the 1st versioned machine is pc-q35-2.4, the dummy table
is not really necessary and it's safe to drop it without breaking
cross version migration in both directions unconditionally.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
hw/i386/acpi-build.c | 18 ++++--------------
1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b17d4a711d..d009176072 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2401,7 +2401,6 @@ static void
build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
{
AcpiTableMcfg *mcfg;
- const char *sig;
int len = sizeof(*mcfg) + 1 * sizeof(mcfg->allocation[0]);
mcfg = acpi_data_push(table_data, len);
@@ -2411,19 +2410,7 @@ build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
mcfg->allocation[0].start_bus_number = 0;
mcfg->allocation[0].end_bus_number = PCIE_MMCFG_BUS(info->mcfg_size - 1);
- /* MCFG is used for ECAM which can be enabled or disabled by guest.
- * To avoid table size changes (which create migration issues),
- * always create the table even if there are no allocations,
- * but set the signature to a reserved value in this case.
- * ACPI spec requires OSPMs to ignore such tables.
- */
- if (info->mcfg_base == PCIE_BASE_ADDR_UNMAPPED) {
- /* Reserved signature: ignored by OSPM */
- sig = "QEMU";
- } else {
- sig = "MCFG";
- }
- build_header(linker, table_data, (void *)mcfg, sig, len, 1, NULL, NULL);
+ build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1, NULL, NULL);
}
/*
@@ -2592,6 +2579,9 @@ static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg)
}
mcfg->mcfg_base = qnum_get_uint(qobject_to(QNum, o));
qobject_unref(o);
+ if (mcfg->mcfg_base == PCIE_BASE_ADDR_UNMAPPED) {
+ return false;
+ }
o = object_property_get_qobject(pci_host, PCIE_HOST_MCFG_SIZE, NULL);
assert(o);
--
2.19.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v3 2/6] hw/arm/virt-acpi-build: remove unnecessary variable mcfg_start
@ 2019-04-17 1:40 ` Wei Yang
0 siblings, 0 replies; 24+ messages in thread
From: Wei Yang @ 2019-04-17 1:40 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: mst, imammedo, shannon.zhaosl, peter.maydell, marcel.apfelbaum,
yang.zhong, philmd, Wei Yang
mcfg_start points to the start of MCFG table and is used in
build_header. While this information could be derived from mcfg.
This patch removes the unnecessary variable mcfg_start.
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/arm/virt-acpi-build.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 857989362a..e09e7eff8d 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -552,7 +552,6 @@ build_mcfg(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
const MemMapEntry *memmap = vms->memmap;
int ecam_id = VIRT_ECAM_ID(vms->highmem_ecam);
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);
@@ -563,8 +562,7 @@ build_mcfg(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
mcfg->allocation[0].end_bus_number =
PCIE_MMCFG_BUS(memmap[ecam_id].size - 1);
- build_header(linker, table_data, (void *)(table_data->data + mcfg_start),
- "MCFG", table_data->len - mcfg_start, 1, NULL, NULL);
+ build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1, NULL, NULL);
}
/* GTDT */
--
2.19.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v3 2/6] hw/arm/virt-acpi-build: remove unnecessary variable mcfg_start
@ 2019-04-17 1:40 ` Wei Yang
0 siblings, 0 replies; 24+ messages in thread
From: Wei Yang @ 2019-04-17 1:40 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: yang.zhong, peter.maydell, mst, shannon.zhaosl, Wei Yang,
imammedo, philmd
mcfg_start points to the start of MCFG table and is used in
build_header. While this information could be derived from mcfg.
This patch removes the unnecessary variable mcfg_start.
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/arm/virt-acpi-build.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 857989362a..e09e7eff8d 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -552,7 +552,6 @@ build_mcfg(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
const MemMapEntry *memmap = vms->memmap;
int ecam_id = VIRT_ECAM_ID(vms->highmem_ecam);
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);
@@ -563,8 +562,7 @@ build_mcfg(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
mcfg->allocation[0].end_bus_number =
PCIE_MMCFG_BUS(memmap[ecam_id].size - 1);
- build_header(linker, table_data, (void *)(table_data->data + mcfg_start),
- "MCFG", table_data->len - mcfg_start, 1, NULL, NULL);
+ build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1, NULL, NULL);
}
/* GTDT */
--
2.19.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v3 3/6] i386, acpi: remove mcfg_ prefix in AcpiMcfgInfo members
@ 2019-04-17 1:40 ` Wei Yang
0 siblings, 0 replies; 24+ messages in thread
From: Wei Yang @ 2019-04-17 1:40 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: mst, imammedo, shannon.zhaosl, peter.maydell, marcel.apfelbaum,
yang.zhong, philmd, Wei Yang
This is obvious the member in AcpiMcfgInfo describe MCFG's property.
Remove the mcfg_ prefix.
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
Suggested-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/i386/acpi-build.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index d009176072..f0d27bffd6 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -87,8 +87,8 @@
#define ACPI_BUILD_IOAPIC_ID 0x0
typedef struct AcpiMcfgInfo {
- uint64_t mcfg_base;
- uint32_t mcfg_size;
+ uint64_t base;
+ uint32_t size;
} AcpiMcfgInfo;
typedef struct AcpiPmInfo {
@@ -2404,11 +2404,11 @@ build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
int len = sizeof(*mcfg) + 1 * sizeof(mcfg->allocation[0]);
mcfg = acpi_data_push(table_data, len);
- mcfg->allocation[0].address = cpu_to_le64(info->mcfg_base);
+ 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->mcfg_size - 1);
+ mcfg->allocation[0].end_bus_number = PCIE_MMCFG_BUS(info->size - 1);
build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1, NULL, NULL);
}
@@ -2577,15 +2577,15 @@ static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg)
if (!o) {
return false;
}
- mcfg->mcfg_base = qnum_get_uint(qobject_to(QNum, o));
+ mcfg->base = qnum_get_uint(qobject_to(QNum, o));
qobject_unref(o);
- if (mcfg->mcfg_base == PCIE_BASE_ADDR_UNMAPPED) {
+ if (mcfg->base == PCIE_BASE_ADDR_UNMAPPED) {
return false;
}
o = object_property_get_qobject(pci_host, PCIE_HOST_MCFG_SIZE, NULL);
assert(o);
- mcfg->mcfg_size = qnum_get_uint(qobject_to(QNum, o));
+ mcfg->size = qnum_get_uint(qobject_to(QNum, o));
qobject_unref(o);
return true;
}
--
2.19.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v3 3/6] i386, acpi: remove mcfg_ prefix in AcpiMcfgInfo members
@ 2019-04-17 1:40 ` Wei Yang
0 siblings, 0 replies; 24+ messages in thread
From: Wei Yang @ 2019-04-17 1:40 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: yang.zhong, peter.maydell, mst, shannon.zhaosl, Wei Yang,
imammedo, philmd
This is obvious the member in AcpiMcfgInfo describe MCFG's property.
Remove the mcfg_ prefix.
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
Suggested-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/i386/acpi-build.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index d009176072..f0d27bffd6 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -87,8 +87,8 @@
#define ACPI_BUILD_IOAPIC_ID 0x0
typedef struct AcpiMcfgInfo {
- uint64_t mcfg_base;
- uint32_t mcfg_size;
+ uint64_t base;
+ uint32_t size;
} AcpiMcfgInfo;
typedef struct AcpiPmInfo {
@@ -2404,11 +2404,11 @@ build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
int len = sizeof(*mcfg) + 1 * sizeof(mcfg->allocation[0]);
mcfg = acpi_data_push(table_data, len);
- mcfg->allocation[0].address = cpu_to_le64(info->mcfg_base);
+ 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->mcfg_size - 1);
+ mcfg->allocation[0].end_bus_number = PCIE_MMCFG_BUS(info->size - 1);
build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1, NULL, NULL);
}
@@ -2577,15 +2577,15 @@ static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg)
if (!o) {
return false;
}
- mcfg->mcfg_base = qnum_get_uint(qobject_to(QNum, o));
+ mcfg->base = qnum_get_uint(qobject_to(QNum, o));
qobject_unref(o);
- if (mcfg->mcfg_base == PCIE_BASE_ADDR_UNMAPPED) {
+ if (mcfg->base == PCIE_BASE_ADDR_UNMAPPED) {
return false;
}
o = object_property_get_qobject(pci_host, PCIE_HOST_MCFG_SIZE, NULL);
assert(o);
- mcfg->mcfg_size = qnum_get_uint(qobject_to(QNum, o));
+ mcfg->size = qnum_get_uint(qobject_to(QNum, o));
qobject_unref(o);
return true;
}
--
2.19.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v3 4/6] hw/arm/virt-acpi-build: pass AcpiMcfgInfo to build_mcfg()
@ 2019-04-17 1:40 ` Wei Yang
0 siblings, 0 replies; 24+ messages in thread
From: Wei Yang @ 2019-04-17 1:40 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: mst, imammedo, shannon.zhaosl, peter.maydell, marcel.apfelbaum,
yang.zhong, philmd, Wei Yang
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>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v3:
* move AcpiMcfgInfo to pci.h
v2:
* for arm platform, construct a AcpiMcfgInfo directly
---
hw/arm/virt-acpi-build.c | 18 +++++++++++-------
hw/i386/acpi-build.c | 6 +-----
include/hw/acpi/pci.h | 33 +++++++++++++++++++++++++++++++++
3 files changed, 45 insertions(+), 12 deletions(-)
create mode 100644 include/hw/acpi/pci.h
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index e09e7eff8d..ebddcde596 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -40,6 +40,7 @@
#include "hw/hw.h"
#include "hw/acpi/aml-build.h"
#include "hw/acpi/utils.h"
+#include "hw/acpi/pci.h"
#include "hw/pci/pcie_host.h"
#include "hw/pci/pci.h"
#include "hw/arm/virt.h"
@@ -546,21 +547,18 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
}
static void
-build_mcfg(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
+build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
{
AcpiTableMcfg *mcfg;
- const MemMapEntry *memmap = vms->memmap;
- int ecam_id = VIRT_ECAM_ID(vms->highmem_ecam);
int len = sizeof(*mcfg) + sizeof(mcfg->allocation[0]);
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->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->size - 1);
build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1, NULL, NULL);
}
@@ -801,7 +799,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);
+ {
+ AcpiMcfgInfo mcfg = {
+ .base = vms->memmap[VIRT_ECAM_ID(vms->highmem_ecam)].base,
+ .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);
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index f0d27bffd6..c2de7f4b01 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -58,6 +58,7 @@
#include "hw/acpi/aml-build.h"
#include "hw/acpi/utils.h"
+#include "hw/acpi/pci.h"
#include "qom/qom-qobject.h"
#include "hw/i386/amd_iommu.h"
@@ -86,11 +87,6 @@
/* Default IOAPIC ID */
#define ACPI_BUILD_IOAPIC_ID 0x0
-typedef struct AcpiMcfgInfo {
- uint64_t base;
- uint32_t size;
-} AcpiMcfgInfo;
-
typedef struct AcpiPmInfo {
bool s3_disabled;
bool s4_disabled;
diff --git a/include/hw/acpi/pci.h b/include/hw/acpi/pci.h
new file mode 100644
index 0000000000..124af7d32a
--- /dev/null
+++ b/include/hw/acpi/pci.h
@@ -0,0 +1,33 @@
+/*
+ * Support for generating PCI related ACPI tables and passing them to Guests
+ *
+ * Copyright (C) 2006 Fabrice Bellard
+ * Copyright (C) 2008-2010 Kevin O'Connor <kevin@koconnor.net>
+ * Copyright (C) 2013-2019 Red Hat Inc
+ * Copyright (C) 2019 Intel Corporation
+ *
+ * Author: Wei Yang <richardw.yang@linux.intel.com>
+ * Author: Michael S. Tsirkin <mst@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef HW_ACPI_PCI_H
+#define HW_ACPI_PCI_H
+
+typedef struct AcpiMcfgInfo {
+ uint64_t base;
+ uint32_t size;
+} AcpiMcfgInfo;
+
+#endif
--
2.19.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v3 4/6] hw/arm/virt-acpi-build: pass AcpiMcfgInfo to build_mcfg()
@ 2019-04-17 1:40 ` Wei Yang
0 siblings, 0 replies; 24+ messages in thread
From: Wei Yang @ 2019-04-17 1:40 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: yang.zhong, peter.maydell, mst, shannon.zhaosl, Wei Yang,
imammedo, philmd
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>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v3:
* move AcpiMcfgInfo to pci.h
v2:
* for arm platform, construct a AcpiMcfgInfo directly
---
hw/arm/virt-acpi-build.c | 18 +++++++++++-------
hw/i386/acpi-build.c | 6 +-----
include/hw/acpi/pci.h | 33 +++++++++++++++++++++++++++++++++
3 files changed, 45 insertions(+), 12 deletions(-)
create mode 100644 include/hw/acpi/pci.h
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index e09e7eff8d..ebddcde596 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -40,6 +40,7 @@
#include "hw/hw.h"
#include "hw/acpi/aml-build.h"
#include "hw/acpi/utils.h"
+#include "hw/acpi/pci.h"
#include "hw/pci/pcie_host.h"
#include "hw/pci/pci.h"
#include "hw/arm/virt.h"
@@ -546,21 +547,18 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
}
static void
-build_mcfg(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
+build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
{
AcpiTableMcfg *mcfg;
- const MemMapEntry *memmap = vms->memmap;
- int ecam_id = VIRT_ECAM_ID(vms->highmem_ecam);
int len = sizeof(*mcfg) + sizeof(mcfg->allocation[0]);
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->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->size - 1);
build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1, NULL, NULL);
}
@@ -801,7 +799,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);
+ {
+ AcpiMcfgInfo mcfg = {
+ .base = vms->memmap[VIRT_ECAM_ID(vms->highmem_ecam)].base,
+ .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);
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index f0d27bffd6..c2de7f4b01 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -58,6 +58,7 @@
#include "hw/acpi/aml-build.h"
#include "hw/acpi/utils.h"
+#include "hw/acpi/pci.h"
#include "qom/qom-qobject.h"
#include "hw/i386/amd_iommu.h"
@@ -86,11 +87,6 @@
/* Default IOAPIC ID */
#define ACPI_BUILD_IOAPIC_ID 0x0
-typedef struct AcpiMcfgInfo {
- uint64_t base;
- uint32_t size;
-} AcpiMcfgInfo;
-
typedef struct AcpiPmInfo {
bool s3_disabled;
bool s4_disabled;
diff --git a/include/hw/acpi/pci.h b/include/hw/acpi/pci.h
new file mode 100644
index 0000000000..124af7d32a
--- /dev/null
+++ b/include/hw/acpi/pci.h
@@ -0,0 +1,33 @@
+/*
+ * Support for generating PCI related ACPI tables and passing them to Guests
+ *
+ * Copyright (C) 2006 Fabrice Bellard
+ * Copyright (C) 2008-2010 Kevin O'Connor <kevin@koconnor.net>
+ * Copyright (C) 2013-2019 Red Hat Inc
+ * Copyright (C) 2019 Intel Corporation
+ *
+ * Author: Wei Yang <richardw.yang@linux.intel.com>
+ * Author: Michael S. Tsirkin <mst@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef HW_ACPI_PCI_H
+#define HW_ACPI_PCI_H
+
+typedef struct AcpiMcfgInfo {
+ uint64_t base;
+ uint32_t size;
+} AcpiMcfgInfo;
+
+#endif
--
2.19.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v3 5/6] hw/acpi: Consolidate build_mcfg to pci.c
@ 2019-04-17 1:40 ` Wei Yang
0 siblings, 0 replies; 24+ messages in thread
From: Wei Yang @ 2019-04-17 1:40 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: mst, imammedo, shannon.zhaosl, peter.maydell, marcel.apfelbaum,
yang.zhong, philmd, Wei Yang
Now we have two identical build_mcfg functions.
Consolidate them in acpi/pci.c.
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
v3:
* adjust changelog based on Igor's suggestion
---
default-configs/arm-softmmu.mak | 1 +
default-configs/i386-softmmu.mak | 1 +
hw/acpi/Kconfig | 4 +++
hw/acpi/Makefile.objs | 1 +
hw/acpi/pci.c | 46 ++++++++++++++++++++++++++++++++
hw/arm/virt-acpi-build.c | 17 ------------
hw/i386/acpi-build.c | 18 +------------
include/hw/acpi/pci.h | 1 +
8 files changed, 55 insertions(+), 34 deletions(-)
create mode 100644 hw/acpi/pci.c
diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 613d19a06d..8f2796e195 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -144,6 +144,7 @@ CONFIG_XIO3130=y
CONFIG_IOH3420=y
CONFIG_I82801B11=y
CONFIG_ACPI=y
+CONFIG_ACPI_PCI=y
CONFIG_ARM_VIRT=y
CONFIG_SMBIOS=y
CONFIG_ASPEED_SOC=y
diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index ba3fb3ff50..cd5ea391e8 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -25,3 +25,4 @@
CONFIG_ISAPC=y
CONFIG_I440FX=y
CONFIG_Q35=y
+CONFIG_ACPI_PCI=y
diff --git a/hw/acpi/Kconfig b/hw/acpi/Kconfig
index eca3beed75..7265843cc3 100644
--- a/hw/acpi/Kconfig
+++ b/hw/acpi/Kconfig
@@ -23,6 +23,10 @@ config ACPI_NVDIMM
bool
depends on ACPI
+config ACPI_PCI
+ bool
+ depends on ACPI
+
config ACPI_VMGENID
bool
default y
diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
index ba93c5b64a..9bb2101e3b 100644
--- a/hw/acpi/Makefile.objs
+++ b/hw/acpi/Makefile.objs
@@ -11,6 +11,7 @@ common-obj-$(call lnot,$(CONFIG_ACPI_X86)) += acpi-stub.o
common-obj-y += acpi_interface.o
common-obj-y += bios-linker-loader.o
common-obj-y += aml-build.o utils.o
+common-obj-$(CONFIG_ACPI_PCI) += pci.o
common-obj-$(CONFIG_TPM) += tpm.o
common-obj-$(CONFIG_IPMI) += ipmi.o
diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
new file mode 100644
index 0000000000..fa0fa30bb9
--- /dev/null
+++ b/hw/acpi/pci.c
@@ -0,0 +1,46 @@
+/*
+ * Support for generating PCI related ACPI tables and passing them to Guests
+ *
+ * Copyright (C) 2006 Fabrice Bellard
+ * Copyright (C) 2008-2010 Kevin O'Connor <kevin@koconnor.net>
+ * Copyright (C) 2013-2019 Red Hat Inc
+ * Copyright (C) 2019 Intel Corporation
+ *
+ * Author: Wei Yang <richardw.yang@linux.intel.com>
+ * Author: Michael S. Tsirkin <mst@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/acpi/aml-build.h"
+#include "hw/acpi/pci.h"
+#include "hw/pci/pcie_host.h"
+
+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);
+}
+
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index ebddcde596..e3353de9e4 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -546,23 +546,6 @@ 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, 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);
-}
-
/* GTDT */
static void
build_gtdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index c2de7f4b01..29980bb3f4 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2393,22 +2393,6 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
table_data->len - srat_start, 1, NULL, NULL);
}
-static void
-build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
-{
- AcpiTableMcfg *mcfg;
- int len = sizeof(*mcfg) + 1 * 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);
-}
-
/*
* VT-d spec 8.1 DMA Remapping Reporting Structure
* (version Oct. 2014 or later)
@@ -2678,7 +2662,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
}
if (acpi_get_mcfg(&mcfg)) {
acpi_add_table(table_offsets, tables_blob);
- build_mcfg_q35(tables_blob, tables->linker, &mcfg);
+ build_mcfg(tables_blob, tables->linker, &mcfg);
}
if (x86_iommu_get_default()) {
IommuType IOMMUType = x86_iommu_get_type();
diff --git a/include/hw/acpi/pci.h b/include/hw/acpi/pci.h
index 124af7d32a..8bbd32cf45 100644
--- a/include/hw/acpi/pci.h
+++ b/include/hw/acpi/pci.h
@@ -30,4 +30,5 @@ typedef struct AcpiMcfgInfo {
uint32_t size;
} AcpiMcfgInfo;
+void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info);
#endif
--
2.19.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v3 5/6] hw/acpi: Consolidate build_mcfg to pci.c
@ 2019-04-17 1:40 ` Wei Yang
0 siblings, 0 replies; 24+ messages in thread
From: Wei Yang @ 2019-04-17 1:40 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: yang.zhong, peter.maydell, mst, shannon.zhaosl, Wei Yang,
imammedo, philmd
Now we have two identical build_mcfg functions.
Consolidate them in acpi/pci.c.
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
v3:
* adjust changelog based on Igor's suggestion
---
default-configs/arm-softmmu.mak | 1 +
default-configs/i386-softmmu.mak | 1 +
hw/acpi/Kconfig | 4 +++
hw/acpi/Makefile.objs | 1 +
hw/acpi/pci.c | 46 ++++++++++++++++++++++++++++++++
hw/arm/virt-acpi-build.c | 17 ------------
hw/i386/acpi-build.c | 18 +------------
include/hw/acpi/pci.h | 1 +
8 files changed, 55 insertions(+), 34 deletions(-)
create mode 100644 hw/acpi/pci.c
diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 613d19a06d..8f2796e195 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -144,6 +144,7 @@ CONFIG_XIO3130=y
CONFIG_IOH3420=y
CONFIG_I82801B11=y
CONFIG_ACPI=y
+CONFIG_ACPI_PCI=y
CONFIG_ARM_VIRT=y
CONFIG_SMBIOS=y
CONFIG_ASPEED_SOC=y
diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index ba3fb3ff50..cd5ea391e8 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -25,3 +25,4 @@
CONFIG_ISAPC=y
CONFIG_I440FX=y
CONFIG_Q35=y
+CONFIG_ACPI_PCI=y
diff --git a/hw/acpi/Kconfig b/hw/acpi/Kconfig
index eca3beed75..7265843cc3 100644
--- a/hw/acpi/Kconfig
+++ b/hw/acpi/Kconfig
@@ -23,6 +23,10 @@ config ACPI_NVDIMM
bool
depends on ACPI
+config ACPI_PCI
+ bool
+ depends on ACPI
+
config ACPI_VMGENID
bool
default y
diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
index ba93c5b64a..9bb2101e3b 100644
--- a/hw/acpi/Makefile.objs
+++ b/hw/acpi/Makefile.objs
@@ -11,6 +11,7 @@ common-obj-$(call lnot,$(CONFIG_ACPI_X86)) += acpi-stub.o
common-obj-y += acpi_interface.o
common-obj-y += bios-linker-loader.o
common-obj-y += aml-build.o utils.o
+common-obj-$(CONFIG_ACPI_PCI) += pci.o
common-obj-$(CONFIG_TPM) += tpm.o
common-obj-$(CONFIG_IPMI) += ipmi.o
diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
new file mode 100644
index 0000000000..fa0fa30bb9
--- /dev/null
+++ b/hw/acpi/pci.c
@@ -0,0 +1,46 @@
+/*
+ * Support for generating PCI related ACPI tables and passing them to Guests
+ *
+ * Copyright (C) 2006 Fabrice Bellard
+ * Copyright (C) 2008-2010 Kevin O'Connor <kevin@koconnor.net>
+ * Copyright (C) 2013-2019 Red Hat Inc
+ * Copyright (C) 2019 Intel Corporation
+ *
+ * Author: Wei Yang <richardw.yang@linux.intel.com>
+ * Author: Michael S. Tsirkin <mst@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/acpi/aml-build.h"
+#include "hw/acpi/pci.h"
+#include "hw/pci/pcie_host.h"
+
+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);
+}
+
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index ebddcde596..e3353de9e4 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -546,23 +546,6 @@ 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, 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);
-}
-
/* GTDT */
static void
build_gtdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index c2de7f4b01..29980bb3f4 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2393,22 +2393,6 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
table_data->len - srat_start, 1, NULL, NULL);
}
-static void
-build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
-{
- AcpiTableMcfg *mcfg;
- int len = sizeof(*mcfg) + 1 * 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);
-}
-
/*
* VT-d spec 8.1 DMA Remapping Reporting Structure
* (version Oct. 2014 or later)
@@ -2678,7 +2662,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
}
if (acpi_get_mcfg(&mcfg)) {
acpi_add_table(table_offsets, tables_blob);
- build_mcfg_q35(tables_blob, tables->linker, &mcfg);
+ build_mcfg(tables_blob, tables->linker, &mcfg);
}
if (x86_iommu_get_default()) {
IommuType IOMMUType = x86_iommu_get_type();
diff --git a/include/hw/acpi/pci.h b/include/hw/acpi/pci.h
index 124af7d32a..8bbd32cf45 100644
--- a/include/hw/acpi/pci.h
+++ b/include/hw/acpi/pci.h
@@ -30,4 +30,5 @@ typedef struct AcpiMcfgInfo {
uint32_t size;
} AcpiMcfgInfo;
+void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info);
#endif
--
2.19.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v3 6/6] acpi: pci: use build_append_foo() API to construct MCFG
@ 2019-04-17 1:40 ` Wei Yang
0 siblings, 0 replies; 24+ messages in thread
From: Wei Yang @ 2019-04-17 1:40 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: mst, imammedo, shannon.zhaosl, peter.maydell, marcel.apfelbaum,
yang.zhong, philmd, Wei Yang
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 <richardw.yang@linux.intel.com>
Suggested-by: Igor Mammedov <imammedo@redhat.com>
---
hw/acpi/pci.c | 30 ++++++++++++++++++------------
include/hw/acpi/acpi-defs.h | 18 ------------------
2 files changed, 18 insertions(+), 30 deletions(-)
diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
index fa0fa30bb9..05050fa087 100644
--- a/hw/acpi/pci.c
+++ b/hw/acpi/pci.c
@@ -30,17 +30,23 @@
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));
+
+ /* PCI fw r3.0 MCFG table. */
+ /* 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];
- AcpiMcfgAllocation allocation[0];
-} QEMU_PACKED;
-typedef struct AcpiTableMcfg AcpiTableMcfg;
-
/*
* TCPA Description Table
*
--
2.19.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v3 6/6] acpi: pci: use build_append_foo() API to construct MCFG
@ 2019-04-17 1:40 ` Wei Yang
0 siblings, 0 replies; 24+ messages in thread
From: Wei Yang @ 2019-04-17 1:40 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: yang.zhong, peter.maydell, mst, shannon.zhaosl, Wei Yang,
imammedo, philmd
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 <richardw.yang@linux.intel.com>
Suggested-by: Igor Mammedov <imammedo@redhat.com>
---
hw/acpi/pci.c | 30 ++++++++++++++++++------------
include/hw/acpi/acpi-defs.h | 18 ------------------
2 files changed, 18 insertions(+), 30 deletions(-)
diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
index fa0fa30bb9..05050fa087 100644
--- a/hw/acpi/pci.c
+++ b/hw/acpi/pci.c
@@ -30,17 +30,23 @@
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));
+
+ /* PCI fw r3.0 MCFG table. */
+ /* 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];
- AcpiMcfgAllocation allocation[0];
-} QEMU_PACKED;
-typedef struct AcpiTableMcfg AcpiTableMcfg;
-
/*
* TCPA Description Table
*
--
2.19.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v3 4/6] hw/arm/virt-acpi-build: pass AcpiMcfgInfo to build_mcfg()
@ 2019-04-18 9:42 ` Igor Mammedov
0 siblings, 0 replies; 24+ messages in thread
From: Igor Mammedov @ 2019-04-18 9:42 UTC (permalink / raw)
To: Wei Yang
Cc: qemu-devel, qemu-arm, mst, shannon.zhaosl, peter.maydell,
marcel.apfelbaum, yang.zhong, philmd
On Wed, 17 Apr 2019 09:40:36 +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>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>
> ---
> v3:
> * move AcpiMcfgInfo to pci.h
> v2:
> * for arm platform, construct a AcpiMcfgInfo directly
> ---
> hw/arm/virt-acpi-build.c | 18 +++++++++++-------
> hw/i386/acpi-build.c | 6 +-----
> include/hw/acpi/pci.h | 33 +++++++++++++++++++++++++++++++++
> 3 files changed, 45 insertions(+), 12 deletions(-)
> create mode 100644 include/hw/acpi/pci.h
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index e09e7eff8d..ebddcde596 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -40,6 +40,7 @@
> #include "hw/hw.h"
> #include "hw/acpi/aml-build.h"
> #include "hw/acpi/utils.h"
> +#include "hw/acpi/pci.h"
> #include "hw/pci/pcie_host.h"
> #include "hw/pci/pci.h"
> #include "hw/arm/virt.h"
> @@ -546,21 +547,18 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> }
>
> static void
> -build_mcfg(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> +build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
> {
> AcpiTableMcfg *mcfg;
> - const MemMapEntry *memmap = vms->memmap;
> - int ecam_id = VIRT_ECAM_ID(vms->highmem_ecam);
> int len = sizeof(*mcfg) + sizeof(mcfg->allocation[0]);
>
> 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->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->size - 1);
>
> build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1, NULL, NULL);
> }
> @@ -801,7 +799,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);
> + {
> + AcpiMcfgInfo mcfg = {
> + .base = vms->memmap[VIRT_ECAM_ID(vms->highmem_ecam)].base,
> + .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);
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index f0d27bffd6..c2de7f4b01 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -58,6 +58,7 @@
>
> #include "hw/acpi/aml-build.h"
> #include "hw/acpi/utils.h"
> +#include "hw/acpi/pci.h"
>
> #include "qom/qom-qobject.h"
> #include "hw/i386/amd_iommu.h"
> @@ -86,11 +87,6 @@
> /* Default IOAPIC ID */
> #define ACPI_BUILD_IOAPIC_ID 0x0
>
> -typedef struct AcpiMcfgInfo {
> - uint64_t base;
> - uint32_t size;
> -} AcpiMcfgInfo;
> -
> typedef struct AcpiPmInfo {
> bool s3_disabled;
> bool s4_disabled;
> diff --git a/include/hw/acpi/pci.h b/include/hw/acpi/pci.h
> new file mode 100644
> index 0000000000..124af7d32a
> --- /dev/null
> +++ b/include/hw/acpi/pci.h
> @@ -0,0 +1,33 @@
> +/*
> + * Support for generating PCI related ACPI tables and passing them to Guests
> + *
> + * Copyright (C) 2006 Fabrice Bellard
> + * Copyright (C) 2008-2010 Kevin O'Connor <kevin@koconnor.net>
> + * Copyright (C) 2013-2019 Red Hat Inc
> + * Copyright (C) 2019 Intel Corporation
> + *
> + * Author: Wei Yang <richardw.yang@linux.intel.com>
> + * Author: Michael S. Tsirkin <mst@redhat.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> +
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> +
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +#ifndef HW_ACPI_PCI_H
> +#define HW_ACPI_PCI_H
> +
> +typedef struct AcpiMcfgInfo {
> + uint64_t base;
> + uint32_t size;
> +} AcpiMcfgInfo;
> +
> +#endif
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v3 4/6] hw/arm/virt-acpi-build: pass AcpiMcfgInfo to build_mcfg()
@ 2019-04-18 9:42 ` Igor Mammedov
0 siblings, 0 replies; 24+ messages in thread
From: Igor Mammedov @ 2019-04-18 9:42 UTC (permalink / raw)
To: Wei Yang
Cc: yang.zhong, peter.maydell, mst, qemu-devel, shannon.zhaosl,
qemu-arm, philmd
On Wed, 17 Apr 2019 09:40:36 +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>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>
> ---
> v3:
> * move AcpiMcfgInfo to pci.h
> v2:
> * for arm platform, construct a AcpiMcfgInfo directly
> ---
> hw/arm/virt-acpi-build.c | 18 +++++++++++-------
> hw/i386/acpi-build.c | 6 +-----
> include/hw/acpi/pci.h | 33 +++++++++++++++++++++++++++++++++
> 3 files changed, 45 insertions(+), 12 deletions(-)
> create mode 100644 include/hw/acpi/pci.h
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index e09e7eff8d..ebddcde596 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -40,6 +40,7 @@
> #include "hw/hw.h"
> #include "hw/acpi/aml-build.h"
> #include "hw/acpi/utils.h"
> +#include "hw/acpi/pci.h"
> #include "hw/pci/pcie_host.h"
> #include "hw/pci/pci.h"
> #include "hw/arm/virt.h"
> @@ -546,21 +547,18 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> }
>
> static void
> -build_mcfg(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> +build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
> {
> AcpiTableMcfg *mcfg;
> - const MemMapEntry *memmap = vms->memmap;
> - int ecam_id = VIRT_ECAM_ID(vms->highmem_ecam);
> int len = sizeof(*mcfg) + sizeof(mcfg->allocation[0]);
>
> 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->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->size - 1);
>
> build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1, NULL, NULL);
> }
> @@ -801,7 +799,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);
> + {
> + AcpiMcfgInfo mcfg = {
> + .base = vms->memmap[VIRT_ECAM_ID(vms->highmem_ecam)].base,
> + .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);
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index f0d27bffd6..c2de7f4b01 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -58,6 +58,7 @@
>
> #include "hw/acpi/aml-build.h"
> #include "hw/acpi/utils.h"
> +#include "hw/acpi/pci.h"
>
> #include "qom/qom-qobject.h"
> #include "hw/i386/amd_iommu.h"
> @@ -86,11 +87,6 @@
> /* Default IOAPIC ID */
> #define ACPI_BUILD_IOAPIC_ID 0x0
>
> -typedef struct AcpiMcfgInfo {
> - uint64_t base;
> - uint32_t size;
> -} AcpiMcfgInfo;
> -
> typedef struct AcpiPmInfo {
> bool s3_disabled;
> bool s4_disabled;
> diff --git a/include/hw/acpi/pci.h b/include/hw/acpi/pci.h
> new file mode 100644
> index 0000000000..124af7d32a
> --- /dev/null
> +++ b/include/hw/acpi/pci.h
> @@ -0,0 +1,33 @@
> +/*
> + * Support for generating PCI related ACPI tables and passing them to Guests
> + *
> + * Copyright (C) 2006 Fabrice Bellard
> + * Copyright (C) 2008-2010 Kevin O'Connor <kevin@koconnor.net>
> + * Copyright (C) 2013-2019 Red Hat Inc
> + * Copyright (C) 2019 Intel Corporation
> + *
> + * Author: Wei Yang <richardw.yang@linux.intel.com>
> + * Author: Michael S. Tsirkin <mst@redhat.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> +
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> +
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +#ifndef HW_ACPI_PCI_H
> +#define HW_ACPI_PCI_H
> +
> +typedef struct AcpiMcfgInfo {
> + uint64_t base;
> + uint32_t size;
> +} AcpiMcfgInfo;
> +
> +#endif
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v3 6/6] acpi: pci: use build_append_foo() API to construct MCFG
@ 2019-04-18 11:02 ` Igor Mammedov
0 siblings, 0 replies; 24+ messages in thread
From: Igor Mammedov @ 2019-04-18 11:02 UTC (permalink / raw)
To: Wei Yang
Cc: qemu-devel, qemu-arm, mst, shannon.zhaosl, peter.maydell,
marcel.apfelbaum, yang.zhong, philmd
On Wed, 17 Apr 2019 09:40:38 +0800
Wei Yang <richardw.yang@linux.intel.com> 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 <richardw.yang@linux.intel.com>
> Suggested-by: Igor Mammedov <imammedo@redhat.com>
Thanks for nice cleanup!
with comment fixed up:
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> hw/acpi/pci.c | 30 ++++++++++++++++++------------
> include/hw/acpi/acpi-defs.h | 18 ------------------
> 2 files changed, 18 insertions(+), 30 deletions(-)
>
> diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
> index fa0fa30bb9..05050fa087 100644
> --- a/hw/acpi/pci.c
> +++ b/hw/acpi/pci.c
> @@ -30,17 +30,23 @@
>
> 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));
> +
> + /* PCI fw r3.0 MCFG table. */
It would be better to put spec name as is here so simple search
could immediately point to it.
like: 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];
> - AcpiMcfgAllocation allocation[0];
> -} QEMU_PACKED;
> -typedef struct AcpiTableMcfg AcpiTableMcfg;
> -
> /*
> * TCPA Description Table
> *
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v3 6/6] acpi: pci: use build_append_foo() API to construct MCFG
@ 2019-04-18 11:02 ` Igor Mammedov
0 siblings, 0 replies; 24+ messages in thread
From: Igor Mammedov @ 2019-04-18 11:02 UTC (permalink / raw)
To: Wei Yang
Cc: yang.zhong, peter.maydell, mst, qemu-devel, shannon.zhaosl,
qemu-arm, philmd
On Wed, 17 Apr 2019 09:40:38 +0800
Wei Yang <richardw.yang@linux.intel.com> 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 <richardw.yang@linux.intel.com>
> Suggested-by: Igor Mammedov <imammedo@redhat.com>
Thanks for nice cleanup!
with comment fixed up:
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> hw/acpi/pci.c | 30 ++++++++++++++++++------------
> include/hw/acpi/acpi-defs.h | 18 ------------------
> 2 files changed, 18 insertions(+), 30 deletions(-)
>
> diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
> index fa0fa30bb9..05050fa087 100644
> --- a/hw/acpi/pci.c
> +++ b/hw/acpi/pci.c
> @@ -30,17 +30,23 @@
>
> 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));
> +
> + /* PCI fw r3.0 MCFG table. */
It would be better to put spec name as is here so simple search
could immediately point to it.
like: 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];
> - AcpiMcfgAllocation allocation[0];
> -} QEMU_PACKED;
> -typedef struct AcpiTableMcfg AcpiTableMcfg;
> -
> /*
> * TCPA Description Table
> *
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/6] q35: acpi: do not create dummy MCFG table
@ 2019-04-18 12:27 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 24+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-04-18 12:27 UTC (permalink / raw)
To: Wei Yang, qemu-devel, qemu-arm
Cc: mst, imammedo, shannon.zhaosl, peter.maydell, marcel.apfelbaum,
yang.zhong
On 4/17/19 3:40 AM, Wei Yang wrote:
> From: Igor Mammedov <imammedo@redhat.com>
>
> Dummy table (with signature "QEMU") creation came from original SeaBIOS
> codebase. And QEMU would have to keep it around if there were Q35 machine
> that depended on keeping ACPI tables blob constant size. Luckily there
> were no versioned Q35 machine types before commit:
> (since 2.3) a1666142db acpi-build: make ROMs RAM blocks resizeable
> which obsoleted need to keep ACPI tables blob the same size on source/destination.
>
> Considering the 1st versioned machine is pc-q35-2.4, the dummy table
> is not really necessary and it's safe to drop it without breaking
> cross version migration in both directions unconditionally.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
> ---
> hw/i386/acpi-build.c | 18 ++++--------------
> 1 file changed, 4 insertions(+), 14 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index b17d4a711d..d009176072 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2401,7 +2401,6 @@ static void
> build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
> {
> AcpiTableMcfg *mcfg;
> - const char *sig;
> int len = sizeof(*mcfg) + 1 * sizeof(mcfg->allocation[0]);
>
> mcfg = acpi_data_push(table_data, len);
> @@ -2411,19 +2410,7 @@ build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
> mcfg->allocation[0].start_bus_number = 0;
> mcfg->allocation[0].end_bus_number = PCIE_MMCFG_BUS(info->mcfg_size - 1);
>
> - /* MCFG is used for ECAM which can be enabled or disabled by guest.
> - * To avoid table size changes (which create migration issues),
> - * always create the table even if there are no allocations,
> - * but set the signature to a reserved value in this case.
> - * ACPI spec requires OSPMs to ignore such tables.
> - */
> - if (info->mcfg_base == PCIE_BASE_ADDR_UNMAPPED) {
> - /* Reserved signature: ignored by OSPM */
> - sig = "QEMU";
> - } else {
> - sig = "MCFG";
> - }
> - build_header(linker, table_data, (void *)mcfg, sig, len, 1, NULL, NULL);
> + build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1, NULL, NULL);
> }
>
> /*
> @@ -2592,6 +2579,9 @@ static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg)
> }
> mcfg->mcfg_base = qnum_get_uint(qobject_to(QNum, o));
> qobject_unref(o);
> + if (mcfg->mcfg_base == PCIE_BASE_ADDR_UNMAPPED) {
> + return false;
> + }
>
> o = object_property_get_qobject(pci_host, PCIE_HOST_MCFG_SIZE, NULL);
> assert(o);
>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/6] q35: acpi: do not create dummy MCFG table
@ 2019-04-18 12:27 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 24+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-04-18 12:27 UTC (permalink / raw)
To: Wei Yang, qemu-devel, qemu-arm
Cc: yang.zhong, peter.maydell, mst, shannon.zhaosl, imammedo
On 4/17/19 3:40 AM, Wei Yang wrote:
> From: Igor Mammedov <imammedo@redhat.com>
>
> Dummy table (with signature "QEMU") creation came from original SeaBIOS
> codebase. And QEMU would have to keep it around if there were Q35 machine
> that depended on keeping ACPI tables blob constant size. Luckily there
> were no versioned Q35 machine types before commit:
> (since 2.3) a1666142db acpi-build: make ROMs RAM blocks resizeable
> which obsoleted need to keep ACPI tables blob the same size on source/destination.
>
> Considering the 1st versioned machine is pc-q35-2.4, the dummy table
> is not really necessary and it's safe to drop it without breaking
> cross version migration in both directions unconditionally.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
> ---
> hw/i386/acpi-build.c | 18 ++++--------------
> 1 file changed, 4 insertions(+), 14 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index b17d4a711d..d009176072 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2401,7 +2401,6 @@ static void
> build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
> {
> AcpiTableMcfg *mcfg;
> - const char *sig;
> int len = sizeof(*mcfg) + 1 * sizeof(mcfg->allocation[0]);
>
> mcfg = acpi_data_push(table_data, len);
> @@ -2411,19 +2410,7 @@ build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
> mcfg->allocation[0].start_bus_number = 0;
> mcfg->allocation[0].end_bus_number = PCIE_MMCFG_BUS(info->mcfg_size - 1);
>
> - /* MCFG is used for ECAM which can be enabled or disabled by guest.
> - * To avoid table size changes (which create migration issues),
> - * always create the table even if there are no allocations,
> - * but set the signature to a reserved value in this case.
> - * ACPI spec requires OSPMs to ignore such tables.
> - */
> - if (info->mcfg_base == PCIE_BASE_ADDR_UNMAPPED) {
> - /* Reserved signature: ignored by OSPM */
> - sig = "QEMU";
> - } else {
> - sig = "MCFG";
> - }
> - build_header(linker, table_data, (void *)mcfg, sig, len, 1, NULL, NULL);
> + build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1, NULL, NULL);
> }
>
> /*
> @@ -2592,6 +2579,9 @@ static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg)
> }
> mcfg->mcfg_base = qnum_get_uint(qobject_to(QNum, o));
> qobject_unref(o);
> + if (mcfg->mcfg_base == PCIE_BASE_ADDR_UNMAPPED) {
> + return false;
> + }
>
> o = object_property_get_qobject(pci_host, PCIE_HOST_MCFG_SIZE, NULL);
> assert(o);
>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v3 6/6] acpi: pci: use build_append_foo() API to construct MCFG
@ 2019-04-18 12:30 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 24+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-04-18 12:30 UTC (permalink / raw)
To: Igor Mammedov, Wei Yang
Cc: qemu-devel, qemu-arm, mst, shannon.zhaosl, peter.maydell,
marcel.apfelbaum, yang.zhong
On 4/18/19 1:02 PM, Igor Mammedov wrote:
> On Wed, 17 Apr 2019 09:40:38 +0800
> Wei Yang <richardw.yang@linux.intel.com> 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 <richardw.yang@linux.intel.com>
>> Suggested-by: Igor Mammedov <imammedo@redhat.com>
> Thanks for nice cleanup!
>
> with comment fixed up:
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>
>> ---
>> hw/acpi/pci.c | 30 ++++++++++++++++++------------
>> include/hw/acpi/acpi-defs.h | 18 ------------------
>> 2 files changed, 18 insertions(+), 30 deletions(-)
>>
>> diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
>> index fa0fa30bb9..05050fa087 100644
>> --- a/hw/acpi/pci.c
>> +++ b/hw/acpi/pci.c
>> @@ -30,17 +30,23 @@
>>
>> 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));
>> +
>> + /* PCI fw r3.0 MCFG table. */
> It would be better to put spec name as is here so simple search
> could immediately point to it.
>
> like: PCI Firmware Specification, Revision 3.0
> 4.1.2 MCFG Table Description
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> + /* 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];
>> - AcpiMcfgAllocation allocation[0];
>> -} QEMU_PACKED;
>> -typedef struct AcpiTableMcfg AcpiTableMcfg;
>> -
>> /*
>> * TCPA Description Table
>> *
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v3 6/6] acpi: pci: use build_append_foo() API to construct MCFG
@ 2019-04-18 12:30 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 24+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-04-18 12:30 UTC (permalink / raw)
To: Igor Mammedov, Wei Yang
Cc: yang.zhong, peter.maydell, mst, qemu-devel, shannon.zhaosl, qemu-arm
On 4/18/19 1:02 PM, Igor Mammedov wrote:
> On Wed, 17 Apr 2019 09:40:38 +0800
> Wei Yang <richardw.yang@linux.intel.com> 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 <richardw.yang@linux.intel.com>
>> Suggested-by: Igor Mammedov <imammedo@redhat.com>
> Thanks for nice cleanup!
>
> with comment fixed up:
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>
>> ---
>> hw/acpi/pci.c | 30 ++++++++++++++++++------------
>> include/hw/acpi/acpi-defs.h | 18 ------------------
>> 2 files changed, 18 insertions(+), 30 deletions(-)
>>
>> diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
>> index fa0fa30bb9..05050fa087 100644
>> --- a/hw/acpi/pci.c
>> +++ b/hw/acpi/pci.c
>> @@ -30,17 +30,23 @@
>>
>> 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));
>> +
>> + /* PCI fw r3.0 MCFG table. */
> It would be better to put spec name as is here so simple search
> could immediately point to it.
>
> like: PCI Firmware Specification, Revision 3.0
> 4.1.2 MCFG Table Description
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> + /* 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];
>> - AcpiMcfgAllocation allocation[0];
>> -} QEMU_PACKED;
>> -typedef struct AcpiTableMcfg AcpiTableMcfg;
>> -
>> /*
>> * TCPA Description Table
>> *
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v3 6/6] acpi: pci: use build_append_foo() API to construct MCFG
@ 2019-04-18 14:20 ` Wei Yang
0 siblings, 0 replies; 24+ messages in thread
From: Wei Yang @ 2019-04-18 14:20 UTC (permalink / raw)
To: Igor Mammedov
Cc: Wei Yang, yang.zhong, peter.maydell, mst, qemu-devel,
shannon.zhaosl, qemu-arm, philmd
On Thu, Apr 18, 2019 at 01:02:41PM +0200, Igor Mammedov wrote:
>On Wed, 17 Apr 2019 09:40:38 +0800
>Wei Yang <richardw.yang@linux.intel.com> 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 <richardw.yang@linux.intel.com>
>> Suggested-by: Igor Mammedov <imammedo@redhat.com>
>Thanks for nice cleanup!
Glad you like it. :-)
>
>with comment fixed up:
>Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>
>> ---
>> hw/acpi/pci.c | 30 ++++++++++++++++++------------
>> include/hw/acpi/acpi-defs.h | 18 ------------------
>> 2 files changed, 18 insertions(+), 30 deletions(-)
>>
>> diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
>> index fa0fa30bb9..05050fa087 100644
>> --- a/hw/acpi/pci.c
>> +++ b/hw/acpi/pci.c
>> @@ -30,17 +30,23 @@
>>
>> 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));
>> +
>> + /* PCI fw r3.0 MCFG table. */
>It would be better to put spec name as is here so simple search
>could immediately point to it.
>
>like: PCI Firmware Specification, Revision 3.0
> 4.1.2 MCFG Table Description
>
Sure, will change to this.
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v3 6/6] acpi: pci: use build_append_foo() API to construct MCFG
@ 2019-04-18 14:20 ` Wei Yang
0 siblings, 0 replies; 24+ messages in thread
From: Wei Yang @ 2019-04-18 14:20 UTC (permalink / raw)
To: Igor Mammedov
Cc: yang.zhong, peter.maydell, mst, qemu-devel, shannon.zhaosl,
qemu-arm, Wei Yang, philmd
On Thu, Apr 18, 2019 at 01:02:41PM +0200, Igor Mammedov wrote:
>On Wed, 17 Apr 2019 09:40:38 +0800
>Wei Yang <richardw.yang@linux.intel.com> 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 <richardw.yang@linux.intel.com>
>> Suggested-by: Igor Mammedov <imammedo@redhat.com>
>Thanks for nice cleanup!
Glad you like it. :-)
>
>with comment fixed up:
>Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>
>> ---
>> hw/acpi/pci.c | 30 ++++++++++++++++++------------
>> include/hw/acpi/acpi-defs.h | 18 ------------------
>> 2 files changed, 18 insertions(+), 30 deletions(-)
>>
>> diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
>> index fa0fa30bb9..05050fa087 100644
>> --- a/hw/acpi/pci.c
>> +++ b/hw/acpi/pci.c
>> @@ -30,17 +30,23 @@
>>
>> 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));
>> +
>> + /* PCI fw r3.0 MCFG table. */
>It would be better to put spec name as is here so simple search
>could immediately point to it.
>
>like: PCI Firmware Specification, Revision 3.0
> 4.1.2 MCFG Table Description
>
Sure, will change to this.
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2019-04-18 14:24 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-17 1:40 [Qemu-devel] [PATCH v3 0/6] Extract build_mcfg Wei Yang
2019-04-17 1:40 ` Wei Yang
2019-04-17 1:40 ` [Qemu-devel] [PATCH v3 1/6] q35: acpi: do not create dummy MCFG table Wei Yang
2019-04-17 1:40 ` Wei Yang
2019-04-18 12:27 ` Philippe Mathieu-Daudé
2019-04-18 12:27 ` Philippe Mathieu-Daudé
2019-04-17 1:40 ` [Qemu-devel] [PATCH v3 2/6] hw/arm/virt-acpi-build: remove unnecessary variable mcfg_start Wei Yang
2019-04-17 1:40 ` Wei Yang
2019-04-17 1:40 ` [Qemu-devel] [PATCH v3 3/6] i386, acpi: remove mcfg_ prefix in AcpiMcfgInfo members Wei Yang
2019-04-17 1:40 ` Wei Yang
2019-04-17 1:40 ` [Qemu-devel] [PATCH v3 4/6] hw/arm/virt-acpi-build: pass AcpiMcfgInfo to build_mcfg() Wei Yang
2019-04-17 1:40 ` Wei Yang
2019-04-18 9:42 ` Igor Mammedov
2019-04-18 9:42 ` Igor Mammedov
2019-04-17 1:40 ` [Qemu-devel] [PATCH v3 5/6] hw/acpi: Consolidate build_mcfg to pci.c Wei Yang
2019-04-17 1:40 ` Wei Yang
2019-04-17 1:40 ` [Qemu-devel] [PATCH v3 6/6] acpi: pci: use build_append_foo() API to construct MCFG Wei Yang
2019-04-17 1:40 ` Wei Yang
2019-04-18 11:02 ` Igor Mammedov
2019-04-18 11:02 ` Igor Mammedov
2019-04-18 12:30 ` Philippe Mathieu-Daudé
2019-04-18 12:30 ` Philippe Mathieu-Daudé
2019-04-18 14:20 ` Wei Yang
2019-04-18 14:20 ` Wei Yang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.