pc: pass pci window data to guests
v2 cover pc: pass pci window data to guests
This makes it possible for bios to load pci window data from host.
This makes it possible for host to make sure setup matches hardware exactly.
This will also make it easier to add more chipsets down the road.
Ranges are passed within a generic GuestInfo structure, can add more fields of interest to Guests in the future.
Note: this is on top of my PCI branch,
if no one objects I'd like to merge it through there as there are some trivial dependencies on that.
Changes from v1:
- fix v1.5-v1.6 migration compatibility
- address Peter Maydell's comments on range.h
- make addresses a bit smaller, compatible to what seabios does at the moment. We can increase the windows, carefully, at a later time.
Michael S. Tsirkin (5):
range: add Range structure
pci: store PCI hole ranges in guestinfo structure
pc: pass PCI hole ranges to Guests
pc: add 1.6 compat type
pc: pci-info add compat support
hw/i386/pc.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++- hw/i386/pc_piix.c | 37 ++++++++++++++++++++++++--- hw/i386/pc_q35.c | 6 ++++- hw/pci-host/q35.c | 4 +++ include/hw/i386/pc.h | 20 ++++++++++++++- include/hw/pci-host/q35.h | 2 ++ include/qemu/range.h | 16 ++++++++++++ include/qemu/typedefs.h | 1 + 8 files changed, 145 insertions(+), 6 deletions(-)
Signature · 1 line
MST
PATCH v2 1/5 range: add Range structure
Sometimes we need to pass ranges around, add a
handy structure for this purpose.
Note: memory.c defines its own concept of AddrRange structure for working with 128 addresses. It's necessary there for doing range math. This is not needed for most users: struct Range is
much simpler, and is only used for passing the range around.
--- include/qemu/range.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
include/qemu/range.h +16 -0
@@ -1,6 +1,22 @@#ifndef QEMU_RANGE_H #define QEMU_RANGE_H +#include <inttypes.h> + +/* +* Operations on 64 bit address ranges. +* Notes: +* - ranges must not wrap around 0, but can include the last byte ~0x0LL. +* - this can not represent a full 0 to ~0x0LL range. +*/ + +/* A structure representing a range of addresses. */ +struct Range { +uint64_t begin;/* First byte of the range, or 0 if empty. */ +uint64_t end;/* 1 + the last byte. 0 if range empty or ends at ~0x0LL. */ +}; +typedef struct Range Range ; +/* Get last byte of a range from offset + length. * Undefined for ranges that wrap around 0. */ static inline uint64_t range_get_last (uint64_t offset,uint64_t len)
Signature · 1 line
MST
Re: [Qemu-devel] [PATCH v2 1/5] range: add Range structure
Quoted 24 lines · Michael S. Tsirkin
Sometimes we need to pass ranges around, add a
handy structure for this purpose.Note:
memory.cdefines its own concept of AddrRange structure for working with 128 addresses. It's necessary there for doing range math. This is not needed for most users: struct Range is
much simpler, and is only used for passing the range around.Cc Peter Maydell <[email protected]>Signed-off-by Michael S. Tsirkin <[email protected]>--- include/qemu/range.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)include/qemu/range.h +2
-0@@ -1,6 +1,22 @@#ifndef QEMU_RANGE_H #define QEMU_RANGE_H +#include <inttypes.h> +
include/qemu/range.h line 15
Quoted 10 lines
include/qemu/range.h +10
-0+/* +* Operations on 64 bit address ranges. +* Notes: +* - ranges must not wrap around 0, but can include the last byte ~0x0LL. +* - this can not represent a full 0 to ~0x0LL range. +*/ + +/* A structure representing a range of addresses. */ +struct Range { +uint64_t begin;/* First byte of the range, or 0 if empty. */
I don't understand, can't a range start from 0?
include/qemu/range.h line 16
Quoted 1 line
include/qemu/range.h +1
-0+uint64_t end;/* 1 + the last byte. 0 if range empty or ends at ~0x0LL. */
How to tell from the two? Instead I suggest this be a length field, whereas 0 means empty.
Quoted 9 lines
include/qemu/range.h +3
-0+}; +typedef struct Range Range ; +/* Get last byte of a range from offset + length. * Undefined for ranges that wrap around 0. */ static inline uint64_t range_get_last (uint64_t offset,uint64_t len)Signature · 1 line
MST
PATCH v2 2/5 pci: store PCI hole ranges in guestinfo structure
In reply to Michael S. TsirkinWill be used to pass hole ranges to guests.
--- hw/i386/pc.c | 39 ++++++++++++++++++++++++++++++++++++++- hw/i386/pc_piix.c | 14 +++++++++++++- hw/i386/pc_q35.c | 6 +++++- hw/pci-host/q35.c | 4 ++++ include/hw/i386/pc.h | 19 ++++++++++++++++++- include/hw/pci-host/q35.h | 2 ++ include/qemu/typedefs.h | 1 + 7 files changed, 81 insertions(+), 4 deletions(-)
hw/i386/pc.c +38 -1
@@ -978,6 +978,41 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge) } } +typedef struct PcGuestInfoState { +PcGuestInfo info; +Notifier machine_done; +}PcGuestInfoState ; + +static +void pc_guest_info_machine_done (Notifier *notifier,void *data) +{ +PcGuestInfoState *guest_info_state =container_of (notifier, + PcGuestInfoState, + machine_done); +} + +PcGuestInfo *pc_guest_info_init (ram_addr_t below_4g_mem_size, +ram_addr_t above_4g_mem_size) +{ +PcGuestInfoState *guest_info_state =g_malloc0 (sizeof *guest_info_state); +PcGuestInfo *guest_info = &guest_info_state->info; + + guest_info->pci_info.w32.end =IO_APIC_DEFAULT_ADDRESS ; +if (sizeof (hwaddr) ==4 ) { + guest_info->pci_info.w64.begin =0 ; + guest_info->pci_info.w64.end =0 ; + }else { + guest_info->pci_info.w64.begin =0x100000000ULL + above_4g_mem_size; + guest_info->pci_info.w64.end = guest_info->pci_info.w64.begin + + (0x1ULL <<62 ); +assert (guest_info->pci_info.w64.begin <= guest_info->pci_info.w64.end); + } + + guest_info_state->machine_done.notify = pc_guest_info_machine_done; +qemu_add_machine_init_done_notifier (&guest_info_state->machine_done); +return guest_info; +} +void pc_acpi_init (const char *default_dsdt) {char *filename;
@@ -1019,7 +1054,8 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,ram_addr_t below_4g_mem_size, ram_addr_t above_4g_mem_size, MemoryRegion *rom_memory, - MemoryRegion **ram_memory) + MemoryRegion **ram_memory, + PcGuestInfo *guest_info) { int linux_boot, i;MemoryRegion *ram, *option_rom_mr;
@@ -1071,6 +1107,7 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,for (i =0 ; i < nb_option_roms; i++) {rom_add_option (option_rom[i].name, option_rom[i].bootindex); } + guest_info->fw_cfg = fw_cfg;return fw_cfg; }
hw/i386/pc_piix.c +13 -1
@@ -90,6 +90,7 @@ static void pc_init1(MemoryRegion *system_memory,MemoryRegion *rom_memory;DeviceState *icc_bridge;FWCfgState *fw_cfg =NULL ; +PcGuestInfo *guest_info; icc_bridge =qdev_create (NULL ,TYPE_ICC_BRIDGE ); object_property_add_child(qdev_get_machine(), "icc-bridge",
@@ -119,12 +120,23 @@ static void pc_init1(MemoryRegion *system_memory, rom_memory = system_memory; } + guest_info =pc_guest_info_init (below_4g_mem_size, above_4g_mem_size); + +/* Set PCI window size the way seabios has always done it. */ +/* TODO: consider just starting at below_4g_mem_size */ +if (ram_size <=0x80000000 ) + guest_info->pci_info.w32.begin =0x80000000 ; +else if (ram_size <=0xc0000000 ) + guest_info->pci_info.w32.begin =0xc0000000 ; +else + guest_info->pci_info.w32.begin =0xe0000000 ; +/* allocate ram and load rom/bios */ if (!xen_enabled ()) { fw_cfg =pc_memory_init (system_memory, kernel_filename, kernel_cmdline, initrd_filename, below_4g_mem_size, above_4g_mem_size, - rom_memory, &ram_memory); + rom_memory, &ram_memory, guest_info); } gsi_state =g_malloc0 (sizeof (*gsi_state));
hw/i386/pc_q35.c +5 -1
@@ -77,6 +77,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)ICH9LPCState *ich9_lpc;PCIDevice *ahci;DeviceState *icc_bridge; +PcGuestInfo *guest_info; icc_bridge =qdev_create (NULL ,TYPE_ICC_BRIDGE ); object_property_add_child(qdev_get_machine(), "icc-bridge",
@@ -105,11 +106,13 @@ static void pc_q35_init(QEMUMachineInitArgs *args) rom_memory =get_system_memory (); } + guest_info =pc_guest_info_init (below_4g_mem_size, above_4g_mem_size); +/* allocate ram and load rom/bios */ if (!xen_enabled ()) {pc_memory_init (get_system_memory (), kernel_filename, kernel_cmdline, initrd_filename, below_4g_mem_size, above_4g_mem_size, - rom_memory, &ram_memory); + rom_memory, &ram_memory, guest_info); }/* irq lines */
@@ -131,6 +134,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args) q35_host->mch.address_space_io =get_system_io (); q35_host->mch.below_4g_mem_size = below_4g_mem_size; q35_host->mch.above_4g_mem_size = above_4g_mem_size; + q35_host->mch.guest_info = guest_info;/* pci */ qdev_init_nofail (DEVICE (q35_host)); host_bus = q35_host->host.pci.bus;
hw/pci-host/q35.c +4 -0
@@ -244,6 +244,10 @@ static int mch_init(PCIDevice *d)hwaddr pci_hole64_size;MCHPCIState *mch =MCH_PCI_DEVICE (d); +/* Leave enough space for the biggest MCFG BAR */ + mch->guest_info->pci_info.w32.begin =MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT + +MCH_HOST_BRIDGE_PCIEXBAR_MAX ; +/* setup pci memory regions */ memory_region_init_alias(&mch->pci_hole, "pci-hole", mch->pci_address_space,
include/hw/i386/pc.h +18 -1
@@ -9,8 +9,20 @@#include "net/net.h" #include "hw/i386/ioapic.h" +#include "qemu/range.h" +/* PC-style peripherals (also used by other machines). */ +typedef struct PcPciInfo { +Range w32; +Range w64; +}PcPciInfo ; + +struct PcGuestInfo { +PcPciInfo pci_info; +FWCfgState *fw_cfg; +}; +/* parallel.c */ static inline bool parallel_init (ISABus *bus,int index,CharDriverState *chr) {
@@ -80,6 +92,10 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level);void pc_cpus_init (const char *cpu_model,DeviceState *icc_bridge);void pc_hot_add_cpu (const int64_t id,Error **errp);void pc_acpi_init (const char *default_dsdt); + +PcGuestInfo *pc_guest_info_init (ram_addr_t below_4g_mem_size, +ram_addr_t above_4g_mem_size); + FWCfgState *pc_memory_init(MemoryRegion *system_memory, const char *kernel_filename, const char *kernel_cmdline,
@@ -87,7 +103,8 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,ram_addr_t below_4g_mem_size, ram_addr_t above_4g_mem_size, MemoryRegion *rom_memory, - MemoryRegion **ram_memory); + MemoryRegion **ram_memory, + PcGuestInfo *guest_info);qemu_irq *pc_allocate_cpu_irq (void );DeviceState *pc_vga_init (ISABus *isa_bus,PCIBus *pci_bus); void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
include/hw/pci-host/q35.h +2 -0
@@ -55,6 +55,7 @@ typedef struct MCHPCIState {uint8_t smm_enabled;ram_addr_t below_4g_mem_size;ram_addr_t above_4g_mem_size; +PcGuestInfo *guest_info; } MCHPCIState; typedef struct Q35PCIHost {
@@ -81,6 +82,7 @@ typedef struct Q35PCIHost {#define MCH_HOST_BRIDGE_PCIEXBAR 0x60/* 64bit register */ #define MCH_HOST_BRIDGE_PCIEXBAR_SIZE 8/* 64bit register */ #define MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT 0xb0000000 +#define MCH_HOST_BRIDGE_PCIEXBAR_MAX (0x10000000)/* 256M */ #define MCH_HOST_BRIDGE_PCIEXBAR_ADMSK Q35_MASK(64, 35, 28)#define MCH_HOST_BRIDGE_PCIEXBAR_128ADMSK ((uint64_t)(1 << 26))#define MCH_HOST_BRIDGE_PCIEXBAR_64ADMSK ((uint64_t)(1 << 25))
include/qemu/typedefs.h +1 -0
@@ -62,5 +62,6 @@ typedef struct VirtIODevice VirtIODevice;typedef struct QEMUSGList QEMUSGList ;typedef struct SHPCDevice SHPCDevice ;typedef struct FWCfgState FWCfgState ; +typedef struct PcGuestInfo PcGuestInfo ;#endif /* QEMU_TYPEDEFS_H */
Signature · 1 line
MST
Re: [Qemu-devel] [PATCH v2 2/5] pci: store PCI hole ranges in guestinfo structure
Hi,
hw/i386/pc.c void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge) · line 1007
Quoted 4 lines
hw/i386/pc.c +4
-0+ }else { + guest_info->pci_info.w64.begin =0x100000000ULL + above_4g_mem_size; + guest_info->pci_info.w64.end = guest_info->pci_info.w64.begin + + (0x1ULL <<62 );
Doesn't this give unaligned windows?
hw/i386/pc_piix.c static void pc_init1(MemoryRegion *system_memory, · line 126
Quoted 2 lines
hw/i386/pc_piix.c +2
-0+/* Set PCI window size the way seabios has always done it. */ +/* TODO: consider just starting at below_4g_mem_size */
Used to be that way. Was changed for alignment reasons (i.e. 1G window starts at 1G border etc).
cheers,
Gerd
Re: [Qemu-devel] [PATCH v2 2/5] pci: store PCI hole ranges in guestinfo structure
Quoted 8 lines · Gerd Hoffmann
Hi,
hw/i386/pc.c void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge) · line 1007
Quoted 4 lines
hw/i386/pc.c +4
-0+ }else { + guest_info->pci_info.w64.begin =0x100000000ULL + above_4g_mem_size; + guest_info->pci_info.w64.end = guest_info->pci_info.w64.begin + + (0x1ULL <<62 );Doesn't this give unaligned windows?
PCI Bridge windows do not need to be size aligned.
In any case, the windows are *exactly* as calculated by seabios - apparently it does not size-align windows either.
Quoted 5 lines
hw/i386/pc_piix.c static void pc_init1(MemoryRegion *system_memory, · line 126
Quoted 2 lines
hw/i386/pc_piix.c +2
-0+/* Set PCI window size the way seabios has always done it. */ +/* TODO: consider just starting at below_4g_mem_size */ Used to be that way. Was changed for alignment reasons (i.e. 1G window starts at 1G border etc).
Where's the alignment requirement coming from?
Quoted 3 lines
cheers,
Gerd
Re: [Qemu-devel] [PATCH v2 2/5] pci: store PCI hole ranges in guestinfo structure
Quoted 8 lines · Michael S. Tsirkin
Quoted 7 lines · Gerd Hoffmann
Hi,
Quoted 4 lines
hw/i386/pc.c +4
-0+ }else { + guest_info->pci_info.w64.begin =0x100000000ULL + above_4g_mem_size; + guest_info->pci_info.w64.end = guest_info->pci_info.w64.begin + + (0x1ULL <<62 );
Quoted 6 lines
Quoted 1 line
Doesn't this give unaligned windows?
PCI Bridge windows do not need to be size aligned.
In any case, the windows are *exactly* as calculated by seabios - apparently it does not size-align windows either.
Surely not. SeaBIOS sizes the 64bit window according to the space needed by the 64bit bars it wants to map there.
Quoted 7 lines
Quoted 5 lines
hw/i386/pc_piix.c static void pc_init1(MemoryRegion *system_memory, · line 126
Quoted 2 lines
hw/i386/pc_piix.c +2
-0+/* Set PCI window size the way seabios has always done it. */ +/* TODO: consider just starting at below_4g_mem_size */ Used to be that way. Was changed for alignment reasons (i.e. 1G window starts at 1G border etc).
Where's the alignment requirement coming from?
seabios creates a mtrr entry for the window, which doesn't work in case it isn't aligned (at least not with a single entry).
Also real hardware tends to do it this way.
cheers,
Gerd
Re: [Qemu-devel] [PATCH v2 2/5] pci: store PCI hole ranges in guestinfo structure
Quoted 11 lines · Gerd Hoffmann
Quoted 10 lines · Michael S. Tsirkin
Quoted 8 lines · Gerd Hoffmann
Hi,
hw/i386/pc.c void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge) · line 1007
Quoted 4 lines
hw/i386/pc.c +4
-0+ }else { + guest_info->pci_info.w64.begin =0x100000000ULL + above_4g_mem_size; + guest_info->pci_info.w64.end = guest_info->pci_info.w64.begin + + (0x1ULL <<62 );Doesn't this give unaligned windows?
Quoted 7 lines
Quoted 4 lines
PCI Bridge windows do not need to be size aligned.
In any case, the windows are *exactly* as calculated by seabios - apparently it does not size-align windows either.
Surely not. SeaBIOS sizes the 64bit window according to the space needed by the 64bit bars it wants to map there.
Ah, it's 64 bit. True. That's a seabios bug by the way:
if we add more devices by hotplug later, we want more pci memory.
Quoted 8 lines
Quoted 7 lines
Quoted 5 lines
hw/i386/pc_piix.c static void pc_init1(MemoryRegion *system_memory, · line 126
Quoted 2 lines
hw/i386/pc_piix.c +2
-0+/* Set PCI window size the way seabios has always done it. */ +/* TODO: consider just starting at below_4g_mem_size */ Used to be that way. Was changed for alignment reasons (i.e. 1G window starts at 1G border etc).
Where's the alignment requirement coming from?
Quoted 7 lines
seabios creates a mtrr entry for the window, which doesn't work in case it isn't aligned (at least not with a single entry).
Also real hardware tends to do it this way.
cheers,
Gerd
I see. I'll figure out the details and add a comment to this end.
But that's for the 32 bit window - I don't see it playing with mtrrs for the 64 bit ranges.
So I'm guessing alignment isn't needed there, right?
Signature · 1 line
MST
Re: [Qemu-devel] [PATCH v2 2/5] pci: store PCI hole ranges in guestinfo structure
Hi,
Quoted 5 lines
I see. I'll figure out the details and add a comment to this end.
But that's for the 32 bit window - I don't see it playing with mtrrs for the 64 bit ranges.
So I'm guessing alignment isn't needed there, right?
mtrr's are a 32bit thing anyway IIRC.
I still would place the 64bit window aligned. Given that 36 physical address lines (-> 64G address space) used to be quite common until recently I think reserving 16G from 64G downwards would be a good pick.
For guests with *lots* of memory we could move the hole to be just
below 1T and maybe also make it larger.
cheers,
Gerd
Re: [Qemu-devel] [PATCH v2 2/5] pci: store PCI hole ranges in guestinfo structure
In reply to Michael S. TsirkinI can't offer any opinion about the values you put into w32 and w64, but I have some remarks.
First, please consider passing -O/path/to/some/order_file to git-format-patch, so that .h files show up at the top of each patch.
Quoted 70 lines · Michael S. Tsirkin
Will be used to pass hole ranges to guests.
Signed-off-by Michael S. Tsirkin <[email protected]>--- hw/i386/pc.c | 39 ++++++++++++++++++++++++++++++++++++++- hw/i386/pc_piix.c | 14 +++++++++++++- hw/i386/pc_q35.c | 6 +++++- hw/pci-host/q35.c | 4 ++++ include/hw/i386/pc.h | 19 ++++++++++++++++++- include/hw/pci-host/q35.h | 2 ++ include/qemu/typedefs.h | 1 + 7 files changed, 81 insertions(+), 4 deletions(-)hw/i386/pc.c +37
-1@@ -978,6 +978,41 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge) } } +typedef struct PcGuestInfoState { +PcGuestInfo info; +Notifier machine_done; +}PcGuestInfoState ; + +static +void pc_guest_info_machine_done (Notifier *notifier,void *data) +{ +PcGuestInfoState *guest_info_state =container_of (notifier, + PcGuestInfoState, + machine_done); +} + +PcGuestInfo *pc_guest_info_init (ram_addr_t below_4g_mem_size, +ram_addr_t above_4g_mem_size) +{ +PcGuestInfoState *guest_info_state =g_malloc0 (sizeof *guest_info_state); +PcGuestInfo *guest_info = &guest_info_state->info; + + guest_info->pci_info.w32.end =IO_APIC_DEFAULT_ADDRESS ; +if (sizeof (hwaddr) ==4 ) { + guest_info->pci_info.w64.begin =0 ; + guest_info->pci_info.w64.end =0 ; + }else { + guest_info->pci_info.w64.begin =0x100000000ULL + above_4g_mem_size; + guest_info->pci_info.w64.end = guest_info->pci_info.w64.begin + + (0x1ULL <<62 ); +assert (guest_info->pci_info.w64.begin <= guest_info->pci_info.w64.end); + } + + guest_info_state->machine_done.notify = pc_guest_info_machine_done; +qemu_add_machine_init_done_notifier (&guest_info_state->machine_done); +return guest_info; +} +void pc_acpi_init (const char *default_dsdt) {char *filename;@@ -1019,7 +1054,8 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,ram_addr_t below_4g_mem_size, ram_addr_t above_4g_mem_size, MemoryRegion *rom_memory, - MemoryRegion **ram_memory) + MemoryRegion **ram_memory, + PcGuestInfo *guest_info) { int linux_boot, i;MemoryRegion *ram, *option_rom_mr;
hw/i386/pc.c FWCfgState *pc_memory_init(MemoryRegion *system_memory, · line 1112
Quoted 6 lines
hw/i386/pc.c +1
-0@@ -1071,6 +1107,7 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,for (i =0 ; i < nb_option_roms; i++) {rom_add_option (option_rom[i].name, option_rom[i].bootindex); } + guest_info->fw_cfg = fw_cfg;return fw_cfg; }
I find this suboptimal style, ie. passing "guest_info" to pc_memory_init() just so that pc_memory_init() can set guest_info->fw_cfg to fw_cfg, when pc_memory_init() returns fw_cfg anyway.
More "philosophically":
Quoted 11 lines
include/hw/i386/pc.h +2
-0@@ -9,8 +9,20 @@#include "net/net.h" #include "hw/i386/ioapic.h" +#include "qemu/range.h" +/* PC-style peripherals (also used by other machines). */
include/hw/i386/pc.h line 24
Quoted 10 lines
include/hw/i386/pc.h +9
-0+typedef struct PcPciInfo { +Range w32; +Range w64; +}PcPciInfo ; + +struct PcGuestInfo { +PcPciInfo pci_info; +FWCfgState *fw_cfg; +};
Are you sure you need a private link to the global fw_cfg object? The pvpanic series added a global lookup possibility in commit 10a584b2 (object_resolve_path()).
Anyway these are just subjective style notes.
Quoted 16 lines
include/hw/i386/pc.h +5
-0+/* parallel.c */ static inline bool parallel_init (ISABus *bus,int index,CharDriverState *chr) {@@ -80,6 +92,10 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level);void pc_cpus_init (const char *cpu_model,DeviceState *icc_bridge);void pc_hot_add_cpu (const int64_t id,Error **errp);void pc_acpi_init (const char *default_dsdt); + +PcGuestInfo *pc_guest_info_init (ram_addr_t below_4g_mem_size, +ram_addr_t above_4g_mem_size); + FWCfgState *pc_memory_init(MemoryRegion *system_memory, const char *kernel_filename, const char *kernel_cmdline,
include/hw/i386/pc.h FWCfgState *pc_memory_init(MemoryRegion *system_memory, · line 110
Quoted 9 lines
include/hw/i386/pc.h +2
-1@@ -87,7 +103,8 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,ram_addr_t below_4g_mem_size, ram_addr_t above_4g_mem_size, MemoryRegion *rom_memory, - MemoryRegion **ram_memory); + MemoryRegion **ram_memory, + PcGuestInfo *guest_info);qemu_irq *pc_allocate_cpu_irq (void );DeviceState *pc_vga_init (ISABus *isa_bus,PCIBus *pci_bus); void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
Going forward:
Quoted 28 lines
hw/i386/pc_piix.c +12
-0@@ -90,6 +90,7 @@ static void pc_init1(MemoryRegion *system_memory,MemoryRegion *rom_memory;DeviceState *icc_bridge;FWCfgState *fw_cfg =NULL ; +PcGuestInfo *guest_info; icc_bridge =qdev_create (NULL ,TYPE_ICC_BRIDGE ); object_property_add_child(qdev_get_machine(), "icc-bridge",@@ -119,12 +120,23 @@ static void pc_init1(MemoryRegion *system_memory, rom_memory = system_memory; } + guest_info =pc_guest_info_init (below_4g_mem_size, above_4g_mem_size); + +/* Set PCI window size the way seabios has always done it. */ +/* TODO: consider just starting at below_4g_mem_size */ +if (ram_size <=0x80000000 ) + guest_info->pci_info.w32.begin =0x80000000 ; +else if (ram_size <=0xc0000000 ) + guest_info->pci_info.w32.begin =0xc0000000 ; +else + guest_info->pci_info.w32.begin =0xe0000000 ; +
hw/i386/pc_piix.c static void pc_init1(MemoryRegion *system_memory, · line 142
Quoted 10 lines
hw/i386/pc_piix.c +1
-1/* allocate ram and load rom/bios */ if (!xen_enabled ()) { fw_cfg =pc_memory_init (system_memory, kernel_filename, kernel_cmdline, initrd_filename, below_4g_mem_size, above_4g_mem_size, - rom_memory, &ram_memory); + rom_memory, &ram_memory, guest_info); } gsi_state =g_malloc0 (sizeof (*gsi_state));
On PIIX you *almost* leak the guest_info structure at once; the only link to it is the machine-done-notifier registered in pc_guest_info_init(). Is this intended? (I guess a later patch in the series will change this.)
Quoted 28 lines
hw/i386/pc_q35.c +4
-1@@ -77,6 +77,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)ICH9LPCState *ich9_lpc;PCIDevice *ahci;DeviceState *icc_bridge; +PcGuestInfo *guest_info; icc_bridge =qdev_create (NULL ,TYPE_ICC_BRIDGE ); object_property_add_child(qdev_get_machine(), "icc-bridge",@@ -105,11 +106,13 @@ static void pc_q35_init(QEMUMachineInitArgs *args) rom_memory =get_system_memory (); } + guest_info =pc_guest_info_init (below_4g_mem_size, above_4g_mem_size); +/* allocate ram and load rom/bios */ if (!xen_enabled ()) {pc_memory_init (get_system_memory (), kernel_filename, kernel_cmdline, initrd_filename, below_4g_mem_size, above_4g_mem_size, - rom_memory, &ram_memory); + rom_memory, &ram_memory, guest_info); }/* irq lines */
hw/i386/pc_q35.c static void pc_q35_init(QEMUMachineInitArgs *args) · line 140
Quoted 7 lines
hw/i386/pc_q35.c +1
-0@@ -131,6 +134,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args) q35_host->mch.address_space_io =get_system_io (); q35_host->mch.below_4g_mem_size = below_4g_mem_size; q35_host->mch.above_4g_mem_size = above_4g_mem_size; + q35_host->mch.guest_info = guest_info;/* pci */ qdev_init_nofail (DEVICE (q35_host)); host_bus = q35_host->host.pci.bus;
OK, a direct (owner) link is established here; which gives birth to the next question:
Quoted 22 lines
hw/pci-host/q35.c +4
-0@@ -244,6 +244,10 @@ static int mch_init(PCIDevice *d)hwaddr pci_hole64_size;MCHPCIState *mch =MCH_PCI_DEVICE (d); +/* Leave enough space for the biggest MCFG BAR */ + mch->guest_info->pci_info.w32.begin =MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT + +MCH_HOST_BRIDGE_PCIEXBAR_MAX ; +/* setup pci memory regions */ memory_region_init_alias(&mch->pci_hole, "pci-hole", mch->pci_address_space,include/hw/pci-host/q35.h +0
-0
include/hw/pci-host/q35.h typedef struct MCHPCIState { · line 59
Quoted 5 lines
include/hw/pci-host/q35.h +1
-0@@ -55,6 +55,7 @@ typedef struct MCHPCIState {uint8_t smm_enabled;ram_addr_t below_4g_mem_size;ram_addr_t above_4g_mem_size; +PcGuestInfo *guest_info; } MCHPCIState;
... how does this relate to migration? (I'm not suggesting anything, I'm just curious.)
Thanks,
Laszlo
Re: [Qemu-devel] [PATCH v2 2/5] pci: store PCI hole ranges in guestinfo structure
Quoted 75 lines · Laszlo Ersek
I can't offer any opinion about the values you put into w32 and w64, but I have some remarks.
First, please consider passing -O/path/to/some/order_file to git-format-patch, so that .h files show up at the top of each patch.
Quoted 70 lines · Michael S. Tsirkin
Will be used to pass hole ranges to guests.
Signed-off-by Michael S. Tsirkin <[email protected]>--- hw/i386/pc.c | 39 ++++++++++++++++++++++++++++++++++++++- hw/i386/pc_piix.c | 14 +++++++++++++- hw/i386/pc_q35.c | 6 +++++- hw/pci-host/q35.c | 4 ++++ include/hw/i386/pc.h | 19 ++++++++++++++++++- include/hw/pci-host/q35.h | 2 ++ include/qemu/typedefs.h | 1 + 7 files changed, 81 insertions(+), 4 deletions(-)hw/i386/pc.c +37
-1@@ -978,6 +978,41 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge) } } +typedef struct PcGuestInfoState { +PcGuestInfo info; +Notifier machine_done; +}PcGuestInfoState ; + +static +void pc_guest_info_machine_done (Notifier *notifier,void *data) +{ +PcGuestInfoState *guest_info_state =container_of (notifier, + PcGuestInfoState, + machine_done); +} + +PcGuestInfo *pc_guest_info_init (ram_addr_t below_4g_mem_size, +ram_addr_t above_4g_mem_size) +{ +PcGuestInfoState *guest_info_state =g_malloc0 (sizeof *guest_info_state); +PcGuestInfo *guest_info = &guest_info_state->info; + + guest_info->pci_info.w32.end =IO_APIC_DEFAULT_ADDRESS ; +if (sizeof (hwaddr) ==4 ) { + guest_info->pci_info.w64.begin =0 ; + guest_info->pci_info.w64.end =0 ; + }else { + guest_info->pci_info.w64.begin =0x100000000ULL + above_4g_mem_size; + guest_info->pci_info.w64.end = guest_info->pci_info.w64.begin + + (0x1ULL <<62 ); +assert (guest_info->pci_info.w64.begin <= guest_info->pci_info.w64.end); + } + + guest_info_state->machine_done.notify = pc_guest_info_machine_done; +qemu_add_machine_init_done_notifier (&guest_info_state->machine_done); +return guest_info; +} +void pc_acpi_init (const char *default_dsdt) {char *filename;@@ -1019,7 +1054,8 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,ram_addr_t below_4g_mem_size, ram_addr_t above_4g_mem_size, MemoryRegion *rom_memory, - MemoryRegion **ram_memory) + MemoryRegion **ram_memory, + PcGuestInfo *guest_info) { int linux_boot, i;MemoryRegion *ram, *option_rom_mr;@@ -1071,6 +1107,7 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,
Quoted 8 lines
hw/i386/pc.c FWCfgState *pc_memory_init(MemoryRegion *system_memory, · line 1112
Quoted 6 lines
hw/i386/pc.c +1
-0for (i =0 ; i < nb_option_roms; i++) {rom_add_option (option_rom[i].name, option_rom[i].bootindex); } + guest_info->fw_cfg = fw_cfg;return fw_cfg; }I find this suboptimal style, ie. passing "
guest_info" topc_memory_init()just so thatpc_memory_init()can setguest_info->fw_cfgtofw_cfg, whenpc_memory_init()returnsfw_cfganyway.
Well otherwise all callers have to remember to set it.
Quoted 26 lines
More "philosophically":
Quoted 11 lines
include/hw/i386/pc.h +2
-0@@ -9,8 +9,20 @@#include "net/net.h" #include "hw/i386/ioapic.h" +#include "qemu/range.h" +/* PC-style peripherals (also used by other machines). */ include/hw/i386/pc.h line 24
Quoted 10 lines
include/hw/i386/pc.h +9
-0+typedef struct PcPciInfo { +Range w32; +Range w64; +}PcPciInfo ; + +struct PcGuestInfo { +PcPciInfo pci_info; +FWCfgState *fw_cfg; +};Are you sure you need a private link to the global
fw_cfgobject? The pvpanic series added a global lookup possibility in commit10a584b2(object_resolve_path()).
Quoted 1 line
Anyway these are just subjective style notes.
Yes.
I personally prefer not using global lookups: passing a pointer makes dependencies clearer IMO.
If we do switch to that, I think it's cleaner to have a wrapper so that everyone does not need to hard-code strings like /machine/fw_cfg.
Quoted 65 lines
Quoted 16 lines
include/hw/i386/pc.h +5
-0+/* parallel.c */ static inline bool parallel_init (ISABus *bus,int index,CharDriverState *chr) {@@ -80,6 +92,10 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level);void pc_cpus_init (const char *cpu_model,DeviceState *icc_bridge);void pc_hot_add_cpu (const int64_t id,Error **errp);void pc_acpi_init (const char *default_dsdt); + +PcGuestInfo *pc_guest_info_init (ram_addr_t below_4g_mem_size, +ram_addr_t above_4g_mem_size); + FWCfgState *pc_memory_init(MemoryRegion *system_memory, const char *kernel_filename, const char *kernel_cmdline,include/hw/i386/pc.h FWCfgState *pc_memory_init(MemoryRegion *system_memory, · line 110
Quoted 9 lines
include/hw/i386/pc.h +2
-1@@ -87,7 +103,8 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,ram_addr_t below_4g_mem_size, ram_addr_t above_4g_mem_size, MemoryRegion *rom_memory, - MemoryRegion **ram_memory); + MemoryRegion **ram_memory, + PcGuestInfo *guest_info);qemu_irq *pc_allocate_cpu_irq (void );DeviceState *pc_vga_init (ISABus *isa_bus,PCIBus *pci_bus); void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,Going forward:
Quoted 37 lines
hw/i386/pc_piix.c +13
-1@@ -90,6 +90,7 @@ static void pc_init1(MemoryRegion *system_memory,MemoryRegion *rom_memory;DeviceState *icc_bridge;FWCfgState *fw_cfg =NULL ; +PcGuestInfo *guest_info; icc_bridge =qdev_create (NULL ,TYPE_ICC_BRIDGE ); object_property_add_child(qdev_get_machine(), "icc-bridge",@@ -119,12 +120,23 @@ static void pc_init1(MemoryRegion *system_memory, rom_memory = system_memory; } + guest_info =pc_guest_info_init (below_4g_mem_size, above_4g_mem_size); + +/* Set PCI window size the way seabios has always done it. */ +/* TODO: consider just starting at below_4g_mem_size */ +if (ram_size <=0x80000000 ) + guest_info->pci_info.w32.begin =0x80000000 ; +else if (ram_size <=0xc0000000 ) + guest_info->pci_info.w32.begin =0xc0000000 ; +else + guest_info->pci_info.w32.begin =0xe0000000 ; +/* allocate ram and load rom/bios */ if (!xen_enabled ()) { fw_cfg =pc_memory_init (system_memory, kernel_filename, kernel_cmdline, initrd_filename, below_4g_mem_size, above_4g_mem_size, - rom_memory, &ram_memory); + rom_memory, &ram_memory, guest_info); }
Quoted 3 lines
hw/i386/pc_piix.c static void pc_init1(MemoryRegion *system_memory, · line 142
Quoted 1 line
hw/i386/pc_piix.c +0
-0gsi_state =g_malloc0 (sizeof (*gsi_state));On PIIX you *almost* leak the
guest_infostructure at once; the only link to it is the machine-done-notifier registered inpc_guest_info_init(). Is this intended? (I guess a later patch in the series will change this.)
Yes. Machine done notifiers generally aren't cleaned up:
there is qemu_add_machine_init_done_notifier but not qemu_del_machine_init_done_notifier,
so there's no way to free it.
Same applies to all other such notifiers.
Quoted 68 lines
Quoted 28 lines
hw/i386/pc_q35.c +4
-1@@ -77,6 +77,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)ICH9LPCState *ich9_lpc;PCIDevice *ahci;DeviceState *icc_bridge; +PcGuestInfo *guest_info; icc_bridge =qdev_create (NULL ,TYPE_ICC_BRIDGE ); object_property_add_child(qdev_get_machine(), "icc-bridge",@@ -105,11 +106,13 @@ static void pc_q35_init(QEMUMachineInitArgs *args) rom_memory =get_system_memory (); } + guest_info =pc_guest_info_init (below_4g_mem_size, above_4g_mem_size); +/* allocate ram and load rom/bios */ if (!xen_enabled ()) {pc_memory_init (get_system_memory (), kernel_filename, kernel_cmdline, initrd_filename, below_4g_mem_size, above_4g_mem_size, - rom_memory, &ram_memory); + rom_memory, &ram_memory, guest_info); }/* irq lines */ hw/i386/pc_q35.c static void pc_q35_init(QEMUMachineInitArgs *args) · line 140
Quoted 7 lines
hw/i386/pc_q35.c +1
-0@@ -131,6 +134,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args) q35_host->mch.address_space_io =get_system_io (); q35_host->mch.below_4g_mem_size = below_4g_mem_size; q35_host->mch.above_4g_mem_size = above_4g_mem_size; + q35_host->mch.guest_info = guest_info;/* pci */ qdev_init_nofail (DEVICE (q35_host)); host_bus = q35_host->host.pci.bus;OK, a direct (owner) link is established here; which gives birth to the next question:
Quoted 22 lines
hw/pci-host/q35.c +4
-0@@ -244,6 +244,10 @@ static int mch_init(PCIDevice *d)hwaddr pci_hole64_size;MCHPCIState *mch =MCH_PCI_DEVICE (d); +/* Leave enough space for the biggest MCFG BAR */ + mch->guest_info->pci_info.w32.begin =MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT + +MCH_HOST_BRIDGE_PCIEXBAR_MAX ; +/* setup pci memory regions */ memory_region_init_alias(&mch->pci_hole, "pci-hole", mch->pci_address_space,include/hw/pci-host/q35.h +0
-0include/hw/pci-host/q35.h typedef struct MCHPCIState { · line 59
Quoted 5 lines
include/hw/pci-host/q35.h +1
-0@@ -55,6 +55,7 @@ typedef struct MCHPCIState {uint8_t smm_enabled;ram_addr_t below_4g_mem_size;ram_addr_t above_4g_mem_size; +PcGuestInfo *guest_info; } MCHPCIState;... how does this relate to migration? (I'm not suggesting anything, I'm just curious.)
Quoted 2 lines
Thanks,
Laszlo
As far as I can tell it doesn't relate to migration in any way.
Signature · 1 line
MST
Re: [Qemu-devel] [PATCH v2 2/5] pci: store PCI hole ranges in guestinfo structure
In reply to Michael S. TsirkinQuoted 39 lines · Michael S. Tsirkin
Will be used to pass hole ranges to guests.
Signed-off-by Michael S. Tsirkin <[email protected]>--- hw/i386/pc.c | 39 ++++++++++++++++++++++++++++++++++++++- hw/i386/pc_piix.c | 14 +++++++++++++- hw/i386/pc_q35.c | 6 +++++- hw/pci-host/q35.c | 4 ++++ include/hw/i386/pc.h | 19 ++++++++++++++++++- include/hw/pci-host/q35.h | 2 ++ include/qemu/typedefs.h | 1 + 7 files changed, 81 insertions(+), 4 deletions(-)hw/i386/pc.c +18
-0@@ -978,6 +978,41 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge) } } +typedef struct PcGuestInfoState { +PcGuestInfo info; +Notifier machine_done; +}PcGuestInfoState ; + +static +void pc_guest_info_machine_done (Notifier *notifier,void *data) +{ +PcGuestInfoState *guest_info_state =container_of (notifier, + PcGuestInfoState, + machine_done); +} + +PcGuestInfo *pc_guest_info_init (ram_addr_t below_4g_mem_size, +ram_addr_t above_4g_mem_size) +{ +PcGuestInfoState *guest_info_state =g_malloc0 (sizeof *guest_info_state); +PcGuestInfo *guest_info = &guest_info_state->info;
hw/i386/pc.c void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge) · line 1008
Quoted 10 lines
hw/i386/pc.c +10
-0+ + guest_info->pci_info.w32.end =IO_APIC_DEFAULT_ADDRESS ; +if (sizeof (hwaddr) ==4 ) { + guest_info->pci_info.w64.begin =0 ; + guest_info->pci_info.w64.end =0 ; + }else { + guest_info->pci_info.w64.begin =0x100000000ULL + above_4g_mem_size; + guest_info->pci_info.w64.end = guest_info->pci_info.w64.begin + + (0x1ULL <<62 ); +assert (guest_info->pci_info.w64.begin <= guest_info->pci_info.w64.end);
It would be better to do this in range_set(), thus users don't need to do it themselves everywhere.
Quoted 105 lines
hw/i386/pc.c +10
-1+ } + + guest_info_state->machine_done.notify = pc_guest_info_machine_done; +qemu_add_machine_init_done_notifier (&guest_info_state->machine_done); +return guest_info; +} +void pc_acpi_init (const char *default_dsdt) {char *filename;@@ -1019,7 +1054,8 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,ram_addr_t below_4g_mem_size, ram_addr_t above_4g_mem_size, MemoryRegion *rom_memory, - MemoryRegion **ram_memory) + MemoryRegion **ram_memory, + PcGuestInfo *guest_info) { int linux_boot, i;MemoryRegion *ram, *option_rom_mr;@@ -1071,6 +1107,7 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,for (i =0 ; i < nb_option_roms; i++) {rom_add_option (option_rom[i].name, option_rom[i].bootindex); } + guest_info->fw_cfg = fw_cfg;return fw_cfg; }hw/i386/pc_piix.c +13
-1@@ -90,6 +90,7 @@ static void pc_init1(MemoryRegion *system_memory,MemoryRegion *rom_memory;DeviceState *icc_bridge;FWCfgState *fw_cfg =NULL ; +PcGuestInfo *guest_info; icc_bridge =qdev_create (NULL ,TYPE_ICC_BRIDGE ); object_property_add_child(qdev_get_machine(), "icc-bridge",@@ -119,12 +120,23 @@ static void pc_init1(MemoryRegion *system_memory, rom_memory = system_memory; } + guest_info =pc_guest_info_init (below_4g_mem_size, above_4g_mem_size); + +/* Set PCI window size the way seabios has always done it. */ +/* TODO: consider just starting at below_4g_mem_size */ +if (ram_size <=0x80000000 ) + guest_info->pci_info.w32.begin =0x80000000 ; +else if (ram_size <=0xc0000000 ) + guest_info->pci_info.w32.begin =0xc0000000 ; +else + guest_info->pci_info.w32.begin =0xe0000000 ; +/* allocate ram and load rom/bios */ if (!xen_enabled ()) { fw_cfg =pc_memory_init (system_memory, kernel_filename, kernel_cmdline, initrd_filename, below_4g_mem_size, above_4g_mem_size, - rom_memory, &ram_memory); + rom_memory, &ram_memory, guest_info); } gsi_state =g_malloc0 (sizeof (*gsi_state));hw/i386/pc_q35.c +5
-1@@ -77,6 +77,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)ICH9LPCState *ich9_lpc;PCIDevice *ahci;DeviceState *icc_bridge; +PcGuestInfo *guest_info; icc_bridge =qdev_create (NULL ,TYPE_ICC_BRIDGE ); object_property_add_child(qdev_get_machine(), "icc-bridge",@@ -105,11 +106,13 @@ static void pc_q35_init(QEMUMachineInitArgs *args) rom_memory =get_system_memory (); } + guest_info =pc_guest_info_init (below_4g_mem_size, above_4g_mem_size); +/* allocate ram and load rom/bios */ if (!xen_enabled ()) {pc_memory_init (get_system_memory (), kernel_filename, kernel_cmdline, initrd_filename, below_4g_mem_size, above_4g_mem_size, - rom_memory, &ram_memory); + rom_memory, &ram_memory, guest_info); }/* irq lines */ @@ -131,6 +134,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args) q35_host->mch.address_space_io =get_system_io (); q35_host->mch.below_4g_mem_size = below_4g_mem_size; q35_host->mch.above_4g_mem_size = above_4g_mem_size; + q35_host->mch.guest_info = guest_info;/* pci */ qdev_init_nofail (DEVICE (q35_host)); host_bus = q35_host->host.pci.bus;hw/pci-host/q35.c +0
-0
hw/pci-host/q35.c static int mch_init(PCIDevice *d) · line 250
Quoted 7 lines
hw/pci-host/q35.c +4
-0@@ -244,6 +244,10 @@ static int mch_init(PCIDevice *d)hwaddr pci_hole64_size;MCHPCIState *mch =MCH_PCI_DEVICE (d); +/* Leave enough space for the biggest MCFG BAR */ + mch->guest_info->pci_info.w32.begin =MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT + +MCH_HOST_BRIDGE_PCIEXBAR_MAX ; +
I'm not sure if this is correct, but mch_update_pciexbar() will update the length according to the length field of PCIEXBAR.
And, is PCI Express configuration space not considered pci hole?
Quoted 9 lines
hw/pci-host/q35.c +0
-0/* setup pci memory regions */ memory_region_init_alias(&mch->pci_hole, "pci-hole", mch->pci_address_space,include/hw/i386/pc.h +0
-0@@ -9,8 +9,20 @@#include "net/net.h"
include/hw/i386/pc.h line 19
Quoted 10 lines
include/hw/i386/pc.h +6
-0#include "hw/i386/ioapic.h" +#include "qemu/range.h" +/* PC-style peripherals (also used by other machines). */ +typedef struct PcPciInfo { +Range w32; +Range w64; +}PcPciInfo ;
PcPciHole as the Subject suggests?
Quoted 64 lines
include/hw/i386/pc.h +12
-1+ +struct PcGuestInfo { +PcPciInfo pci_info; +FWCfgState *fw_cfg; +}; +/* parallel.c */ static inline bool parallel_init (ISABus *bus,int index,CharDriverState *chr) {@@ -80,6 +92,10 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level);void pc_cpus_init (const char *cpu_model,DeviceState *icc_bridge);void pc_hot_add_cpu (const int64_t id,Error **errp);void pc_acpi_init (const char *default_dsdt); + +PcGuestInfo *pc_guest_info_init (ram_addr_t below_4g_mem_size, +ram_addr_t above_4g_mem_size); + FWCfgState *pc_memory_init(MemoryRegion *system_memory, const char *kernel_filename, const char *kernel_cmdline,@@ -87,7 +103,8 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,ram_addr_t below_4g_mem_size, ram_addr_t above_4g_mem_size, MemoryRegion *rom_memory, - MemoryRegion **ram_memory); + MemoryRegion **ram_memory, + PcGuestInfo *guest_info);qemu_irq *pc_allocate_cpu_irq (void );DeviceState *pc_vga_init (ISABus *isa_bus,PCIBus *pci_bus); void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,include/hw/pci-host/q35.h +2
-0@@ -55,6 +55,7 @@ typedef struct MCHPCIState {uint8_t smm_enabled;ram_addr_t below_4g_mem_size;ram_addr_t above_4g_mem_size; +PcGuestInfo *guest_info; } MCHPCIState; typedef struct Q35PCIHost {@@ -81,6 +82,7 @@ typedef struct Q35PCIHost {#define MCH_HOST_BRIDGE_PCIEXBAR 0x60/* 64bit register */ #define MCH_HOST_BRIDGE_PCIEXBAR_SIZE 8/* 64bit register */ #define MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT 0xb0000000 +#define MCH_HOST_BRIDGE_PCIEXBAR_MAX (0x10000000)/* 256M */ #define MCH_HOST_BRIDGE_PCIEXBAR_ADMSK Q35_MASK(64, 35, 28)#define MCH_HOST_BRIDGE_PCIEXBAR_128ADMSK ((uint64_t)(1 << 26))#define MCH_HOST_BRIDGE_PCIEXBAR_64ADMSK ((uint64_t)(1 << 25))include/qemu/typedefs.h +1
-0@@ -62,5 +62,6 @@ typedef struct VirtIODevice VirtIODevice;typedef struct QEMUSGList QEMUSGList ;typedef struct SHPCDevice SHPCDevice ;typedef struct FWCfgState FWCfgState ; +typedef struct PcGuestInfo PcGuestInfo ;#endif /* QEMU_TYPEDEFS_H */ Signature · 1 line
MST
PATCH v2 3/5 pc: pass PCI hole ranges to Guests
In reply to Michael S. TsirkinGuest currently has to jump through lots of hoops to guess the PCI hole ranges. It's fragile, and makes us change BIOS each time we add a new chipset. Let's report the window in a ROM file, to make BIOS do exactly what QEMU intends.
--- hw/i386/pc.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
hw/i386/pc.c +21 -0
@@ -978,6 +978,26 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge) } } +/* pci-info ROM file. Little endian format */ +typedef struct PcRomPciInfo { +uint64_t w32_min; +uint64_t w32_max; +uint64_t w64_min; +uint64_t w64_max; +}PcRomPciInfo ; + +static void pc_fw_cfg_guest_info (PcGuestInfo *guest_info) +{ +PcRomPciInfo *info =g_malloc (sizeof *info); + info->w32_min =cpu_to_le64 (guest_info->pci_info.w32.begin); + info->w32_max =cpu_to_le64 (guest_info->pci_info.w32.end); + info->w64_min =cpu_to_le64 (guest_info->pci_info.w64.begin); + info->w64_max =cpu_to_le64 (guest_info->pci_info.w64.end); +/* Pass PCI hole info to guest via a side channel. +* Required so guest PCI enumeration does the right thing. */ +fw_cfg_add_file (guest_info->fw_cfg,"etc/pci-info" , info,sizeof *info); +} + typedef struct PcGuestInfoState { PcGuestInfo info; Notifier machine_done;
@@ -989,6 +1009,7 @@ void pc_guest_info_machine_done(Notifier *notifier, void *data)PcGuestInfoState *guest_info_state =container_of (notifier, PcGuestInfoState, machine_done); +pc_fw_cfg_guest_info (&guest_info_state->info); } PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size,
Signature · 1 line
MST
PATCH v2 4/5 pc: add 1.6 compat type
In reply to Michael S. TsirkinIdentical to 1.5 ATM, but changes will accumulate.
--- hw/i386/pc_piix.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
hw/i386/pc_piix.c +12 -1
@@ -57,6 +57,7 @@ static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };static const int ide_irq[MAX_IDE_BUS ] = {14 ,15 };static bool has_pvpanic = true; +static bool has_pci_info = true;/* PC hardware initialisation */ static void pc_init1(MemoryRegion *system_memory,
@@ -340,9 +341,19 @@ static void pc_xen_hvm_init(QEMUMachineInitArgs *args) }#endif +static QEMUMachine pc_i440fx_machine_v1_6 = { + .name ="pc-i440fx-1.6" , + .alias ="pc" , + .desc ="Standard PC (i440FX + PIIX, 1996)" , + .init = pc_init_pci, + .hot_add_cpu = pc_hot_add_cpu, + .max_cpus =255 , + .is_default =1 , +DEFAULT_MACHINE_OPTIONS , +}; +static QEMUMachine pc_i440fx_machine_v1_5 = { .name ="pc-i440fx-1.5" , - .alias ="pc" , .desc ="Standard PC (i440FX + PIIX, 1996)" , .init = pc_init_pci, .hot_add_cpu = pc_hot_add_cpu,
Signature · 1 line
MST
Re: [Qemu-devel] [PATCH v2 4/5] pc: add 1.6 compat type
Am 30.05.2013 13:07, schrieb Michael S. Tsirkin:
Quoted 3 lines
Identical to 1.5 ATM, but changes will accumulate.
Signed-off-by Michael S. Tsirkin <[email protected]>
You did notice that Eduardo had a similar patch yesterday or so? Yours does not handle q35 by comparison.
hw/i386/pc_piix.c static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 }; · line 60
Quoted 13 lines
--- hw/i386/pc_piix.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)hw/i386/pc_piix.c +1
-0@@ -57,6 +57,7 @@ static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };static const int ide_irq[MAX_IDE_BUS ] = {14 ,15 };static bool has_pvpanic = true; +static bool has_pci_info = true;
I don't see this used here, and it's replaced in next patch.
Andreas
Quoted 25 lines
hw/i386/pc_piix.c +11
-1/* PC hardware initialisation */ static void pc_init1(MemoryRegion *system_memory,@@ -340,9 +341,19 @@ static void pc_xen_hvm_init(QEMUMachineInitArgs *args) }#endif +static QEMUMachine pc_i440fx_machine_v1_6 = { + .name ="pc-i440fx-1.6" , + .alias ="pc" , + .desc ="Standard PC (i440FX + PIIX, 1996)" , + .init = pc_init_pci, + .hot_add_cpu = pc_hot_add_cpu, + .max_cpus =255 , + .is_default =1 , +DEFAULT_MACHINE_OPTIONS , +}; +static QEMUMachine pc_i440fx_machine_v1_5 = { .name ="pc-i440fx-1.5" , - .alias ="pc" , .desc ="Standard PC (i440FX + PIIX, 1996)" , .init = pc_init_pci, .hot_add_cpu = pc_hot_add_cpu,
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH v2 4/5] pc: add 1.6 compat type
Quoted 7 lines · Andreas Färber
Am 30.05.2013 13:07, schrieb Michael S. Tsirkin:
Quoted 3 lines
Identical to 1.5 ATM, but changes will accumulate.
Signed-off-by Michael S. Tsirkin <[email protected]>You did notice that Eduardo had a similar patch yesterday or so? Yours does not handle q35 by comparison.
Hmm I'll have to figure out the dependency.
Quoted 48 lines
hw/i386/pc_piix.c static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 }; · line 60
Quoted 13 lines
--- hw/i386/pc_piix.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)hw/i386/pc_piix.c +1
-0@@ -57,6 +57,7 @@ static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };static const int ide_irq[MAX_IDE_BUS ] = {14 ,15 };static bool has_pvpanic = true; +static bool has_pci_info = true;I don't see this used here, and it's replaced in next patch.
Andreas
Quoted 25 lines
hw/i386/pc_piix.c +11
-1/* PC hardware initialisation */ static void pc_init1(MemoryRegion *system_memory,@@ -340,9 +341,19 @@ static void pc_xen_hvm_init(QEMUMachineInitArgs *args) }#endif +static QEMUMachine pc_i440fx_machine_v1_6 = { + .name ="pc-i440fx-1.6" , + .alias ="pc" , + .desc ="Standard PC (i440FX + PIIX, 1996)" , + .init = pc_init_pci, + .hot_add_cpu = pc_hot_add_cpu, + .max_cpus =255 , + .is_default =1 , +DEFAULT_MACHINE_OPTIONS , +}; +static QEMUMachine pc_i440fx_machine_v1_5 = { .name ="pc-i440fx-1.5" , - .alias ="pc" , .desc ="Standard PC (i440FX + PIIX, 1996)" , .init = pc_init_pci, .hot_add_cpu = pc_hot_add_cpu,Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
PATCH v2 5/5 pc: pci-info add compat support
In reply to Michael S. TsirkinWe can't change fw cfg entries or add new ones without breaking cross version migration. Add a flag to skip adding new entry when running with 1.5 compat machine type.
--- hw/i386/pc.c | 7 ++++++- hw/i386/pc_piix.c | 12 ++++++++++-- include/hw/i386/pc.h | 1 + 3 files changed, 17 insertions(+), 3 deletions(-)
hw/i386/pc.c +6 -1
@@ -988,7 +988,12 @@ typedef struct PcRomPciInfo {static void pc_fw_cfg_guest_info (PcGuestInfo *guest_info) { -PcRomPciInfo *info =g_malloc (sizeof *info); +PcRomPciInfo *info; +if (guest_info->compat_v1_5) { +return ; + } + + info =g_malloc (sizeof *info); info->w32_min =cpu_to_le64 (guest_info->pci_info.w32.begin); info->w32_max =cpu_to_le64 (guest_info->pci_info.w32.end); info->w64_min =cpu_to_le64 (guest_info->pci_info.w64.begin);
hw/i386/pc_piix.c +10 -2
@@ -57,7 +57,7 @@ static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };static const int ide_irq[MAX_IDE_BUS ] = {14 ,15 };static bool has_pvpanic = true; -static bool has_pci_info = true; +static bool guest_info_compat_v1_5 = false;/* PC hardware initialisation */ static void pc_init1(MemoryRegion *system_memory,
@@ -122,6 +122,7 @@ static void pc_init1(MemoryRegion *system_memory, } guest_info =pc_guest_info_init (below_4g_mem_size, above_4g_mem_size); + guest_info->compat_v1_5 = guest_info_compat_v1_5;/* Set PCI window size the way seabios has always done it. */ /* TODO: consider just starting at below_4g_mem_size */
@@ -259,6 +260,12 @@ static void pc_init_pci(QEMUMachineInitArgs *args) initrd_filename, cpu_model, 1, 1); } +static void pc_init_pci_1_5 (QEMUMachineInitArgs *args) +{ + guest_info_compat_v1_5 = true; +pc_init_pci (args); +} +static void pc_init_pci_1_4 (QEMUMachineInitArgs *args) { has_pvpanic = false;
@@ -355,7 +362,7 @@ static QEMUMachine pc_i440fx_machine_v1_6 = {static QEMUMachine pc_i440fx_machine_v1_5 = { .name ="pc-i440fx-1.5" , .desc ="Standard PC (i440FX + PIIX, 1996)" , - .init = pc_init_pci, + .init = pc_init_pci_1_5, .hot_add_cpu = pc_hot_add_cpu, .max_cpus =255 , .is_default =1 ,
@@ -759,6 +766,7 @@ static QEMUMachine xenfv_machine = {static void pc_machine_init (void ) { +qemu_register_machine (&pc_i440fx_machine_v1_6);qemu_register_machine (&pc_i440fx_machine_v1_5);qemu_register_machine (&pc_i440fx_machine_v1_4);qemu_register_machine (&pc_machine_v1_3);
include/hw/i386/pc.h +1 -0
@@ -20,6 +20,7 @@ typedef struct PcPciInfo {struct PcGuestInfo {PcPciInfo pci_info; +bool compat_v1_5;FWCfgState *fw_cfg; };
Signature · 1 line
MST
Re: [Qemu-devel] [PATCH v2 5/5] pc: pci-info add compat support
hw/i386/pc_piix.c static void pc_init1(MemoryRegion *system_memory, · line 125
Quoted 7 lines · Michael S. Tsirkin
/* PC hardware initialisation */
static void pc_init1(MemoryRegion*system_memory,hw/i386/pc_piix.c +1
-0@@ -122,6 +122,7 @@ static void pc_init1(MemoryRegion *system_memory, } guest_info =pc_guest_info_init (below_4g_mem_size, above_4g_mem_size); + guest_info->compat_v1_5 = guest_info_compat_v1_5;
I believe I can see the advantage of delaying this "compat_v1_5" until init-done-notifier time: init code gradually building up / rewriting guest_info doesn't have to tiptoe around conditions.
Style: would it be worth passing "guest_info_compat_v1_5" as a parameter to pc_guest_info_init()? Currently you have an _init() function that partially initializes the struct, and right after _init() returns you fill in what's still missing form basic initialization.
No more comments for the series.
Thanks,
Laszlo
Re: [Qemu-devel] [PATCH v2 5/5] pc: pci-info add compat support
Quoted 14 lines · Laszlo Ersek
hw/i386/pc_piix.c static void pc_init1(MemoryRegion *system_memory, · line 125
Quoted 7 lines · Michael S. Tsirkin
/* PC hardware initialisation */
static void pc_init1(MemoryRegion*system_memory,hw/i386/pc_piix.c +1
-0@@ -122,6 +122,7 @@ static void pc_init1(MemoryRegion *system_memory, } guest_info =pc_guest_info_init (below_4g_mem_size, above_4g_mem_size); + guest_info->compat_v1_5 = guest_info_compat_v1_5;I believe I can see the advantage of delaying this "
compat_v1_5" until init-done-notifier time: init code gradually building up / rewritingguest_infodoesn't have to tiptoe around conditions.
Quoted 4 lines
Style: would it be worth passing "
guest_info_compat_v1_5" as a parameter topc_guest_info_init()? Currently you have an_init()function that partially initializes the struct, and right after_init()returns you fill in what's still missing form basic initialization.
This seems to be the style used otherwise in this file ...
Quoted 4 lines
No more comments for the series.
Thanks,
Laszlo