|
| From: | Alexey Kardashevskiy |
| Subject: | Re: [Qemu-devel] [PATCH qemu] spapr-pci: Make MMIO spacing a machine property and increase it |
| Date: | Fri, 4 Mar 2016 15:13:12 +1100 |
| User-agent: | Mozilla/5.0 (X11; Linux i686 on x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 |
On 03/04/2016 02:39 PM, David Gibson wrote:
On Thu, Mar 03, 2016 at 12:42:53PM +1100, Alexey Kardashevskiy wrote:The pseries machine supports multiple PHBs. Each PHB's MMIO/IO space is mapped to the CPU address space starting at SPAPR_PCI_WINDOW_BASE plus some offset which is calculated from PHB's index and SPAPR_PCI_WINDOW_SPACING which is defined now as 64GB. Since the default 32bit DMA window is using first 2GB of MMIO space, the amount of MMIO which the PCI devices can actually use is reduced to 62GB. This is a problem if the user wants to use devices with huge BARs. For example, 2 PCI functions of a NVIDIA K80 adapter being passed through will exceed this limit as they have 16M + 16G + 32M BARs which (when aligned) will need 64GB. This converts SPAPR_PCI_WINDOW_BASE and SPAPR_PCI_WINDOW_SPACING to sPAPRMachineState properties. This uses old values for pseries machine before 2.6 and increases the spacing to 128GB so MMIO space becomes 126GB. This changes the default value of sPAPRPHBState::mem_win_size to -1 for pseries-2.6 and adds setup to spapr_phb_realize. Signed-off-by: Alexey Kardashevskiy <address@hidden>So, in theory I dislike the spapr_pci device reaching into the machine type to get the spacing configuration. But.. I don't know of a better way to achieve the desired outcome.
We could drop @index and spacing; and request the user to specify the MMIO window start (at least) for every additional PHB.
A couple of other details concern me a little more.--- hw/ppc/spapr.c | 43 ++++++++++++++++++++++++++++++++++++++++++- hw/ppc/spapr_pci.c | 14 ++++++++++---- include/hw/pci-host/spapr.h | 4 +--- include/hw/ppc/spapr.h | 1 + 4 files changed, 54 insertions(+), 8 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index e9d4abf..d21ad8a 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -40,6 +40,7 @@ #include "migration/migration.h" #include "mmu-hash64.h" #include "qom/cpu.h" +#include "qapi/visitor.h" #include "hw/boards.h" #include "hw/ppc/ppc.h" @@ -2100,6 +2101,29 @@ static void spapr_set_kvm_type(Object *obj, const char *value, Error **errp) spapr->kvm_type = g_strdup(value); } +static void spapr_prop_get_uint64(Object *obj, Visitor *v, const char *name, + void *opaque, Error **errp) +{ + uint64_t value = *(uint64_t *)opaque; + visit_type_uint64(v, name, &value, errp); +} + +static void spapr_prop_set_uint64(Object *obj, Visitor *v, const char *name, + void *opaque, Error **errp) +{ + uint64_t value = -1; + visit_type_uint64(v, name, &value, errp); + *(uint64_t *)opaque = value; +}Pity there aren't standard helpers for this.+static void spapr_prop_add_uint64(Object *obj, const char *name, + uint64_t *pval, const char *desc) +{ + object_property_add(obj, name, "uint64", spapr_prop_get_uint64, + spapr_prop_set_uint64, NULL, pval, NULL); + object_property_set_description(obj, name, desc, NULL); +} + static void spapr_machine_initfn(Object *obj) { sPAPRMachineState *spapr = SPAPR_MACHINE(obj); @@ -2110,6 +2134,10 @@ static void spapr_machine_initfn(Object *obj) object_property_set_description(obj, "kvm-type", "Specifies the KVM virtualization mode (HV, PR)", NULL); + spapr_prop_add_uint64(obj, "phb-mmio-base", &spapr->phb_mmio_base, + "Base address for PCI host bridge MMIO"); + spapr_prop_add_uint64(obj, "phb-mmio-spacing", &spapr->phb_mmio_spacing, + "Amount of MMIO space per PCI host bridge");Hmm.. what happens if someone tries to change these propertis at runtime with qom-set? That sounds bad.
What is the problem here exactly? These are the properties for new PHBs, if/when we add an ability to hotplug PHBs, changes to these properties will reflect in new PHB properties.
Likewise writing to "kvm-type" does not switch from HV to PR and vice versa.
} static void spapr_machine_finalizefn(Object *obj) @@ -2357,6 +2385,10 @@ static const TypeInfo spapr_machine_info = { */ static void spapr_machine_2_6_instance_options(MachineState *machine) { + sPAPRMachineState *spapr = SPAPR_MACHINE(machine); + + spapr->phb_mmio_base = SPAPR_PCI_WINDOW_BASE; + spapr->phb_mmio_spacing = SPAPR_PCI_WINDOW_SPACING; } static void spapr_machine_2_6_class_options(MachineClass *mc) @@ -2370,10 +2402,19 @@ DEFINE_SPAPR_MACHINE(2_6, "2.6", true); * pseries-2.5 */ #define SPAPR_COMPAT_2_5 \ - HW_COMPAT_2_5 + HW_COMPAT_2_5 \ + {\ + .driver = TYPE_SPAPR_PCI_HOST_BRIDGE,\ + .property = "mem_win_size",\ + .value = "0x1000000000",\ + }, static void spapr_machine_2_5_instance_options(MachineState *machine) { + sPAPRMachineState *spapr = SPAPR_MACHINE(machine); + + spapr->phb_mmio_base = 0x10000000000ULL; + spapr->phb_mmio_spacing = 0x1000000000ULL; } static void spapr_machine_2_5_class_options(MachineClass *mc) diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index e8edad3..bae01dd 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -1260,9 +1260,11 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) sphb->buid = SPAPR_PCI_BASE_BUID + sphb->index; sphb->dma_liobn = SPAPR_PCI_LIOBN(sphb->index, 0); - windows_base = SPAPR_PCI_WINDOW_BASE - + sphb->index * SPAPR_PCI_WINDOW_SPACING; + windows_base = spapr->phb_mmio_base + + sphb->index * spapr->phb_mmio_spacing; sphb->mem_win_addr = windows_base + SPAPR_PCI_MMIO_WIN_OFF; + sphb->mem_win_size = spapr->phb_mmio_spacing - + SPAPR_PCI_MEM_WIN_BUS_OFFSET; sphb->io_win_addr = windows_base + SPAPR_PCI_IO_WIN_OFF; } @@ -1281,6 +1283,11 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) return; } + if (sphb->mem_win_size == (hwaddr)-1) { + error_setg(errp, "Memory window size not specified for PHB"); + return; + } + if (sphb->io_win_addr == (hwaddr)-1) { error_setg(errp, "IO window address not specified for PHB"); return; @@ -1441,8 +1448,7 @@ static Property spapr_phb_properties[] = { DEFINE_PROP_UINT64("buid", sPAPRPHBState, buid, -1), DEFINE_PROP_UINT32("liobn", sPAPRPHBState, dma_liobn, -1), DEFINE_PROP_UINT64("mem_win_addr", sPAPRPHBState, mem_win_addr, -1), - DEFINE_PROP_UINT64("mem_win_size", sPAPRPHBState, mem_win_size, - SPAPR_PCI_MMIO_WIN_SIZE), + DEFINE_PROP_UINT64("mem_win_size", sPAPRPHBState, mem_win_size, -1), DEFINE_PROP_UINT64("io_win_addr", sPAPRPHBState, io_win_addr, -1), DEFINE_PROP_UINT64("io_win_size", sPAPRPHBState, io_win_size, SPAPR_PCI_IO_WIN_SIZE), diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h index 7de5e02..b828c31 100644 --- a/include/hw/pci-host/spapr.h +++ b/include/hw/pci-host/spapr.h @@ -107,10 +107,8 @@ struct sPAPRPHBVFIOState { #define SPAPR_PCI_MEM_WIN_BUS_OFFSET 0x80000000ULL #define SPAPR_PCI_WINDOW_BASE 0x10000000000ULL -#define SPAPR_PCI_WINDOW_SPACING 0x1000000000ULL +#define SPAPR_PCI_WINDOW_SPACING 0x2000000000ULL #define SPAPR_PCI_MMIO_WIN_OFF 0xA0000000 -#define SPAPR_PCI_MMIO_WIN_SIZE (SPAPR_PCI_WINDOW_SPACING - \ - SPAPR_PCI_MEM_WIN_BUS_OFFSET) #define SPAPR_PCI_IO_WIN_OFF 0x80000000 #define SPAPR_PCI_IO_WIN_SIZE 0x10000 diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index 098d85d..8b1369e 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -48,6 +48,7 @@ struct sPAPRMachineState { struct VIOsPAPRBus *vio_bus; QLIST_HEAD(, sPAPRPHBState) phbs; + uint64_t phb_mmio_base, phb_mmio_spacing; struct sPAPRNVRAM *nvram; XICSState *icp; DeviceState *rtc;
-- Alexey
| [Prev in Thread] | Current Thread | [Next in Thread] |