* [Qemu-devel] [PATCH 1/4] hw/pc: extract reserved memory end computation to a standalone function
2016-05-02 15:37 [Qemu-devel] [PATCH 0/4] pci: better support for 64-bit MMIO allocation Marcel Apfelbaum
@ 2016-05-02 15:37 ` Marcel Apfelbaum
2016-05-02 15:37 ` [Qemu-devel] [PATCH 2/4] pci: reserve 64 bit MMIO range for PCI hotplug Marcel Apfelbaum
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Marcel Apfelbaum @ 2016-05-02 15:37 UTC (permalink / raw)
To: qemu-devel; +Cc: marcel, mst, imammedo, lersek, ehabkost
This code will be reused when calculating 64-bit MMIO hotplug ranges.
Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
hw/i386/pc.c | 29 +++++++++++++++++++++--------
include/hw/i386/pc.h | 1 +
2 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2ac97c4..ae4a13d 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1280,6 +1280,7 @@ void pc_memory_init(PCMachineState *pcms,
FWCfgState *fw_cfg;
MachineState *machine = MACHINE(pcms);
PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
+ uint64_t res_mem_end;
assert(machine->ram_size == pcms->below_4g_mem_size +
pcms->above_4g_mem_size);
@@ -1375,15 +1376,10 @@ void pc_memory_init(PCMachineState *pcms,
rom_set_fw(fw_cfg);
- if (pcmc->has_reserved_memory && pcms->hotplug_memory.base) {
+ res_mem_end = pc_machine_get_reserved_memory_end(pcms);
+ if (res_mem_end) {
uint64_t *val = g_malloc(sizeof(*val));
- PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
- uint64_t res_mem_end = pcms->hotplug_memory.base;
-
- if (!pcmc->broken_reserved_end) {
- res_mem_end += memory_region_size(&pcms->hotplug_memory.mr);
- }
- *val = cpu_to_le64(ROUND_UP(res_mem_end, 0x1ULL << 30));
+ *val = cpu_to_le64(res_mem_end);
fw_cfg_add_file(fw_cfg, "etc/reserved-memory-end", val, sizeof(*val));
}
@@ -1849,6 +1845,23 @@ bool pc_machine_is_smm_enabled(PCMachineState *pcms)
return false;
}
+uint64_t pc_machine_get_reserved_memory_end(PCMachineState *pcms)
+{
+ PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
+ uint64_t res_mem_end = 0;
+
+ if (pcmc->has_reserved_memory && pcms->hotplug_memory.base) {
+ res_mem_end = pcms->hotplug_memory.base;
+
+ if (!pcmc->broken_reserved_end) {
+ res_mem_end += memory_region_size(&pcms->hotplug_memory.mr);
+ }
+ res_mem_end = ROUND_UP(res_mem_end, 0x1ULL << 30);
+ }
+
+ return res_mem_end;
+}
+
static void pc_machine_get_smm(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 96f0b66..7c25814 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -223,6 +223,7 @@ void i8042_setup_a20_line(ISADevice *dev, qemu_irq *a20_out);
extern int fd_bootchk;
bool pc_machine_is_smm_enabled(PCMachineState *pcms);
+uint64_t pc_machine_get_reserved_memory_end(PCMachineState *pcms);
void pc_register_ferr_irq(qemu_irq irq);
void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
--
2.4.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Qemu-devel] [PATCH 2/4] pci: reserve 64 bit MMIO range for PCI hotplug
2016-05-02 15:37 [Qemu-devel] [PATCH 0/4] pci: better support for 64-bit MMIO allocation Marcel Apfelbaum
2016-05-02 15:37 ` [Qemu-devel] [PATCH 1/4] hw/pc: extract reserved memory end computation to a standalone function Marcel Apfelbaum
@ 2016-05-02 15:37 ` Marcel Apfelbaum
2016-05-02 15:37 ` [Qemu-devel] [PATCH 3/4] acpi: refactor pxb crs computation Marcel Apfelbaum
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Marcel Apfelbaum @ 2016-05-02 15:37 UTC (permalink / raw)
To: qemu-devel; +Cc: marcel, mst, imammedo, lersek, ehabkost
Using the firmware assigned MMIO ranges for 64-bit PCI window
leads to zero space for hot-plugging PCI devices over 4G.
PC machines can use the whole CPU addressable range after
the space reserved for memory-hotplug.
Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
hw/pci/pci.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index bb605ef..44dd949 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -41,6 +41,7 @@
#include "hw/hotplug.h"
#include "hw/boards.h"
#include "qemu/cutils.h"
+#include "hw/i386/pc.h"
//#define DEBUG_PCI
#ifdef DEBUG_PCI
@@ -2467,8 +2468,19 @@ static void pci_dev_get_w64(PCIBus *b, PCIDevice *dev, void *opaque)
void pci_bus_get_w64_range(PCIBus *bus, Range *range)
{
- range->begin = range->end = 0;
- pci_for_each_device_under_bus(bus, pci_dev_get_w64, range);
+ Object *machine = qdev_get_machine();
+ if (object_dynamic_cast(machine, TYPE_PC_MACHINE)) {
+ PCMachineState *pcms = PC_MACHINE(machine);
+ range->begin = pc_machine_get_reserved_memory_end(pcms);
+ if (!range->begin) {
+ range->begin = ROUND_UP(0x100000000ULL + pcms->above_4g_mem_size,
+ 1ULL << 30);
+ }
+ range->end = 1ULL << 40; /* 40 bits physical */
+ } else {
+ range->begin = range->end = 0;
+ pci_for_each_device_under_bus(bus, pci_dev_get_w64, range);
+ }
}
static bool pcie_has_upstream_port(PCIDevice *dev)
--
2.4.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Qemu-devel] [PATCH 3/4] acpi: refactor pxb crs computation
2016-05-02 15:37 [Qemu-devel] [PATCH 0/4] pci: better support for 64-bit MMIO allocation Marcel Apfelbaum
2016-05-02 15:37 ` [Qemu-devel] [PATCH 1/4] hw/pc: extract reserved memory end computation to a standalone function Marcel Apfelbaum
2016-05-02 15:37 ` [Qemu-devel] [PATCH 2/4] pci: reserve 64 bit MMIO range for PCI hotplug Marcel Apfelbaum
@ 2016-05-02 15:37 ` Marcel Apfelbaum
2016-05-02 15:37 ` [Qemu-devel] [PATCH 4/4] hw/apci: handle 64-bit MMIO regions correctly Marcel Apfelbaum
2016-05-09 16:50 ` [Qemu-devel] [PATCH 0/4] pci: better support for 64-bit MMIO allocation Laszlo Ersek
4 siblings, 0 replies; 9+ messages in thread
From: Marcel Apfelbaum @ 2016-05-02 15:37 UTC (permalink / raw)
To: qemu-devel; +Cc: marcel, mst, imammedo, lersek, ehabkost
Instead of always passing both IO and MEM ranges when
computing CRS ranges, define a new CrsRangeSet structure
that include them both.
This is done before introducing a third type of range,
64-bit MEM, so it will be easier to pass them all around.
Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
hw/i386/acpi-build.c | 82 ++++++++++++++++++++++++++++++++--------------------
1 file changed, 51 insertions(+), 31 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 35180ef..be002d7 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -736,6 +736,23 @@ static void crs_range_free(gpointer data)
g_free(entry);
}
+typedef struct CrsRangeSet {
+ GPtrArray *io_ranges;
+ GPtrArray *mem_ranges;
+ } CrsRangeSet;
+
+static void crs_range_set_init(CrsRangeSet *range_set)
+{
+ range_set->io_ranges = g_ptr_array_new_with_free_func(crs_range_free);
+ range_set->mem_ranges = g_ptr_array_new_with_free_func(crs_range_free);
+}
+
+static void crs_range_set_free(CrsRangeSet *range_set)
+{
+ g_ptr_array_free(range_set->io_ranges, true);
+ g_ptr_array_free(range_set->mem_ranges, true);
+}
+
static gint crs_range_compare(gconstpointer a, gconstpointer b)
{
CrsRangeEntry *entry_a = *(CrsRangeEntry **)a;
@@ -820,18 +837,17 @@ static void crs_range_merge(GPtrArray *range)
g_ptr_array_free(tmp, true);
}
-static Aml *build_crs(PCIHostState *host,
- GPtrArray *io_ranges, GPtrArray *mem_ranges)
+static Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set)
{
Aml *crs = aml_resource_template();
- GPtrArray *host_io_ranges = g_ptr_array_new_with_free_func(crs_range_free);
- GPtrArray *host_mem_ranges = g_ptr_array_new_with_free_func(crs_range_free);
+ CrsRangeSet temp_range_set;
CrsRangeEntry *entry;
uint8_t max_bus = pci_bus_num(host->bus);
uint8_t type;
int devfn;
int i;
+ crs_range_set_init(&temp_range_set);
for (devfn = 0; devfn < ARRAY_SIZE(host->bus->devices); devfn++) {
uint64_t range_base, range_limit;
PCIDevice *dev = host->bus->devices[devfn];
@@ -855,9 +871,11 @@ static Aml *build_crs(PCIHostState *host,
}
if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
- crs_range_insert(host_io_ranges, range_base, range_limit);
+ crs_range_insert(temp_range_set.io_ranges,
+ range_base, range_limit);
} else { /* "memory" */
- crs_range_insert(host_mem_ranges, range_base, range_limit);
+ crs_range_insert(temp_range_set.mem_ranges,
+ range_base, range_limit);
}
}
@@ -876,7 +894,8 @@ static Aml *build_crs(PCIHostState *host,
* that do not support multiple root buses
*/
if (range_base && range_base <= range_limit) {
- crs_range_insert(host_io_ranges, range_base, range_limit);
+ crs_range_insert(temp_range_set.io_ranges,
+ range_base, range_limit);
}
range_base =
@@ -889,7 +908,8 @@ static Aml *build_crs(PCIHostState *host,
* that do not support multiple root buses
*/
if (range_base && range_base <= range_limit) {
- crs_range_insert(host_mem_ranges, range_base, range_limit);
+ crs_range_insert(temp_range_set.mem_ranges,
+ range_base, range_limit);
}
range_base =
@@ -902,35 +922,36 @@ static Aml *build_crs(PCIHostState *host,
* that do not support multiple root buses
*/
if (range_base && range_base <= range_limit) {
- crs_range_insert(host_mem_ranges, range_base, range_limit);
+ crs_range_insert(temp_range_set.mem_ranges,
+ range_base, range_limit);
}
}
}
- crs_range_merge(host_io_ranges);
- for (i = 0; i < host_io_ranges->len; i++) {
- entry = g_ptr_array_index(host_io_ranges, i);
+ crs_range_merge(temp_range_set.io_ranges);
+ for (i = 0; i < temp_range_set.io_ranges->len; i++) {
+ entry = g_ptr_array_index(temp_range_set.io_ranges, i);
aml_append(crs,
aml_word_io(AML_MIN_FIXED, AML_MAX_FIXED,
AML_POS_DECODE, AML_ENTIRE_RANGE,
0, entry->base, entry->limit, 0,
entry->limit - entry->base + 1));
- crs_range_insert(io_ranges, entry->base, entry->limit);
+ crs_range_insert(range_set->io_ranges, entry->base, entry->limit);
}
- g_ptr_array_free(host_io_ranges, true);
- crs_range_merge(host_mem_ranges);
- for (i = 0; i < host_mem_ranges->len; i++) {
- entry = g_ptr_array_index(host_mem_ranges, i);
+ crs_range_merge(temp_range_set.mem_ranges);
+ for (i = 0; i < temp_range_set.mem_ranges->len; i++) {
+ entry = g_ptr_array_index(temp_range_set.mem_ranges, i);
aml_append(crs,
aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED,
AML_MAX_FIXED, AML_NON_CACHEABLE,
AML_READ_WRITE,
0, entry->base, entry->limit, 0,
entry->limit - entry->base + 1));
- crs_range_insert(mem_ranges, entry->base, entry->limit);
+ crs_range_insert(range_set->mem_ranges, entry->base, entry->limit);
}
- g_ptr_array_free(host_mem_ranges, true);
+
+ crs_range_set_free(&temp_range_set);
aml_append(crs,
aml_word_bus_number(AML_MIN_FIXED, AML_MAX_FIXED, AML_POS_DECODE,
@@ -1985,8 +2006,7 @@ build_dsdt(GArray *table_data, GArray *linker,
{
CrsRangeEntry *entry;
Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs;
- GPtrArray *mem_ranges = g_ptr_array_new_with_free_func(crs_range_free);
- GPtrArray *io_ranges = g_ptr_array_new_with_free_func(crs_range_free);
+ CrsRangeSet crs_range_set;
PCMachineState *pcms = PC_MACHINE(machine);
uint32_t nr_mem = machine->ram_slots;
int root_bus_limit = 0xFF;
@@ -2087,6 +2107,7 @@ build_dsdt(GArray *table_data, GArray *linker,
}
aml_append(dsdt, scope);
+ crs_range_set_init(&crs_range_set);
bus = PC_MACHINE(machine)->bus;
if (bus) {
QLIST_FOREACH(bus, &bus->child, sibling) {
@@ -2113,8 +2134,7 @@ build_dsdt(GArray *table_data, GArray *linker,
}
aml_append(dev, build_prt(false));
- crs = build_crs(PCI_HOST_BRIDGE(BUS(bus)->parent),
- io_ranges, mem_ranges);
+ crs = build_crs(PCI_HOST_BRIDGE(BUS(bus)->parent), &crs_range_set);
aml_append(dev, aml_name_decl("_CRS", crs));
aml_append(scope, dev);
aml_append(dsdt, scope);
@@ -2135,9 +2155,9 @@ build_dsdt(GArray *table_data, GArray *linker,
AML_POS_DECODE, AML_ENTIRE_RANGE,
0x0000, 0x0000, 0x0CF7, 0x0000, 0x0CF8));
- crs_replace_with_free_ranges(io_ranges, 0x0D00, 0xFFFF);
- for (i = 0; i < io_ranges->len; i++) {
- entry = g_ptr_array_index(io_ranges, i);
+ crs_replace_with_free_ranges(crs_range_set.io_ranges, 0x0D00, 0xFFFF);
+ for (i = 0; i < crs_range_set.io_ranges->len; i++) {
+ entry = g_ptr_array_index(crs_range_set.io_ranges, i);
aml_append(crs,
aml_word_io(AML_MIN_FIXED, AML_MAX_FIXED,
AML_POS_DECODE, AML_ENTIRE_RANGE,
@@ -2150,9 +2170,10 @@ build_dsdt(GArray *table_data, GArray *linker,
AML_CACHEABLE, AML_READ_WRITE,
0, 0x000A0000, 0x000BFFFF, 0, 0x00020000));
- crs_replace_with_free_ranges(mem_ranges, pci->w32.begin, pci->w32.end - 1);
- for (i = 0; i < mem_ranges->len; i++) {
- entry = g_ptr_array_index(mem_ranges, i);
+ crs_replace_with_free_ranges(crs_range_set.mem_ranges,
+ pci->w32.begin, pci->w32.end - 1);
+ for (i = 0; i < crs_range_set. mem_ranges->len; i++) {
+ entry = g_ptr_array_index(crs_range_set.mem_ranges, i);
aml_append(crs,
aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
AML_NON_CACHEABLE, AML_READ_WRITE,
@@ -2182,8 +2203,7 @@ build_dsdt(GArray *table_data, GArray *linker,
aml_append(dev, aml_name_decl("_CRS", crs));
aml_append(scope, dev);
- g_ptr_array_free(io_ranges, true);
- g_ptr_array_free(mem_ranges, true);
+ crs_range_set_free(&crs_range_set);
/* reserve PCIHP resources */
if (pm->pcihp_io_len) {
--
2.4.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Qemu-devel] [PATCH 4/4] hw/apci: handle 64-bit MMIO regions correctly
2016-05-02 15:37 [Qemu-devel] [PATCH 0/4] pci: better support for 64-bit MMIO allocation Marcel Apfelbaum
` (2 preceding siblings ...)
2016-05-02 15:37 ` [Qemu-devel] [PATCH 3/4] acpi: refactor pxb crs computation Marcel Apfelbaum
@ 2016-05-02 15:37 ` Marcel Apfelbaum
2016-05-09 16:54 ` Laszlo Ersek
2016-05-09 16:50 ` [Qemu-devel] [PATCH 0/4] pci: better support for 64-bit MMIO allocation Laszlo Ersek
4 siblings, 1 reply; 9+ messages in thread
From: Marcel Apfelbaum @ 2016-05-02 15:37 UTC (permalink / raw)
To: qemu-devel; +Cc: marcel, mst, imammedo, lersek, ehabkost
In build_crs(), the calculation and merging of the ranges already happens
in 64-bit, but the entry boundaries are silently truncated to 32-bit in the
call to aml_dword_memory(). Fix it by handling the 64-bit MMIO ranges separately.
This fixes 64-bit BARs behind PXBs.
Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
hw/i386/acpi-build.c | 61 +++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 48 insertions(+), 13 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index be002d7..af8b0d3 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -739,18 +739,22 @@ static void crs_range_free(gpointer data)
typedef struct CrsRangeSet {
GPtrArray *io_ranges;
GPtrArray *mem_ranges;
+ GPtrArray *mem_64bit_ranges;
} CrsRangeSet;
static void crs_range_set_init(CrsRangeSet *range_set)
{
range_set->io_ranges = g_ptr_array_new_with_free_func(crs_range_free);
range_set->mem_ranges = g_ptr_array_new_with_free_func(crs_range_free);
+ range_set->mem_64bit_ranges =
+ g_ptr_array_new_with_free_func(crs_range_free);
}
static void crs_range_set_free(CrsRangeSet *range_set)
{
g_ptr_array_free(range_set->io_ranges, true);
g_ptr_array_free(range_set->mem_ranges, true);
+ g_ptr_array_free(range_set->mem_64bit_ranges, true);
}
static gint crs_range_compare(gconstpointer a, gconstpointer b)
@@ -908,8 +912,14 @@ static Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set)
* that do not support multiple root buses
*/
if (range_base && range_base <= range_limit) {
- crs_range_insert(temp_range_set.mem_ranges,
- range_base, range_limit);
+ uint64_t length = range_limit - range_base + 1;
+ if (range_limit <= UINT32_MAX && length <= UINT32_MAX) {
+ crs_range_insert(temp_range_set.mem_ranges,
+ range_base, range_limit);
+ } else {
+ crs_range_insert(temp_range_set.mem_64bit_ranges,
+ range_base, range_limit);
+ }
}
range_base =
@@ -922,8 +932,14 @@ static Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set)
* that do not support multiple root buses
*/
if (range_base && range_base <= range_limit) {
- crs_range_insert(temp_range_set.mem_ranges,
- range_base, range_limit);
+ uint64_t length = range_limit - range_base + 1;
+ if (range_limit <= UINT32_MAX && length <= UINT32_MAX) {
+ crs_range_insert(temp_range_set.mem_ranges,
+ range_base, range_limit);
+ } else {
+ crs_range_insert(temp_range_set.mem_64bit_ranges,
+ range_base, range_limit);
+ }
}
}
}
@@ -944,13 +960,26 @@ static Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set)
entry = g_ptr_array_index(temp_range_set.mem_ranges, i);
aml_append(crs,
aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED,
- AML_MAX_FIXED, AML_NON_CACHEABLE,
- AML_READ_WRITE,
- 0, entry->base, entry->limit, 0,
- entry->limit - entry->base + 1));
+ AML_MAX_FIXED, AML_NON_CACHEABLE,
+ AML_READ_WRITE,
+ 0, entry->base, entry->limit, 0,
+ entry->limit - entry->base + 1));
crs_range_insert(range_set->mem_ranges, entry->base, entry->limit);
}
+ crs_range_merge(temp_range_set.mem_64bit_ranges);
+ for (i = 0; i < temp_range_set.mem_64bit_ranges->len; i++) {
+ entry = g_ptr_array_index(temp_range_set.mem_64bit_ranges, i);
+ aml_append(crs,
+ aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED,
+ AML_MAX_FIXED, AML_NON_CACHEABLE,
+ AML_READ_WRITE,
+ 0, entry->base, entry->limit, 0,
+ entry->limit - entry->base + 1));
+ crs_range_insert(range_set->mem_64bit_ranges,
+ entry->base, entry->limit);
+ }
+
crs_range_set_free(&temp_range_set);
aml_append(crs,
@@ -2182,11 +2211,17 @@ build_dsdt(GArray *table_data, GArray *linker,
}
if (pci->w64.begin) {
- aml_append(crs,
- aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
- AML_CACHEABLE, AML_READ_WRITE,
- 0, pci->w64.begin, pci->w64.end - 1, 0,
- pci->w64.end - pci->w64.begin));
+ crs_replace_with_free_ranges(crs_range_set.mem_64bit_ranges,
+ pci->w64.begin, pci->w64.end - 1);
+ for (i = 0; i < crs_range_set. mem_64bit_ranges->len; i++) {
+ entry = g_ptr_array_index(crs_range_set.mem_64bit_ranges, i);
+ aml_append(crs,
+ aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED,
+ AML_MAX_FIXED,
+ AML_CACHEABLE, AML_READ_WRITE,
+ 0, entry->base, entry->limit,
+ 0, entry->limit - entry->base + 1));
+ }
}
aml_append(scope, aml_name_decl("_CRS", crs));
--
2.4.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH 4/4] hw/apci: handle 64-bit MMIO regions correctly
2016-05-02 15:37 ` [Qemu-devel] [PATCH 4/4] hw/apci: handle 64-bit MMIO regions correctly Marcel Apfelbaum
@ 2016-05-09 16:54 ` Laszlo Ersek
2016-05-09 17:49 ` Marcel Apfelbaum
0 siblings, 1 reply; 9+ messages in thread
From: Laszlo Ersek @ 2016-05-09 16:54 UTC (permalink / raw)
To: Marcel Apfelbaum, qemu-devel; +Cc: imammedo, ehabkost, mst
On 05/02/16 17:37, Marcel Apfelbaum wrote:
> In build_crs(), the calculation and merging of the ranges already happens
> in 64-bit, but the entry boundaries are silently truncated to 32-bit in the
> call to aml_dword_memory(). Fix it by handling the 64-bit MMIO ranges separately.
> This fixes 64-bit BARs behind PXBs.
>
> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
> ---
> hw/i386/acpi-build.c | 61 +++++++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 48 insertions(+), 13 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index be002d7..af8b0d3 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -739,18 +739,22 @@ static void crs_range_free(gpointer data)
> typedef struct CrsRangeSet {
> GPtrArray *io_ranges;
> GPtrArray *mem_ranges;
> + GPtrArray *mem_64bit_ranges;
> } CrsRangeSet;
>
> static void crs_range_set_init(CrsRangeSet *range_set)
> {
> range_set->io_ranges = g_ptr_array_new_with_free_func(crs_range_free);
> range_set->mem_ranges = g_ptr_array_new_with_free_func(crs_range_free);
> + range_set->mem_64bit_ranges =
> + g_ptr_array_new_with_free_func(crs_range_free);
> }
>
> static void crs_range_set_free(CrsRangeSet *range_set)
> {
> g_ptr_array_free(range_set->io_ranges, true);
> g_ptr_array_free(range_set->mem_ranges, true);
> + g_ptr_array_free(range_set->mem_64bit_ranges, true);
> }
>
> static gint crs_range_compare(gconstpointer a, gconstpointer b)
> @@ -908,8 +912,14 @@ static Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set)
> * that do not support multiple root buses
> */
> if (range_base && range_base <= range_limit) {
> - crs_range_insert(temp_range_set.mem_ranges,
> - range_base, range_limit);
> + uint64_t length = range_limit - range_base + 1;
> + if (range_limit <= UINT32_MAX && length <= UINT32_MAX) {
> + crs_range_insert(temp_range_set.mem_ranges,
> + range_base, range_limit);
> + } else {
> + crs_range_insert(temp_range_set.mem_64bit_ranges,
> + range_base, range_limit);
> + }
> }
>
> range_base =
> @@ -922,8 +932,14 @@ static Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set)
> * that do not support multiple root buses
> */
> if (range_base && range_base <= range_limit) {
> - crs_range_insert(temp_range_set.mem_ranges,
> - range_base, range_limit);
> + uint64_t length = range_limit - range_base + 1;
> + if (range_limit <= UINT32_MAX && length <= UINT32_MAX) {
> + crs_range_insert(temp_range_set.mem_ranges,
> + range_base, range_limit);
> + } else {
> + crs_range_insert(temp_range_set.mem_64bit_ranges,
> + range_base, range_limit);
> + }
> }
> }
> }
> @@ -944,13 +960,26 @@ static Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set)
> entry = g_ptr_array_index(temp_range_set.mem_ranges, i);
> aml_append(crs,
> aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED,
> - AML_MAX_FIXED, AML_NON_CACHEABLE,
> - AML_READ_WRITE,
> - 0, entry->base, entry->limit, 0,
> - entry->limit - entry->base + 1));
> + AML_MAX_FIXED, AML_NON_CACHEABLE,
> + AML_READ_WRITE,
> + 0, entry->base, entry->limit, 0,
> + entry->limit - entry->base + 1));
(Thanks for the opportunity for a little word play here:)
Unintented unindent?
Laszlo
> crs_range_insert(range_set->mem_ranges, entry->base, entry->limit);
> }
>
> + crs_range_merge(temp_range_set.mem_64bit_ranges);
> + for (i = 0; i < temp_range_set.mem_64bit_ranges->len; i++) {
> + entry = g_ptr_array_index(temp_range_set.mem_64bit_ranges, i);
> + aml_append(crs,
> + aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED,
> + AML_MAX_FIXED, AML_NON_CACHEABLE,
> + AML_READ_WRITE,
> + 0, entry->base, entry->limit, 0,
> + entry->limit - entry->base + 1));
> + crs_range_insert(range_set->mem_64bit_ranges,
> + entry->base, entry->limit);
> + }
> +
> crs_range_set_free(&temp_range_set);
>
> aml_append(crs,
> @@ -2182,11 +2211,17 @@ build_dsdt(GArray *table_data, GArray *linker,
> }
>
> if (pci->w64.begin) {
> - aml_append(crs,
> - aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
> - AML_CACHEABLE, AML_READ_WRITE,
> - 0, pci->w64.begin, pci->w64.end - 1, 0,
> - pci->w64.end - pci->w64.begin));
> + crs_replace_with_free_ranges(crs_range_set.mem_64bit_ranges,
> + pci->w64.begin, pci->w64.end - 1);
> + for (i = 0; i < crs_range_set. mem_64bit_ranges->len; i++) {
> + entry = g_ptr_array_index(crs_range_set.mem_64bit_ranges, i);
> + aml_append(crs,
> + aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED,
> + AML_MAX_FIXED,
> + AML_CACHEABLE, AML_READ_WRITE,
> + 0, entry->base, entry->limit,
> + 0, entry->limit - entry->base + 1));
> + }
> }
> aml_append(scope, aml_name_decl("_CRS", crs));
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH 4/4] hw/apci: handle 64-bit MMIO regions correctly
2016-05-09 16:54 ` Laszlo Ersek
@ 2016-05-09 17:49 ` Marcel Apfelbaum
0 siblings, 0 replies; 9+ messages in thread
From: Marcel Apfelbaum @ 2016-05-09 17:49 UTC (permalink / raw)
To: Laszlo Ersek, qemu-devel; +Cc: imammedo, ehabkost, mst
On 05/09/2016 07:54 PM, Laszlo Ersek wrote:
> On 05/02/16 17:37, Marcel Apfelbaum wrote:
>> In build_crs(), the calculation and merging of the ranges already happens
>> in 64-bit, but the entry boundaries are silently truncated to 32-bit in the
>> call to aml_dword_memory(). Fix it by handling the 64-bit MMIO ranges separately.
>> This fixes 64-bit BARs behind PXBs.
>>
>> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
>> ---
>> hw/i386/acpi-build.c | 61 +++++++++++++++++++++++++++++++++++++++++-----------
>> 1 file changed, 48 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index be002d7..af8b0d3 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -739,18 +739,22 @@ static void crs_range_free(gpointer data)
>> typedef struct CrsRangeSet {
>> GPtrArray *io_ranges;
>> GPtrArray *mem_ranges;
>> + GPtrArray *mem_64bit_ranges;
>> } CrsRangeSet;
>>
>> static void crs_range_set_init(CrsRangeSet *range_set)
>> {
>> range_set->io_ranges = g_ptr_array_new_with_free_func(crs_range_free);
>> range_set->mem_ranges = g_ptr_array_new_with_free_func(crs_range_free);
>> + range_set->mem_64bit_ranges =
>> + g_ptr_array_new_with_free_func(crs_range_free);
>> }
>>
>> static void crs_range_set_free(CrsRangeSet *range_set)
>> {
>> g_ptr_array_free(range_set->io_ranges, true);
>> g_ptr_array_free(range_set->mem_ranges, true);
>> + g_ptr_array_free(range_set->mem_64bit_ranges, true);
>> }
>>
>> static gint crs_range_compare(gconstpointer a, gconstpointer b)
>> @@ -908,8 +912,14 @@ static Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set)
>> * that do not support multiple root buses
>> */
>> if (range_base && range_base <= range_limit) {
>> - crs_range_insert(temp_range_set.mem_ranges,
>> - range_base, range_limit);
>> + uint64_t length = range_limit - range_base + 1;
>> + if (range_limit <= UINT32_MAX && length <= UINT32_MAX) {
>> + crs_range_insert(temp_range_set.mem_ranges,
>> + range_base, range_limit);
>> + } else {
>> + crs_range_insert(temp_range_set.mem_64bit_ranges,
>> + range_base, range_limit);
>> + }
>> }
>>
>> range_base =
>> @@ -922,8 +932,14 @@ static Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set)
>> * that do not support multiple root buses
>> */
>> if (range_base && range_base <= range_limit) {
>> - crs_range_insert(temp_range_set.mem_ranges,
>> - range_base, range_limit);
>> + uint64_t length = range_limit - range_base + 1;
>> + if (range_limit <= UINT32_MAX && length <= UINT32_MAX) {
>> + crs_range_insert(temp_range_set.mem_ranges,
>> + range_base, range_limit);
>> + } else {
>> + crs_range_insert(temp_range_set.mem_64bit_ranges,
>> + range_base, range_limit);
>> + }
>> }
>> }
>> }
>> @@ -944,13 +960,26 @@ static Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set)
>> entry = g_ptr_array_index(temp_range_set.mem_ranges, i);
>> aml_append(crs,
>> aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED,
>> - AML_MAX_FIXED, AML_NON_CACHEABLE,
>> - AML_READ_WRITE,
>> - 0, entry->base, entry->limit, 0,
>> - entry->limit - entry->base + 1));
>> + AML_MAX_FIXED, AML_NON_CACHEABLE,
>> + AML_READ_WRITE,
>> + 0, entry->base, entry->limit, 0,
>> + entry->limit - entry->base + 1));
>
> (Thanks for the opportunity for a little word play here:)
>
> Unintented unindent?
Please enjoy :)
I'll fix it before re-spin.
Thanks,
Marcel
>
> Laszlo
>
>> crs_range_insert(range_set->mem_ranges, entry->base, entry->limit);
>> }
>>
>> + crs_range_merge(temp_range_set.mem_64bit_ranges);
>> + for (i = 0; i < temp_range_set.mem_64bit_ranges->len; i++) {
>> + entry = g_ptr_array_index(temp_range_set.mem_64bit_ranges, i);
>> + aml_append(crs,
>> + aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED,
>> + AML_MAX_FIXED, AML_NON_CACHEABLE,
>> + AML_READ_WRITE,
>> + 0, entry->base, entry->limit, 0,
>> + entry->limit - entry->base + 1));
>> + crs_range_insert(range_set->mem_64bit_ranges,
>> + entry->base, entry->limit);
>> + }
>> +
>> crs_range_set_free(&temp_range_set);
>>
>> aml_append(crs,
>> @@ -2182,11 +2211,17 @@ build_dsdt(GArray *table_data, GArray *linker,
>> }
>>
>> if (pci->w64.begin) {
>> - aml_append(crs,
>> - aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
>> - AML_CACHEABLE, AML_READ_WRITE,
>> - 0, pci->w64.begin, pci->w64.end - 1, 0,
>> - pci->w64.end - pci->w64.begin));
>> + crs_replace_with_free_ranges(crs_range_set.mem_64bit_ranges,
>> + pci->w64.begin, pci->w64.end - 1);
>> + for (i = 0; i < crs_range_set. mem_64bit_ranges->len; i++) {
>> + entry = g_ptr_array_index(crs_range_set.mem_64bit_ranges, i);
>> + aml_append(crs,
>> + aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED,
>> + AML_MAX_FIXED,
>> + AML_CACHEABLE, AML_READ_WRITE,
>> + 0, entry->base, entry->limit,
>> + 0, entry->limit - entry->base + 1));
>> + }
>> }
>> aml_append(scope, aml_name_decl("_CRS", crs));
>>
>>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 0/4] pci: better support for 64-bit MMIO allocation
2016-05-02 15:37 [Qemu-devel] [PATCH 0/4] pci: better support for 64-bit MMIO allocation Marcel Apfelbaum
` (3 preceding siblings ...)
2016-05-02 15:37 ` [Qemu-devel] [PATCH 4/4] hw/apci: handle 64-bit MMIO regions correctly Marcel Apfelbaum
@ 2016-05-09 16:50 ` Laszlo Ersek
2016-05-09 18:16 ` Marcel Apfelbaum
4 siblings, 1 reply; 9+ messages in thread
From: Laszlo Ersek @ 2016-05-09 16:50 UTC (permalink / raw)
To: Marcel Apfelbaum, qemu-devel; +Cc: imammedo, ehabkost, mst
Hi Marcel,
On 05/02/16 17:37, Marcel Apfelbaum wrote:
> Hi,
>
> First two patches allocate (max_reserved_ram - max_addr_cpu_addressable) range for PCI hotplug
> (for PC Machines) instead of the previous 64-bit PCI window that included only
> the ranges allocated by the firmware.
>
> The next two patches fix 64-bit CRS computations.
>
> Thank you,
> Marcel
>
> Marcel Apfelbaum (4):
> hw/pc: extract reserved memory end computation to a standalone
> function
> pci: reserve 64 bit MMIO range for PCI hotplug
> acpi: refactor pxb crs computation
> hw/apci: handle 64-bit MMIO regions correctly
>
> hw/i386/acpi-build.c | 135 ++++++++++++++++++++++++++++++++++++---------------
> hw/i386/pc.c | 29 ++++++++---
> hw/pci/pci.c | 16 +++++-
> include/hw/i386/pc.h | 1 +
> 4 files changed, 131 insertions(+), 50 deletions(-)
>
I just tried to test this series, on top of v2.6.0-rc5. (My intent was to dump and decompile the DSDT in the guest, and to compare the versions before vs. after.)
The series doesn't apply.
You had posted it on May 2nd, so I tried to apply the series to v2.6.0-rc4 (2016-05-02) and v2.6.0-rc3 too (2016-04-21); neither worked.
The error message from "git am --reject" is, for patch #4:
Checking patch hw/i386/acpi-build.c...
error: while searching for:
}
if (pci->w64.begin) {
aml_append(crs,
aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
AML_CACHEABLE, AML_READ_WRITE,
0, pci->w64.begin, pci->w64.end - 1, 0,
pci->w64.end - pci->w64.begin));
}
aml_append(scope, aml_name_decl("_CRS", crs));
Looking at the trailing context in your patch #4, in particular
aml_append(scope, aml_name_decl("_CRS", crs));
I think that the conflict is caused by this intervening commit:
commit 2b1c2e8e5f1990f0a201a8cbf9d366fca60f4aa8
Author: Igor Mammedov <imammedo@redhat.com>
Date: Fri Apr 8 13:23:13 2016 +0200
pc: acpi: tpm: add missing MMIO resource to PCI0._CRS
Which was part of v2.6.0-rc2.
... Actually, that's a pretty small patch, so maybe I can apply your series at 2b1c2e8e5f19^, then rebase it to v2.6.0-rc5 from there... Yes, that works.
... as far as rebasing is concerned, but when it comes to compiling, the patches seem to break the arm-softmmu and aarch64-softmmu targets:
hw/pci/pci.c:2474: undefined reference to `pc_machine_get_reserved_memory_end'
This is because patch #2 adds a pc_machine_get_reserved_memory_end() call to the generic pci_bus_get_w64_range() function, which is included for other targets as well.
Anyway, for now I'm testing with the x86_64-softmmu target. So this is my QEMU command line:
VIRTIO10=1
ISO=/mnt/data/isos/fedora/23/Fedora-Live-Workstation-x86_64-23-10.iso
CODE=/home/virt-images/OVMF_CODE.fd
TMPL=/home/virt-images/OVMF_VARS.fd
TFTP=/var/lib/dnsmasq
BF=shim.efi
if [ $VIRTIO10 -eq 0 ]; then
MODERN=disable-legacy=off,disable-modern=on
else
MODERN=disable-legacy=on,disable-modern=off
fi
cp $TMPL vars3.fd
/opt/qemu-installed/bin/qemu-system-x86_64 \
-m 2048 \
-M pc \
-enable-kvm \
-device qxl-vga \
-drive if=pflash,readonly,format=raw,file=$CODE \
-drive if=pflash,format=raw,file=vars3.fd \
-drive id=cdrom,if=none,readonly,format=raw,cache=writethrough,file=$ISO \
\
-debugcon file:debug3.log \
-global isa-debugcon.iobase=0x402 \
\
-chardev stdio,signal=off,mux=on,id=char0 \
-mon chardev=char0,mode=readline,default \
-serial chardev:char0 \
\
-device pxb,bus=pci.0,id=bridge1,bus_nr=11 \
-device pxb,bus=pci.0,id=bridge2,bus_nr=7 \
-device pxb,bus=pci.0,id=bridge3,bus_nr=15 \
\
-device virtio-scsi-pci,id=scsi0,bus=bridge1 \
-device scsi-cd,bus=scsi0.0,drive=cdrom,bootindex=1 \
\
-netdev user,id=netdev0,hostfwd=tcp:127.0.0.1:2227-:22,tftp=$TFTP,bootfile=$BF \
-device virtio-net-pci,netdev=netdev0,romfile=,bootindex=2,bus=bridge1,addr=2 \
\
-netdev user,id=netdev1,hostfwd=tcp:127.0.0.1:2228-:22 \
-device virtio-net-pci,netdev=netdev1,romfile=,$MODERN,bootindex=3,bus=bridge2,addr=3 \
\
-netdev user,id=netdev2,hostfwd=tcp:127.0.0.1:2229-:22 \
-device virtio-net-pci,netdev=netdev2,romfile=,$MODERN,bootindex=4,bus=bridge3,addr=4 \
\
-global PIIX4_PM.disable_s3=0
Without your patches, the guest dmesg contains the messages
pci 0000:0f:00.0: can't claim BAR 15 [mem 0x800800000-0x800ffffff 64bit pref]: no compatible bridge window
pci 0000:07:00.0: can't claim BAR 15 [mem 0x800000000-0x8007fffff 64bit pref]: no compatible bridge window
pci 0000:10:04.0: can't claim BAR 4 [mem 0x800800000-0x800ffffff 64bit pref]: no compatible bridge window
pci 0000:08:03.0: can't claim BAR 4 [mem 0x800000000-0x8007fffff 64bit pref]: no compatible bridge window
With your patches, those messages are gone, so that looks fine.
Comparing the decompiled DSDTs, before and after, in the _CRS objects of the PC0F and PC07 devices (see above "id=bridge2,bus_nr=7" and "id=bridge3,bus_nr=15"), the truncation is fixed; the DWordMemory entries are replaced with the right QWordMemory entries. Great.
I also see some changes in the _CRS of the PCI0 device:
DWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, NonCacheable, ReadWrite,
0x00000000, // Granularity
0x88400000, // Range Minimum
- 0xFFFFFFFF, // Range Maximum
+ 0xFEBFFFFF, // Range Maximum
0x00000000, // Translation Offset
- 0x77C00000, // Length
+ 0x76800000, // Length
This change looks correct (the IO-APIC area starts at 0xFEC00000), but I can't connect it to your patches. Can you explain what code does this?
+ ,, , AddressRangeMemory, TypeStatic)
+ QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, Cacheable, ReadWrite,
+ 0x0000000000000000, // Granularity
+ 0x0000000100000000, // Range Minimum
+ 0x00000007FFFFFFFF, // Range Maximum
+ 0x0000000000000000, // Translation Offset
+ 0x0000000700000000, // Length
+ ,, , AddressRangeMemory, TypeStatic)
+ QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, Cacheable, ReadWrite,
+ 0x0000000000000000, // Granularity
+ 0x0000000801000000, // Range Minimum
+ 0x000000FFFFFFFFFF, // Range Maximum
+ 0x0000000000000000, // Translation Offset
+ 0x000000F7FF000000, // Length
,, , AddressRangeMemory, TypeStatic)
})
This looks like the range from 4GB to 1TB, from which the allocated MMIO64 resources have been punched out. This is the room for the hotplugged devices, right?
Thanks,
Laszlo
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH 0/4] pci: better support for 64-bit MMIO allocation
2016-05-09 16:50 ` [Qemu-devel] [PATCH 0/4] pci: better support for 64-bit MMIO allocation Laszlo Ersek
@ 2016-05-09 18:16 ` Marcel Apfelbaum
0 siblings, 0 replies; 9+ messages in thread
From: Marcel Apfelbaum @ 2016-05-09 18:16 UTC (permalink / raw)
To: Laszlo Ersek, qemu-devel; +Cc: imammedo, ehabkost, mst
On 05/09/2016 07:50 PM, Laszlo Ersek wrote:
> Hi Marcel,
>
> On 05/02/16 17:37, Marcel Apfelbaum wrote:
>> Hi,
>>
>> First two patches allocate (max_reserved_ram - max_addr_cpu_addressable) range for PCI hotplug
>> (for PC Machines) instead of the previous 64-bit PCI window that included only
>> the ranges allocated by the firmware.
>>
>> The next two patches fix 64-bit CRS computations.
>>
>> Thank you,
>> Marcel
>>
>> Marcel Apfelbaum (4):
>> hw/pc: extract reserved memory end computation to a standalone
>> function
>> pci: reserve 64 bit MMIO range for PCI hotplug
>> acpi: refactor pxb crs computation
>> hw/apci: handle 64-bit MMIO regions correctly
>>
>> hw/i386/acpi-build.c | 135 ++++++++++++++++++++++++++++++++++++---------------
>> hw/i386/pc.c | 29 ++++++++---
>> hw/pci/pci.c | 16 +++++-
>> include/hw/i386/pc.h | 1 +
>> 4 files changed, 131 insertions(+), 50 deletions(-)
>>
>
> I just tried to test this series, on top of v2.6.0-rc5. (My intent was to dump and decompile the DSDT in the guest, and to compare the versions before vs. after.)
>
> The series doesn't apply.
>
> You had posted it on May 2nd, so I tried to apply the series to v2.6.0-rc4 (2016-05-02) and v2.6.0-rc3 too (2016-04-21); neither worked.
>
> The error message from "git am --reject" is, for patch #4:
>
> Checking patch hw/i386/acpi-build.c...
> error: while searching for:
> }
>
> if (pci->w64.begin) {
> aml_append(crs,
> aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
> AML_CACHEABLE, AML_READ_WRITE,
> 0, pci->w64.begin, pci->w64.end - 1, 0,
> pci->w64.end - pci->w64.begin));
> }
> aml_append(scope, aml_name_decl("_CRS", crs));
>
> Looking at the trailing context in your patch #4, in particular
>
> aml_append(scope, aml_name_decl("_CRS", crs));
>
> I think that the conflict is caused by this intervening commit:
>
> commit 2b1c2e8e5f1990f0a201a8cbf9d366fca60f4aa8
> Author: Igor Mammedov <imammedo@redhat.com>
> Date: Fri Apr 8 13:23:13 2016 +0200
>
> pc: acpi: tpm: add missing MMIO resource to PCI0._CRS
>
> Which was part of v2.6.0-rc2.
>
> ... Actually, that's a pretty small patch, so maybe I can apply your series at 2b1c2e8e5f19^, then rebase it to v2.6.0-rc5 from there... Yes, that works.
>
> ... as far as rebasing is concerned, but when it comes to compiling, the patches seem to break the arm-softmmu and aarch64-softmmu targets:
>
> hw/pci/pci.c:2474: undefined reference to `pc_machine_get_reserved_memory_end'
>
> This is because patch #2 adds a pc_machine_get_reserved_memory_end() call to the generic pci_bus_get_w64_range() function, which is included for other targets as well.
Hi Laszlo,
Sorry for all that, I'll rebase and re-send. Next time just ping me and I'll do the homework.
>
>
> Anyway, for now I'm testing with the x86_64-softmmu target. So this is my QEMU command line:
>
> VIRTIO10=1
> ISO=/mnt/data/isos/fedora/23/Fedora-Live-Workstation-x86_64-23-10.iso
> CODE=/home/virt-images/OVMF_CODE.fd
> TMPL=/home/virt-images/OVMF_VARS.fd
> TFTP=/var/lib/dnsmasq
> BF=shim.efi
>
> if [ $VIRTIO10 -eq 0 ]; then
> MODERN=disable-legacy=off,disable-modern=on
> else
> MODERN=disable-legacy=on,disable-modern=off
> fi
>
> cp $TMPL vars3.fd
>
> /opt/qemu-installed/bin/qemu-system-x86_64 \
> -m 2048 \
> -M pc \
> -enable-kvm \
> -device qxl-vga \
> -drive if=pflash,readonly,format=raw,file=$CODE \
> -drive if=pflash,format=raw,file=vars3.fd \
> -drive id=cdrom,if=none,readonly,format=raw,cache=writethrough,file=$ISO \
> \
> -debugcon file:debug3.log \
> -global isa-debugcon.iobase=0x402 \
> \
> -chardev stdio,signal=off,mux=on,id=char0 \
> -mon chardev=char0,mode=readline,default \
> -serial chardev:char0 \
> \
> -device pxb,bus=pci.0,id=bridge1,bus_nr=11 \
> -device pxb,bus=pci.0,id=bridge2,bus_nr=7 \
> -device pxb,bus=pci.0,id=bridge3,bus_nr=15 \
> \
> -device virtio-scsi-pci,id=scsi0,bus=bridge1 \
> -device scsi-cd,bus=scsi0.0,drive=cdrom,bootindex=1 \
> \
> -netdev user,id=netdev0,hostfwd=tcp:127.0.0.1:2227-:22,tftp=$TFTP,bootfile=$BF \
> -device virtio-net-pci,netdev=netdev0,romfile=,bootindex=2,bus=bridge1,addr=2 \
> \
> -netdev user,id=netdev1,hostfwd=tcp:127.0.0.1:2228-:22 \
> -device virtio-net-pci,netdev=netdev1,romfile=,$MODERN,bootindex=3,bus=bridge2,addr=3 \
> \
> -netdev user,id=netdev2,hostfwd=tcp:127.0.0.1:2229-:22 \
> -device virtio-net-pci,netdev=netdev2,romfile=,$MODERN,bootindex=4,bus=bridge3,addr=4 \
> \
> -global PIIX4_PM.disable_s3=0
>
> Without your patches, the guest dmesg contains the messages
>
> pci 0000:0f:00.0: can't claim BAR 15 [mem 0x800800000-0x800ffffff 64bit pref]: no compatible bridge window
> pci 0000:07:00.0: can't claim BAR 15 [mem 0x800000000-0x8007fffff 64bit pref]: no compatible bridge window
> pci 0000:10:04.0: can't claim BAR 4 [mem 0x800800000-0x800ffffff 64bit pref]: no compatible bridge window
> pci 0000:08:03.0: can't claim BAR 4 [mem 0x800000000-0x8007fffff 64bit pref]: no compatible bridge window
>
> With your patches, those messages are gone, so that looks fine.
>
> Comparing the decompiled DSDTs, before and after, in the _CRS objects of the PC0F and PC07 devices (see above "id=bridge2,bus_nr=7" and "id=bridge3,bus_nr=15"), the truncation is fixed; the DWordMemory entries are replaced with the right QWordMemory entries. Great.
>
Good!
> I also see some changes in the _CRS of the PCI0 device:
>
> DWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, NonCacheable, ReadWrite,
> 0x00000000, // Granularity
> 0x88400000, // Range Minimum
> - 0xFFFFFFFF, // Range Maximum
> + 0xFEBFFFFF, // Range Maximum
> 0x00000000, // Translation Offset
> - 0x77C00000, // Length
> + 0x76800000, // Length
>
> This change looks correct (the IO-APIC area starts at 0xFEC00000), but I can't connect it to your patches. Can you explain what code does this?
The only interesting patch with regard to the above range is Patch 4/4. However, it deals only with 64-bit MMIO ranges.
Looking closer, it is possible we had a bug before this patch: The code assumed all the ranges assigned to PXBs are 32-bit. Adding a 64-bit
may cause a mismatch beyond the wrong ACPI table.
The way it works is (for each resource type IO/MEM/ and now 64-bit MEM):
1. build_crs is called for each PXB, the firmware assigned ranges are added placed temp list CrsRangeSet
2. we call crs_range_merge to try to merge adjacent ranges to shrink the CRS (we don't want a resource for each devices)
3. finally we add the ranges to the ACPI tables
4. once all the PXBs are 'done', we call crs_replace_with_free_ranges to replace the ranges "used" by PXBs with free ones
to be assigned to bus 0.
5. We add the bus 0 resources to ACPI tables.
It is possible that steps 2 and/or 4 may be affected by (not anticipated) 64-bit ranges, but this patch completely separates them.
>
> + ,, , AddressRangeMemory, TypeStatic)
> + QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, Cacheable, ReadWrite,
> + 0x0000000000000000, // Granularity
> + 0x0000000100000000, // Range Minimum
> + 0x00000007FFFFFFFF, // Range Maximum
> + 0x0000000000000000, // Translation Offset
> + 0x0000000700000000, // Length
> + ,, , AddressRangeMemory, TypeStatic)
> + QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, Cacheable, ReadWrite,
> + 0x0000000000000000, // Granularity
> + 0x0000000801000000, // Range Minimum
> + 0x000000FFFFFFFFFF, // Range Maximum
> + 0x0000000000000000, // Translation Offset
> + 0x000000F7FF000000, // Length
> ,, , AddressRangeMemory, TypeStatic)
> })
>
> This looks like the range from 4GB to 1TB, from which the allocated MMIO64 resources have been punched out. This is the room for the hotplugged devices, right?
Exactly. For the moment all this range is allocated to bus 0, but once PXBs will support hotplug we'll need to divide it somehow between them.
I tried to hotplug an ivshmem device with a huge BAR (sadly I don't have a real device) and after this series it can be hot-plugged like a charm.
Thanks,
Marcel
>
> Thanks,
> Laszlo
>
^ permalink raw reply [flat|nested] 9+ messages in thread