* [Qemu-devel] [PATCH 1/4] pc: sanitize i440fx_init() arguments
2013-10-16 8:49 [Qemu-devel] [PATCH 0/4] pc: inform SeaBIOS where 64-bit PCI hole start and PCI mappings cleanup Igor Mammedov
@ 2013-10-16 8:49 ` Igor Mammedov
2013-10-16 8:49 ` [Qemu-devel] [PATCH 2/4] pc: consolidate mapping of PCI address space into system address space Igor Mammedov
` (3 subsequent siblings)
4 siblings, 0 replies; 14+ messages in thread
From: Igor Mammedov @ 2013-10-16 8:49 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, armbru, blauwirbel, kraxel, aliguori, pbonzini, afaerber
* rename pci_hole_start to below_4g_mem_size to reflect what is
really passed in and move pci_hole_size calculation inside
i440fx.
* remove ram_size arg from function signature, since it could be
retrieved as below_4g_mem_size + above_4g_mem_size sum,
internally.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
hw/i386/pc_piix.c | 3 +--
hw/pci-host/piix.c | 11 ++++++-----
include/hw/i386/pc.h | 4 +---
3 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index c6042c7..2fed5fd 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -145,9 +145,8 @@ static void pc_init1(QEMUMachineInitArgs *args,
if (pci_enabled) {
pci_bus = i440fx_init(&i440fx_state, &piix3_devfn, &isa_bus, gsi,
- system_memory, system_io, args->ram_size,
+ system_memory, system_io,
below_4g_mem_size,
- 0x100000000ULL - below_4g_mem_size,
above_4g_mem_size,
pci_memory, ram_memory);
} else {
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index c041149..5b9cc1f 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -311,9 +311,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
ISABus **isa_bus, qemu_irq *pic,
MemoryRegion *address_space_mem,
MemoryRegion *address_space_io,
- ram_addr_t ram_size,
- hwaddr pci_hole_start,
- hwaddr pci_hole_size,
+ hwaddr below_4g_mem_size,
ram_addr_t above_4g_mem_size,
MemoryRegion *pci_address_space,
MemoryRegion *ram_memory)
@@ -327,6 +325,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
unsigned i;
I440FXState *i440fx;
uint64_t pci_hole64_size;
+ ram_addr_t ram_size = below_4g_mem_size + above_4g_mem_size;
dev = qdev_create(NULL, TYPE_I440FX_PCI_HOST_BRIDGE);
s = PCI_HOST_BRIDGE(dev);
@@ -355,8 +354,10 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
}
memory_region_init_alias(&f->pci_hole, OBJECT(d), "pci-hole", f->pci_address_space,
- pci_hole_start, pci_hole_size);
- memory_region_add_subregion(f->system_memory, pci_hole_start, &f->pci_hole);
+ below_4g_mem_size,
+ 0x100000000ULL - below_4g_mem_size);
+ memory_region_add_subregion(f->system_memory, below_4g_mem_size,
+ &f->pci_hole);
pci_hole64_size = pci_host_get_hole64_size(i440fx->pci_hole64_size);
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 6083839..95fc6cc 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -166,9 +166,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn,
ISABus **isa_bus, qemu_irq *pic,
MemoryRegion *address_space_mem,
MemoryRegion *address_space_io,
- ram_addr_t ram_size,
- hwaddr pci_hole_start,
- hwaddr pci_hole_size,
+ hwaddr below_4g_mem_size,
ram_addr_t above_4g_mem_size,
MemoryRegion *pci_memory,
MemoryRegion *ram_memory);
--
1.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH 2/4] pc: consolidate mapping of PCI address space into system address space
2013-10-16 8:49 [Qemu-devel] [PATCH 0/4] pc: inform SeaBIOS where 64-bit PCI hole start and PCI mappings cleanup Igor Mammedov
2013-10-16 8:49 ` [Qemu-devel] [PATCH 1/4] pc: sanitize i440fx_init() arguments Igor Mammedov
@ 2013-10-16 8:49 ` Igor Mammedov
2013-10-16 8:49 ` [Qemu-devel] [PATCH 3/4] fw_cfg: make cast macro available to world Igor Mammedov
` (2 subsequent siblings)
4 siblings, 0 replies; 14+ messages in thread
From: Igor Mammedov @ 2013-10-16 8:49 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, armbru, blauwirbel, kraxel, aliguori, pbonzini, afaerber
Reduce code duplication by moving PCI address space mappings
into a common helper, and reuse code in PIIX and Q35.
Also rename variables to reflect that PCI memory region mappings
are not the same as PCI holes which are programmed by BIOS to
avoid confusion with PCI holes.
PS:
Although keep user visible via "info mtree" names for regions
"pci-hole" and "pci-hole64" to avoid management tools regressions.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
hw/i386/pc.c | 40 ++++++++++++++++++++++++++++------------
hw/pci-host/piix.c | 28 ++++++++--------------------
hw/pci-host/q35.c | 28 ++++++++--------------------
include/hw/i386/pc.h | 15 +++++----------
4 files changed, 49 insertions(+), 62 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 0c313fe..7ecb028 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1053,21 +1053,37 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size,
return guest_info;
}
-void pc_init_pci64_hole(PcPciInfo *pci_info, uint64_t pci_hole64_start,
- uint64_t pci_hole64_size)
+/* setup pci memory regions mappings into system address space */
+void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory,
+ MemoryRegion *pci_address_space,
+ MemoryRegion *pci32_as, MemoryRegion *pci64_as,
+ uint32_t pci32_as_start, uint32_t pci32_as_size,
+ uint64_t pci64_as_start, uint64_t pci64_as_size)
{
- if ((sizeof(hwaddr) == 4) || (!pci_hole64_size)) {
+ memory_region_init_alias(pci32_as, owner, "pci-hole", pci_address_space,
+ pci32_as_start, pci32_as_size);
+ memory_region_add_subregion(system_memory, pci32_as_start, pci32_as);
+
+ if (sizeof(hwaddr) == 4) {
return;
}
- /*
- * BIOS does not set MTRR entries for the 64 bit window, so no need to
- * align address to power of two. Align address at 1G, this makes sure
- * it can be exactly covered with a PAT entry even when using huge
- * pages.
- */
- pci_info->w64.begin = ROUND_UP(pci_hole64_start, 0x1ULL << 30);
- pci_info->w64.end = pci_info->w64.begin + pci_hole64_size;
- assert(pci_info->w64.begin <= pci_info->w64.end);
+
+ if (pci64_as_size == DEFAULT_PCI_HOLE64_SIZE) {
+ pci64_as_size = 1ULL << 62;
+ }
+ memory_region_init_alias(pci64_as, owner, "pci-hole64", pci_address_space,
+ pci64_as_start, pci64_as_size);
+ if (pci64_as_size) {
+ /*
+ * BIOS does not set MTRR entries for the 64 bit window, so no need to
+ * align address to power of two. Align address at 1G, this makes sure
+ * it can be exactly covered with a PAT entry even when using huge
+ * pages.
+ */
+ memory_region_add_subregion(system_memory,
+ ROUND_UP(pci64_as_start, 0x1ULL << 30),
+ pci64_as);
+ }
}
void pc_acpi_init(const char *default_dsdt)
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 5b9cc1f..d422b25 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -324,7 +324,6 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
PCII440FXState *f;
unsigned i;
I440FXState *i440fx;
- uint64_t pci_hole64_size;
ram_addr_t ram_size = below_4g_mem_size + above_4g_mem_size;
dev = qdev_create(NULL, TYPE_I440FX_PCI_HOST_BRIDGE);
@@ -353,25 +352,14 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
i440fx->pci_info.w32.begin = 0xe0000000;
}
- memory_region_init_alias(&f->pci_hole, OBJECT(d), "pci-hole", f->pci_address_space,
- below_4g_mem_size,
- 0x100000000ULL - below_4g_mem_size);
- memory_region_add_subregion(f->system_memory, below_4g_mem_size,
- &f->pci_hole);
-
- pci_hole64_size = pci_host_get_hole64_size(i440fx->pci_hole64_size);
-
- pc_init_pci64_hole(&i440fx->pci_info, 0x100000000ULL + above_4g_mem_size,
- pci_hole64_size);
- memory_region_init_alias(&f->pci_hole_64bit, OBJECT(d), "pci-hole64",
- f->pci_address_space,
- i440fx->pci_info.w64.begin,
- pci_hole64_size);
- if (pci_hole64_size) {
- memory_region_add_subregion(f->system_memory,
- i440fx->pci_info.w64.begin,
- &f->pci_hole_64bit);
- }
+ pc_pci_as_mapping_init(OBJECT(d), f->system_memory,
+ f->pci_address_space,
+ &f->pci_hole, &f->pci_hole_64bit,
+ below_4g_mem_size,
+ 0x100000000ULL - below_4g_mem_size,
+ 0x100000000ULL + above_4g_mem_size,
+ i440fx->pci_hole64_size);
+
memory_region_init_alias(&f->smram_region, OBJECT(d), "smram-region",
f->pci_address_space, 0xa0000, 0x20000);
memory_region_add_subregion_overlap(f->system_memory, 0xa0000,
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index ad703a4..cb8aea0 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -336,28 +336,16 @@ static int mch_init(PCIDevice *d)
{
int i;
MCHPCIState *mch = MCH_PCI_DEVICE(d);
- uint64_t pci_hole64_size;
/* setup pci memory regions */
- memory_region_init_alias(&mch->pci_hole, OBJECT(mch), "pci-hole",
- mch->pci_address_space,
- mch->below_4g_mem_size,
- 0x100000000ULL - mch->below_4g_mem_size);
- memory_region_add_subregion(mch->system_memory, mch->below_4g_mem_size,
- &mch->pci_hole);
-
- pci_hole64_size = pci_host_get_hole64_size(mch->pci_hole64_size);
- pc_init_pci64_hole(&mch->pci_info, 0x100000000ULL + mch->above_4g_mem_size,
- pci_hole64_size);
- memory_region_init_alias(&mch->pci_hole_64bit, OBJECT(mch), "pci-hole64",
- mch->pci_address_space,
- mch->pci_info.w64.begin,
- pci_hole64_size);
- if (pci_hole64_size) {
- memory_region_add_subregion(mch->system_memory,
- mch->pci_info.w64.begin,
- &mch->pci_hole_64bit);
- }
+ pc_pci_as_mapping_init(OBJECT(mch), mch->system_memory,
+ mch->pci_address_space,
+ &mch->pci_hole, &mch->pci_hole_64bit,
+ mch->below_4g_mem_size,
+ 0x100000000ULL - mch->below_4g_mem_size,
+ 0x100000000ULL + mch->above_4g_mem_size,
+ mch->pci_hole64_size);
+
/* smram */
cpu_smm_register(&mch_set_smm, mch);
memory_region_init_alias(&mch->smram_region, OBJECT(mch), "smram-region",
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 95fc6cc..ed86642 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -108,17 +108,12 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size,
#define PCI_HOST_PROP_PCI_HOLE64_SIZE "pci-hole64-size"
#define DEFAULT_PCI_HOLE64_SIZE (~0x0ULL)
-static inline uint64_t pci_host_get_hole64_size(uint64_t pci_hole64_size)
-{
- if (pci_hole64_size == DEFAULT_PCI_HOLE64_SIZE) {
- return 1ULL << 62;
- } else {
- return pci_hole64_size;
- }
-}
-void pc_init_pci64_hole(PcPciInfo *pci_info, uint64_t pci_hole64_start,
- uint64_t pci_hole64_size);
+void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory,
+ MemoryRegion *pci_address_space,
+ MemoryRegion *pci32_as, MemoryRegion *pci64_as,
+ uint32_t pci32_as_start, uint32_t pci32_as_size,
+ uint64_t pci64_as_start, uint64_t pci64_as_size);
FWCfgState *pc_memory_init(MemoryRegion *system_memory,
const char *kernel_filename,
--
1.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH 3/4] fw_cfg: make cast macro available to world
2013-10-16 8:49 [Qemu-devel] [PATCH 0/4] pc: inform SeaBIOS where 64-bit PCI hole start and PCI mappings cleanup Igor Mammedov
2013-10-16 8:49 ` [Qemu-devel] [PATCH 1/4] pc: sanitize i440fx_init() arguments Igor Mammedov
2013-10-16 8:49 ` [Qemu-devel] [PATCH 2/4] pc: consolidate mapping of PCI address space into system address space Igor Mammedov
@ 2013-10-16 8:49 ` Igor Mammedov
2013-10-16 8:49 ` [Qemu-devel] [PATCH 4/4] pc: add 'etc/pcimem64-minimum-address' fw_cfg interface to SeaBIOS Igor Mammedov
2013-10-16 9:20 ` [Qemu-devel] [PATCH 0/4] pc: inform SeaBIOS where 64-bit PCI hole start and PCI mappings cleanup Michael S. Tsirkin
4 siblings, 0 replies; 14+ messages in thread
From: Igor Mammedov @ 2013-10-16 8:49 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, armbru, blauwirbel, kraxel, aliguori, pbonzini, afaerber
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
hw/nvram/fw_cfg.c | 4 ----
include/hw/nvram/fw_cfg.h | 7 +++++++
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index d0820e5..6c31e24 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -32,10 +32,6 @@
#define FW_CFG_SIZE 2
#define FW_CFG_DATA_SIZE 1
-#define TYPE_FW_CFG "fw_cfg"
-#define FW_CFG_NAME "fw_cfg"
-#define FW_CFG_PATH "/machine/" FW_CFG_NAME
-#define FW_CFG(obj) OBJECT_CHECK(FWCfgState, (obj), TYPE_FW_CFG)
typedef struct FWCfgEntry {
uint32_t len;
diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
index f60dd67..651fe21 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -6,6 +6,7 @@
#include <stddef.h>
#include "exec/hwaddr.h"
+#include "qom/object.h"
#include "qemu/typedefs.h"
#endif
@@ -47,6 +48,12 @@
#define FW_CFG_INVALID 0xffff
#ifndef NO_QEMU_PROTOS
+
+#define TYPE_FW_CFG "fw_cfg"
+#define FW_CFG_NAME "fw_cfg"
+#define FW_CFG_PATH "/machine/" FW_CFG_NAME
+#define FW_CFG(obj) OBJECT_CHECK(FWCfgState, (obj), TYPE_FW_CFG)
+
typedef struct FWCfgFile {
uint32_t size; /* file size */
uint16_t select; /* write this to 0x510 to read it */
--
1.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH 4/4] pc: add 'etc/pcimem64-minimum-address' fw_cfg interface to SeaBIOS
2013-10-16 8:49 [Qemu-devel] [PATCH 0/4] pc: inform SeaBIOS where 64-bit PCI hole start and PCI mappings cleanup Igor Mammedov
` (2 preceding siblings ...)
2013-10-16 8:49 ` [Qemu-devel] [PATCH 3/4] fw_cfg: make cast macro available to world Igor Mammedov
@ 2013-10-16 8:49 ` Igor Mammedov
2013-10-16 9:27 ` Michael S. Tsirkin
2013-10-16 9:29 ` Michael S. Tsirkin
2013-10-16 9:20 ` [Qemu-devel] [PATCH 0/4] pc: inform SeaBIOS where 64-bit PCI hole start and PCI mappings cleanup Michael S. Tsirkin
4 siblings, 2 replies; 14+ messages in thread
From: Igor Mammedov @ 2013-10-16 8:49 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, armbru, blauwirbel, kraxel, aliguori, pbonzini, afaerber
'etc/pcimem64-minimum-address' will allow QEMU to communicate to BIOS
where PCI memory address space mapping starts in high memory.
Allowing BIOS start mapping 64-bit PCI BARs at address where QEMU
placed this mapping vs. hardcoded value right after highmem RAM.
That will allow QEMU to reserve extra address space before
64-bit PCI hole for memory hotplug.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
* SeaBIOS patch: http://patchwork.ozlabs.org/patch/283623/
---
hw/i386/pc.c | 9 ++++++++-
hw/i386/pc_piix.c | 2 +-
hw/pci-host/piix.c | 5 +++--
hw/pci-host/q35.c | 4 +++-
include/hw/i386/pc.h | 5 +++--
5 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 7ecb028..ac99ae1 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1055,7 +1055,7 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size,
/* setup pci memory regions mappings into system address space */
void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory,
- MemoryRegion *pci_address_space,
+ MemoryRegion *pci_address_space, FWCfgState *fw_cfg,
MemoryRegion *pci32_as, MemoryRegion *pci64_as,
uint32_t pci32_as_start, uint32_t pci32_as_size,
uint64_t pci64_as_start, uint64_t pci64_as_size)
@@ -1083,6 +1083,13 @@ void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory,
memory_region_add_subregion(system_memory,
ROUND_UP(pci64_as_start, 0x1ULL << 30),
pci64_as);
+ if (fw_cfg) {
+ uint64_t *pcimem64_start = g_malloc(sizeof(*pcimem64_start));
+
+ *pcimem64_start = cpu_to_le64(pci64_as_start);
+ fw_cfg_add_file(fw_cfg, "etc/pcimem64-minimum-address",
+ pcimem64_start, sizeof(*pcimem64_start));
+ }
}
}
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 2fed5fd..966c48a 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -148,7 +148,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
system_memory, system_io,
below_4g_mem_size,
above_4g_mem_size,
- pci_memory, ram_memory);
+ pci_memory, ram_memory, fw_cfg);
} else {
pci_bus = NULL;
i440fx_state = NULL;
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index d422b25..9347099 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -314,7 +314,8 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
hwaddr below_4g_mem_size,
ram_addr_t above_4g_mem_size,
MemoryRegion *pci_address_space,
- MemoryRegion *ram_memory)
+ MemoryRegion *ram_memory,
+ FWCfgState *fw_cfg)
{
DeviceState *dev;
PCIBus *b;
@@ -353,7 +354,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
}
pc_pci_as_mapping_init(OBJECT(d), f->system_memory,
- f->pci_address_space,
+ f->pci_address_space, fw_cfg,
&f->pci_hole, &f->pci_hole_64bit,
below_4g_mem_size,
0x100000000ULL - below_4g_mem_size,
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index cb8aea0..583585c 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -30,6 +30,7 @@
#include "hw/hw.h"
#include "hw/pci-host/q35.h"
#include "qapi/visitor.h"
+#include "hw/nvram/fw_cfg.h"
/****************************************************************************
* Q35 host
@@ -336,10 +337,11 @@ static int mch_init(PCIDevice *d)
{
int i;
MCHPCIState *mch = MCH_PCI_DEVICE(d);
+ FWCfgState *fw_cfg = FW_CFG(object_resolve_path(FW_CFG_PATH, NULL));
/* setup pci memory regions */
pc_pci_as_mapping_init(OBJECT(mch), mch->system_memory,
- mch->pci_address_space,
+ mch->pci_address_space, fw_cfg,
&mch->pci_hole, &mch->pci_hole_64bit,
mch->below_4g_mem_size,
0x100000000ULL - mch->below_4g_mem_size,
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index ed86642..6d0d8c9 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -110,7 +110,7 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size,
void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory,
- MemoryRegion *pci_address_space,
+ MemoryRegion *pci_address_space, FWCfgState *fw_cfg,
MemoryRegion *pci32_as, MemoryRegion *pci64_as,
uint32_t pci32_as_start, uint32_t pci32_as_size,
uint64_t pci64_as_start, uint64_t pci64_as_size);
@@ -164,7 +164,8 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn,
hwaddr below_4g_mem_size,
ram_addr_t above_4g_mem_size,
MemoryRegion *pci_memory,
- MemoryRegion *ram_memory);
+ MemoryRegion *ram_memory,
+ FWCfgState *fw_cfg);
/* piix4.c */
extern PCIDevice *piix4_dev;
--
1.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH 4/4] pc: add 'etc/pcimem64-minimum-address' fw_cfg interface to SeaBIOS
2013-10-16 8:49 ` [Qemu-devel] [PATCH 4/4] pc: add 'etc/pcimem64-minimum-address' fw_cfg interface to SeaBIOS Igor Mammedov
@ 2013-10-16 9:27 ` Michael S. Tsirkin
2013-10-16 9:29 ` Michael S. Tsirkin
1 sibling, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2013-10-16 9:27 UTC (permalink / raw)
To: Igor Mammedov
Cc: armbru, qemu-devel, blauwirbel, kraxel, aliguori, pbonzini, afaerber
On Wed, Oct 16, 2013 at 10:49:14AM +0200, Igor Mammedov wrote:
> 'etc/pcimem64-minimum-address' will allow QEMU to communicate to BIOS
> where PCI memory address space mapping starts in high memory.
>
> Allowing BIOS start mapping 64-bit PCI BARs at address where QEMU
> placed this mapping vs. hardcoded value right after highmem RAM.
>
> That will allow QEMU to reserve extra address space before
> 64-bit PCI hole for memory hotplug.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
So I don't like the name (seems to tell bios what to do instead of
describing hardware - it's really just the end of hot-pluggable RAM)
but it's not critical.
However I don't like that it touches pci-host devices.
Esp if my suggestion works out and we can remove the "pci hole"
concept from pci-host code, this interface also should be localized in
PC code.
> ---
> * SeaBIOS patch: http://patchwork.ozlabs.org/patch/283623/
>
> ---
> hw/i386/pc.c | 9 ++++++++-
> hw/i386/pc_piix.c | 2 +-
> hw/pci-host/piix.c | 5 +++--
> hw/pci-host/q35.c | 4 +++-
> include/hw/i386/pc.h | 5 +++--
> 5 files changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 7ecb028..ac99ae1 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1055,7 +1055,7 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size,
>
> /* setup pci memory regions mappings into system address space */
> void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory,
> - MemoryRegion *pci_address_space,
> + MemoryRegion *pci_address_space, FWCfgState *fw_cfg,
> MemoryRegion *pci32_as, MemoryRegion *pci64_as,
> uint32_t pci32_as_start, uint32_t pci32_as_size,
> uint64_t pci64_as_start, uint64_t pci64_as_size)
> @@ -1083,6 +1083,13 @@ void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory,
> memory_region_add_subregion(system_memory,
> ROUND_UP(pci64_as_start, 0x1ULL << 30),
> pci64_as);
> + if (fw_cfg) {
> + uint64_t *pcimem64_start = g_malloc(sizeof(*pcimem64_start));
> +
> + *pcimem64_start = cpu_to_le64(pci64_as_start);
> + fw_cfg_add_file(fw_cfg, "etc/pcimem64-minimum-address",
> + pcimem64_start, sizeof(*pcimem64_start));
> + }
> }
> }
>
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 2fed5fd..966c48a 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -148,7 +148,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
> system_memory, system_io,
> below_4g_mem_size,
> above_4g_mem_size,
> - pci_memory, ram_memory);
> + pci_memory, ram_memory, fw_cfg);
> } else {
> pci_bus = NULL;
> i440fx_state = NULL;
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index d422b25..9347099 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -314,7 +314,8 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
> hwaddr below_4g_mem_size,
> ram_addr_t above_4g_mem_size,
> MemoryRegion *pci_address_space,
> - MemoryRegion *ram_memory)
> + MemoryRegion *ram_memory,
> + FWCfgState *fw_cfg)
> {
> DeviceState *dev;
> PCIBus *b;
> @@ -353,7 +354,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
> }
>
> pc_pci_as_mapping_init(OBJECT(d), f->system_memory,
> - f->pci_address_space,
> + f->pci_address_space, fw_cfg,
> &f->pci_hole, &f->pci_hole_64bit,
> below_4g_mem_size,
> 0x100000000ULL - below_4g_mem_size,
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index cb8aea0..583585c 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -30,6 +30,7 @@
> #include "hw/hw.h"
> #include "hw/pci-host/q35.h"
> #include "qapi/visitor.h"
> +#include "hw/nvram/fw_cfg.h"
>
> /****************************************************************************
> * Q35 host
> @@ -336,10 +337,11 @@ static int mch_init(PCIDevice *d)
> {
> int i;
> MCHPCIState *mch = MCH_PCI_DEVICE(d);
> + FWCfgState *fw_cfg = FW_CFG(object_resolve_path(FW_CFG_PATH, NULL));
>
> /* setup pci memory regions */
> pc_pci_as_mapping_init(OBJECT(mch), mch->system_memory,
> - mch->pci_address_space,
> + mch->pci_address_space, fw_cfg,
> &mch->pci_hole, &mch->pci_hole_64bit,
> mch->below_4g_mem_size,
> 0x100000000ULL - mch->below_4g_mem_size,
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index ed86642..6d0d8c9 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -110,7 +110,7 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size,
>
>
> void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory,
> - MemoryRegion *pci_address_space,
> + MemoryRegion *pci_address_space, FWCfgState *fw_cfg,
> MemoryRegion *pci32_as, MemoryRegion *pci64_as,
> uint32_t pci32_as_start, uint32_t pci32_as_size,
> uint64_t pci64_as_start, uint64_t pci64_as_size);
> @@ -164,7 +164,8 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn,
> hwaddr below_4g_mem_size,
> ram_addr_t above_4g_mem_size,
> MemoryRegion *pci_memory,
> - MemoryRegion *ram_memory);
> + MemoryRegion *ram_memory,
> + FWCfgState *fw_cfg);
>
> /* piix4.c */
> extern PCIDevice *piix4_dev;
> --
> 1.7.1
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH 4/4] pc: add 'etc/pcimem64-minimum-address' fw_cfg interface to SeaBIOS
2013-10-16 8:49 ` [Qemu-devel] [PATCH 4/4] pc: add 'etc/pcimem64-minimum-address' fw_cfg interface to SeaBIOS Igor Mammedov
2013-10-16 9:27 ` Michael S. Tsirkin
@ 2013-10-16 9:29 ` Michael S. Tsirkin
2013-10-16 12:19 ` Igor Mammedov
1 sibling, 1 reply; 14+ messages in thread
From: Michael S. Tsirkin @ 2013-10-16 9:29 UTC (permalink / raw)
To: Igor Mammedov
Cc: armbru, qemu-devel, blauwirbel, kraxel, aliguori, pbonzini, afaerber
On Wed, Oct 16, 2013 at 10:49:14AM +0200, Igor Mammedov wrote:
> 'etc/pcimem64-minimum-address' will allow QEMU to communicate to BIOS
> where PCI memory address space mapping starts in high memory.
>
> Allowing BIOS start mapping 64-bit PCI BARs at address where QEMU
> placed this mapping vs. hardcoded value right after highmem RAM.
>
> That will allow QEMU to reserve extra address space before
> 64-bit PCI hole for memory hotplug.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> * SeaBIOS patch: http://patchwork.ozlabs.org/patch/283623/
>
> ---
> hw/i386/pc.c | 9 ++++++++-
> hw/i386/pc_piix.c | 2 +-
> hw/pci-host/piix.c | 5 +++--
> hw/pci-host/q35.c | 4 +++-
> include/hw/i386/pc.h | 5 +++--
> 5 files changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 7ecb028..ac99ae1 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1055,7 +1055,7 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size,
>
> /* setup pci memory regions mappings into system address space */
> void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory,
> - MemoryRegion *pci_address_space,
> + MemoryRegion *pci_address_space, FWCfgState *fw_cfg,
> MemoryRegion *pci32_as, MemoryRegion *pci64_as,
> uint32_t pci32_as_start, uint32_t pci32_as_size,
> uint64_t pci64_as_start, uint64_t pci64_as_size)
> @@ -1083,6 +1083,13 @@ void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory,
> memory_region_add_subregion(system_memory,
> ROUND_UP(pci64_as_start, 0x1ULL << 30),
> pci64_as);
> + if (fw_cfg) {
> + uint64_t *pcimem64_start = g_malloc(sizeof(*pcimem64_start));
> +
> + *pcimem64_start = cpu_to_le64(pci64_as_start);
> + fw_cfg_add_file(fw_cfg, "etc/pcimem64-minimum-address",
> + pcimem64_start, sizeof(*pcimem64_start));
> + }
> }
> }
>
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 2fed5fd..966c48a 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -148,7 +148,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
> system_memory, system_io,
> below_4g_mem_size,
> above_4g_mem_size,
> - pci_memory, ram_memory);
> + pci_memory, ram_memory, fw_cfg);
> } else {
> pci_bus = NULL;
> i440fx_state = NULL;
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index d422b25..9347099 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -314,7 +314,8 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
> hwaddr below_4g_mem_size,
> ram_addr_t above_4g_mem_size,
> MemoryRegion *pci_address_space,
> - MemoryRegion *ram_memory)
> + MemoryRegion *ram_memory,
> + FWCfgState *fw_cfg)
> {
> DeviceState *dev;
> PCIBus *b;
> @@ -353,7 +354,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
> }
>
> pc_pci_as_mapping_init(OBJECT(d), f->system_memory,
> - f->pci_address_space,
> + f->pci_address_space, fw_cfg,
> &f->pci_hole, &f->pci_hole_64bit,
> below_4g_mem_size,
> 0x100000000ULL - below_4g_mem_size,
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index cb8aea0..583585c 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -30,6 +30,7 @@
> #include "hw/hw.h"
> #include "hw/pci-host/q35.h"
> #include "qapi/visitor.h"
> +#include "hw/nvram/fw_cfg.h"
>
> /****************************************************************************
> * Q35 host
> @@ -336,10 +337,11 @@ static int mch_init(PCIDevice *d)
> {
> int i;
> MCHPCIState *mch = MCH_PCI_DEVICE(d);
> + FWCfgState *fw_cfg = FW_CFG(object_resolve_path(FW_CFG_PATH, NULL));
Seems to duplicate code from fw_cfg_find().
Why not just use it?
>
> /* setup pci memory regions */
> pc_pci_as_mapping_init(OBJECT(mch), mch->system_memory,
> - mch->pci_address_space,
> + mch->pci_address_space, fw_cfg,
> &mch->pci_hole, &mch->pci_hole_64bit,
> mch->below_4g_mem_size,
> 0x100000000ULL - mch->below_4g_mem_size,
Or call fw_cfg_find internally in pc_pci_as_mapping_init?
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index ed86642..6d0d8c9 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -110,7 +110,7 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size,
>
>
> void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory,
> - MemoryRegion *pci_address_space,
> + MemoryRegion *pci_address_space, FWCfgState *fw_cfg,
> MemoryRegion *pci32_as, MemoryRegion *pci64_as,
> uint32_t pci32_as_start, uint32_t pci32_as_size,
> uint64_t pci64_as_start, uint64_t pci64_as_size);
> @@ -164,7 +164,8 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn,
> hwaddr below_4g_mem_size,
> ram_addr_t above_4g_mem_size,
> MemoryRegion *pci_memory,
> - MemoryRegion *ram_memory);
> + MemoryRegion *ram_memory,
> + FWCfgState *fw_cfg);
>
> /* piix4.c */
> extern PCIDevice *piix4_dev;
> --
> 1.7.1
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH 4/4] pc: add 'etc/pcimem64-minimum-address' fw_cfg interface to SeaBIOS
2013-10-16 9:29 ` Michael S. Tsirkin
@ 2013-10-16 12:19 ` Igor Mammedov
2013-10-16 13:42 ` Michael S. Tsirkin
0 siblings, 1 reply; 14+ messages in thread
From: Igor Mammedov @ 2013-10-16 12:19 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: armbru, qemu-devel, blauwirbel, kraxel, aliguori, pbonzini, afaerber
On Wed, 16 Oct 2013 12:29:48 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Wed, Oct 16, 2013 at 10:49:14AM +0200, Igor Mammedov wrote:
> > 'etc/pcimem64-minimum-address' will allow QEMU to communicate to BIOS
> > where PCI memory address space mapping starts in high memory.
> >
> > Allowing BIOS start mapping 64-bit PCI BARs at address where QEMU
> > placed this mapping vs. hardcoded value right after highmem RAM.
> >
> > That will allow QEMU to reserve extra address space before
> > 64-bit PCI hole for memory hotplug.
> >
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > * SeaBIOS patch: http://patchwork.ozlabs.org/patch/283623/
> >
> > ---
> > hw/i386/pc.c | 9 ++++++++-
> > hw/i386/pc_piix.c | 2 +-
> > hw/pci-host/piix.c | 5 +++--
> > hw/pci-host/q35.c | 4 +++-
> > include/hw/i386/pc.h | 5 +++--
> > 5 files changed, 18 insertions(+), 7 deletions(-)
> >
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 7ecb028..ac99ae1 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -1055,7 +1055,7 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size,
> >
> > /* setup pci memory regions mappings into system address space */
> > void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory,
> > - MemoryRegion *pci_address_space,
> > + MemoryRegion *pci_address_space, FWCfgState *fw_cfg,
> > MemoryRegion *pci32_as, MemoryRegion *pci64_as,
> > uint32_t pci32_as_start, uint32_t pci32_as_size,
> > uint64_t pci64_as_start, uint64_t pci64_as_size)
> > @@ -1083,6 +1083,13 @@ void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory,
> > memory_region_add_subregion(system_memory,
> > ROUND_UP(pci64_as_start, 0x1ULL << 30),
> > pci64_as);
> > + if (fw_cfg) {
> > + uint64_t *pcimem64_start = g_malloc(sizeof(*pcimem64_start));
> > +
> > + *pcimem64_start = cpu_to_le64(pci64_as_start);
> > + fw_cfg_add_file(fw_cfg, "etc/pcimem64-minimum-address",
> > + pcimem64_start, sizeof(*pcimem64_start));
> > + }
> > }
> > }
> >
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index 2fed5fd..966c48a 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -148,7 +148,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
> > system_memory, system_io,
> > below_4g_mem_size,
> > above_4g_mem_size,
> > - pci_memory, ram_memory);
> > + pci_memory, ram_memory, fw_cfg);
> > } else {
> > pci_bus = NULL;
> > i440fx_state = NULL;
> > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> > index d422b25..9347099 100644
> > --- a/hw/pci-host/piix.c
> > +++ b/hw/pci-host/piix.c
> > @@ -314,7 +314,8 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
> > hwaddr below_4g_mem_size,
> > ram_addr_t above_4g_mem_size,
> > MemoryRegion *pci_address_space,
> > - MemoryRegion *ram_memory)
> > + MemoryRegion *ram_memory,
> > + FWCfgState *fw_cfg)
> > {
> > DeviceState *dev;
> > PCIBus *b;
> > @@ -353,7 +354,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
> > }
> >
> > pc_pci_as_mapping_init(OBJECT(d), f->system_memory,
> > - f->pci_address_space,
> > + f->pci_address_space, fw_cfg,
> > &f->pci_hole, &f->pci_hole_64bit,
> > below_4g_mem_size,
> > 0x100000000ULL - below_4g_mem_size,
> > diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> > index cb8aea0..583585c 100644
> > --- a/hw/pci-host/q35.c
> > +++ b/hw/pci-host/q35.c
> > @@ -30,6 +30,7 @@
> > #include "hw/hw.h"
> > #include "hw/pci-host/q35.h"
> > #include "qapi/visitor.h"
> > +#include "hw/nvram/fw_cfg.h"
> >
> > /****************************************************************************
> > * Q35 host
> > @@ -336,10 +337,11 @@ static int mch_init(PCIDevice *d)
> > {
> > int i;
> > MCHPCIState *mch = MCH_PCI_DEVICE(d);
> > + FWCfgState *fw_cfg = FW_CFG(object_resolve_path(FW_CFG_PATH, NULL));
>
> Seems to duplicate code from fw_cfg_find().
> Why not just use it?
I've thought CAST(object_resolve_path()) is preferred method to do it,
But I can surely use fw_cfg_find().
>
> >
> > /* setup pci memory regions */
> > pc_pci_as_mapping_init(OBJECT(mch), mch->system_memory,
> > - mch->pci_address_space,
> > + mch->pci_address_space, fw_cfg,
> > &mch->pci_hole, &mch->pci_hole_64bit,
> > mch->below_4g_mem_size,
> > 0x100000000ULL - mch->below_4g_mem_size,
>
> Or call fw_cfg_find internally in pc_pci_as_mapping_init?
I'll do it, thanks!
>
> > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > index ed86642..6d0d8c9 100644
> > --- a/include/hw/i386/pc.h
> > +++ b/include/hw/i386/pc.h
> > @@ -110,7 +110,7 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size,
> >
> >
> > void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory,
> > - MemoryRegion *pci_address_space,
> > + MemoryRegion *pci_address_space, FWCfgState *fw_cfg,
> > MemoryRegion *pci32_as, MemoryRegion *pci64_as,
> > uint32_t pci32_as_start, uint32_t pci32_as_size,
> > uint64_t pci64_as_start, uint64_t pci64_as_size);
> > @@ -164,7 +164,8 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn,
> > hwaddr below_4g_mem_size,
> > ram_addr_t above_4g_mem_size,
> > MemoryRegion *pci_memory,
> > - MemoryRegion *ram_memory);
> > + MemoryRegion *ram_memory,
> > + FWCfgState *fw_cfg);
> >
> > /* piix4.c */
> > extern PCIDevice *piix4_dev;
> > --
> > 1.7.1
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH 4/4] pc: add 'etc/pcimem64-minimum-address' fw_cfg interface to SeaBIOS
2013-10-16 12:19 ` Igor Mammedov
@ 2013-10-16 13:42 ` Michael S. Tsirkin
0 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2013-10-16 13:42 UTC (permalink / raw)
To: Igor Mammedov
Cc: armbru, qemu-devel, blauwirbel, kraxel, aliguori, pbonzini, afaerber
On Wed, Oct 16, 2013 at 02:19:13PM +0200, Igor Mammedov wrote:
> On Wed, 16 Oct 2013 12:29:48 +0300
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > On Wed, Oct 16, 2013 at 10:49:14AM +0200, Igor Mammedov wrote:
> > > 'etc/pcimem64-minimum-address' will allow QEMU to communicate to BIOS
> > > where PCI memory address space mapping starts in high memory.
> > >
> > > Allowing BIOS start mapping 64-bit PCI BARs at address where QEMU
> > > placed this mapping vs. hardcoded value right after highmem RAM.
> > >
> > > That will allow QEMU to reserve extra address space before
> > > 64-bit PCI hole for memory hotplug.
> > >
> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > ---
> > > * SeaBIOS patch: http://patchwork.ozlabs.org/patch/283623/
> > >
> > > ---
> > > hw/i386/pc.c | 9 ++++++++-
> > > hw/i386/pc_piix.c | 2 +-
> > > hw/pci-host/piix.c | 5 +++--
> > > hw/pci-host/q35.c | 4 +++-
> > > include/hw/i386/pc.h | 5 +++--
> > > 5 files changed, 18 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > > index 7ecb028..ac99ae1 100644
> > > --- a/hw/i386/pc.c
> > > +++ b/hw/i386/pc.c
> > > @@ -1055,7 +1055,7 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size,
> > >
> > > /* setup pci memory regions mappings into system address space */
> > > void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory,
> > > - MemoryRegion *pci_address_space,
> > > + MemoryRegion *pci_address_space, FWCfgState *fw_cfg,
> > > MemoryRegion *pci32_as, MemoryRegion *pci64_as,
> > > uint32_t pci32_as_start, uint32_t pci32_as_size,
> > > uint64_t pci64_as_start, uint64_t pci64_as_size)
> > > @@ -1083,6 +1083,13 @@ void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory,
> > > memory_region_add_subregion(system_memory,
> > > ROUND_UP(pci64_as_start, 0x1ULL << 30),
> > > pci64_as);
> > > + if (fw_cfg) {
> > > + uint64_t *pcimem64_start = g_malloc(sizeof(*pcimem64_start));
> > > +
> > > + *pcimem64_start = cpu_to_le64(pci64_as_start);
> > > + fw_cfg_add_file(fw_cfg, "etc/pcimem64-minimum-address",
> > > + pcimem64_start, sizeof(*pcimem64_start));
> > > + }
> > > }
> > > }
> > >
> > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > > index 2fed5fd..966c48a 100644
> > > --- a/hw/i386/pc_piix.c
> > > +++ b/hw/i386/pc_piix.c
> > > @@ -148,7 +148,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
> > > system_memory, system_io,
> > > below_4g_mem_size,
> > > above_4g_mem_size,
> > > - pci_memory, ram_memory);
> > > + pci_memory, ram_memory, fw_cfg);
> > > } else {
> > > pci_bus = NULL;
> > > i440fx_state = NULL;
> > > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> > > index d422b25..9347099 100644
> > > --- a/hw/pci-host/piix.c
> > > +++ b/hw/pci-host/piix.c
> > > @@ -314,7 +314,8 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
> > > hwaddr below_4g_mem_size,
> > > ram_addr_t above_4g_mem_size,
> > > MemoryRegion *pci_address_space,
> > > - MemoryRegion *ram_memory)
> > > + MemoryRegion *ram_memory,
> > > + FWCfgState *fw_cfg)
> > > {
> > > DeviceState *dev;
> > > PCIBus *b;
> > > @@ -353,7 +354,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
> > > }
> > >
> > > pc_pci_as_mapping_init(OBJECT(d), f->system_memory,
> > > - f->pci_address_space,
> > > + f->pci_address_space, fw_cfg,
> > > &f->pci_hole, &f->pci_hole_64bit,
> > > below_4g_mem_size,
> > > 0x100000000ULL - below_4g_mem_size,
> > > diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> > > index cb8aea0..583585c 100644
> > > --- a/hw/pci-host/q35.c
> > > +++ b/hw/pci-host/q35.c
> > > @@ -30,6 +30,7 @@
> > > #include "hw/hw.h"
> > > #include "hw/pci-host/q35.h"
> > > #include "qapi/visitor.h"
> > > +#include "hw/nvram/fw_cfg.h"
> > >
> > > /****************************************************************************
> > > * Q35 host
> > > @@ -336,10 +337,11 @@ static int mch_init(PCIDevice *d)
> > > {
> > > int i;
> > > MCHPCIState *mch = MCH_PCI_DEVICE(d);
> > > + FWCfgState *fw_cfg = FW_CFG(object_resolve_path(FW_CFG_PATH, NULL));
> >
> > Seems to duplicate code from fw_cfg_find().
> > Why not just use it?
> I've thought CAST(object_resolve_path()) is preferred method to do it,
> But I can surely use fw_cfg_find().
I've no idea what's preferable, but duplicating code generally
is considered a bad idea in C.
> >
> > >
> > > /* setup pci memory regions */
> > > pc_pci_as_mapping_init(OBJECT(mch), mch->system_memory,
> > > - mch->pci_address_space,
> > > + mch->pci_address_space, fw_cfg,
> > > &mch->pci_hole, &mch->pci_hole_64bit,
> > > mch->below_4g_mem_size,
> > > 0x100000000ULL - mch->below_4g_mem_size,
> >
> > Or call fw_cfg_find internally in pc_pci_as_mapping_init?
> I'll do it, thanks!
>
> >
> > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > > index ed86642..6d0d8c9 100644
> > > --- a/include/hw/i386/pc.h
> > > +++ b/include/hw/i386/pc.h
> > > @@ -110,7 +110,7 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size,
> > >
> > >
> > > void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory,
> > > - MemoryRegion *pci_address_space,
> > > + MemoryRegion *pci_address_space, FWCfgState *fw_cfg,
> > > MemoryRegion *pci32_as, MemoryRegion *pci64_as,
> > > uint32_t pci32_as_start, uint32_t pci32_as_size,
> > > uint64_t pci64_as_start, uint64_t pci64_as_size);
> > > @@ -164,7 +164,8 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn,
> > > hwaddr below_4g_mem_size,
> > > ram_addr_t above_4g_mem_size,
> > > MemoryRegion *pci_memory,
> > > - MemoryRegion *ram_memory);
> > > + MemoryRegion *ram_memory,
> > > + FWCfgState *fw_cfg);
> > >
> > > /* piix4.c */
> > > extern PCIDevice *piix4_dev;
> > > --
> > > 1.7.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 0/4] pc: inform SeaBIOS where 64-bit PCI hole start and PCI mappings cleanup
2013-10-16 8:49 [Qemu-devel] [PATCH 0/4] pc: inform SeaBIOS where 64-bit PCI hole start and PCI mappings cleanup Igor Mammedov
` (3 preceding siblings ...)
2013-10-16 8:49 ` [Qemu-devel] [PATCH 4/4] pc: add 'etc/pcimem64-minimum-address' fw_cfg interface to SeaBIOS Igor Mammedov
@ 2013-10-16 9:20 ` Michael S. Tsirkin
2013-10-29 10:08 ` Igor Mammedov
4 siblings, 1 reply; 14+ messages in thread
From: Michael S. Tsirkin @ 2013-10-16 9:20 UTC (permalink / raw)
To: Igor Mammedov
Cc: armbru, qemu-devel, blauwirbel, kraxel, aliguori, pbonzini, afaerber
On Wed, Oct 16, 2013 at 10:49:10AM +0200, Igor Mammedov wrote:
> * simplify PCI address space mapping into system address space,
> replacing code duplication in piix/q53 PCs with helper function
I think this does not go far enough.
I was always wondering about PCI hole in QEMU.
Some real PCs have a "PCI hole" where PCI
masks real memory, but PIIX does not do this,
instead PCI is whenever RAM does not mask it.
So it looks like the hole concept is a left-over
from when we didn't have priorities in the memory API.
How about we remove them?
See patch below.
I did a quick boot test and it seems to work, of course
it needs much more testing.
It's on top of Marcel's series adding negative priorities,
so works on top of the acpi branch or the pci branch.
I'm also wondering about the smram region - it uses
priority 1 but does not say why.
Need to check what does it overlap with, and why.
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index bad3953..988516a 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -102,8 +102,6 @@ struct PCII440FXState {
MemoryRegion *system_memory;
MemoryRegion *pci_address_space;
MemoryRegion *ram_memory;
- MemoryRegion pci_hole;
- MemoryRegion pci_hole_64bit;
PAMMemoryRegion pam_regions[13];
MemoryRegion smram_region;
uint8_t smm_enabled;
@@ -326,7 +324,6 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
PCII440FXState *f;
unsigned i;
I440FXState *i440fx;
- uint64_t pci_hole64_size;
dev = qdev_create(NULL, TYPE_I440FX_PCI_HOST_BRIDGE);
s = PCI_HOST_BRIDGE(dev);
@@ -354,23 +351,9 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
i440fx->pci_info.w32.begin = 0xe0000000;
}
- memory_region_init_alias(&f->pci_hole, OBJECT(d), "pci-hole", f->pci_address_space,
- pci_hole_start, pci_hole_size);
- memory_region_add_subregion(f->system_memory, pci_hole_start, &f->pci_hole);
-
- pci_hole64_size = pci_host_get_hole64_size(i440fx->pci_hole64_size);
-
- pc_init_pci64_hole(&i440fx->pci_info, 0x100000000ULL + above_4g_mem_size,
- pci_hole64_size);
- memory_region_init_alias(&f->pci_hole_64bit, OBJECT(d), "pci-hole64",
- f->pci_address_space,
- i440fx->pci_info.w64.begin,
- pci_hole64_size);
- if (pci_hole64_size) {
- memory_region_add_subregion(f->system_memory,
- i440fx->pci_info.w64.begin,
- &f->pci_hole_64bit);
- }
+ /* Set to lower priority than RAM */
+ memory_region_add_subregion_overlap(f->system_memory, 0x0,
+ f->pci_address_space, -1);
memory_region_init_alias(&f->smram_region, OBJECT(d), "smram-region",
f->pci_address_space, 0xa0000, 0x20000);
memory_region_add_subregion_overlap(f->system_memory, 0xa0000,
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH 0/4] pc: inform SeaBIOS where 64-bit PCI hole start and PCI mappings cleanup
2013-10-16 9:20 ` [Qemu-devel] [PATCH 0/4] pc: inform SeaBIOS where 64-bit PCI hole start and PCI mappings cleanup Michael S. Tsirkin
@ 2013-10-29 10:08 ` Igor Mammedov
2013-10-29 10:22 ` Michael S. Tsirkin
2013-10-29 11:49 ` Michael S. Tsirkin
0 siblings, 2 replies; 14+ messages in thread
From: Igor Mammedov @ 2013-10-29 10:08 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: armbru, qemu-devel, blauwirbel, kraxel, aliguori, pbonzini, afaerber
On Wed, 16 Oct 2013 12:20:45 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Wed, Oct 16, 2013 at 10:49:10AM +0200, Igor Mammedov wrote:
> > * simplify PCI address space mapping into system address space,
> > replacing code duplication in piix/q53 PCs with helper function
>
> I think this does not go far enough.
>
> I was always wondering about PCI hole in QEMU.
> Some real PCs have a "PCI hole" where PCI
> masks real memory, but PIIX does not do this,
> instead PCI is whenever RAM does not mask it.
>
> So it looks like the hole concept is a left-over
> from when we didn't have priorities in the memory API.
> How about we remove them?
> See patch below.
> I did a quick boot test and it seems to work, of course
> it needs much more testing.
> It's on top of Marcel's series adding negative priorities,
> so works on top of the acpi branch or the pci branch.
I have done quite thorough testing and it works well except of
one caveat, it breaks migration due to different memory regions
layout.
So we'll have to keep an old aliasing scheme at least for old
machine types. Having that in mind do we still want to add
an extra implementation as you suggested?
>
> I'm also wondering about the smram region - it uses
> priority 1 but does not say why.
> Need to check what does it overlap with, and why.
>
>
>
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index bad3953..988516a 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -102,8 +102,6 @@ struct PCII440FXState {
> MemoryRegion *system_memory;
> MemoryRegion *pci_address_space;
> MemoryRegion *ram_memory;
> - MemoryRegion pci_hole;
> - MemoryRegion pci_hole_64bit;
> PAMMemoryRegion pam_regions[13];
> MemoryRegion smram_region;
> uint8_t smm_enabled;
> @@ -326,7 +324,6 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
> PCII440FXState *f;
> unsigned i;
> I440FXState *i440fx;
> - uint64_t pci_hole64_size;
>
> dev = qdev_create(NULL, TYPE_I440FX_PCI_HOST_BRIDGE);
> s = PCI_HOST_BRIDGE(dev);
> @@ -354,23 +351,9 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
> i440fx->pci_info.w32.begin = 0xe0000000;
> }
>
> - memory_region_init_alias(&f->pci_hole, OBJECT(d), "pci-hole", f->pci_address_space,
> - pci_hole_start, pci_hole_size);
> - memory_region_add_subregion(f->system_memory, pci_hole_start, &f->pci_hole);
> -
> - pci_hole64_size = pci_host_get_hole64_size(i440fx->pci_hole64_size);
> -
> - pc_init_pci64_hole(&i440fx->pci_info, 0x100000000ULL + above_4g_mem_size,
> - pci_hole64_size);
> - memory_region_init_alias(&f->pci_hole_64bit, OBJECT(d), "pci-hole64",
> - f->pci_address_space,
> - i440fx->pci_info.w64.begin,
> - pci_hole64_size);
> - if (pci_hole64_size) {
> - memory_region_add_subregion(f->system_memory,
> - i440fx->pci_info.w64.begin,
> - &f->pci_hole_64bit);
> - }
> + /* Set to lower priority than RAM */
> + memory_region_add_subregion_overlap(f->system_memory, 0x0,
> + f->pci_address_space, -1);
> memory_region_init_alias(&f->smram_region, OBJECT(d), "smram-region",
> f->pci_address_space, 0xa0000, 0x20000);
> memory_region_add_subregion_overlap(f->system_memory, 0xa0000,
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH 0/4] pc: inform SeaBIOS where 64-bit PCI hole start and PCI mappings cleanup
2013-10-29 10:08 ` Igor Mammedov
@ 2013-10-29 10:22 ` Michael S. Tsirkin
2013-10-29 11:17 ` Igor Mammedov
2013-10-29 11:49 ` Michael S. Tsirkin
1 sibling, 1 reply; 14+ messages in thread
From: Michael S. Tsirkin @ 2013-10-29 10:22 UTC (permalink / raw)
To: Igor Mammedov
Cc: armbru, qemu-devel, blauwirbel, kraxel, aliguori, pbonzini, afaerber
On Tue, Oct 29, 2013 at 11:08:56AM +0100, Igor Mammedov wrote:
> On Wed, 16 Oct 2013 12:20:45 +0300
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > On Wed, Oct 16, 2013 at 10:49:10AM +0200, Igor Mammedov wrote:
> > > * simplify PCI address space mapping into system address space,
> > > replacing code duplication in piix/q53 PCs with helper function
> >
> > I think this does not go far enough.
> >
> > I was always wondering about PCI hole in QEMU.
> > Some real PCs have a "PCI hole" where PCI
> > masks real memory, but PIIX does not do this,
> > instead PCI is whenever RAM does not mask it.
> >
> > So it looks like the hole concept is a left-over
> > from when we didn't have priorities in the memory API.
> > How about we remove them?
> > See patch below.
> > I did a quick boot test and it seems to work, of course
> > it needs much more testing.
> > It's on top of Marcel's series adding negative priorities,
> > so works on top of the acpi branch or the pci branch.
> I have done quite thorough testing and it works well except of
> one caveat, it breaks migration due to different memory regions
> layout.
Interesting. We don't migrate PCI memory regions so what's going on?
> So we'll have to keep an old aliasing scheme at least for old
> machine types. Having that in mind do we still want to add
> an extra implementation as you suggested?
>
> >
> > I'm also wondering about the smram region - it uses
> > priority 1 but does not say why.
> > Need to check what does it overlap with, and why.
> >
> >
> >
> > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> > index bad3953..988516a 100644
> > --- a/hw/pci-host/piix.c
> > +++ b/hw/pci-host/piix.c
> > @@ -102,8 +102,6 @@ struct PCII440FXState {
> > MemoryRegion *system_memory;
> > MemoryRegion *pci_address_space;
> > MemoryRegion *ram_memory;
> > - MemoryRegion pci_hole;
> > - MemoryRegion pci_hole_64bit;
> > PAMMemoryRegion pam_regions[13];
> > MemoryRegion smram_region;
> > uint8_t smm_enabled;
> > @@ -326,7 +324,6 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
> > PCII440FXState *f;
> > unsigned i;
> > I440FXState *i440fx;
> > - uint64_t pci_hole64_size;
> >
> > dev = qdev_create(NULL, TYPE_I440FX_PCI_HOST_BRIDGE);
> > s = PCI_HOST_BRIDGE(dev);
> > @@ -354,23 +351,9 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
> > i440fx->pci_info.w32.begin = 0xe0000000;
> > }
> >
> > - memory_region_init_alias(&f->pci_hole, OBJECT(d), "pci-hole", f->pci_address_space,
> > - pci_hole_start, pci_hole_size);
> > - memory_region_add_subregion(f->system_memory, pci_hole_start, &f->pci_hole);
> > -
> > - pci_hole64_size = pci_host_get_hole64_size(i440fx->pci_hole64_size);
> > -
> > - pc_init_pci64_hole(&i440fx->pci_info, 0x100000000ULL + above_4g_mem_size,
> > - pci_hole64_size);
> > - memory_region_init_alias(&f->pci_hole_64bit, OBJECT(d), "pci-hole64",
> > - f->pci_address_space,
> > - i440fx->pci_info.w64.begin,
> > - pci_hole64_size);
> > - if (pci_hole64_size) {
> > - memory_region_add_subregion(f->system_memory,
> > - i440fx->pci_info.w64.begin,
> > - &f->pci_hole_64bit);
> > - }
> > + /* Set to lower priority than RAM */
> > + memory_region_add_subregion_overlap(f->system_memory, 0x0,
> > + f->pci_address_space, -1);
> > memory_region_init_alias(&f->smram_region, OBJECT(d), "smram-region",
> > f->pci_address_space, 0xa0000, 0x20000);
> > memory_region_add_subregion_overlap(f->system_memory, 0xa0000,
> >
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH 0/4] pc: inform SeaBIOS where 64-bit PCI hole start and PCI mappings cleanup
2013-10-29 10:22 ` Michael S. Tsirkin
@ 2013-10-29 11:17 ` Igor Mammedov
0 siblings, 0 replies; 14+ messages in thread
From: Igor Mammedov @ 2013-10-29 11:17 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: armbru, qemu-devel, blauwirbel, kraxel, aliguori, pbonzini, afaerber
On Tue, 29 Oct 2013 12:22:09 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Tue, Oct 29, 2013 at 11:08:56AM +0100, Igor Mammedov wrote:
> > On Wed, 16 Oct 2013 12:20:45 +0300
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >
> > > On Wed, Oct 16, 2013 at 10:49:10AM +0200, Igor Mammedov wrote:
> > > > * simplify PCI address space mapping into system address space,
> > > > replacing code duplication in piix/q53 PCs with helper function
> > >
> > > I think this does not go far enough.
> > >
> > > I was always wondering about PCI hole in QEMU.
> > > Some real PCs have a "PCI hole" where PCI
> > > masks real memory, but PIIX does not do this,
> > > instead PCI is whenever RAM does not mask it.
> > >
> > > So it looks like the hole concept is a left-over
> > > from when we didn't have priorities in the memory API.
> > > How about we remove them?
> > > See patch below.
> > > I did a quick boot test and it seems to work, of course
> > > it needs much more testing.
> > > It's on top of Marcel's series adding negative priorities,
> > > so works on top of the acpi branch or the pci branch.
> > I have done quite thorough testing and it works well except of
> > one caveat, it breaks migration due to different memory regions
> > layout.
>
> Interesting. We don't migrate PCI memory regions so what's going on?
I'm sorry for noise, it was a false alarm due to wrong testing
configuration. It was failing with:
"
qemu-system-x86_64: pci_add_option_rom: failed to find romfile "efi-e1000.rom"
Unknown ramblock "0000:03.0/e1000.rom", cannot accept migration
"
Caused by incorrect -L option. It works fine when correct BIOS path is
provided.
One more questions about whether we should disable "etc/pci-info" fw_cfg
for 1.7 again since Seabios doesn't use it (and it looks like it's not
going to do so)?
>
> > So we'll have to keep an old aliasing scheme at least for old
> > machine types. Having that in mind do we still want to add
> > an extra implementation as you suggested?
> >
> > >
> > > I'm also wondering about the smram region - it uses
> > > priority 1 but does not say why.
> > > Need to check what does it overlap with, and why.
> > >
> > >
> > >
> > > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> > > index bad3953..988516a 100644
> > > --- a/hw/pci-host/piix.c
> > > +++ b/hw/pci-host/piix.c
> > > @@ -102,8 +102,6 @@ struct PCII440FXState {
> > > MemoryRegion *system_memory;
> > > MemoryRegion *pci_address_space;
> > > MemoryRegion *ram_memory;
> > > - MemoryRegion pci_hole;
> > > - MemoryRegion pci_hole_64bit;
> > > PAMMemoryRegion pam_regions[13];
> > > MemoryRegion smram_region;
> > > uint8_t smm_enabled;
> > > @@ -326,7 +324,6 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
> > > PCII440FXState *f;
> > > unsigned i;
> > > I440FXState *i440fx;
> > > - uint64_t pci_hole64_size;
> > >
> > > dev = qdev_create(NULL, TYPE_I440FX_PCI_HOST_BRIDGE);
> > > s = PCI_HOST_BRIDGE(dev);
> > > @@ -354,23 +351,9 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
> > > i440fx->pci_info.w32.begin = 0xe0000000;
> > > }
> > >
> > > - memory_region_init_alias(&f->pci_hole, OBJECT(d), "pci-hole", f->pci_address_space,
> > > - pci_hole_start, pci_hole_size);
> > > - memory_region_add_subregion(f->system_memory, pci_hole_start, &f->pci_hole);
> > > -
> > > - pci_hole64_size = pci_host_get_hole64_size(i440fx->pci_hole64_size);
> > > -
> > > - pc_init_pci64_hole(&i440fx->pci_info, 0x100000000ULL + above_4g_mem_size,
> > > - pci_hole64_size);
> > > - memory_region_init_alias(&f->pci_hole_64bit, OBJECT(d), "pci-hole64",
> > > - f->pci_address_space,
> > > - i440fx->pci_info.w64.begin,
> > > - pci_hole64_size);
> > > - if (pci_hole64_size) {
> > > - memory_region_add_subregion(f->system_memory,
> > > - i440fx->pci_info.w64.begin,
> > > - &f->pci_hole_64bit);
> > > - }
> > > + /* Set to lower priority than RAM */
> > > + memory_region_add_subregion_overlap(f->system_memory, 0x0,
> > > + f->pci_address_space, -1);
> > > memory_region_init_alias(&f->smram_region, OBJECT(d), "smram-region",
> > > f->pci_address_space, 0xa0000, 0x20000);
> > > memory_region_add_subregion_overlap(f->system_memory, 0xa0000,
> > >
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 0/4] pc: inform SeaBIOS where 64-bit PCI hole start and PCI mappings cleanup
2013-10-29 10:08 ` Igor Mammedov
2013-10-29 10:22 ` Michael S. Tsirkin
@ 2013-10-29 11:49 ` Michael S. Tsirkin
1 sibling, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2013-10-29 11:49 UTC (permalink / raw)
To: Igor Mammedov
Cc: armbru, qemu-devel, blauwirbel, kraxel, aliguori, pbonzini, afaerber
On Tue, Oct 29, 2013 at 11:08:56AM +0100, Igor Mammedov wrote:
> On Wed, 16 Oct 2013 12:20:45 +0300
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > On Wed, Oct 16, 2013 at 10:49:10AM +0200, Igor Mammedov wrote:
> > > * simplify PCI address space mapping into system address space,
> > > replacing code duplication in piix/q53 PCs with helper function
> >
> > I think this does not go far enough.
> >
> > I was always wondering about PCI hole in QEMU.
> > Some real PCs have a "PCI hole" where PCI
> > masks real memory, but PIIX does not do this,
> > instead PCI is whenever RAM does not mask it.
> >
> > So it looks like the hole concept is a left-over
> > from when we didn't have priorities in the memory API.
> > How about we remove them?
> > See patch below.
> > I did a quick boot test and it seems to work, of course
> > it needs much more testing.
> > It's on top of Marcel's series adding negative priorities,
> > so works on top of the acpi branch or the pci branch.
> I have done quite thorough testing and it works well except of
> one caveat, it breaks migration due to different memory regions
> layout.
>
> So we'll have to keep an old aliasing scheme at least for old
> machine types. Having that in mind do we still want to add
> an extra implementation as you suggested?
Sorry I didn't answer this question.
I think it's a bug - PCI hole should not affect migration.
^ permalink raw reply [flat|nested] 14+ messages in thread