* [Qemu-devel] [PATCH for-1.8 0/2 v3] pc: inform SeaBIOS where 64-bit PCI hole begins
@ 2013-11-12 13:58 Igor Mammedov
2013-11-12 13:58 ` [Qemu-devel] [PATCH 1/2] pc: map PCI address space as catchall region for not mapped addresses Igor Mammedov
2013-11-12 13:58 ` [Qemu-devel] [PATCH 2/2] pc: add 'etc/reserved-memory-end' fw_cfg interface for SeaBIOS Igor Mammedov
0 siblings, 2 replies; 13+ messages in thread
From: Igor Mammedov @ 2013-11-12 13:58 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, kraxel, afaerber, aliguori, mst
* simplify PCI address space mapping into system address space,
replacing code duplication in piix/q53 PCs with a helper function
* add fw_cfg 'etc/reserved-memory-end' to allow QEMU reserve
additional address space before 64-bit PCI hole. Which will be
need for reserving memory hotplug region in highmem.
v3:
* rebased on top of current master (1.7-rc0), wich includes dependency
"memory: Change MemoryRegion priorities from unsigned to signed"
* s/pcimem64-minimum-address/reserved-memory-end/ per Michael suggestion
v2:
* use negative priority to map PCI address space under RAM memory
regions which allows simplify code by removing pci_hole &
pci_hole64 memory region aliases
Igor Mammedov (1):
pc: add 'etc/reserved-memory-end' fw_cfg interface for SeaBIOS
Michael S. Tsirkin (1):
pc: map PCI address space as catchall region for not mapped addresses
hw/i386/pc.c | 28 ++++++++++++++++------------
hw/i386/pc_piix.c | 2 --
hw/pci-host/piix.c | 27 +++++----------------------
hw/pci-host/q35.c | 28 ++++++----------------------
include/hw/i386/pc.h | 15 +++------------
include/hw/pci-host/q35.h | 2 --
6 files changed, 30 insertions(+), 72 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH 1/2] pc: map PCI address space as catchall region for not mapped addresses 2013-11-12 13:58 [Qemu-devel] [PATCH for-1.8 0/2 v3] pc: inform SeaBIOS where 64-bit PCI hole begins Igor Mammedov @ 2013-11-12 13:58 ` Igor Mammedov 2013-11-12 16:29 ` Laszlo Ersek 2013-11-12 13:58 ` [Qemu-devel] [PATCH 2/2] pc: add 'etc/reserved-memory-end' fw_cfg interface for SeaBIOS Igor Mammedov 1 sibling, 1 reply; 13+ messages in thread From: Igor Mammedov @ 2013-11-12 13:58 UTC (permalink / raw) To: qemu-devel; +Cc: pbonzini, kraxel, afaerber, aliguori, mst From: "Michael S. Tsirkin" <mst@redhat.com> With a help of negative memory region priority PCI address space is mapped underneath RAM regions effectively catching every access to addresses not mapped by any other region. It simplifies PCI address space mapping into system address space. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- hw/i386/pc.c | 20 ++++++-------------- hw/i386/pc_piix.c | 2 -- hw/pci-host/piix.c | 26 ++++---------------------- hw/pci-host/q35.c | 27 +++++---------------------- include/hw/i386/pc.h | 14 ++------------ include/hw/pci-host/q35.h | 2 -- 6 files changed, 17 insertions(+), 74 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 12c436e..6c82ada 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1093,21 +1093,13 @@ 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 address space mapping into system address space */ +void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory, + MemoryRegion *pci_address_space) { - if ((sizeof(hwaddr) == 4) || (!pci_hole64_size)) { - 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); + /* Set to lower priority than RAM */ + memory_region_add_subregion_overlap(system_memory, 0x0, + pci_address_space, -1); } void pc_acpi_init(const char *default_dsdt) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 4fdb7b6..23e4857 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -150,8 +150,6 @@ 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, - 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 bad3953..5d4e290 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; @@ -312,8 +310,6 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, MemoryRegion *address_space_mem, MemoryRegion *address_space_io, ram_addr_t ram_size, - hwaddr pci_hole_start, - hwaddr pci_hole_size, ram_addr_t above_4g_mem_size, MemoryRegion *pci_address_space, MemoryRegion *ram_memory) @@ -326,7 +322,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 +349,10 @@ 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); - } + /* setup pci memory mapping */ + pc_pci_as_mapping_init(OBJECT(f), f->system_memory, + f->pci_address_space); + 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 b8feed1..d1792de 100644 --- a/hw/pci-host/q35.c +++ b/hw/pci-host/q35.c @@ -350,28 +350,11 @@ 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); - } + + /* setup pci memory mapping */ + pc_pci_as_mapping_init(OBJECT(mch), mch->system_memory, + mch->pci_address_space); + /* 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 03cc0ba..8b3be3c 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -128,17 +128,9 @@ 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); FWCfgState *pc_memory_init(MemoryRegion *system_memory, const char *kernel_filename, @@ -187,8 +179,6 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn, MemoryRegion *address_space_mem, MemoryRegion *address_space_io, ram_addr_t ram_size, - hwaddr pci_hole_start, - hwaddr pci_hole_size, ram_addr_t above_4g_mem_size, MemoryRegion *pci_memory, MemoryRegion *ram_memory); diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h index aee91aa..28a623c 100644 --- a/include/hw/pci-host/q35.h +++ b/include/hw/pci-host/q35.h @@ -53,8 +53,6 @@ typedef struct MCHPCIState { MemoryRegion *address_space_io; PAMMemoryRegion pam_regions[13]; MemoryRegion smram_region; - MemoryRegion pci_hole; - MemoryRegion pci_hole_64bit; PcPciInfo pci_info; uint8_t smm_enabled; ram_addr_t below_4g_mem_size; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] pc: map PCI address space as catchall region for not mapped addresses 2013-11-12 13:58 ` [Qemu-devel] [PATCH 1/2] pc: map PCI address space as catchall region for not mapped addresses Igor Mammedov @ 2013-11-12 16:29 ` Laszlo Ersek 0 siblings, 0 replies; 13+ messages in thread From: Laszlo Ersek @ 2013-11-12 16:29 UTC (permalink / raw) To: Igor Mammedov; +Cc: mst, qemu-devel, kraxel, aliguori, pbonzini, afaerber On 11/12/13 14:58, Igor Mammedov wrote: > From: "Michael S. Tsirkin" <mst@redhat.com> > > With a help of negative memory region priority PCI address space > is mapped underneath RAM regions effectively catching every access > to addresses not mapped by any other region. > It simplifies PCI address space mapping into system address space. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > hw/i386/pc.c | 20 ++++++-------------- > hw/i386/pc_piix.c | 2 -- > hw/pci-host/piix.c | 26 ++++---------------------- > hw/pci-host/q35.c | 27 +++++---------------------- > include/hw/i386/pc.h | 14 ++------------ > include/hw/pci-host/q35.h | 2 -- > 6 files changed, 17 insertions(+), 74 deletions(-) Tested with OVMF running from flash (and storing nvvars there). Also tested with my pending series "i440fx-test: check firmware visibility" (reviews welcome :)). GTest: result: OK GTest: run: /i440fx/firmware/bios (MSG: qemu cmdline: -S -display none -bios /tmp/fw_blob_UAGE6W) GTest: result: OK GTest: run: /i440fx/firmware/pflash (MSG: qemu cmdline: -S -display none -pflash /tmp/fw_blob_DHXN6W) GTest: result: OK Tested-by: Laszlo Ersek <lersek@redhat.com> Thanks! Laszlo ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 2/2] pc: add 'etc/reserved-memory-end' fw_cfg interface for SeaBIOS 2013-11-12 13:58 [Qemu-devel] [PATCH for-1.8 0/2 v3] pc: inform SeaBIOS where 64-bit PCI hole begins Igor Mammedov 2013-11-12 13:58 ` [Qemu-devel] [PATCH 1/2] pc: map PCI address space as catchall region for not mapped addresses Igor Mammedov @ 2013-11-12 13:58 ` Igor Mammedov 2013-11-12 18:26 ` Paolo Bonzini 1 sibling, 1 reply; 13+ messages in thread From: Igor Mammedov @ 2013-11-12 13:58 UTC (permalink / raw) To: qemu-devel; +Cc: pbonzini, kraxel, afaerber, aliguori, mst 'etc/reserved-memory-end' will allow QEMU to tell BIOS where PCI BARs mapping could safely start in high memory. Allowing BIOS to start mapping 64-bit PCI BARs at address where it wouldn't conflict with other mappings QEMU might place before it. That permits QEMU to reserve extra address space before 64-bit PCI hole for memory hotplug. Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- hw/i386/pc.c | 14 +++++++++++++- hw/pci-host/piix.c | 3 ++- hw/pci-host/q35.c | 3 ++- include/hw/i386/pc.h | 3 ++- 4 files changed, 19 insertions(+), 4 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 6c82ada..b504047 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1095,11 +1095,23 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size, /* setup pci memory address space mapping into system address space */ void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory, - MemoryRegion *pci_address_space) + MemoryRegion *pci_address_space, + uint64_t reserved_memory_end) { + uint64_t *val; + FWCfgState *fw_cfg = fw_cfg_find(); + /* Set to lower priority than RAM */ memory_region_add_subregion_overlap(system_memory, 0x0, pci_address_space, -1); + g_assert(fw_cfg); + /* + * Align address at 1G, this makes sure it can be exactly covered + * with a PAT entry even when using huge pages. + */ + val = g_malloc(sizeof(*val)); + *val = cpu_to_le64(ROUND_UP(reserved_memory_end, 0x1ULL << 30)); + fw_cfg_add_file(fw_cfg, "etc/reserved-memory-end", val, sizeof(*val)); } void pc_acpi_init(const char *default_dsdt) diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c index 5d4e290..16205e7 100644 --- a/hw/pci-host/piix.c +++ b/hw/pci-host/piix.c @@ -351,7 +351,8 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, /* setup pci memory mapping */ pc_pci_as_mapping_init(OBJECT(f), f->system_memory, - f->pci_address_space); + f->pci_address_space, + 0x100000000ULL + above_4g_mem_size); memory_region_init_alias(&f->smram_region, OBJECT(d), "smram-region", f->pci_address_space, 0xa0000, 0x20000); diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c index d1792de..1293353 100644 --- a/hw/pci-host/q35.c +++ b/hw/pci-host/q35.c @@ -353,7 +353,8 @@ static int mch_init(PCIDevice *d) /* setup pci memory mapping */ pc_pci_as_mapping_init(OBJECT(mch), mch->system_memory, - mch->pci_address_space); + mch->pci_address_space, + 0x100000000ULL + mch->above_4g_mem_size); /* smram */ cpu_smm_register(&mch_set_smm, mch); diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 8b3be3c..2663046 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -130,7 +130,8 @@ 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, + uint64_t reserved_memory_end); FWCfgState *pc_memory_init(MemoryRegion *system_memory, const char *kernel_filename, -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] pc: add 'etc/reserved-memory-end' fw_cfg interface for SeaBIOS 2013-11-12 13:58 ` [Qemu-devel] [PATCH 2/2] pc: add 'etc/reserved-memory-end' fw_cfg interface for SeaBIOS Igor Mammedov @ 2013-11-12 18:26 ` Paolo Bonzini 2013-11-12 20:17 ` Igor Mammedov 2013-11-12 22:10 ` Michael S. Tsirkin 0 siblings, 2 replies; 13+ messages in thread From: Paolo Bonzini @ 2013-11-12 18:26 UTC (permalink / raw) To: Igor Mammedov; +Cc: mst, aliguori, qemu-devel, afaerber, kraxel Il 12/11/2013 14:58, Igor Mammedov ha scritto: > 'etc/reserved-memory-end' will allow QEMU to tell BIOS where PCI > BARs mapping could safely start in high memory. > > Allowing BIOS to start mapping 64-bit PCI BARs at address where it > wouldn't conflict with other mappings QEMU might place before it. > > That permits QEMU to reserve extra address space before > 64-bit PCI hole for memory hotplug. I may be royally wrong, but I think the new file should only be added to new machine types. Otherwise, after migrating old machine types from new QEMU to old QEMU, you may end up with PCI BARs mapped outside the "PCI windows" that exist until before patch 1/2 of this series. Does this make sense? Also, would it make sense to use the e820 interface that Gerd has added, instead of a new fw_cfg file? Thanks, Paolo > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > hw/i386/pc.c | 14 +++++++++++++- > hw/pci-host/piix.c | 3 ++- > hw/pci-host/q35.c | 3 ++- > include/hw/i386/pc.h | 3 ++- > 4 files changed, 19 insertions(+), 4 deletions(-) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 6c82ada..b504047 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1095,11 +1095,23 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size, > > /* setup pci memory address space mapping into system address space */ > void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory, > - MemoryRegion *pci_address_space) > + MemoryRegion *pci_address_space, > + uint64_t reserved_memory_end) > { > + uint64_t *val; > + FWCfgState *fw_cfg = fw_cfg_find(); > + > /* Set to lower priority than RAM */ > memory_region_add_subregion_overlap(system_memory, 0x0, > pci_address_space, -1); > + g_assert(fw_cfg); > + /* > + * Align address at 1G, this makes sure it can be exactly covered > + * with a PAT entry even when using huge pages. > + */ > + val = g_malloc(sizeof(*val)); > + *val = cpu_to_le64(ROUND_UP(reserved_memory_end, 0x1ULL << 30)); > + fw_cfg_add_file(fw_cfg, "etc/reserved-memory-end", val, sizeof(*val)); > } > > void pc_acpi_init(const char *default_dsdt) > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c > index 5d4e290..16205e7 100644 > --- a/hw/pci-host/piix.c > +++ b/hw/pci-host/piix.c > @@ -351,7 +351,8 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, > > /* setup pci memory mapping */ > pc_pci_as_mapping_init(OBJECT(f), f->system_memory, > - f->pci_address_space); > + f->pci_address_space, > + 0x100000000ULL + above_4g_mem_size); > > memory_region_init_alias(&f->smram_region, OBJECT(d), "smram-region", > f->pci_address_space, 0xa0000, 0x20000); > diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c > index d1792de..1293353 100644 > --- a/hw/pci-host/q35.c > +++ b/hw/pci-host/q35.c > @@ -353,7 +353,8 @@ static int mch_init(PCIDevice *d) > > /* setup pci memory mapping */ > pc_pci_as_mapping_init(OBJECT(mch), mch->system_memory, > - mch->pci_address_space); > + mch->pci_address_space, > + 0x100000000ULL + mch->above_4g_mem_size); > > /* smram */ > cpu_smm_register(&mch_set_smm, mch); > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 8b3be3c..2663046 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -130,7 +130,8 @@ 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, > + uint64_t reserved_memory_end); > > FWCfgState *pc_memory_init(MemoryRegion *system_memory, > const char *kernel_filename, > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] pc: add 'etc/reserved-memory-end' fw_cfg interface for SeaBIOS 2013-11-12 18:26 ` Paolo Bonzini @ 2013-11-12 20:17 ` Igor Mammedov 2013-11-12 22:10 ` Michael S. Tsirkin 1 sibling, 0 replies; 13+ messages in thread From: Igor Mammedov @ 2013-11-12 20:17 UTC (permalink / raw) To: Paolo Bonzini; +Cc: mst, aliguori, qemu-devel, afaerber, kraxel On Tue, 12 Nov 2013 19:26:02 +0100 Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 12/11/2013 14:58, Igor Mammedov ha scritto: > > 'etc/reserved-memory-end' will allow QEMU to tell BIOS where PCI > > BARs mapping could safely start in high memory. > > > > Allowing BIOS to start mapping 64-bit PCI BARs at address where it > > wouldn't conflict with other mappings QEMU might place before it. > > > > That permits QEMU to reserve extra address space before > > 64-bit PCI hole for memory hotplug. > > I may be royally wrong, but I think the new file should only be added to > new machine types. Otherwise, after migrating old machine types from > new QEMU to old QEMU, you may end up with PCI BARs mapped outside the > "PCI windows" that exist until before patch 1/2 of this series. > > Does this make sense? it will shift BARs only when 'etc/reserved-memory-end' != above4gram_end, presently 'etc/reserved-memory-end' == above4gram_end, so it doesn't affect new->old or old->new migration. Just to be sure I've done new->old and old->new migration testing, it looks like nothing is broken by patch. When 'etc/reserved-memory-end' > above4gram_end, new->old migration is unlikely (unrealistically) to be broken due to default 64bit PCI hole size in old QEMU is 1ULL << 62, so to become broken guest should have a BAR that will go beyond 1ULL << 62 border. But your question have reminded me to make sure that memory hotplug should be disabled for old machine type to avoid clash of memory hotplug region and old 64-bit PCI window. > > Also, would it make sense to use the e820 interface that Gerd has added, > instead of a new fw_cfg file? Gerd suggested it before, but we've come to agreement that dedicated file is cleaner approach than extending standard e820 table with QEMU specific extensions. http://www.mail-archive.com/qemu-devel@nongnu.org/msg200359.html > > Thanks, > > Paolo > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > --- > > hw/i386/pc.c | 14 +++++++++++++- > > hw/pci-host/piix.c | 3 ++- > > hw/pci-host/q35.c | 3 ++- > > include/hw/i386/pc.h | 3 ++- > > 4 files changed, 19 insertions(+), 4 deletions(-) > > > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > > index 6c82ada..b504047 100644 > > --- a/hw/i386/pc.c > > +++ b/hw/i386/pc.c > > @@ -1095,11 +1095,23 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size, > > > > /* setup pci memory address space mapping into system address space */ > > void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory, > > - MemoryRegion *pci_address_space) > > + MemoryRegion *pci_address_space, > > + uint64_t reserved_memory_end) > > { > > + uint64_t *val; > > + FWCfgState *fw_cfg = fw_cfg_find(); > > + > > /* Set to lower priority than RAM */ > > memory_region_add_subregion_overlap(system_memory, 0x0, > > pci_address_space, -1); > > + g_assert(fw_cfg); > > + /* > > + * Align address at 1G, this makes sure it can be exactly covered > > + * with a PAT entry even when using huge pages. > > + */ > > + val = g_malloc(sizeof(*val)); > > + *val = cpu_to_le64(ROUND_UP(reserved_memory_end, 0x1ULL << 30)); > > + fw_cfg_add_file(fw_cfg, "etc/reserved-memory-end", val, sizeof(*val)); > > } > > > > void pc_acpi_init(const char *default_dsdt) > > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c > > index 5d4e290..16205e7 100644 > > --- a/hw/pci-host/piix.c > > +++ b/hw/pci-host/piix.c > > @@ -351,7 +351,8 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, > > > > /* setup pci memory mapping */ > > pc_pci_as_mapping_init(OBJECT(f), f->system_memory, > > - f->pci_address_space); > > + f->pci_address_space, > > + 0x100000000ULL + above_4g_mem_size); > > > > memory_region_init_alias(&f->smram_region, OBJECT(d), "smram-region", > > f->pci_address_space, 0xa0000, 0x20000); > > diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c > > index d1792de..1293353 100644 > > --- a/hw/pci-host/q35.c > > +++ b/hw/pci-host/q35.c > > @@ -353,7 +353,8 @@ static int mch_init(PCIDevice *d) > > > > /* setup pci memory mapping */ > > pc_pci_as_mapping_init(OBJECT(mch), mch->system_memory, > > - mch->pci_address_space); > > + mch->pci_address_space, > > + 0x100000000ULL + mch->above_4g_mem_size); > > > > /* smram */ > > cpu_smm_register(&mch_set_smm, mch); > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > > index 8b3be3c..2663046 100644 > > --- a/include/hw/i386/pc.h > > +++ b/include/hw/i386/pc.h > > @@ -130,7 +130,8 @@ 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, > > + uint64_t reserved_memory_end); > > > > FWCfgState *pc_memory_init(MemoryRegion *system_memory, > > const char *kernel_filename, > > > -- Regards, Igor ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] pc: add 'etc/reserved-memory-end' fw_cfg interface for SeaBIOS 2013-11-12 18:26 ` Paolo Bonzini 2013-11-12 20:17 ` Igor Mammedov @ 2013-11-12 22:10 ` Michael S. Tsirkin 2013-11-12 23:03 ` Paolo Bonzini 2013-11-13 12:04 ` Igor Mammedov 1 sibling, 2 replies; 13+ messages in thread From: Michael S. Tsirkin @ 2013-11-12 22:10 UTC (permalink / raw) To: Paolo Bonzini; +Cc: Igor Mammedov, aliguori, qemu-devel, afaerber, kraxel On Tue, Nov 12, 2013 at 07:26:02PM +0100, Paolo Bonzini wrote: > Il 12/11/2013 14:58, Igor Mammedov ha scritto: > > 'etc/reserved-memory-end' will allow QEMU to tell BIOS where PCI > > BARs mapping could safely start in high memory. > > > > Allowing BIOS to start mapping 64-bit PCI BARs at address where it > > wouldn't conflict with other mappings QEMU might place before it. > > > > That permits QEMU to reserve extra address space before > > 64-bit PCI hole for memory hotplug. > > I may be royally wrong, but I think the new file should only be added to > new machine types. Otherwise, after migrating old machine types from > new QEMU to old QEMU, you may end up with PCI BARs mapped outside the > "PCI windows" that exist until before patch 1/2 of this series. > > Does this make sense? Yes. Generally FW CFG must not be added/removed for a given machine types, otherwise guest that is migrated while reading it will get a corrupted result: half old and half new. > Also, would it make sense to use the e820 interface that Gerd has added, > instead of a new fw_cfg file? > > Thanks, > > Paolo This was already discussed on previous revisions of this patch. Have you seen that discussion? > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > --- > > hw/i386/pc.c | 14 +++++++++++++- > > hw/pci-host/piix.c | 3 ++- > > hw/pci-host/q35.c | 3 ++- > > include/hw/i386/pc.h | 3 ++- > > 4 files changed, 19 insertions(+), 4 deletions(-) > > > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > > index 6c82ada..b504047 100644 > > --- a/hw/i386/pc.c > > +++ b/hw/i386/pc.c > > @@ -1095,11 +1095,23 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size, > > > > /* setup pci memory address space mapping into system address space */ > > void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory, > > - MemoryRegion *pci_address_space) > > + MemoryRegion *pci_address_space, > > + uint64_t reserved_memory_end) > > { > > + uint64_t *val; > > + FWCfgState *fw_cfg = fw_cfg_find(); > > + > > /* Set to lower priority than RAM */ > > memory_region_add_subregion_overlap(system_memory, 0x0, > > pci_address_space, -1); > > + g_assert(fw_cfg); > > + /* > > + * Align address at 1G, this makes sure it can be exactly covered > > + * with a PAT entry even when using huge pages. > > + */ > > + val = g_malloc(sizeof(*val)); > > + *val = cpu_to_le64(ROUND_UP(reserved_memory_end, 0x1ULL << 30)); > > + fw_cfg_add_file(fw_cfg, "etc/reserved-memory-end", val, sizeof(*val)); > > } > > > > void pc_acpi_init(const char *default_dsdt) > > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c > > index 5d4e290..16205e7 100644 > > --- a/hw/pci-host/piix.c > > +++ b/hw/pci-host/piix.c > > @@ -351,7 +351,8 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, > > > > /* setup pci memory mapping */ > > pc_pci_as_mapping_init(OBJECT(f), f->system_memory, > > - f->pci_address_space); > > + f->pci_address_space, > > + 0x100000000ULL + above_4g_mem_size); > > > > memory_region_init_alias(&f->smram_region, OBJECT(d), "smram-region", > > f->pci_address_space, 0xa0000, 0x20000); > > diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c > > index d1792de..1293353 100644 > > --- a/hw/pci-host/q35.c > > +++ b/hw/pci-host/q35.c > > @@ -353,7 +353,8 @@ static int mch_init(PCIDevice *d) > > > > /* setup pci memory mapping */ > > pc_pci_as_mapping_init(OBJECT(mch), mch->system_memory, > > - mch->pci_address_space); > > + mch->pci_address_space, > > + 0x100000000ULL + mch->above_4g_mem_size); > > > > /* smram */ > > cpu_smm_register(&mch_set_smm, mch); > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > > index 8b3be3c..2663046 100644 > > --- a/include/hw/i386/pc.h > > +++ b/include/hw/i386/pc.h > > @@ -130,7 +130,8 @@ 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, > > + uint64_t reserved_memory_end); > > > > FWCfgState *pc_memory_init(MemoryRegion *system_memory, > > const char *kernel_filename, > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] pc: add 'etc/reserved-memory-end' fw_cfg interface for SeaBIOS 2013-11-12 22:10 ` Michael S. Tsirkin @ 2013-11-12 23:03 ` Paolo Bonzini 2013-11-13 12:04 ` Igor Mammedov 1 sibling, 0 replies; 13+ messages in thread From: Paolo Bonzini @ 2013-11-12 23:03 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: Igor Mammedov, kraxel, qemu-devel, aliguori, afaerber Il 12/11/2013 23:10, Michael S. Tsirkin ha scritto: > This was already discussed on previous revisions of this patch. > Have you seen that discussion? Obviously not. :) Paolo ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] pc: add 'etc/reserved-memory-end' fw_cfg interface for SeaBIOS 2013-11-12 22:10 ` Michael S. Tsirkin 2013-11-12 23:03 ` Paolo Bonzini @ 2013-11-13 12:04 ` Igor Mammedov 2013-11-14 7:40 ` Michael S. Tsirkin 1 sibling, 1 reply; 13+ messages in thread From: Igor Mammedov @ 2013-11-13 12:04 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: Paolo Bonzini, aliguori, qemu-devel, afaerber, kraxel On Wed, 13 Nov 2013 00:10:39 +0200 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Tue, Nov 12, 2013 at 07:26:02PM +0100, Paolo Bonzini wrote: > > Il 12/11/2013 14:58, Igor Mammedov ha scritto: > > > 'etc/reserved-memory-end' will allow QEMU to tell BIOS where PCI > > > BARs mapping could safely start in high memory. > > > > > > Allowing BIOS to start mapping 64-bit PCI BARs at address where it > > > wouldn't conflict with other mappings QEMU might place before it. > > > > > > That permits QEMU to reserve extra address space before > > > 64-bit PCI hole for memory hotplug. > > > > I may be royally wrong, but I think the new file should only be added to > > new machine types. Otherwise, after migrating old machine types from > > new QEMU to old QEMU, you may end up with PCI BARs mapped outside the > > "PCI windows" that exist until before patch 1/2 of this series. > > > > Does this make sense? > > Yes. > Generally FW CFG must not be added/removed for a given machine types, > otherwise guest that is migrated while reading it will > get a corrupted result: half old and half new. Is it true for a file 'etc/reserved-memory-end' though? I've debugged SeaBIOS to learn more about it, and new->old migration with following reboot, showed that file is not found by SeaBIOS (well since old QEMU doesn't have it), as result SeaBIOS fallbacks to the old behavior placing 64-PCI bars right above ram_over_4G as it was intended. And with 'etc/reserved-memory-end' == ram_over_4G_end as it is in this patch, there isn't issue whatsoever. Looks like there is no migrations issues with files, provided SeaBIOS knows how to deal with a missing file. -- Regards, Igor ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] pc: add 'etc/reserved-memory-end' fw_cfg interface for SeaBIOS 2013-11-13 12:04 ` Igor Mammedov @ 2013-11-14 7:40 ` Michael S. Tsirkin 2013-11-14 13:37 ` Igor Mammedov 2013-11-15 1:07 ` [Qemu-devel] [PATCH 2/2 v4] " Igor Mammedov 0 siblings, 2 replies; 13+ messages in thread From: Michael S. Tsirkin @ 2013-11-14 7:40 UTC (permalink / raw) To: Igor Mammedov; +Cc: Paolo Bonzini, aliguori, qemu-devel, afaerber, kraxel On Wed, Nov 13, 2013 at 01:04:10PM +0100, Igor Mammedov wrote: > On Wed, 13 Nov 2013 00:10:39 +0200 > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > On Tue, Nov 12, 2013 at 07:26:02PM +0100, Paolo Bonzini wrote: > > > Il 12/11/2013 14:58, Igor Mammedov ha scritto: > > > > 'etc/reserved-memory-end' will allow QEMU to tell BIOS where PCI > > > > BARs mapping could safely start in high memory. > > > > > > > > Allowing BIOS to start mapping 64-bit PCI BARs at address where it > > > > wouldn't conflict with other mappings QEMU might place before it. > > > > > > > > That permits QEMU to reserve extra address space before > > > > 64-bit PCI hole for memory hotplug. > > > > > > I may be royally wrong, but I think the new file should only be added to > > > new machine types. Otherwise, after migrating old machine types from > > > new QEMU to old QEMU, you may end up with PCI BARs mapped outside the > > > "PCI windows" that exist until before patch 1/2 of this series. > > > > > > Does this make sense? > > > > Yes. > > Generally FW CFG must not be added/removed for a given machine types, > > otherwise guest that is migrated while reading it will > > get a corrupted result: half old and half new. > > Is it true for a file 'etc/reserved-memory-end' though? It's true for any FW CFG entry. > I've debugged SeaBIOS to learn more about it, and new->old migration with > following reboot, showed that file is not found by SeaBIOS (well since old > QEMU doesn't have it), as result SeaBIOS fallbacks to the old behavior > placing 64-PCI bars right above ram_over_4G as it was intended. > > And with 'etc/reserved-memory-end' == ram_over_4G_end as it is in this > patch, there isn't issue whatsoever. > > Looks like there is no migrations issues with files, provided SeaBIOS knows > how to deal with a missing file. Here's an example of the issue: qemu_cfg_read_entry(&count, QEMU_CFG_FILE_DIR, sizeof(count)); 1. <<<<<< count = be32_to_cpu(count); u32 e; for (e = 0; e < count; e++) { struct QemuCfgFile qfile; qemu_cfg_read(&qfile, sizeof(qfile)); qemu_romfile_add(qfile.name, be16_to_cpu(qfile.select) , 0, be32_to_cpu(qfile.size)); } 2. <<<<<<< If migration happens during qemu_cfg_read_entry before point 1, you can get a byte from old count and a byte from new, resulting in a corrupt count. If migration happens at point 2, you will get incorrect mapping between file selector and name. System will likely fail to boot. There are probably other issues. Bottom line: FW CFG is guest visible state. Migration must not change it. > -- > Regards, > Igor ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] pc: add 'etc/reserved-memory-end' fw_cfg interface for SeaBIOS 2013-11-14 7:40 ` Michael S. Tsirkin @ 2013-11-14 13:37 ` Igor Mammedov 2013-11-15 1:07 ` [Qemu-devel] [PATCH 2/2 v4] " Igor Mammedov 1 sibling, 0 replies; 13+ messages in thread From: Igor Mammedov @ 2013-11-14 13:37 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: Paolo Bonzini, aliguori, qemu-devel, afaerber, kraxel On Thu, 14 Nov 2013 09:40:12 +0200 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Wed, Nov 13, 2013 at 01:04:10PM +0100, Igor Mammedov wrote: > > On Wed, 13 Nov 2013 00:10:39 +0200 > > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > On Tue, Nov 12, 2013 at 07:26:02PM +0100, Paolo Bonzini wrote: > > > > Il 12/11/2013 14:58, Igor Mammedov ha scritto: > > > > > 'etc/reserved-memory-end' will allow QEMU to tell BIOS where PCI > > > > > BARs mapping could safely start in high memory. > > > > > > > > > > Allowing BIOS to start mapping 64-bit PCI BARs at address where it > > > > > wouldn't conflict with other mappings QEMU might place before it. > > > > > > > > > > That permits QEMU to reserve extra address space before > > > > > 64-bit PCI hole for memory hotplug. > > > > > > > > I may be royally wrong, but I think the new file should only be added to > > > > new machine types. Otherwise, after migrating old machine types from > > > > new QEMU to old QEMU, you may end up with PCI BARs mapped outside the > > > > "PCI windows" that exist until before patch 1/2 of this series. > > > > > > > > Does this make sense? > > > > > > Yes. > > > Generally FW CFG must not be added/removed for a given machine types, > > > otherwise guest that is migrated while reading it will > > > get a corrupted result: half old and half new. > > > > Is it true for a file 'etc/reserved-memory-end' though? > > It's true for any FW CFG entry. > > > I've debugged SeaBIOS to learn more about it, and new->old migration with > > following reboot, showed that file is not found by SeaBIOS (well since old > > QEMU doesn't have it), as result SeaBIOS fallbacks to the old behavior > > placing 64-PCI bars right above ram_over_4G as it was intended. > > > > And with 'etc/reserved-memory-end' == ram_over_4G_end as it is in this > > patch, there isn't issue whatsoever. > > > > Looks like there is no migrations issues with files, provided SeaBIOS knows > > how to deal with a missing file. > > Here's an example of the issue: > > qemu_cfg_read_entry(&count, QEMU_CFG_FILE_DIR, sizeof(count)); > > 1. <<<<<< > > count = be32_to_cpu(count); > > u32 e; > for (e = 0; e < count; e++) { > struct QemuCfgFile qfile; > qemu_cfg_read(&qfile, sizeof(qfile)); > qemu_romfile_add(qfile.name, be16_to_cpu(qfile.select) > , 0, be32_to_cpu(qfile.size)); > } > > 2. <<<<<<< > > > If migration happens during qemu_cfg_read_entry before point 1, you can > get a byte from old count and a byte from new, resulting in a corrupt > count. > > If migration happens at point 2, you will get incorrect > mapping between file selector and name. System will likely > fail to boot. > > There are probably other issues. > > Bottom line: FW CFG is guest visible state. Migration must not change > it. Thanks for explanation, I'll resend this patch with machine compatibility changes. > > > -- > > Regards, > > Igor -- Regards, Igor ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 2/2 v4] pc: add 'etc/reserved-memory-end' fw_cfg interface for SeaBIOS 2013-11-14 7:40 ` Michael S. Tsirkin 2013-11-14 13:37 ` Igor Mammedov @ 2013-11-15 1:07 ` Igor Mammedov 1 sibling, 0 replies; 13+ messages in thread From: Igor Mammedov @ 2013-11-15 1:07 UTC (permalink / raw) To: qemu-devel; +Cc: pbonzini, mst, kraxel, aliguori, afaerber 'etc/reserved-memory-end' will allow QEMU to tell BIOS where PCI BARs mapping could safely start in high memory. Allowing BIOS to start mapping 64-bit PCI BARs at address where it wouldn't conflict with other mappings QEMU might place before it. That permits QEMU to reserve extra address space before 64-bit PCI hole for memory hotplug. Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- v2: * disable 'etc/reserved-memory-end' for 1.7 and older machine types --- hw/i386/pc.c | 27 +++++++++++++++++++++++++-- hw/i386/pc_piix.c | 1 + hw/i386/pc_q35.c | 1 + hw/pci-host/piix.c | 3 ++- hw/pci-host/q35.c | 3 ++- include/hw/i386/pc.h | 11 +++++++++-- 6 files changed, 40 insertions(+), 6 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 6c82ada..ba82c67 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1094,14 +1094,37 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size, } /* setup pci memory address space mapping into system address space */ -void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory, - MemoryRegion *pci_address_space) +void pc_pci_as_mapping_init_1_7(Object *owner, MemoryRegion *system_memory, + MemoryRegion *pci_address_space, + uint64_t reserved_memory_end) { /* Set to lower priority than RAM */ memory_region_add_subregion_overlap(system_memory, 0x0, pci_address_space, -1); } +static +void pc_pci_as_mapping_init_1_8(Object *owner, MemoryRegion *system_memory, + MemoryRegion *pci_address_space, + uint64_t reserved_memory_end) +{ + uint64_t *val; + FWCfgState *fw_cfg = fw_cfg_find(); + g_assert(fw_cfg); + /* + * Align address at 1G, this makes sure it can be exactly covered + * with a PAT entry even when using huge pages. + */ + val = g_malloc(sizeof(*val)); + *val = cpu_to_le64(ROUND_UP(reserved_memory_end, 0x1ULL << 30)); + fw_cfg_add_file(fw_cfg, "etc/reserved-memory-end", val, sizeof(*val)); + + pc_pci_as_mapping_init_1_7(owner, system_memory, pci_address_space, + reserved_memory_end); +} + +pc_pci_as_mapping_init_fn pc_pci_as_mapping_init = pc_pci_as_mapping_init_1_8; + void pc_acpi_init(const char *default_dsdt) { char *filename; diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 0456ab1..c0904f9 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -240,6 +240,7 @@ static void pc_init_pci(QEMUMachineInitArgs *args) static void pc_compat_1_7(QEMUMachineInitArgs *args) { + pc_pci_as_mapping_init = pc_pci_as_mapping_init_1_7; } static void pc_compat_1_6(QEMUMachineInitArgs *args) diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index b18e966..c165843 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -224,6 +224,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args) static void pc_compat_1_7(QEMUMachineInitArgs *args) { + pc_pci_as_mapping_init = pc_pci_as_mapping_init_1_7; } static void pc_compat_1_6(QEMUMachineInitArgs *args) diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c index 5d4e290..16205e7 100644 --- a/hw/pci-host/piix.c +++ b/hw/pci-host/piix.c @@ -351,7 +351,8 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, /* setup pci memory mapping */ pc_pci_as_mapping_init(OBJECT(f), f->system_memory, - f->pci_address_space); + f->pci_address_space, + 0x100000000ULL + above_4g_mem_size); memory_region_init_alias(&f->smram_region, OBJECT(d), "smram-region", f->pci_address_space, 0xa0000, 0x20000); diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c index d1792de..1293353 100644 --- a/hw/pci-host/q35.c +++ b/hw/pci-host/q35.c @@ -353,7 +353,8 @@ static int mch_init(PCIDevice *d) /* setup pci memory mapping */ pc_pci_as_mapping_init(OBJECT(mch), mch->system_memory, - mch->pci_address_space); + mch->pci_address_space, + 0x100000000ULL + mch->above_4g_mem_size); /* smram */ cpu_smm_register(&mch_set_smm, mch); diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 8b3be3c..bf1b168 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -129,8 +129,15 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size, #define DEFAULT_PCI_HOLE64_SIZE (~0x0ULL) -void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory, - MemoryRegion *pci_address_space); +typedef +void (*pc_pci_as_mapping_init_fn)(Object *owner, MemoryRegion *system_memory, + MemoryRegion *pci_address_space, + uint64_t reserved_memory_end); +extern pc_pci_as_mapping_init_fn pc_pci_as_mapping_init; + +void pc_pci_as_mapping_init_1_7(Object *owner, MemoryRegion *system_memory, + MemoryRegion *pci_address_space, + uint64_t reserved_memory_end); FWCfgState *pc_memory_init(MemoryRegion *system_memory, const char *kernel_filename, -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 0/2 v2] pc: inform SeaBIOS where 64-bit PCI hole begins
@ 2013-10-29 12:57 Igor Mammedov
2013-10-29 12:57 ` [Qemu-devel] [PATCH 1/2] pc: map PCI address space as catchall region for not mapped addresses Igor Mammedov
0 siblings, 1 reply; 13+ messages in thread
From: Igor Mammedov @ 2013-10-29 12:57 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, afaerber, aliguori, mst
* simplify PCI address space mapping into system address space,
replacing code duplication in piix/q53 PCs with a helper function
* add fw_cfg 'etc/pcimem64-minimum-address' to allow QEMU reserve
additional address space before 64-bit PCI hole. Which will be
need for reserving memory hotplug region in highmem.
SeaBIOS counterpart: http://patchwork.ozlabs.org/patch/283623/
v2:
* use negative priority to map PCI address space under RAM memory
regions which allows simplify code by removing pci_hole &
pci_hole64 memory region aliases
Series depends on:
"memory: Change MemoryRegion priorities from unsigned to signed:
Git tree for testing:
https://github.com/imammedo/qemu/commits/pcimem64-minimum-address-v2
Igor Mammedov (1):
pc: add 'etc/pcimem64-minimum-address' fw_cfg interface to SeaBIOS
Michael S. Tsirkin (1):
pc: map PCI address space as catchall region for not mapped addresses
hw/i386/pc.c | 28 ++++++++++++++++------------
hw/i386/pc_piix.c | 2 --
hw/pci-host/piix.c | 27 +++++----------------------
hw/pci-host/q35.c | 28 ++++++----------------------
include/hw/i386/pc.h | 15 +++------------
include/hw/pci-host/q35.h | 2 --
6 files changed, 30 insertions(+), 72 deletions(-)
^ permalink raw reply [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH 1/2] pc: map PCI address space as catchall region for not mapped addresses 2013-10-29 12:57 [Qemu-devel] [PATCH 0/2 v2] pc: inform SeaBIOS where 64-bit PCI hole begins Igor Mammedov @ 2013-10-29 12:57 ` Igor Mammedov 0 siblings, 0 replies; 13+ messages in thread From: Igor Mammedov @ 2013-10-29 12:57 UTC (permalink / raw) To: qemu-devel; +Cc: pbonzini, afaerber, aliguori, mst From: Michael S. Tsirkin <mst@redhat.com> With a help of negative memory region priority PCI address space is mapped underneath RAM regions effectively catching every access to addresses not mapped by any other region. It simplifies PCI address space mapping into system address space. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- hw/i386/pc.c | 20 ++++++-------------- hw/i386/pc_piix.c | 2 -- hw/pci-host/piix.c | 26 ++++---------------------- hw/pci-host/q35.c | 27 +++++---------------------- include/hw/i386/pc.h | 14 ++------------ include/hw/pci-host/q35.h | 2 -- 6 files changed, 17 insertions(+), 74 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 0c313fe..340c696 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1053,21 +1053,13 @@ 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 address space mapping into system address space */ +void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory, + MemoryRegion *pci_address_space) { - if ((sizeof(hwaddr) == 4) || (!pci_hole64_size)) { - 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); + /* Set to lower priority than RAM */ + memory_region_add_subregion_overlap(system_memory, 0x0, + pci_address_space, -1); } void pc_acpi_init(const char *default_dsdt) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index c6042c7..73053cb 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -146,8 +146,6 @@ 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, - 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..d34d20b 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; @@ -312,8 +310,6 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, MemoryRegion *address_space_mem, MemoryRegion *address_space_io, ram_addr_t ram_size, - hwaddr pci_hole_start, - hwaddr pci_hole_size, ram_addr_t above_4g_mem_size, MemoryRegion *pci_address_space, MemoryRegion *ram_memory) @@ -326,7 +322,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 +349,10 @@ 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); - } + /* setup pci memory mapping */ + pc_pci_as_mapping_init(OBJECT(f), f->system_memory, + f->pci_address_space); + 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..3e82caf 100644 --- a/hw/pci-host/q35.c +++ b/hw/pci-host/q35.c @@ -336,28 +336,11 @@ 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); - } + + /* setup pci memory mapping */ + pc_pci_as_mapping_init(OBJECT(mch), mch->system_memory, + mch->pci_address_space); + /* 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 6083839..de5a347 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -108,17 +108,9 @@ 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); FWCfgState *pc_memory_init(MemoryRegion *system_memory, const char *kernel_filename, @@ -167,8 +159,6 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn, MemoryRegion *address_space_mem, MemoryRegion *address_space_io, ram_addr_t ram_size, - hwaddr pci_hole_start, - hwaddr pci_hole_size, ram_addr_t above_4g_mem_size, MemoryRegion *pci_memory, MemoryRegion *ram_memory); diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h index 56de92e..fd53ff9 100644 --- a/include/hw/pci-host/q35.h +++ b/include/hw/pci-host/q35.h @@ -53,8 +53,6 @@ typedef struct MCHPCIState { MemoryRegion *address_space_io; PAMMemoryRegion pam_regions[13]; MemoryRegion smram_region; - MemoryRegion pci_hole; - MemoryRegion pci_hole_64bit; PcPciInfo pci_info; uint8_t smm_enabled; ram_addr_t below_4g_mem_size; -- 1.7.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2013-11-15 1:08 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-11-12 13:58 [Qemu-devel] [PATCH for-1.8 0/2 v3] pc: inform SeaBIOS where 64-bit PCI hole begins Igor Mammedov 2013-11-12 13:58 ` [Qemu-devel] [PATCH 1/2] pc: map PCI address space as catchall region for not mapped addresses Igor Mammedov 2013-11-12 16:29 ` Laszlo Ersek 2013-11-12 13:58 ` [Qemu-devel] [PATCH 2/2] pc: add 'etc/reserved-memory-end' fw_cfg interface for SeaBIOS Igor Mammedov 2013-11-12 18:26 ` Paolo Bonzini 2013-11-12 20:17 ` Igor Mammedov 2013-11-12 22:10 ` Michael S. Tsirkin 2013-11-12 23:03 ` Paolo Bonzini 2013-11-13 12:04 ` Igor Mammedov 2013-11-14 7:40 ` Michael S. Tsirkin 2013-11-14 13:37 ` Igor Mammedov 2013-11-15 1:07 ` [Qemu-devel] [PATCH 2/2 v4] " Igor Mammedov -- strict thread matches above, loose matches on Subject: below -- 2013-10-29 12:57 [Qemu-devel] [PATCH 0/2 v2] pc: inform SeaBIOS where 64-bit PCI hole begins Igor Mammedov 2013-10-29 12:57 ` [Qemu-devel] [PATCH 1/2] pc: map PCI address space as catchall region for not mapped addresses Igor Mammedov
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.