* [Qemu-devel] [PATCH qemu v5 00/12] spapr: vfio: Enable Dynamic DMA windows (DDW)
@ 2015-03-31 5:28 Alexey Kardashevskiy
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 01/12] linux headers update for DDW on SPAPR Alexey Kardashevskiy
` (11 more replies)
0 siblings, 12 replies; 26+ messages in thread
From: Alexey Kardashevskiy @ 2015-03-31 5:28 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Kardashevskiy, Alex Williamson, qemu-ppc, Alexander Graf,
David Gibson
Each Partitionable Endpoint (IOMMU group) has an address range on a PCI bus
where devices are allowed to do DMA. These ranges are called DMA windows.
By default, there is a single DMA window, 1 or 2GB big, mapped at zero
on a PCI bus.
PAPR defines a DDW RTAS API which allows pseries guests
querying the hypervisor about DDW support and capabilities (page size mask
for now). A pseries guest may request an additional (to the default)
DMA windows using this RTAS API.
The existing pseries Linux guests request an additional window as big as
the guest RAM and map the entire guest window which effectively creates
direct mapping of the guest memory to a PCI bus.
This patchset reworks PPC64 IOMMU code and adds necessary structures
to support big windows.
Once a Linux guest discovers the presence of DDW, it does:
1. query hypervisor about number of available windows and page size masks;
2. create a window with the biggest possible page size (today 4K/64K/16M);
3. map the entire guest RAM via H_PUT_TCE* hypercalls;
4. switche dma_ops to direct_dma_ops on the selected PE.
Once this is done, H_PUT_TCE is not called anymore for 64bit devices and
the guest does not waste time on DMA map/unmap operations.
Note that 32bit devices won't use DDW and will keep using the default
DMA window so KVM optimizations will be required (to be posted later).
This patchset adds DDW support for pseries. The host kernel changes are
required, posted as:
[PATCH kernel v7 00/31] powerpc/iommu/vfio: Enable Dynamic DMA windows
This patchset is based on git://github.com/dgibson/qemu.git spapr-next branch.
Please comment. Thanks!
Changes:
v5:
* TCE tables got "enabled" state and are persistent, i.e. not recreated
every reboot
* added v2 of SPAPR_TCE_IOMMU
* fixed migration for emulated PHB with enabled DDW
* huge pile of other changes
v4:
* reimplemented the whole thing
* machine reset and ddw-reset RTAS call both remove all TCE tables and
create the default one
* IOMMU group id is not needed to use VFIO PHB anymore, multiple groups
are supported on the same VFIO container and virtual PHB
v3:
* removed "reset" from API now
* reworked machine versions
* applied multiple comments
* includes David's machine QOM rework as this patchset adds a new machine type
v2:
* tested on emulated PHB
* removed "ddw" machine property, now it is PHB property
* disabled by default
* defined "pseries-2.2" machine which enables DDW by default
* fixed reset() and reference counting
Alexey Kardashevskiy (12):
linux headers update for DDW on SPAPR
vmstate: Define VARRAY with VMS_ALLOC
spapr_pci: Make find_phb()/find_dev() public
spapr_pci_vfio: Enable multiple groups per container
vfio: spapr: Move SPAPR-related code to a separate file
vfio: spapr: Add SPAPR IOMMU v2 support (DMA memory preregistering)
spapr_iommu: Rework TCE table initialization
spapr_pci: Rework reset to reset DMA configuration
spapr_iommu: Add root memory region
spapr_pci: Rework finish_realize()
spapr_pci: Disable all DMA windows on reset
spapr_pci/spapr_pci_vfio: Support Dynamic DMA Windows (DDW)
hw/ppc/Makefile.objs | 3 +
hw/ppc/spapr.c | 5 +
hw/ppc/spapr_iommu.c | 140 +++++++++++++------
hw/ppc/spapr_pci.c | 118 ++++++++++++----
hw/ppc/spapr_pci_vfio.c | 149 ++++++++++++++------
hw/ppc/spapr_rtas_ddw.c | 314 ++++++++++++++++++++++++++++++++++++++++++
hw/ppc/spapr_vio.c | 10 +-
hw/vfio/Makefile.objs | 1 +
hw/vfio/common.c | 186 +++++--------------------
hw/vfio/spapr.c | 301 ++++++++++++++++++++++++++++++++++++++++
include/hw/pci-host/spapr.h | 21 ++-
include/hw/ppc/spapr.h | 31 ++++-
include/hw/vfio/vfio-common.h | 16 +++
include/hw/vfio/vfio.h | 2 +-
include/migration/vmstate.h | 10 ++
linux-headers/linux/vfio.h | 88 +++++++++++-
trace-events | 5 +
17 files changed, 1130 insertions(+), 270 deletions(-)
create mode 100644 hw/ppc/spapr_rtas_ddw.c
create mode 100644 hw/vfio/spapr.c
--
2.0.0
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH qemu v5 01/12] linux headers update for DDW on SPAPR
2015-03-31 5:28 [Qemu-devel] [PATCH qemu v5 00/12] spapr: vfio: Enable Dynamic DMA windows (DDW) Alexey Kardashevskiy
@ 2015-03-31 5:28 ` Alexey Kardashevskiy
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 02/12] vmstate: Define VARRAY with VMS_ALLOC Alexey Kardashevskiy
` (10 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Alexey Kardashevskiy @ 2015-03-31 5:28 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Kardashevskiy, Alex Williamson, qemu-ppc, Alexander Graf,
David Gibson
Since the changes are not in upstream yet, no tag or branch is specified here.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
| 88 ++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 85 insertions(+), 3 deletions(-)
--git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
index 95ba870..ce05371 100644
--- a/linux-headers/linux/vfio.h
+++ b/linux-headers/linux/vfio.h
@@ -36,6 +36,8 @@
/* Two-stage IOMMU */
#define VFIO_TYPE1_NESTING_IOMMU 6 /* Implies v2 */
+#define VFIO_SPAPR_TCE_v2_IOMMU 7
+
/*
* The IOCTL interface is designed for extensibility by embedding the
* structure length (argsz) and flags into structures passed between
@@ -441,6 +443,23 @@ struct vfio_iommu_type1_dma_unmap {
/* -------- Additional API for SPAPR TCE (Server POWERPC) IOMMU -------- */
/*
+ * The SPAPR TCE DDW info struct provides the information about
+ * the details of Dynamic DMA window capability.
+ *
+ * @pgsizes contains a page size bitmask, 4K/64K/16M are supported.
+ * @max_dynamic_windows_supported tells the maximum number of windows
+ * which the platform can create.
+ * @levels tells the maximum number of levels in multi-level IOMMU tables;
+ * this allows splitting a table into smaller chunks which reduces
+ * the amount of physically contiguous memory required for the table.
+ */
+struct vfio_iommu_spapr_tce_ddw_info {
+ __u64 pgsizes; /* Bitmap of supported page sizes */
+ __u32 max_dynamic_windows_supported;
+ __u32 levels;
+};
+
+/*
* The SPAPR TCE info struct provides the information about the PCI bus
* address ranges available for DMA, these values are programmed into
* the hardware so the guest has to know that information.
@@ -450,14 +469,17 @@ struct vfio_iommu_type1_dma_unmap {
* addresses too so the window works as a filter rather than an offset
* for IOVA addresses.
*
- * A flag will need to be added if other page sizes are supported,
- * so as defined here, it is always 4k.
+ * Flags supported:
+ * - VFIO_IOMMU_SPAPR_INFO_DDW: informs the userspace that dynamic DMA windows
+ * (DDW) support is present. @ddw is only supported when DDW is present.
*/
struct vfio_iommu_spapr_tce_info {
__u32 argsz;
- __u32 flags; /* reserved for future use */
+ __u32 flags;
+#define VFIO_IOMMU_SPAPR_INFO_DDW (1 << 0) /* DDW supported */
__u32 dma32_window_start; /* 32 bit window start (bytes) */
__u32 dma32_window_size; /* 32 bit window size (bytes) */
+ struct vfio_iommu_spapr_tce_ddw_info ddw;
};
#define VFIO_IOMMU_SPAPR_TCE_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 12)
@@ -493,6 +515,66 @@ struct vfio_eeh_pe_op {
#define VFIO_EEH_PE_OP _IO(VFIO_TYPE, VFIO_BASE + 21)
+/**
+ * VFIO_IOMMU_SPAPR_REGISTER_MEMORY - _IOW(VFIO_TYPE, VFIO_BASE + 17, struct vfio_iommu_spapr_register_memory)
+ *
+ * Registers user space memory where DMA is allowed. It pins
+ * user pages and does the locked memory accounting so
+ * subsequent VFIO_IOMMU_MAP_DMA/VFIO_IOMMU_UNMAP_DMA calls
+ * get faster.
+ */
+struct vfio_iommu_spapr_register_memory {
+ __u32 argsz;
+ __u32 flags;
+ __u64 vaddr; /* Process virtual address */
+ __u64 size; /* Size of mapping (bytes) */
+};
+#define VFIO_IOMMU_SPAPR_REGISTER_MEMORY _IO(VFIO_TYPE, VFIO_BASE + 17)
+
+/**
+ * VFIO_IOMMU_SPAPR_UNREGISTER_MEMORY - _IOW(VFIO_TYPE, VFIO_BASE + 18, struct vfio_iommu_spapr_register_memory)
+ *
+ * Unregisters user space memory registered with
+ * VFIO_IOMMU_SPAPR_REGISTER_MEMORY.
+ * Uses vfio_iommu_spapr_register_memory for parameters.
+ */
+#define VFIO_IOMMU_SPAPR_UNREGISTER_MEMORY _IO(VFIO_TYPE, VFIO_BASE + 18)
+
+/**
+ * VFIO_IOMMU_SPAPR_TCE_CREATE - _IOWR(VFIO_TYPE, VFIO_BASE + 19, struct vfio_iommu_spapr_tce_create)
+ *
+ * Creates an additional TCE table and programs it (sets a new DMA window)
+ * to every IOMMU group in the container. It receives page shift, window
+ * size and number of levels in the TCE table being created.
+ *
+ * It allocates and returns an offset on a PCI bus of the new DMA window.
+ */
+struct vfio_iommu_spapr_tce_create {
+ __u32 argsz;
+ __u32 flags;
+ /* in */
+ __u32 page_shift;
+ __u64 window_size;
+ __u32 levels;
+ /* out */
+ __u64 start_addr;
+};
+#define VFIO_IOMMU_SPAPR_TCE_CREATE _IO(VFIO_TYPE, VFIO_BASE + 19)
+
+/**
+ * VFIO_IOMMU_SPAPR_TCE_REMOVE - _IOW(VFIO_TYPE, VFIO_BASE + 20, struct vfio_iommu_spapr_tce_remove)
+ *
+ * Unprograms a TCE table from all groups in the container and destroys it.
+ * It receives a PCI bus offset as a window id.
+ */
+struct vfio_iommu_spapr_tce_remove {
+ __u32 argsz;
+ __u32 flags;
+ /* in */
+ __u64 start_addr;
+};
+#define VFIO_IOMMU_SPAPR_TCE_REMOVE _IO(VFIO_TYPE, VFIO_BASE + 20)
+
/* ***************************************************************** */
#endif /* VFIO_H */
--
2.0.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH qemu v5 02/12] vmstate: Define VARRAY with VMS_ALLOC
2015-03-31 5:28 [Qemu-devel] [PATCH qemu v5 00/12] spapr: vfio: Enable Dynamic DMA windows (DDW) Alexey Kardashevskiy
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 01/12] linux headers update for DDW on SPAPR Alexey Kardashevskiy
@ 2015-03-31 5:28 ` Alexey Kardashevskiy
2015-04-08 1:55 ` David Gibson
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 03/12] spapr_pci: Make find_phb()/find_dev() public Alexey Kardashevskiy
` (9 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Alexey Kardashevskiy @ 2015-03-31 5:28 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Kardashevskiy, Alex Williamson, qemu-ppc, Alexander Graf,
David Gibson
This allows dynamic allocation for migrating arrays.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
include/migration/vmstate.h | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index bc7616a..73b9d67 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -299,6 +299,16 @@ extern const VMStateInfo vmstate_info_bitmap;
.offset = vmstate_offset_pointer(_state, _field, _type), \
}
+#define VMSTATE_VARRAY_UINT32_ALLOC(_field, _state, _field_num, _version, _info, _type) {\
+ .name = (stringify(_field)), \
+ .version_id = (_version), \
+ .num_offset = vmstate_offset_value(_state, _field_num, uint32_t),\
+ .info = &(_info), \
+ .size = sizeof(_type), \
+ .flags = VMS_VARRAY_UINT32|VMS_POINTER|VMS_ALLOC, \
+ .offset = vmstate_offset_pointer(_state, _field, _type), \
+}
+
#define VMSTATE_VARRAY_UINT16_UNSAFE(_field, _state, _field_num, _version, _info, _type) {\
.name = (stringify(_field)), \
.version_id = (_version), \
--
2.0.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH qemu v5 03/12] spapr_pci: Make find_phb()/find_dev() public
2015-03-31 5:28 [Qemu-devel] [PATCH qemu v5 00/12] spapr: vfio: Enable Dynamic DMA windows (DDW) Alexey Kardashevskiy
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 01/12] linux headers update for DDW on SPAPR Alexey Kardashevskiy
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 02/12] vmstate: Define VARRAY with VMS_ALLOC Alexey Kardashevskiy
@ 2015-03-31 5:28 ` Alexey Kardashevskiy
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 04/12] spapr_pci_vfio: Enable multiple groups per container Alexey Kardashevskiy
` (8 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Alexey Kardashevskiy @ 2015-03-31 5:28 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Kardashevskiy, Alex Williamson, qemu-ppc, Alexander Graf,
David Gibson
This makes find_phb()/find_dev() public and changed its names
to spapr_pci_find_phb()/spapr_pci_find_dev() as they are going to
be used from other parts of QEMU such as VFIO DDW (dynamic DMA window)
or VFIO PCI error injection or VFIO EEH handling - in all these
cases there are RTAS calls which are addressed to BUID+config_addr
in IEEE1275 format.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/ppc/spapr_pci.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 609a8ae..52c5c73 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -426,7 +426,7 @@ static void rtas_ibm_set_eeh_option(PowerPCCPU *cpu,
addr = rtas_ld(args, 0);
option = rtas_ld(args, 3);
- sphb = find_phb(spapr, buid);
+ sphb = spapr_pci_find_phb(spapr, buid);
if (!sphb) {
goto param_error_exit;
}
@@ -461,7 +461,7 @@ static void rtas_ibm_get_config_addr_info2(PowerPCCPU *cpu,
}
buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
- sphb = find_phb(spapr, buid);
+ sphb = spapr_pci_find_phb(spapr, buid);
if (!sphb) {
goto param_error_exit;
}
@@ -479,7 +479,7 @@ static void rtas_ibm_get_config_addr_info2(PowerPCCPU *cpu,
switch (option) {
case RTAS_GET_PE_ADDR:
addr = rtas_ld(args, 0);
- pdev = find_dev(spapr, buid, addr);
+ pdev = spapr_pci_find_dev(spapr, buid, addr);
if (!pdev) {
goto param_error_exit;
}
@@ -516,7 +516,7 @@ static void rtas_ibm_read_slot_reset_state2(PowerPCCPU *cpu,
}
buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
- sphb = find_phb(spapr, buid);
+ sphb = spapr_pci_find_phb(spapr, buid);
if (!sphb) {
goto param_error_exit;
}
@@ -562,7 +562,7 @@ static void rtas_ibm_set_slot_reset(PowerPCCPU *cpu,
buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
option = rtas_ld(args, 3);
- sphb = find_phb(spapr, buid);
+ sphb = spapr_pci_find_phb(spapr, buid);
if (!sphb) {
goto param_error_exit;
}
@@ -596,7 +596,7 @@ static void rtas_ibm_configure_pe(PowerPCCPU *cpu,
}
buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
- sphb = find_phb(spapr, buid);
+ sphb = spapr_pci_find_phb(spapr, buid);
if (!sphb) {
goto param_error_exit;
}
@@ -631,7 +631,7 @@ static void rtas_ibm_slot_error_detail(PowerPCCPU *cpu,
}
buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
- sphb = find_phb(spapr, buid);
+ sphb = spapr_pci_find_phb(spapr, buid);
if (!sphb) {
goto param_error_exit;
}
--
2.0.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH qemu v5 04/12] spapr_pci_vfio: Enable multiple groups per container
2015-03-31 5:28 [Qemu-devel] [PATCH qemu v5 00/12] spapr: vfio: Enable Dynamic DMA windows (DDW) Alexey Kardashevskiy
` (2 preceding siblings ...)
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 03/12] spapr_pci: Make find_phb()/find_dev() public Alexey Kardashevskiy
@ 2015-03-31 5:28 ` Alexey Kardashevskiy
2015-04-08 2:01 ` David Gibson
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 05/12] vfio: spapr: Move SPAPR-related code to a separate file Alexey Kardashevskiy
` (7 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Alexey Kardashevskiy @ 2015-03-31 5:28 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Kardashevskiy, Alex Williamson, qemu-ppc, Alexander Graf,
David Gibson
This enables multiple IOMMU groups in one VFIO container which means
that multiple devices from different groups can share the same IOMMU
table (or tables if DDW).
This removes a group id from vfio_container_ioctl(). The kernel support
is required for this; if the host kernel does not have the support,
it will allow only one group per container. The PHB's "iommuid" property
is ignored.
This adds a sanity check that there is just one VFIO container per
PHB address space.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
hw/ppc/spapr_pci_vfio.c | 17 ++++++-----------
hw/vfio/common.c | 28 ++++++++++++++--------------
include/hw/vfio/vfio.h | 2 +-
3 files changed, 21 insertions(+), 26 deletions(-)
diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
index 99a1be5..f8b503e 100644
--- a/hw/ppc/spapr_pci_vfio.c
+++ b/hw/ppc/spapr_pci_vfio.c
@@ -35,12 +35,7 @@ static void spapr_phb_vfio_finish_realize(sPAPRPHBState *sphb, Error **errp)
sPAPRTCETable *tcet;
uint32_t liobn = svphb->phb.dma_liobn;
- if (svphb->iommugroupid == -1) {
- error_setg(errp, "Wrong IOMMU group ID %d", svphb->iommugroupid);
- return;
- }
-
- ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
+ ret = vfio_container_ioctl(&svphb->phb.iommu_as,
VFIO_CHECK_EXTENSION,
(void *) VFIO_SPAPR_TCE_IOMMU);
if (ret != 1) {
@@ -49,7 +44,7 @@ static void spapr_phb_vfio_finish_realize(sPAPRPHBState *sphb, Error **errp)
return;
}
- ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
+ ret = vfio_container_ioctl(&sphb->iommu_as,
VFIO_IOMMU_SPAPR_TCE_GET_INFO, &info);
if (ret) {
error_setg_errno(errp, -ret,
@@ -116,7 +111,7 @@ static int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
return RTAS_OUT_PARAM_ERROR;
}
- ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
+ ret = vfio_container_ioctl(&svphb->phb.iommu_as,
VFIO_EEH_PE_OP, &op);
if (ret < 0) {
return RTAS_OUT_HW_ERROR;
@@ -132,7 +127,7 @@ static int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state)
int ret;
op.op = VFIO_EEH_PE_GET_STATE;
- ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
+ ret = vfio_container_ioctl(&svphb->phb.iommu_as,
VFIO_EEH_PE_OP, &op);
if (ret < 0) {
return RTAS_OUT_PARAM_ERROR;
@@ -162,7 +157,7 @@ static int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option)
return RTAS_OUT_PARAM_ERROR;
}
- ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
+ ret = vfio_container_ioctl(&svphb->phb.iommu_as,
VFIO_EEH_PE_OP, &op);
if (ret < 0) {
return RTAS_OUT_HW_ERROR;
@@ -178,7 +173,7 @@ static int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb)
int ret;
op.op = VFIO_EEH_PE_CONFIGURE;
- ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
+ ret = vfio_container_ioctl(&svphb->phb.iommu_as,
VFIO_EEH_PE_OP, &op);
if (ret < 0) {
return RTAS_OUT_PARAM_ERROR;
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index b012620..99e1900 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -915,21 +915,23 @@ void vfio_put_base_device(VFIODevice *vbasedev)
close(vbasedev->fd);
}
-static int vfio_container_do_ioctl(AddressSpace *as, int32_t groupid,
+static int vfio_container_do_ioctl(AddressSpace *as,
int req, void *param)
{
- VFIOGroup *group;
VFIOContainer *container;
- int ret = -1;
+ int ret;
+ VFIOAddressSpace *space;
- group = vfio_get_group(groupid, as);
- if (!group) {
- error_report("vfio: group %d not registered", groupid);
- return ret;
- }
+ space = vfio_get_address_space(as);
+ container = QLIST_FIRST(&space->containers);
- container = group->container;
- if (group->container) {
+ if (!container) {
+ error_report("vfio: container is not set");
+ return -1;
+ } else if (QLIST_NEXT(container, next)) {
+ error_report("vfio: multiple containers per PHB are not supported");
+ return -1;
+ } else {
ret = ioctl(container->fd, req, param);
if (ret < 0) {
error_report("vfio: failed to ioctl %d to container: ret=%d, %s",
@@ -937,12 +939,10 @@ static int vfio_container_do_ioctl(AddressSpace *as, int32_t groupid,
}
}
- vfio_put_group(group);
-
return ret;
}
-int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
+int vfio_container_ioctl(AddressSpace *as,
int req, void *param)
{
/* We allow only certain ioctls to the container */
@@ -957,5 +957,5 @@ int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
return -1;
}
- return vfio_container_do_ioctl(as, groupid, req, param);
+ return vfio_container_do_ioctl(as, req, param);
}
diff --git a/include/hw/vfio/vfio.h b/include/hw/vfio/vfio.h
index 0b26cd8..76b5744 100644
--- a/include/hw/vfio/vfio.h
+++ b/include/hw/vfio/vfio.h
@@ -3,7 +3,7 @@
#include "qemu/typedefs.h"
-extern int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
+extern int vfio_container_ioctl(AddressSpace *as,
int req, void *param);
#endif
--
2.0.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH qemu v5 05/12] vfio: spapr: Move SPAPR-related code to a separate file
2015-03-31 5:28 [Qemu-devel] [PATCH qemu v5 00/12] spapr: vfio: Enable Dynamic DMA windows (DDW) Alexey Kardashevskiy
` (3 preceding siblings ...)
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 04/12] spapr_pci_vfio: Enable multiple groups per container Alexey Kardashevskiy
@ 2015-03-31 5:28 ` Alexey Kardashevskiy
2015-04-08 2:05 ` David Gibson
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 06/12] vfio: spapr: Add SPAPR IOMMU v2 support (DMA memory preregistering) Alexey Kardashevskiy
` (6 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Alexey Kardashevskiy @ 2015-03-31 5:28 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Kardashevskiy, Alex Williamson, qemu-ppc, Alexander Graf,
David Gibson
This moves SPAPR bits to a separate file to avoid pollution of x86 code.
This is a mechanical patch.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
hw/vfio/Makefile.objs | 1 +
hw/vfio/common.c | 134 ++-----------------------
hw/vfio/spapr.c | 226 ++++++++++++++++++++++++++++++++++++++++++
include/hw/vfio/vfio-common.h | 13 +++
4 files changed, 246 insertions(+), 128 deletions(-)
create mode 100644 hw/vfio/spapr.c
diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs
index e31f30e..b987ffb 100644
--- a/hw/vfio/Makefile.objs
+++ b/hw/vfio/Makefile.objs
@@ -1,4 +1,5 @@
ifeq ($(CONFIG_LINUX), y)
obj-$(CONFIG_SOFTMMU) += common.o
obj-$(CONFIG_PCI) += pci.o
+obj-$(CONFIG_SOFTMMU) += spapr.o
endif
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 99e1900..a71f881 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -190,8 +190,8 @@ const MemoryRegionOps vfio_region_ops = {
/*
* DMA - Mapping and unmapping for the "type1" IOMMU interface used on x86
*/
-static int vfio_dma_unmap(VFIOContainer *container,
- hwaddr iova, ram_addr_t size)
+int vfio_dma_unmap(VFIOContainer *container,
+ hwaddr iova, ram_addr_t size)
{
struct vfio_iommu_type1_dma_unmap unmap = {
.argsz = sizeof(unmap),
@@ -208,8 +208,8 @@ static int vfio_dma_unmap(VFIOContainer *container,
return 0;
}
-static int vfio_dma_map(VFIOContainer *container, hwaddr iova,
- ram_addr_t size, void *vaddr, bool readonly)
+int vfio_dma_map(VFIOContainer *container, hwaddr iova,
+ ram_addr_t size, void *vaddr, bool readonly)
{
struct vfio_iommu_type1_dma_map map = {
.argsz = sizeof(map),
@@ -238,7 +238,7 @@ static int vfio_dma_map(VFIOContainer *container, hwaddr iova,
return -errno;
}
-static bool vfio_listener_skipped_section(MemoryRegionSection *section)
+bool vfio_listener_skipped_section(MemoryRegionSection *section)
{
return (!memory_region_is_ram(section->mr) &&
!memory_region_is_iommu(section->mr)) ||
@@ -251,64 +251,6 @@ static bool vfio_listener_skipped_section(MemoryRegionSection *section)
section->offset_within_address_space & (1ULL << 63);
}
-static void vfio_iommu_map_notify(Notifier *n, void *data)
-{
- VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n);
- VFIOContainer *container = giommu->container;
- IOMMUTLBEntry *iotlb = data;
- MemoryRegion *mr;
- hwaddr xlat;
- hwaddr len = iotlb->addr_mask + 1;
- void *vaddr;
- int ret;
-
- trace_vfio_iommu_map_notify(iotlb->iova,
- iotlb->iova + iotlb->addr_mask);
-
- /*
- * The IOMMU TLB entry we have just covers translation through
- * this IOMMU to its immediate target. We need to translate
- * it the rest of the way through to memory.
- */
- mr = address_space_translate(&address_space_memory,
- iotlb->translated_addr,
- &xlat, &len, iotlb->perm & IOMMU_WO);
- if (!memory_region_is_ram(mr)) {
- error_report("iommu map to non memory area %"HWADDR_PRIx"",
- xlat);
- return;
- }
- /*
- * Translation truncates length to the IOMMU page size,
- * check that it did not truncate too much.
- */
- if (len & iotlb->addr_mask) {
- error_report("iommu has granularity incompatible with target AS");
- return;
- }
-
- if ((iotlb->perm & IOMMU_RW) != IOMMU_NONE) {
- vaddr = memory_region_get_ram_ptr(mr) + xlat;
- ret = vfio_dma_map(container, iotlb->iova,
- iotlb->addr_mask + 1, vaddr,
- !(iotlb->perm & IOMMU_WO) || mr->readonly);
- if (ret) {
- error_report("vfio_dma_map(%p, 0x%"HWADDR_PRIx", "
- "0x%"HWADDR_PRIx", %p) = %d (%m)",
- container, iotlb->iova,
- iotlb->addr_mask + 1, vaddr, ret);
- }
- } else {
- ret = vfio_dma_unmap(container, iotlb->iova, iotlb->addr_mask + 1);
- if (ret) {
- error_report("vfio_dma_unmap(%p, 0x%"HWADDR_PRIx", "
- "0x%"HWADDR_PRIx") = %d (%m)",
- container, iotlb->iova,
- iotlb->addr_mask + 1, ret);
- }
- }
-}
-
static void vfio_listener_region_add(MemoryListener *listener,
MemoryRegionSection *section)
{
@@ -344,45 +286,6 @@ static void vfio_listener_region_add(MemoryListener *listener,
memory_region_ref(section->mr);
- if (memory_region_is_iommu(section->mr)) {
- VFIOGuestIOMMU *giommu;
-
- trace_vfio_listener_region_add_iommu(iova,
- int128_get64(int128_sub(llend, int128_one())));
- /*
- * FIXME: We should do some checking to see if the
- * capabilities of the host VFIO IOMMU are adequate to model
- * the guest IOMMU
- *
- * FIXME: For VFIO iommu types which have KVM acceleration to
- * avoid bouncing all map/unmaps through qemu this way, this
- * would be the right place to wire that up (tell the KVM
- * device emulation the VFIO iommu handles to use).
- */
- /*
- * This assumes that the guest IOMMU is empty of
- * mappings at this point.
- *
- * One way of doing this is:
- * 1. Avoid sharing IOMMUs between emulated devices or different
- * IOMMU groups.
- * 2. Implement VFIO_IOMMU_ENABLE in the host kernel to fail if
- * there are some mappings in IOMMU.
- *
- * VFIO on SPAPR does that. Other IOMMU models may do that different,
- * they must make sure there are no existing mappings or
- * loop through existing mappings to map them into VFIO.
- */
- giommu = g_malloc0(sizeof(*giommu));
- giommu->iommu = section->mr;
- giommu->container = container;
- giommu->n.notify = vfio_iommu_map_notify;
- QLIST_INSERT_HEAD(&container->giommu_list, giommu, giommu_next);
- memory_region_register_iommu_notifier(giommu->iommu, &giommu->n);
-
- return;
- }
-
/* Here we assume that memory_region_is_ram(section->mr)==true */
end = int128_get64(llend);
@@ -435,27 +338,6 @@ static void vfio_listener_region_del(MemoryListener *listener,
return;
}
- if (memory_region_is_iommu(section->mr)) {
- VFIOGuestIOMMU *giommu;
-
- QLIST_FOREACH(giommu, &container->giommu_list, giommu_next) {
- if (giommu->iommu == section->mr) {
- memory_region_unregister_iommu_notifier(&giommu->n);
- QLIST_REMOVE(giommu, giommu_next);
- g_free(giommu);
- break;
- }
- }
-
- /*
- * FIXME: We assume the one big unmap below is adequate to
- * remove any individual page mappings in the IOMMU which
- * might have been copied into VFIO. This works for a page table
- * based IOMMU where a big unmap flattens a large range of IO-PTEs.
- * That may not be true for all IOMMU types.
- */
- }
-
iova = TARGET_PAGE_ALIGN(section->offset_within_address_space);
end = (section->offset_within_address_space + int128_get64(section->size)) &
TARGET_PAGE_MASK;
@@ -721,11 +603,7 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as)
goto free_container_exit;
}
- container->iommu_data.type1.listener = vfio_memory_listener;
- container->iommu_data.release = vfio_listener_release;
-
- memory_listener_register(&container->iommu_data.type1.listener,
- container->space->as);
+ spapr_memory_listener_register(container);
} else {
error_report("vfio: No available IOMMU models");
diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
new file mode 100644
index 0000000..5f79194
--- /dev/null
+++ b/hw/vfio/spapr.c
@@ -0,0 +1,226 @@
+/*
+ * QEMU sPAPR VFIO IOMMU
+ *
+ * Copyright (c) 2015 Alexey Kardashevskiy, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License,
+ * or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "hw/vfio/vfio-common.h"
+#include "qemu/error-report.h"
+#include "trace.h"
+
+static void vfio_iommu_map_notify(Notifier *n, void *data)
+{
+ VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n);
+ VFIOContainer *container = giommu->container;
+ IOMMUTLBEntry *iotlb = data;
+ MemoryRegion *mr;
+ hwaddr xlat;
+ hwaddr len = iotlb->addr_mask + 1;
+ void *vaddr;
+ int ret;
+
+ trace_vfio_iommu_map_notify(iotlb->iova,
+ iotlb->iova + iotlb->addr_mask);
+
+ /*
+ * The IOMMU TLB entry we have just covers translation through
+ * this IOMMU to its immediate target. We need to translate
+ * it the rest of the way through to memory.
+ */
+ mr = address_space_translate(&address_space_memory,
+ iotlb->translated_addr,
+ &xlat, &len, iotlb->perm & IOMMU_WO);
+ if (!memory_region_is_ram(mr)) {
+ error_report("iommu map to non memory area %"HWADDR_PRIx,
+ xlat);
+ return;
+ }
+ /*
+ * Translation truncates length to the IOMMU page size,
+ * check that it did not truncate too much.
+ */
+ if (len & iotlb->addr_mask) {
+ error_report("iommu has granularity incompatible with target AS");
+ return;
+ }
+
+ if ((iotlb->perm & IOMMU_RW) != IOMMU_NONE) {
+ vaddr = memory_region_get_ram_ptr(mr) + xlat;
+ ret = vfio_dma_map(container, iotlb->iova,
+ iotlb->addr_mask + 1, vaddr,
+ !(iotlb->perm & IOMMU_WO) || mr->readonly);
+ if (ret) {
+ error_report("vfio_dma_map(%p, 0x%"HWADDR_PRIx", "
+ "0x%"HWADDR_PRIx", %p) = %d (%m)",
+ container, iotlb->iova,
+ iotlb->addr_mask + 1, vaddr, ret);
+ }
+ } else {
+ ret = vfio_dma_unmap(container, iotlb->iova, iotlb->addr_mask + 1);
+ if (ret) {
+ error_report("vfio_dma_unmap(%p, 0x%"HWADDR_PRIx", "
+ "0x%"HWADDR_PRIx") = %d (%m)",
+ container, iotlb->iova,
+ iotlb->addr_mask + 1, ret);
+ }
+ }
+}
+
+static void vfio_spapr_listener_region_add(MemoryListener *listener,
+ MemoryRegionSection *section)
+{
+ VFIOContainer *container = container_of(listener, VFIOContainer,
+ iommu_data.spapr.listener);
+ hwaddr iova;
+ Int128 llend;
+ VFIOGuestIOMMU *giommu;
+
+ if (vfio_listener_skipped_section(section)) {
+ trace_vfio_listener_region_add_skip(
+ section->offset_within_address_space,
+ section->offset_within_address_space +
+ int128_get64(int128_sub(section->size, int128_one())));
+ return;
+ }
+
+ if (unlikely((section->offset_within_address_space & ~TARGET_PAGE_MASK) !=
+ (section->offset_within_region & ~TARGET_PAGE_MASK))) {
+ error_report("%s received unaligned region", __func__);
+ return;
+ }
+
+ iova = TARGET_PAGE_ALIGN(section->offset_within_address_space);
+ llend = int128_make64(section->offset_within_address_space);
+ llend = int128_add(llend, section->size);
+ llend = int128_and(llend, int128_exts64(TARGET_PAGE_MASK));
+
+ if (int128_ge(int128_make64(iova), llend)) {
+ return;
+ }
+
+ memory_region_ref(section->mr);
+
+ trace_vfio_listener_region_add_iommu(iova,
+ int128_get64(int128_sub(llend, int128_one())));
+ /*
+ * FIXME: We should do some checking to see if the
+ * capabilities of the host VFIO IOMMU are adequate to model
+ * the guest IOMMU
+ *
+ * FIXME: For VFIO iommu types which have KVM acceleration to
+ * avoid bouncing all map/unmaps through qemu this way, this
+ * would be the right place to wire that up (tell the KVM
+ * device emulation the VFIO iommu handles to use).
+ */
+ /*
+ * This assumes that the guest IOMMU is empty of
+ * mappings at this point.
+ *
+ * One way of doing this is:
+ * 1. Avoid sharing IOMMUs between emulated devices or different
+ * IOMMU groups.
+ * 2. Implement VFIO_IOMMU_ENABLE in the host kernel to fail if
+ * there are some mappings in IOMMU.
+ *
+ * VFIO on SPAPR does that. Other IOMMU models may do that different,
+ * they must make sure there are no existing mappings or
+ * loop through existing mappings to map them into VFIO.
+ */
+ giommu = g_malloc0(sizeof(*giommu));
+ giommu->iommu = section->mr;
+ giommu->container = container;
+ giommu->n.notify = vfio_iommu_map_notify;
+ QLIST_INSERT_HEAD(&container->giommu_list, giommu, giommu_next);
+ memory_region_register_iommu_notifier(giommu->iommu, &giommu->n);
+}
+
+static void vfio_spapr_listener_region_del(MemoryListener *listener,
+ MemoryRegionSection *section)
+{
+ VFIOContainer *container = container_of(listener, VFIOContainer,
+ iommu_data.spapr.listener);
+ hwaddr iova, end;
+ int ret;
+ VFIOGuestIOMMU *giommu;
+
+ if (vfio_listener_skipped_section(section)) {
+ trace_vfio_listener_region_del_skip(
+ section->offset_within_address_space,
+ section->offset_within_address_space +
+ int128_get64(int128_sub(section->size, int128_one())));
+ return;
+ }
+
+ if (unlikely((section->offset_within_address_space & ~TARGET_PAGE_MASK) !=
+ (section->offset_within_region & ~TARGET_PAGE_MASK))) {
+ error_report("%s received unaligned region", __func__);
+ return;
+ }
+
+ QLIST_FOREACH(giommu, &container->giommu_list, giommu_next) {
+ if (giommu->iommu == section->mr) {
+ memory_region_unregister_iommu_notifier(&giommu->n);
+ QLIST_REMOVE(giommu, giommu_next);
+ g_free(giommu);
+ break;
+ }
+ }
+
+ /*
+ * FIXME: We assume the one big unmap below is adequate to
+ * remove any individual page mappings in the IOMMU which
+ * might have been copied into VFIO. This works for a page table
+ * based IOMMU where a big unmap flattens a large range of IO-PTEs.
+ * That may not be true for all IOMMU types.
+ */
+
+ iova = TARGET_PAGE_ALIGN(section->offset_within_address_space);
+ end = (section->offset_within_address_space + int128_get64(section->size)) &
+ TARGET_PAGE_MASK;
+
+ if (iova >= end) {
+ return;
+ }
+
+ trace_vfio_listener_region_del(iova, end - 1);
+
+ ret = vfio_dma_unmap(container, iova, end - iova);
+ memory_region_unref(section->mr);
+ if (ret) {
+ error_report("vfio_dma_unmap(%p, 0x%"HWADDR_PRIx", "
+ "0x%"HWADDR_PRIx") = %d (%m)",
+ container, iova, end - iova, ret);
+ }
+}
+
+static const MemoryListener vfio_spapr_memory_listener = {
+ .region_add = vfio_spapr_listener_region_add,
+ .region_del = vfio_spapr_listener_region_del,
+};
+
+static void vfio_spapr_listener_release(VFIOContainer *container)
+{
+ memory_listener_unregister(&container->iommu_data.spapr.listener);
+}
+
+void spapr_memory_listener_register(VFIOContainer *container)
+{
+ container->iommu_data.spapr.listener = vfio_spapr_memory_listener;
+ container->iommu_data.release = vfio_spapr_listener_release;
+
+ memory_listener_register(&container->iommu_data.spapr.listener,
+ container->space->as);
+}
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index cfdfb6a..d0b831c 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -69,6 +69,10 @@ typedef struct VFIOType1 {
bool initialized;
} VFIOType1;
+typedef struct VFIOSPAPR {
+ MemoryListener listener;
+} VFIOSPAPR;
+
typedef struct VFIOContainer {
VFIOAddressSpace *space;
int fd; /* /dev/vfio/vfio, empowered by the attached groups */
@@ -76,6 +80,7 @@ typedef struct VFIOContainer {
/* enable abstraction to support various iommu backends */
union {
VFIOType1 type1;
+ VFIOSPAPR spapr;
};
void (*release)(struct VFIOContainer *);
} iommu_data;
@@ -145,4 +150,12 @@ extern const MemoryRegionOps vfio_region_ops;
extern QLIST_HEAD(vfio_group_head, VFIOGroup) vfio_group_list;
extern QLIST_HEAD(vfio_as_head, VFIOAddressSpace) vfio_address_spaces;
+extern int vfio_dma_map(VFIOContainer *container, hwaddr iova,
+ ram_addr_t size, void *vaddr, bool readonly);
+extern int vfio_dma_unmap(VFIOContainer *container,
+ hwaddr iova, ram_addr_t size);
+bool vfio_listener_skipped_section(MemoryRegionSection *section);
+
+extern void spapr_memory_listener_register(VFIOContainer *container);
+
#endif /* !HW_VFIO_VFIO_COMMON_H */
--
2.0.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH qemu v5 06/12] vfio: spapr: Add SPAPR IOMMU v2 support (DMA memory preregistering)
2015-03-31 5:28 [Qemu-devel] [PATCH qemu v5 00/12] spapr: vfio: Enable Dynamic DMA windows (DDW) Alexey Kardashevskiy
` (4 preceding siblings ...)
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 05/12] vfio: spapr: Move SPAPR-related code to a separate file Alexey Kardashevskiy
@ 2015-03-31 5:28 ` Alexey Kardashevskiy
2015-04-08 2:15 ` David Gibson
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 07/12] spapr_iommu: Rework TCE table initialization Alexey Kardashevskiy
` (5 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Alexey Kardashevskiy @ 2015-03-31 5:28 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Kardashevskiy, Alex Williamson, qemu-ppc, Alexander Graf,
David Gibson
This makes use of the new "memory registering" feature. The idea is
to provide the guest ability to notify the host kernel about pages which
are going to be used for DMA. Having this information, the host kernel
can pin them all once per user process, do locked pages accounting (once)
and not spent time on doing that in real time with possible failures
which cannot be handled nicely in some cases.
This adds a guest RAM memory listener which notifies a VFIO container
about memory which needs to be pinned/unpinned. VFIO MMIO regions
(i.e. "skip dump" regions) are skipped.
The feature is only enabled for SPAPR IOMMU v2. The host kernel changes
are required. Since v2 does not need/support VFIO_IOMMU_ENABLE, this does
not call it when v2 is detected and enabled.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v5:
* simplified the patch
* added trace points
* added round_up() for the size
* SPAPR IOMMU v2 used
---
hw/vfio/common.c | 26 +++++++++-----
hw/vfio/spapr.c | 79 +++++++++++++++++++++++++++++++++++++++++--
include/hw/vfio/vfio-common.h | 5 ++-
trace-events | 1 +
4 files changed, 100 insertions(+), 11 deletions(-)
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index a71f881..e35e478 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -577,14 +577,18 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as)
container->iommu_data.type1.initialized = true;
- } else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU)) {
+ } else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU) ||
+ ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_v2_IOMMU)) {
+ bool v2 = !!ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_v2_IOMMU);
+
ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, &fd);
if (ret) {
error_report("vfio: failed to set group container: %m");
ret = -errno;
goto free_container_exit;
}
- ret = ioctl(fd, VFIO_SET_IOMMU, VFIO_SPAPR_TCE_IOMMU);
+ ret = ioctl(fd, VFIO_SET_IOMMU,
+ v2 ? VFIO_SPAPR_TCE_v2_IOMMU : VFIO_SPAPR_TCE_IOMMU);
if (ret) {
error_report("vfio: failed to set iommu for container: %m");
ret = -errno;
@@ -596,14 +600,20 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as)
* when container fd is closed so we do not call it explicitly
* in this file.
*/
- ret = ioctl(fd, VFIO_IOMMU_ENABLE);
- if (ret) {
- error_report("vfio: failed to enable container: %m");
- ret = -errno;
- goto free_container_exit;
+ if (!v2) {
+ ret = ioctl(fd, VFIO_IOMMU_ENABLE);
+ if (ret) {
+ error_report("vfio: failed to enable container: %m");
+ ret = -errno;
+ goto free_container_exit;
+ }
}
- spapr_memory_listener_register(container);
+ ret = spapr_memory_listener_register(container, v2 ? 2 : 1);
+ if (ret) {
+ error_report("vfio: RAM memory listener initialization failed for container");
+ goto listener_release_exit;
+ }
} else {
error_report("vfio: No available IOMMU models");
diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
index 5f79194..a670907 100644
--- a/hw/vfio/spapr.c
+++ b/hw/vfio/spapr.c
@@ -17,6 +17,9 @@
* along with this program; if not, see <http://www.gnu.org/licenses/>.
*/
+#include <sys/ioctl.h>
+#include <linux/vfio.h>
+
#include "hw/vfio/vfio-common.h"
#include "qemu/error-report.h"
#include "trace.h"
@@ -211,16 +214,88 @@ static const MemoryListener vfio_spapr_memory_listener = {
.region_del = vfio_spapr_listener_region_del,
};
+static void vfio_ram_do_region(VFIOContainer *container,
+ MemoryRegionSection *section, unsigned long req)
+{
+ int ret;
+ struct vfio_iommu_spapr_register_memory reg = { .argsz = sizeof(reg) };
+
+ if (!memory_region_is_ram(section->mr) ||
+ memory_region_is_skip_dump(section->mr)) {
+ return;
+ }
+
+ reg.vaddr = (__u64) memory_region_get_ram_ptr(section->mr) +
+ section->offset_within_region;
+ reg.size = ROUND_UP(int128_get64(section->size), TARGET_PAGE_SIZE);
+
+ ret = ioctl(container->fd, req, ®);
+ trace_vfio_ram_register(_IOC_NR(req) - VFIO_BASE, reg.vaddr, reg.size, ret);
+
+ /*
+ * On the initfn path, store the first error in the container so we
+ * can gracefully fail. Runtime, there's not much we can do other
+ * than throw a hardware error.
+ */
+ if (!container->iommu_data.spapr.ram_reg_initialized) {
+ if (!container->iommu_data.spapr.ram_reg_error) {
+ container->iommu_data.spapr.ram_reg_error = ret;
+ }
+ } else {
+ hw_error("vfio: RAM registering failed, unable to continue");
+ }
+}
+
+static void vfio_spapr_ram_listener_region_add(MemoryListener *listener,
+ MemoryRegionSection *section)
+{
+ VFIOContainer *container = container_of(listener, VFIOContainer,
+ iommu_data.spapr.ramlistener);
+ memory_region_ref(section->mr);
+ vfio_ram_do_region(container, section, VFIO_IOMMU_SPAPR_REGISTER_MEMORY);
+}
+
+static void vfio_spapr_ram_listener_region_del(MemoryListener *listener,
+ MemoryRegionSection *section)
+{
+ VFIOContainer *container = container_of(listener, VFIOContainer,
+ iommu_data.spapr.ramlistener);
+ memory_region_unref(section->mr);
+ vfio_ram_do_region(container, section, VFIO_IOMMU_SPAPR_UNREGISTER_MEMORY);
+}
+
+static const MemoryListener vfio_spapr_ram_memory_listener = {
+ .region_add = vfio_spapr_ram_listener_region_add,
+ .region_del = vfio_spapr_ram_listener_region_del,
+};
+
static void vfio_spapr_listener_release(VFIOContainer *container)
{
memory_listener_unregister(&container->iommu_data.spapr.listener);
}
-void spapr_memory_listener_register(VFIOContainer *container)
+static void vfio_spapr_listener_release_v2(VFIOContainer *container)
+{
+ memory_listener_unregister(&container->iommu_data.spapr.listener);
+ vfio_spapr_listener_release(container);
+}
+
+int spapr_memory_listener_register(VFIOContainer *container, int ver)
{
container->iommu_data.spapr.listener = vfio_spapr_memory_listener;
container->iommu_data.release = vfio_spapr_listener_release;
-
memory_listener_register(&container->iommu_data.spapr.listener,
container->space->as);
+ if (ver < 2) {
+ return 0;
+ }
+
+ container->iommu_data.spapr.ramlistener = vfio_spapr_ram_memory_listener;
+ container->iommu_data.release = vfio_spapr_listener_release_v2;
+ memory_listener_register(&container->iommu_data.spapr.ramlistener,
+ &address_space_memory);
+
+ container->iommu_data.spapr.ram_reg_initialized = true;
+
+ return container->iommu_data.spapr.ram_reg_error;
}
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index d0b831c..b5ef446 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -71,6 +71,9 @@ typedef struct VFIOType1 {
typedef struct VFIOSPAPR {
MemoryListener listener;
+ MemoryListener ramlistener;
+ int ram_reg_error;
+ bool ram_reg_initialized;
} VFIOSPAPR;
typedef struct VFIOContainer {
@@ -156,6 +159,6 @@ extern int vfio_dma_unmap(VFIOContainer *container,
hwaddr iova, ram_addr_t size);
bool vfio_listener_skipped_section(MemoryRegionSection *section);
-extern void spapr_memory_listener_register(VFIOContainer *container);
+extern int spapr_memory_listener_register(VFIOContainer *container, int ver);
#endif /* !HW_VFIO_VFIO_COMMON_H */
diff --git a/trace-events b/trace-events
index 1231ba4..2739140 100644
--- a/trace-events
+++ b/trace-events
@@ -1563,6 +1563,7 @@ vfio_disconnect_container(int fd) "close container->fd=%d"
vfio_put_group(int fd) "close group->fd=%d"
vfio_get_device(const char * name, unsigned int flags, unsigned int num_regions, unsigned int num_irqs) "Device %s flags: %u, regions: %u, irqs: %u"
vfio_put_base_device(int fd) "close vdev->fd=%d"
+vfio_ram_register(int req, uint64_t va, uint64_t size, int ret) "req=%d va=%"PRIx64" size=%"PRIx64" ret=%d"
#hw/acpi/memory_hotplug.c
mhp_acpi_invalid_slot_selected(uint32_t slot) "0x%"PRIx32
--
2.0.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH qemu v5 07/12] spapr_iommu: Rework TCE table initialization
2015-03-31 5:28 [Qemu-devel] [PATCH qemu v5 00/12] spapr: vfio: Enable Dynamic DMA windows (DDW) Alexey Kardashevskiy
` (5 preceding siblings ...)
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 06/12] vfio: spapr: Add SPAPR IOMMU v2 support (DMA memory preregistering) Alexey Kardashevskiy
@ 2015-03-31 5:28 ` Alexey Kardashevskiy
2015-04-08 2:35 ` David Gibson
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 08/12] spapr_pci: Rework reset to reset DMA configuration Alexey Kardashevskiy
` (4 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Alexey Kardashevskiy @ 2015-03-31 5:28 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Kardashevskiy, Alex Williamson, qemu-ppc, Alexander Graf,
David Gibson
Currently TCE tables are created once at start and their size never
changes. We are going to change that by introducing a Dynamic DMA windows
support where DMA configuration may change during the guest execution.
This changes spapr_tce_new_table() to create an empty stub object. Only
LIOBN is assigned by the time of creation. It still will be called once
at the owner object (VIO or PHB) creation.
This introduces spapr_tce_set_props() to set the table size, start and
page size. It only assigns the properties. It will be called at the owner
object creation OR later from the "ibm,create-pe-dma-window" RTAS handler
so the table's parameters can change.
This introduces an "enabled" state for TCE table objects with two
helper functions - spapr_tce_table_enable()/spapr_tce_table_disable().
spapr_tce_table_enable() allocates the guest view of the TCE table
(in the user space or KVM). spapr_tce_table_disable() disposes the table.
Follow up patches will disable+enable tables on reset (system reset
or DDW reset).
No visible change in behaviour is expected except the actual table
will be reallocated every reset. We might optimize this later.
The other way to implement this would be dynamically create/remove
the TCE table QOM objects but this would make migration impossible
as migration expects all QOM objects to exist at the receiver
so we have to have TCE table objects created when migration begins.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
hw/ppc/spapr_iommu.c | 98 +++++++++++++++++++++++++++++++------------------
hw/ppc/spapr_pci.c | 8 ++--
hw/ppc/spapr_pci_vfio.c | 11 ++++--
hw/ppc/spapr_vio.c | 10 ++---
include/hw/ppc/spapr.h | 12 +++---
5 files changed, 87 insertions(+), 52 deletions(-)
diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
index a14cdc4..a015357 100644
--- a/hw/ppc/spapr_iommu.c
+++ b/hw/ppc/spapr_iommu.c
@@ -126,25 +126,6 @@ static MemoryRegionIOMMUOps spapr_iommu_ops = {
static int spapr_tce_table_realize(DeviceState *dev)
{
sPAPRTCETable *tcet = SPAPR_TCE_TABLE(dev);
- uint64_t window_size = (uint64_t)tcet->nb_table << tcet->page_shift;
-
- if (kvm_enabled() && !(window_size >> 32)) {
- tcet->table = kvmppc_create_spapr_tce(tcet->liobn,
- window_size,
- &tcet->fd,
- tcet->vfio_accel);
- }
-
- if (!tcet->table) {
- size_t table_size = tcet->nb_table * sizeof(uint64_t);
- tcet->table = g_malloc0(table_size);
- }
-
- trace_spapr_iommu_new_table(tcet->liobn, tcet, tcet->table, tcet->fd);
-
- memory_region_init_iommu(&tcet->iommu, OBJECT(dev), &spapr_iommu_ops,
- "iommu-spapr",
- (uint64_t)tcet->nb_table << tcet->page_shift);
QLIST_INSERT_HEAD(&spapr_tce_tables, tcet, list);
@@ -154,11 +135,7 @@ static int spapr_tce_table_realize(DeviceState *dev)
return 0;
}
-sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn,
- uint64_t bus_offset,
- uint32_t page_shift,
- uint32_t nb_table,
- bool vfio_accel)
+sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn)
{
sPAPRTCETable *tcet;
char tmp[64];
@@ -169,36 +146,87 @@ sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn,
return NULL;
}
- if (!nb_table) {
- return NULL;
- }
-
tcet = SPAPR_TCE_TABLE(object_new(TYPE_SPAPR_TCE_TABLE));
tcet->liobn = liobn;
- tcet->bus_offset = bus_offset;
- tcet->page_shift = page_shift;
- tcet->nb_table = nb_table;
- tcet->vfio_accel = vfio_accel;
snprintf(tmp, sizeof(tmp), "tce-table-%x", liobn);
object_property_add_child(OBJECT(owner), tmp, OBJECT(tcet), NULL);
object_property_set_bool(OBJECT(tcet), true, "realized", NULL);
+ trace_spapr_iommu_new_table(tcet->liobn, tcet, tcet->table, tcet->fd);
+
return tcet;
}
-static void spapr_tce_table_unrealize(DeviceState *dev, Error **errp)
+void spapr_tce_set_props(sPAPRTCETable *tcet, uint64_t bus_offset,
+ uint32_t page_shift, uint32_t nb_table,
+ bool vfio_accel)
{
- sPAPRTCETable *tcet = SPAPR_TCE_TABLE(dev);
+ if (tcet->enabled) {
+ return;
+ }
+ tcet->bus_offset = bus_offset;
+ tcet->page_shift = page_shift;
+ tcet->nb_table = nb_table;
+ tcet->vfio_accel = vfio_accel;
+}
- QLIST_REMOVE(tcet, list);
+void spapr_tce_table_enable(sPAPRTCETable *tcet)
+{
+ uint64_t window_size = (uint64_t)tcet->nb_table << tcet->page_shift;
+
+ if (tcet->enabled) {
+ return;
+ }
+
+ if (!tcet->nb_table) {
+ return;
+ }
+
+ if (kvm_enabled() && !(window_size >> 32)) {
+ tcet->table = kvmppc_create_spapr_tce(tcet->liobn,
+ window_size,
+ &tcet->fd,
+ tcet->vfio_accel);
+ }
+
+ if (!tcet->table) {
+ size_t table_size = tcet->nb_table * sizeof(uint64_t);
+ tcet->table = g_malloc0(table_size);
+ }
+
+ memory_region_init_iommu(&tcet->iommu, OBJECT(tcet), &spapr_iommu_ops,
+ "iommu-spapr",
+ (uint64_t)tcet->nb_table << tcet->page_shift);
+
+ tcet->enabled = true;
+}
+
+void spapr_tce_table_disable(sPAPRTCETable *tcet)
+{
+ if (!tcet->enabled) {
+ return;
+ }
if (!kvm_enabled() ||
(kvmppc_remove_spapr_tce(tcet->table, tcet->fd,
tcet->nb_table) != 0)) {
+ tcet->fd = -1;
g_free(tcet->table);
}
+ tcet->table = NULL;
+ tcet->enabled = false;
+ spapr_tce_set_props(tcet, 0, 0, 0, false);
+}
+
+static void spapr_tce_table_unrealize(DeviceState *dev, Error **errp)
+{
+ sPAPRTCETable *tcet = SPAPR_TCE_TABLE(dev);
+
+ QLIST_REMOVE(tcet, list);
+
+ spapr_tce_table_disable(tcet);
}
MemoryRegion *spapr_tce_get_iommu(sPAPRTCETable *tcet)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 52c5c73..acfdbe5 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -895,15 +895,17 @@ static void spapr_phb_finish_realize(sPAPRPHBState *sphb, Error **errp)
sPAPRTCETable *tcet;
uint32_t nb_table;
- nb_table = SPAPR_PCI_DMA32_SIZE >> SPAPR_TCE_PAGE_SHIFT;
- tcet = spapr_tce_new_table(DEVICE(sphb), sphb->dma_liobn,
- 0, SPAPR_TCE_PAGE_SHIFT, nb_table, false);
+ tcet = spapr_tce_new_table(DEVICE(sphb), sphb->dma_liobn);
if (!tcet) {
error_setg(errp, "Unable to create TCE table for %s",
sphb->dtbusname);
return ;
}
+ nb_table = SPAPR_PCI_DMA32_SIZE >> SPAPR_TCE_PAGE_SHIFT;
+ spapr_tce_set_props(tcet, 0, SPAPR_TCE_PAGE_SHIFT, nb_table, false);
+ spapr_tce_table_enable(tcet);
+
/* Register default 32bit DMA window */
memory_region_add_subregion(&sphb->iommu_root, 0,
spapr_tce_get_iommu(tcet));
diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
index f8b503e..6c9adb5 100644
--- a/hw/ppc/spapr_pci_vfio.c
+++ b/hw/ppc/spapr_pci_vfio.c
@@ -34,6 +34,7 @@ static void spapr_phb_vfio_finish_realize(sPAPRPHBState *sphb, Error **errp)
int ret;
sPAPRTCETable *tcet;
uint32_t liobn = svphb->phb.dma_liobn;
+ uint32_t nb_table;
ret = vfio_container_ioctl(&svphb->phb.iommu_as,
VFIO_CHECK_EXTENSION,
@@ -52,16 +53,18 @@ static void spapr_phb_vfio_finish_realize(sPAPRPHBState *sphb, Error **errp)
return;
}
- tcet = spapr_tce_new_table(DEVICE(sphb), liobn, info.dma32_window_start,
- SPAPR_TCE_PAGE_SHIFT,
- info.dma32_window_size >> SPAPR_TCE_PAGE_SHIFT,
- true);
+ tcet = spapr_tce_new_table(DEVICE(sphb), liobn);
if (!tcet) {
error_setg(errp, "spapr-vfio: failed to create VFIO TCE table");
return;
}
/* Register default 32bit DMA window */
+ nb_table = info.dma32_window_size >> SPAPR_TCE_PAGE_SHIFT;
+ spapr_tce_set_props(tcet, info.dma32_window_start, SPAPR_TCE_PAGE_SHIFT,
+ nb_table, true);
+ spapr_tce_table_enable(tcet);
+
memory_region_add_subregion(&sphb->iommu_root, tcet->bus_offset,
spapr_tce_get_iommu(tcet));
}
diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
index 174033d..6394527 100644
--- a/hw/ppc/spapr_vio.c
+++ b/hw/ppc/spapr_vio.c
@@ -479,11 +479,11 @@ static void spapr_vio_busdev_realize(DeviceState *qdev, Error **errp)
memory_region_add_subregion_overlap(&dev->mrroot, 0, &dev->mrbypass, 1);
address_space_init(&dev->as, &dev->mrroot, qdev->id);
- dev->tcet = spapr_tce_new_table(qdev, liobn,
- 0,
- SPAPR_TCE_PAGE_SHIFT,
- pc->rtce_window_size >>
- SPAPR_TCE_PAGE_SHIFT, false);
+ dev->tcet = spapr_tce_new_table(qdev, liobn);
+ spapr_tce_set_props(dev->tcet, 0, SPAPR_TCE_PAGE_SHIFT,
+ pc->rtce_window_size >> SPAPR_TCE_PAGE_SHIFT,
+ false);
+ spapr_tce_table_enable(dev->tcet);
dev->tcet->vdev = dev;
memory_region_add_subregion_overlap(&dev->mrroot, 0,
spapr_tce_get_iommu(dev->tcet), 2);
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 7d9ab9d..6e33b9b 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -498,6 +498,7 @@ typedef struct sPAPRTCETable sPAPRTCETable;
struct sPAPRTCETable {
DeviceState parent;
+ bool enabled;
uint32_t liobn;
uint32_t nb_table;
uint64_t bus_offset;
@@ -515,11 +516,12 @@ sPAPRTCETable *spapr_tce_find_by_liobn(uint32_t liobn);
void spapr_events_init(sPAPREnvironment *spapr);
void spapr_events_fdt_skel(void *fdt, uint32_t epow_irq);
int spapr_h_cas_compose_response(target_ulong addr, target_ulong size);
-sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn,
- uint64_t bus_offset,
- uint32_t page_shift,
- uint32_t nb_table,
- bool vfio_accel);
+sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn);
+void spapr_tce_set_props(sPAPRTCETable *tcet, uint64_t bus_offset,
+ uint32_t page_shift, uint32_t nb_table,
+ bool vfio_accel);
+void spapr_tce_table_enable(sPAPRTCETable *tcet);
+void spapr_tce_table_disable(sPAPRTCETable *tcet);
MemoryRegion *spapr_tce_get_iommu(sPAPRTCETable *tcet);
int spapr_dma_dt(void *fdt, int node_off, const char *propname,
uint32_t liobn, uint64_t window, uint32_t size);
--
2.0.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH qemu v5 08/12] spapr_pci: Rework reset to reset DMA configuration
2015-03-31 5:28 [Qemu-devel] [PATCH qemu v5 00/12] spapr: vfio: Enable Dynamic DMA windows (DDW) Alexey Kardashevskiy
` (6 preceding siblings ...)
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 07/12] spapr_iommu: Rework TCE table initialization Alexey Kardashevskiy
@ 2015-03-31 5:28 ` Alexey Kardashevskiy
2015-04-08 2:42 ` David Gibson
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 09/12] spapr_iommu: Add root memory region Alexey Kardashevskiy
` (3 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Alexey Kardashevskiy @ 2015-03-31 5:28 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Kardashevskiy, Alex Williamson, qemu-ppc, Alexander Graf,
David Gibson
On a system reset, DMA configuration has to reset too. At the moment
it clears the table content. This is enough for the single table case
but with DDW, we will also have to disable all DMA windows except
the default one. Furthermore according to sPAPR, if the guest removed
the default window and created a huge one at the same zero offset on
a PCI bus, the reset handler has to recreate the default window with
the default properties (2GB big, 4K pages).
This reworks SPAPR PHB code to disable the existing DMA window on reset
and then configure and enable the default window.
Without DDW that means that the same window will be disabled and then
enabled with no other change in behaviour.
This changes the table creation to do it in one place in PHB (VFIO PHB
just inherits the behaviour from PHB). The actual table allocation is
done from the reset handler and this is where finish_realize() is called.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
hw/ppc/spapr_pci.c | 42 ++++++++++++++++++++++++------------------
hw/ppc/spapr_pci_vfio.c | 17 +----------------
include/hw/pci-host/spapr.h | 1 +
3 files changed, 26 insertions(+), 34 deletions(-)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index acfdbe5..57bbc82 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -722,6 +722,22 @@ static const MemoryRegionOps spapr_msi_ops = {
};
/*
+ * DMA windows
+ */
+int spapr_phb_dma_reset(sPAPRPHBState *sphb)
+{
+ const uint32_t liobn = SPAPR_PCI_LIOBN(sphb->index, 0);
+ sPAPRTCETable *tcet = spapr_tce_find_by_liobn(liobn);
+ sPAPRPHBClass *spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
+ Error *err = NULL;
+
+ spapr_tce_table_disable(tcet);
+ spc->finish_realize(sphb, &err);
+
+ return 0;
+}
+
+/*
* PHB PCI device
*/
static AddressSpace *spapr_pci_dma_iommu(PCIBus *bus, void *opaque, int devfn)
@@ -736,11 +752,11 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
SysBusDevice *s = SYS_BUS_DEVICE(dev);
sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(s);
PCIHostState *phb = PCI_HOST_BRIDGE(s);
- sPAPRPHBClass *info = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(s);
char *namebuf;
int i;
PCIBus *bus;
uint64_t msi_window_size = 4096;
+ sPAPRTCETable *tcet;
if (sphb->index != (uint32_t)-1) {
hwaddr windows_base;
@@ -880,12 +896,10 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
sphb->lsi_table[i].irq = irq;
}
- if (!info->finish_realize) {
- error_setg(errp, "finish_realize not defined");
- return;
- }
-
- info->finish_realize(sphb, errp);
+ /* Create default DMA window */
+ tcet = spapr_tce_new_table(DEVICE(sphb), sphb->dma_liobn);
+ memory_region_add_subregion_overlap(&sphb->iommu_root, 0,
+ spapr_tce_get_iommu(tcet), 0);
sphb->msi = g_hash_table_new_full(g_int_hash, g_int_equal, g_free, g_free);
}
@@ -895,20 +909,10 @@ static void spapr_phb_finish_realize(sPAPRPHBState *sphb, Error **errp)
sPAPRTCETable *tcet;
uint32_t nb_table;
- tcet = spapr_tce_new_table(DEVICE(sphb), sphb->dma_liobn);
- if (!tcet) {
- error_setg(errp, "Unable to create TCE table for %s",
- sphb->dtbusname);
- return ;
- }
-
+ tcet = spapr_tce_find_by_liobn(sphb->dma_liobn);
nb_table = SPAPR_PCI_DMA32_SIZE >> SPAPR_TCE_PAGE_SHIFT;
spapr_tce_set_props(tcet, 0, SPAPR_TCE_PAGE_SHIFT, nb_table, false);
spapr_tce_table_enable(tcet);
-
- /* Register default 32bit DMA window */
- memory_region_add_subregion(&sphb->iommu_root, 0,
- spapr_tce_get_iommu(tcet));
}
static int spapr_phb_children_reset(Object *child, void *opaque)
@@ -924,6 +928,8 @@ static int spapr_phb_children_reset(Object *child, void *opaque)
static void spapr_phb_reset(DeviceState *qdev)
{
+ spapr_phb_dma_reset(SPAPR_PCI_HOST_BRIDGE(qdev));
+
/* Reset the IOMMU state */
object_child_foreach(OBJECT(qdev), spapr_phb_children_reset, NULL);
}
diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
index 6c9adb5..1657f6b 100644
--- a/hw/ppc/spapr_pci_vfio.c
+++ b/hw/ppc/spapr_pci_vfio.c
@@ -53,25 +53,11 @@ static void spapr_phb_vfio_finish_realize(sPAPRPHBState *sphb, Error **errp)
return;
}
- tcet = spapr_tce_new_table(DEVICE(sphb), liobn);
- if (!tcet) {
- error_setg(errp, "spapr-vfio: failed to create VFIO TCE table");
- return;
- }
-
- /* Register default 32bit DMA window */
+ tcet = spapr_tce_find_by_liobn(liobn);
nb_table = info.dma32_window_size >> SPAPR_TCE_PAGE_SHIFT;
spapr_tce_set_props(tcet, info.dma32_window_start, SPAPR_TCE_PAGE_SHIFT,
nb_table, true);
spapr_tce_table_enable(tcet);
-
- memory_region_add_subregion(&sphb->iommu_root, tcet->bus_offset,
- spapr_tce_get_iommu(tcet));
-}
-
-static void spapr_phb_vfio_reset(DeviceState *qdev)
-{
- /* Do nothing */
}
static int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
@@ -191,7 +177,6 @@ static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data)
sPAPRPHBClass *spc = SPAPR_PCI_HOST_BRIDGE_CLASS(klass);
dc->props = spapr_phb_vfio_properties;
- dc->reset = spapr_phb_vfio_reset;
spc->finish_realize = spapr_phb_vfio_finish_realize;
spc->eeh_set_option = spapr_phb_vfio_eeh_set_option;
spc->eeh_get_state = spapr_phb_vfio_eeh_get_state;
diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
index 5b497ce..f592276 100644
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -134,5 +134,6 @@ void spapr_pci_rtas_init(void);
sPAPRPHBState *spapr_pci_find_phb(sPAPREnvironment *spapr, uint64_t buid);
PCIDevice *spapr_pci_find_dev(sPAPREnvironment *spapr, uint64_t buid,
uint32_t config_addr);
+int spapr_phb_dma_reset(sPAPRPHBState *sphb);
#endif /* __HW_SPAPR_PCI_H__ */
--
2.0.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH qemu v5 09/12] spapr_iommu: Add root memory region
2015-03-31 5:28 [Qemu-devel] [PATCH qemu v5 00/12] spapr: vfio: Enable Dynamic DMA windows (DDW) Alexey Kardashevskiy
` (7 preceding siblings ...)
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 08/12] spapr_pci: Rework reset to reset DMA configuration Alexey Kardashevskiy
@ 2015-03-31 5:28 ` Alexey Kardashevskiy
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 10/12] spapr_pci: Rework finish_realize() Alexey Kardashevskiy
` (2 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Alexey Kardashevskiy @ 2015-03-31 5:28 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Kardashevskiy, Alex Williamson, qemu-ppc, Alexander Graf,
David Gibson
We are going to have multiple DMA windows at different offsets on
a PCI bus. For the sake of migration, we will have as many TCE table
objects pre-created as many windows supported.
So we need a way to map windows dynamically onto a PCI bus
when migration of a table is completed but at this stage a TCE table
object does not have access to a PHB to ask it to map a DMA window
backed by just migrated TCE table.
This adds a "root" memory region (UINT64_MAX long) to the TCE object.
This new region is mapped on a PCI bus with enabled overlapping as
there will be one root MR per TCE table, each of them mapped at 0.
The actual IOMMU memory region is a subregion of the root region and
a TCE table enables/disables this subregion and maps it at
the specific offset inside the root MR which is 1:1 mapping of
a PCI address space.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
hw/ppc/spapr_iommu.c | 9 ++++++++-
include/hw/ppc/spapr.h | 2 +-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
index a015357..d880818 100644
--- a/hw/ppc/spapr_iommu.c
+++ b/hw/ppc/spapr_iommu.c
@@ -150,6 +150,8 @@ sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn)
tcet->liobn = liobn;
snprintf(tmp, sizeof(tmp), "tce-table-%x", liobn);
+ memory_region_init(&tcet->root, OBJECT(tcet), tmp, UINT64_MAX);
+
object_property_add_child(OBJECT(owner), tmp, OBJECT(tcet), NULL);
object_property_set_bool(OBJECT(tcet), true, "realized", NULL);
@@ -200,6 +202,8 @@ void spapr_tce_table_enable(sPAPRTCETable *tcet)
"iommu-spapr",
(uint64_t)tcet->nb_table << tcet->page_shift);
+ memory_region_add_subregion(&tcet->root, tcet->bus_offset, &tcet->iommu);
+
tcet->enabled = true;
}
@@ -209,6 +213,8 @@ void spapr_tce_table_disable(sPAPRTCETable *tcet)
return;
}
+ memory_region_del_subregion(&tcet->root, &tcet->iommu);
+
if (!kvm_enabled() ||
(kvmppc_remove_spapr_tce(tcet->table, tcet->fd,
tcet->nb_table) != 0)) {
@@ -216,6 +222,7 @@ void spapr_tce_table_disable(sPAPRTCETable *tcet)
g_free(tcet->table);
}
tcet->table = NULL;
+ object_unref(OBJECT(&tcet->iommu));
tcet->enabled = false;
spapr_tce_set_props(tcet, 0, 0, 0, false);
}
@@ -231,7 +238,7 @@ static void spapr_tce_table_unrealize(DeviceState *dev, Error **errp)
MemoryRegion *spapr_tce_get_iommu(sPAPRTCETable *tcet)
{
- return &tcet->iommu;
+ return &tcet->root;
}
static void spapr_tce_reset(DeviceState *dev)
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 6e33b9b..5a45c3f 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -507,7 +507,7 @@ struct sPAPRTCETable {
bool bypass;
bool vfio_accel;
int fd;
- MemoryRegion iommu;
+ MemoryRegion root, iommu;
struct VIOsPAPRDevice *vdev; /* for @bypass migration compatibility only */
QLIST_ENTRY(sPAPRTCETable) list;
};
--
2.0.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH qemu v5 10/12] spapr_pci: Rework finish_realize()
2015-03-31 5:28 [Qemu-devel] [PATCH qemu v5 00/12] spapr: vfio: Enable Dynamic DMA windows (DDW) Alexey Kardashevskiy
` (8 preceding siblings ...)
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 09/12] spapr_iommu: Add root memory region Alexey Kardashevskiy
@ 2015-03-31 5:28 ` Alexey Kardashevskiy
2015-04-08 5:08 ` David Gibson
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 11/12] spapr_pci: Disable all DMA windows on reset Alexey Kardashevskiy
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 12/12] spapr_pci/spapr_pci_vfio: Support Dynamic DMA Windows (DDW) Alexey Kardashevskiy
11 siblings, 1 reply; 26+ messages in thread
From: Alexey Kardashevskiy @ 2015-03-31 5:28 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Kardashevskiy, Alex Williamson, qemu-ppc, Alexander Graf,
David Gibson
This renames and reworks finish_realize() which used to finalize DMA
setup with an assumption that it will not change later.
The new callback supports multiple windows and supports various
parameters such as page and windows sizes.
This is a mechanical change so no change in behaviour is expected.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
hw/ppc/spapr_pci.c | 14 +++++++++-----
hw/ppc/spapr_pci_vfio.c | 24 +++++++++++++-----------
include/hw/pci-host/spapr.h | 3 ++-
3 files changed, 24 insertions(+), 17 deletions(-)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 57bbc82..b8c2488 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -729,10 +729,9 @@ int spapr_phb_dma_reset(sPAPRPHBState *sphb)
const uint32_t liobn = SPAPR_PCI_LIOBN(sphb->index, 0);
sPAPRTCETable *tcet = spapr_tce_find_by_liobn(liobn);
sPAPRPHBClass *spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
- Error *err = NULL;
spapr_tce_table_disable(tcet);
- spc->finish_realize(sphb, &err);
+ spc->dma_init_window(sphb, liobn, SPAPR_TCE_PAGE_SHIFT, 0);
return 0;
}
@@ -904,15 +903,20 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
sphb->msi = g_hash_table_new_full(g_int_hash, g_int_equal, g_free, g_free);
}
-static void spapr_phb_finish_realize(sPAPRPHBState *sphb, Error **errp)
+static int spapr_phb_dma_init_window(sPAPRPHBState *sphb, uint32_t liobn,
+ uint32_t page_shift,
+ uint64_t window_size_hint)
{
sPAPRTCETable *tcet;
uint32_t nb_table;
+ uint64_t bus_offset = 0;
tcet = spapr_tce_find_by_liobn(sphb->dma_liobn);
nb_table = SPAPR_PCI_DMA32_SIZE >> SPAPR_TCE_PAGE_SHIFT;
- spapr_tce_set_props(tcet, 0, SPAPR_TCE_PAGE_SHIFT, nb_table, false);
+ spapr_tce_set_props(tcet, bus_offset, page_shift, nb_table, false);
spapr_tce_table_enable(tcet);
+
+ return 0;
}
static int spapr_phb_children_reset(Object *child, void *opaque)
@@ -1065,7 +1069,7 @@ static void spapr_phb_class_init(ObjectClass *klass, void *data)
dc->vmsd = &vmstate_spapr_pci;
set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
dc->cannot_instantiate_with_device_add_yet = false;
- spc->finish_realize = spapr_phb_finish_realize;
+ spc->dma_init_window = spapr_phb_dma_init_window;
}
static const TypeInfo spapr_phb_info = {
diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
index 1657f6b..a7e32f6 100644
--- a/hw/ppc/spapr_pci_vfio.c
+++ b/hw/ppc/spapr_pci_vfio.c
@@ -21,43 +21,45 @@
#include "hw/pci-host/spapr.h"
#include "linux/vfio.h"
#include "hw/vfio/vfio.h"
+#include "qemu/error-report.h"
static Property spapr_phb_vfio_properties[] = {
DEFINE_PROP_INT32("iommu", sPAPRPHBVFIOState, iommugroupid, -1),
DEFINE_PROP_END_OF_LIST(),
};
-static void spapr_phb_vfio_finish_realize(sPAPRPHBState *sphb, Error **errp)
+static int spapr_phb_vfio_dma_init_window(sPAPRPHBState *sphb, uint32_t liobn,
+ uint32_t page_shift,
+ uint64_t window_size_hint)
{
sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
struct vfio_iommu_spapr_tce_info info = { .argsz = sizeof(info) };
int ret;
- sPAPRTCETable *tcet;
- uint32_t liobn = svphb->phb.dma_liobn;
uint32_t nb_table;
+ sPAPRTCETable *tcet = spapr_tce_find_by_liobn(liobn);
ret = vfio_container_ioctl(&svphb->phb.iommu_as,
VFIO_CHECK_EXTENSION,
(void *) VFIO_SPAPR_TCE_IOMMU);
if (ret != 1) {
- error_setg_errno(errp, -ret,
- "spapr-vfio: SPAPR extension is not supported");
- return;
+ error_report("spapr-vfio: SPAPR extension is not supported: %m");
+ return ret;
}
ret = vfio_container_ioctl(&sphb->iommu_as,
VFIO_IOMMU_SPAPR_TCE_GET_INFO, &info);
if (ret) {
- error_setg_errno(errp, -ret,
- "spapr-vfio: get info from container failed");
- return;
+ error_report("spapr-vfio: get info from container failed: %m");
+ return ret;
}
tcet = spapr_tce_find_by_liobn(liobn);
nb_table = info.dma32_window_size >> SPAPR_TCE_PAGE_SHIFT;
- spapr_tce_set_props(tcet, info.dma32_window_start, SPAPR_TCE_PAGE_SHIFT,
+ spapr_tce_set_props(tcet, info.dma32_window_start, page_shift,
nb_table, true);
spapr_tce_table_enable(tcet);
+
+ return ret;
}
static int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
@@ -177,7 +179,7 @@ static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data)
sPAPRPHBClass *spc = SPAPR_PCI_HOST_BRIDGE_CLASS(klass);
dc->props = spapr_phb_vfio_properties;
- spc->finish_realize = spapr_phb_vfio_finish_realize;
+ spc->dma_init_window = spapr_phb_vfio_dma_init_window;
spc->eeh_set_option = spapr_phb_vfio_eeh_set_option;
spc->eeh_get_state = spapr_phb_vfio_eeh_get_state;
spc->eeh_reset = spapr_phb_vfio_eeh_reset;
diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
index f592276..f556a41 100644
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -48,7 +48,8 @@ typedef struct sPAPRPHBVFIOState sPAPRPHBVFIOState;
struct sPAPRPHBClass {
PCIHostBridgeClass parent_class;
- void (*finish_realize)(sPAPRPHBState *sphb, Error **errp);
+ int (*dma_init_window)(sPAPRPHBState *sphb, uint32_t liobn,
+ uint32_t page_shift, uint64_t window_size_hint);
int (*eeh_set_option)(sPAPRPHBState *sphb, unsigned int addr, int option);
int (*eeh_get_state)(sPAPRPHBState *sphb, int *state);
int (*eeh_reset)(sPAPRPHBState *sphb, int option);
--
2.0.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH qemu v5 11/12] spapr_pci: Disable all DMA windows on reset
2015-03-31 5:28 [Qemu-devel] [PATCH qemu v5 00/12] spapr: vfio: Enable Dynamic DMA windows (DDW) Alexey Kardashevskiy
` (9 preceding siblings ...)
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 10/12] spapr_pci: Rework finish_realize() Alexey Kardashevskiy
@ 2015-03-31 5:28 ` Alexey Kardashevskiy
2015-04-08 5:09 ` David Gibson
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 12/12] spapr_pci/spapr_pci_vfio: Support Dynamic DMA Windows (DDW) Alexey Kardashevskiy
11 siblings, 1 reply; 26+ messages in thread
From: Alexey Kardashevskiy @ 2015-03-31 5:28 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Kardashevskiy, Alex Williamson, qemu-ppc, Alexander Graf,
David Gibson
This disables all DMA windows on a PHB reset. It does not make any
difference now as there is just one DMA window but it will later with DDW
patches.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
hw/ppc/spapr_pci.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index b8c2488..a85289e 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -724,13 +724,24 @@ static const MemoryRegionOps spapr_msi_ops = {
/*
* DMA windows
*/
+static int spapr_phb_disable_dma_windows(Object *child, void *opaque)
+{
+ sPAPRTCETable *tcet = (sPAPRTCETable *)
+ object_dynamic_cast(child, TYPE_SPAPR_TCE_TABLE);
+
+ if (tcet) {
+ spapr_tce_table_disable(tcet);
+ }
+
+ return 0;
+}
+
int spapr_phb_dma_reset(sPAPRPHBState *sphb)
{
const uint32_t liobn = SPAPR_PCI_LIOBN(sphb->index, 0);
- sPAPRTCETable *tcet = spapr_tce_find_by_liobn(liobn);
sPAPRPHBClass *spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
- spapr_tce_table_disable(tcet);
+ object_child_foreach(OBJECT(sphb), spapr_phb_disable_dma_windows, NULL);
spc->dma_init_window(sphb, liobn, SPAPR_TCE_PAGE_SHIFT, 0);
return 0;
--
2.0.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH qemu v5 12/12] spapr_pci/spapr_pci_vfio: Support Dynamic DMA Windows (DDW)
2015-03-31 5:28 [Qemu-devel] [PATCH qemu v5 00/12] spapr: vfio: Enable Dynamic DMA windows (DDW) Alexey Kardashevskiy
` (10 preceding siblings ...)
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 11/12] spapr_pci: Disable all DMA windows on reset Alexey Kardashevskiy
@ 2015-03-31 5:28 ` Alexey Kardashevskiy
11 siblings, 0 replies; 26+ messages in thread
From: Alexey Kardashevskiy @ 2015-03-31 5:28 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Kardashevskiy, Alex Williamson, qemu-ppc, Alexander Graf,
David Gibson
This adds support for Dynamic DMA Windows (DDW) option defined by
the SPAPR specification which allows to have additional DMA window(s)
This implements DDW for emulated and VFIO PHB.
This reserves RTAS token numbers for DDW calls.
This adds callbacks definitions which PHB needs to implement in order to
support dynamic DMA windows (DDW).
This implements ddw_capabilities_update() callback which updates
DMA-related capabilities stored in sPAPRPHBState.
This extends init_dma_window() callbacks to support a second DMA window
which is going to contain mapping of the entire guest RAM in most cases.
This changes the TCE table migration descriptor to support dynamic
tables as from now on, PHB will create as many stub TCE table objects
as PHB can possibly support but not all of them might be initialized at
the time of migration because DDW might or might not be requested by
the guest.
The "ddw" property is enabled by default on a PHB but for compatibility
the pseries-2.3 machine and older disable it.
This implements DDW for VFIO. The host kernel support is required.
This adds a "levels" property to VFIO PHB to control the number of levels
in the actual TCE table allocated by the host kernel, 0 is the default
value to tell QEMU to calculate the correct value. Current hardware
supports up to 5 levels.
The existing linux guests try creating one additional huge DMA window
with 64K or 16MB pages and map the entire guest RAM to. If succeeded,
the guest switches to dma_direct_ops and never calls TCE hypercalls
(H_PUT_TCE,...) again. This enables VFIO devices to use the entire RAM
and not waste time on map/unmap later.
This adds 4 RTAS handlers:
* ibm,query-pe-dma-window
* ibm,create-pe-dma-window
* ibm,remove-pe-dma-window
* ibm,reset-pe-dma-window
These are registered from type_init() callback.
These RTAS handlers are implemented in a separate file to avoid polluting
spapr_iommu.c with PCI.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v5:
* total rework
* enabled for machines >2.3
* fixed migration
* merged rtas handlers here
v4:
* reset handler is back in generalized form
v3:
* removed reset
* windows_num is now 1 or bigger rather than 0-based value and it is only
changed in PHB code, not in RTAS
* added page mask check in create()
* added SPAPR_PCI_DDW_MAX_WINDOWS to track how many windows are already
created
v2:
* tested on hacked emulated E1000
* implemented DDW reset on the PHB reset
* spapr_pci_ddw_remove/spapr_pci_ddw_reset are public for reuse by VFIO
---
hw/ppc/Makefile.objs | 3 +
hw/ppc/spapr.c | 5 +
hw/ppc/spapr_iommu.c | 33 ++++-
hw/ppc/spapr_pci.c | 59 ++++++++-
hw/ppc/spapr_pci_vfio.c | 116 +++++++++++++---
hw/ppc/spapr_rtas_ddw.c | 314 ++++++++++++++++++++++++++++++++++++++++++++
include/hw/pci-host/spapr.h | 17 +++
include/hw/ppc/spapr.h | 17 ++-
trace-events | 4 +
9 files changed, 541 insertions(+), 27 deletions(-)
create mode 100644 hw/ppc/spapr_rtas_ddw.c
diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
index 437955d..c6b344f 100644
--- a/hw/ppc/Makefile.objs
+++ b/hw/ppc/Makefile.objs
@@ -7,6 +7,9 @@ obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o
ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
obj-y += spapr_pci_vfio.o
endif
+ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES), yy)
+obj-y += spapr_rtas_ddw.o
+endif
# PowerPC 4xx boards
obj-y += ppc405_boards.o ppc4xx_devs.o ppc405_uc.o ppc440_bamboo.o
obj-y += ppc4xx_pci.o
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 7febff7..98e00c5 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1806,6 +1806,11 @@ static const TypeInfo spapr_machine_info = {
.driver = TYPE_SPAPR_PCI_HOST_BRIDGE,\
.property = "mem_win_size",\
.value = "0x20000000",\
+ },\
+ {\
+ .driver = TYPE_SPAPR_PCI_HOST_BRIDGE,\
+ .property = "ddw",\
+ .value = stringify(off),\
}
#define SPAPR_COMPAT_2_1 \
diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
index d880818..f3e49cd 100644
--- a/hw/ppc/spapr_iommu.c
+++ b/hw/ppc/spapr_iommu.c
@@ -90,6 +90,13 @@ static IOMMUTLBEntry spapr_tce_translate_iommu(MemoryRegion *iommu, hwaddr addr,
return ret;
}
+static void spapr_tce_table_pre_save(void *opaque)
+{
+ sPAPRTCETable *tcet = SPAPR_TCE_TABLE(opaque);
+
+ tcet->migtable = tcet->table;
+}
+
static int spapr_tce_table_post_load(void *opaque, int version_id)
{
sPAPRTCETable *tcet = SPAPR_TCE_TABLE(opaque);
@@ -98,22 +105,42 @@ static int spapr_tce_table_post_load(void *opaque, int version_id)
spapr_vio_set_bypass(tcet->vdev, tcet->bypass);
}
+ if (!tcet->migtable) {
+ return 0;
+ }
+
+ if (tcet->enabled) {
+ if (!tcet->table) {
+ tcet->enabled = false;
+ spapr_tce_table_enable(tcet);
+ }
+ memcpy(tcet->table, tcet->migtable,
+ tcet->nb_table * sizeof(tcet->table[0]));
+ free(tcet->migtable);
+ tcet->migtable = NULL;
+ }
+
return 0;
}
static const VMStateDescription vmstate_spapr_tce_table = {
.name = "spapr_iommu",
- .version_id = 2,
+ .version_id = 3,
.minimum_version_id = 2,
+ .pre_save = spapr_tce_table_pre_save,
.post_load = spapr_tce_table_post_load,
.fields = (VMStateField []) {
/* Sanity check */
VMSTATE_UINT32_EQUAL(liobn, sPAPRTCETable),
- VMSTATE_UINT32_EQUAL(nb_table, sPAPRTCETable),
/* IOMMU state */
+ VMSTATE_BOOL_V(enabled, sPAPRTCETable, 3),
+ VMSTATE_UINT64_V(bus_offset, sPAPRTCETable, 3),
+ VMSTATE_UINT32_V(page_shift, sPAPRTCETable, 3),
+ VMSTATE_UINT32(nb_table, sPAPRTCETable),
VMSTATE_BOOL(bypass, sPAPRTCETable),
- VMSTATE_VARRAY_UINT32(table, sPAPRTCETable, nb_table, 0, vmstate_info_uint64, uint64_t),
+ VMSTATE_VARRAY_UINT32_ALLOC(migtable, sPAPRTCETable, nb_table, 0,
+ vmstate_info_uint64, uint64_t),
VMSTATE_END_OF_LIST()
},
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index a85289e..126909f 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -730,6 +730,11 @@ static int spapr_phb_disable_dma_windows(Object *child, void *opaque)
object_dynamic_cast(child, TYPE_SPAPR_TCE_TABLE);
if (tcet) {
+ sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(opaque);
+ sPAPRPHBClass *spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(opaque);
+ if (spc->dma_remove_window) {
+ spc->dma_remove_window(sphb, tcet);
+ }
spapr_tce_table_disable(tcet);
}
@@ -741,12 +746,23 @@ int spapr_phb_dma_reset(sPAPRPHBState *sphb)
const uint32_t liobn = SPAPR_PCI_LIOBN(sphb->index, 0);
sPAPRPHBClass *spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
- object_child_foreach(OBJECT(sphb), spapr_phb_disable_dma_windows, NULL);
+ spc->dma_capabilities_update(sphb);
+ object_child_foreach(OBJECT(sphb), spapr_phb_disable_dma_windows, sphb);
spc->dma_init_window(sphb, liobn, SPAPR_TCE_PAGE_SHIFT, 0);
return 0;
}
+static int spapr_pci_dma_capabilities_update(sPAPRPHBState *sphb)
+{
+ sphb->windows_supported = SPAPR_PCI_DMA_MAX_WINDOWS;
+ sphb->page_size_mask = (1 << 12) | (1 << 16) | (1 << 24);
+ sphb->dma32_window_size = SPAPR_PCI_DMA32_SIZE;
+ sphb->dma64_window_size = pow2ceil(ram_size);
+
+ return 0;
+}
+
/*
* PHB PCI device
*/
@@ -906,10 +922,12 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
sphb->lsi_table[i].irq = irq;
}
- /* Create default DMA window */
- tcet = spapr_tce_new_table(DEVICE(sphb), sphb->dma_liobn);
- memory_region_add_subregion_overlap(&sphb->iommu_root, 0,
- spapr_tce_get_iommu(tcet), 0);
+ for (i = 0; i < SPAPR_PCI_DMA_MAX_WINDOWS; ++i) {
+ tcet = spapr_tce_new_table(DEVICE(sphb),
+ SPAPR_PCI_LIOBN(sphb->index, i));
+ memory_region_add_subregion_overlap(&sphb->iommu_root, 0,
+ spapr_tce_get_iommu(tcet), 0);
+ }
sphb->msi = g_hash_table_new_full(g_int_hash, g_int_equal, g_free, g_free);
}
@@ -922,8 +940,16 @@ static int spapr_phb_dma_init_window(sPAPRPHBState *sphb, uint32_t liobn,
uint32_t nb_table;
uint64_t bus_offset = 0;
- tcet = spapr_tce_find_by_liobn(sphb->dma_liobn);
- nb_table = SPAPR_PCI_DMA32_SIZE >> SPAPR_TCE_PAGE_SHIFT;
+ tcet = spapr_tce_find_by_liobn(liobn);
+
+ if (!window_size_hint && !SPAPR_PCI_DMA_WINDOW_NUM(liobn)) {
+ window_size_hint = SPAPR_PCI_DMA32_SIZE;
+ }
+ if (SPAPR_PCI_DMA_WINDOW_NUM(liobn)) {
+ bus_offset = SPAPR_PCI_DMA64_START;
+ }
+
+ nb_table = window_size_hint >> page_shift;
spapr_tce_set_props(tcet, bus_offset, page_shift, nb_table, false);
spapr_tce_table_enable(tcet);
@@ -959,6 +985,7 @@ static Property spapr_phb_properties[] = {
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),
+ DEFINE_PROP_BOOL("ddw", sPAPRPHBState, ddw_enabled, true),
DEFINE_PROP_END_OF_LIST(),
};
@@ -1081,6 +1108,7 @@ static void spapr_phb_class_init(ObjectClass *klass, void *data)
set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
dc->cannot_instantiate_with_device_add_yet = false;
spc->dma_init_window = spapr_phb_dma_init_window;
+ spc->dma_capabilities_update = spapr_pci_dma_capabilities_update;
}
static const TypeInfo spapr_phb_info = {
@@ -1151,6 +1179,15 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
uint32_t interrupt_map_mask[] = {
cpu_to_be32(b_ddddd(-1)|b_fff(0)), 0x0, 0x0, cpu_to_be32(-1)};
uint32_t interrupt_map[PCI_SLOT_MAX * PCI_NUM_PINS][7];
+ uint32_t ddw_applicable[] = {
+ cpu_to_be32(RTAS_IBM_QUERY_PE_DMA_WINDOW),
+ cpu_to_be32(RTAS_IBM_CREATE_PE_DMA_WINDOW),
+ cpu_to_be32(RTAS_IBM_REMOVE_PE_DMA_WINDOW)
+ };
+ uint32_t ddw_extensions[] = {
+ cpu_to_be32(1),
+ cpu_to_be32(RTAS_IBM_RESET_PE_DMA_WINDOW)
+ };
sPAPRTCETable *tcet;
/* Start populating the FDT */
@@ -1181,6 +1218,14 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
_FDT(fdt_setprop_cell(fdt, bus_off, "ibm,pci-config-space-type", 0x1));
_FDT(fdt_setprop_cell(fdt, bus_off, "ibm,pe-total-#msi", XICS_IRQS));
+ /* Dynamic DMA window */
+ if (phb->ddw_enabled) {
+ _FDT(fdt_setprop(fdt, bus_off, "ibm,ddw-applicable", &ddw_applicable,
+ sizeof(ddw_applicable)));
+ _FDT(fdt_setprop(fdt, bus_off, "ibm,ddw-extensions",
+ &ddw_extensions, sizeof(ddw_extensions)));
+ }
+
/* Build the interrupt-map, this must matches what is done
* in pci_spapr_map_irq
*/
diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
index a7e32f6..0f53ac4 100644
--- a/hw/ppc/spapr_pci_vfio.c
+++ b/hw/ppc/spapr_pci_vfio.c
@@ -25,40 +25,122 @@
static Property spapr_phb_vfio_properties[] = {
DEFINE_PROP_INT32("iommu", sPAPRPHBVFIOState, iommugroupid, -1),
+ DEFINE_PROP_UINT8("levels", sPAPRPHBVFIOState, levels, 0),
DEFINE_PROP_END_OF_LIST(),
};
+static int spapr_pci_vfio_dma_capabilities_update(sPAPRPHBState *sphb)
+{
+ struct vfio_iommu_spapr_tce_info info = { .argsz = sizeof(info) };
+ sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
+ int ret;
+
+ ret = vfio_container_ioctl(&sphb->iommu_as,
+ VFIO_IOMMU_SPAPR_TCE_GET_INFO, &info);
+ if (ret) {
+ return ret;
+ }
+
+ sphb->dma32_window_size = info.dma32_window_size;
+
+ if (sphb->ddw_enabled && (info.flags & VFIO_IOMMU_SPAPR_INFO_DDW)) {
+ sphb->windows_supported = info.ddw.max_dynamic_windows_supported;
+ sphb->page_size_mask = info.ddw.pgsizes;
+ sphb->dma64_window_size = pow2ceil(ram_size);
+ svphb->max_levels = info.ddw.levels;
+ } else {
+ /* If VFIO_IOMMU_INFO_DDW is not set, disable DDW */
+ sphb->ddw_enabled = false;
+ }
+
+ return ret;
+}
+
+static int spapr_phb_vfio_levels(uint32_t entries)
+{
+ unsigned pages = (entries * sizeof(uint64_t)) / getpagesize();
+ int levels;
+
+ if (pages <= 64) {
+ levels = 1;
+ } else if (pages <= 64*64) {
+ levels = 2;
+ } else if (pages <= 64*64*64) {
+ levels = 3;
+ } else {
+ levels = 4;
+ }
+
+ return levels;
+}
+
static int spapr_phb_vfio_dma_init_window(sPAPRPHBState *sphb, uint32_t liobn,
uint32_t page_shift,
uint64_t window_size_hint)
{
sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
- struct vfio_iommu_spapr_tce_info info = { .argsz = sizeof(info) };
int ret;
uint32_t nb_table;
sPAPRTCETable *tcet = spapr_tce_find_by_liobn(liobn);
+ struct vfio_iommu_spapr_tce_create create = {
+ .argsz = sizeof(create),
+ .page_shift = page_shift,
+ .window_size = 0,
+ .levels = svphb->levels,
+ .start_addr = 0,
+ };
- ret = vfio_container_ioctl(&svphb->phb.iommu_as,
- VFIO_CHECK_EXTENSION,
- (void *) VFIO_SPAPR_TCE_IOMMU);
- if (ret != 1) {
- error_report("spapr-vfio: SPAPR extension is not supported: %m");
- return ret;
+ if (!window_size_hint && !SPAPR_PCI_DMA_WINDOW_NUM(liobn)) {
+ create.window_size = sphb->dma32_window_size;
}
+ if (window_size_hint) {
+ create.window_size = window_size_hint;
+ }
+
+ if (sphb->ddw_enabled) {
+ /*
+ * Dynamic windows are supported, that means that there is no
+ * pre-created window and we have to create one.
+ */
+ if (!create.levels) {
+ create.levels = spapr_phb_vfio_levels(create.window_size >>
+ page_shift);
+ }
+
+ if (create.levels > svphb->max_levels) {
+ return -EINVAL;
+ }
- ret = vfio_container_ioctl(&sphb->iommu_as,
- VFIO_IOMMU_SPAPR_TCE_GET_INFO, &info);
- if (ret) {
- error_report("spapr-vfio: get info from container failed: %m");
- return ret;
+ ret = vfio_container_ioctl(&sphb->iommu_as,
+ VFIO_IOMMU_SPAPR_TCE_CREATE, &create);
+ if (ret) {
+ return ret;
+ }
}
- tcet = spapr_tce_find_by_liobn(liobn);
- nb_table = info.dma32_window_size >> SPAPR_TCE_PAGE_SHIFT;
- spapr_tce_set_props(tcet, info.dma32_window_start, page_shift,
- nb_table, true);
+ nb_table = create.window_size >> page_shift;
+ spapr_tce_set_props(tcet, create.start_addr, page_shift, nb_table, true);
spapr_tce_table_enable(tcet);
+ return 0;
+}
+
+static int spapr_pci_vfio_dma_remove_window(sPAPRPHBState *sphb,
+ sPAPRTCETable *tcet)
+{
+ struct vfio_iommu_spapr_tce_remove remove = {
+ .argsz = sizeof(remove),
+ .start_addr = tcet->bus_offset
+ };
+ int ret;
+
+ if (!sphb->ddw_enabled) {
+ return 0;
+ }
+
+ ret = vfio_container_ioctl(&sphb->iommu_as,
+ VFIO_IOMMU_SPAPR_TCE_REMOVE, &remove);
+
return ret;
}
@@ -180,6 +262,8 @@ static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data)
dc->props = spapr_phb_vfio_properties;
spc->dma_init_window = spapr_phb_vfio_dma_init_window;
+ spc->dma_capabilities_update = spapr_pci_vfio_dma_capabilities_update;
+ spc->dma_remove_window = spapr_pci_vfio_dma_remove_window;
spc->eeh_set_option = spapr_phb_vfio_eeh_set_option;
spc->eeh_get_state = spapr_phb_vfio_eeh_get_state;
spc->eeh_reset = spapr_phb_vfio_eeh_reset;
diff --git a/hw/ppc/spapr_rtas_ddw.c b/hw/ppc/spapr_rtas_ddw.c
new file mode 100644
index 0000000..2347c1e
--- /dev/null
+++ b/hw/ppc/spapr_rtas_ddw.c
@@ -0,0 +1,314 @@
+/*
+ * QEMU sPAPR Dynamic DMA windows support
+ *
+ * Copyright (c) 2014 Alexey Kardashevskiy, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License,
+ * or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/error-report.h"
+#include "hw/ppc/spapr.h"
+#include "hw/pci-host/spapr.h"
+#include "trace.h"
+
+static int spapr_phb_get_active_win_num_cb(Object *child, void *opaque)
+{
+ sPAPRTCETable *tcet;
+
+ tcet = (sPAPRTCETable *) object_dynamic_cast(child, TYPE_SPAPR_TCE_TABLE);
+ if (tcet && tcet->enabled) {
+ ++*(unsigned *)opaque;
+ }
+ return 0;
+}
+
+static unsigned spapr_phb_get_active_win_num(sPAPRPHBState *sphb)
+{
+ unsigned ret = 0;
+
+ object_child_foreach(OBJECT(sphb), spapr_phb_get_active_win_num_cb, &ret);
+
+ return ret;
+}
+
+static int spapr_phb_get_free_liobn_cb(Object *child, void *opaque)
+{
+ sPAPRTCETable *tcet;
+
+ tcet = (sPAPRTCETable *) object_dynamic_cast(child, TYPE_SPAPR_TCE_TABLE);
+ if (tcet && !tcet->enabled) {
+ *(uint32_t *)opaque = tcet->liobn;
+ return 1;
+ }
+ return 0;
+}
+
+static unsigned spapr_phb_get_free_liobn(sPAPRPHBState *sphb)
+{
+ uint32_t liobn = 0;
+
+ object_child_foreach(OBJECT(sphb), spapr_phb_get_free_liobn_cb, &liobn);
+
+ return liobn;
+}
+
+static uint32_t spapr_query_mask(struct ppc_one_seg_page_size *sps,
+ uint64_t page_mask)
+{
+ int i, j;
+ uint32_t mask = 0;
+ const struct { int shift; uint32_t mask; } masks[] = {
+ { 12, RTAS_DDW_PGSIZE_4K },
+ { 16, RTAS_DDW_PGSIZE_64K },
+ { 24, RTAS_DDW_PGSIZE_16M },
+ { 25, RTAS_DDW_PGSIZE_32M },
+ { 26, RTAS_DDW_PGSIZE_64M },
+ { 27, RTAS_DDW_PGSIZE_128M },
+ { 28, RTAS_DDW_PGSIZE_256M },
+ { 34, RTAS_DDW_PGSIZE_16G },
+ };
+
+ for (i = 0; i < PPC_PAGE_SIZES_MAX_SZ; i++) {
+ for (j = 0; j < ARRAY_SIZE(masks); ++j) {
+ if ((sps[i].page_shift == masks[j].shift) &&
+ (page_mask & (1ULL << masks[j].shift))) {
+ mask |= masks[j].mask;
+ }
+ }
+ }
+
+ return mask;
+}
+
+static void rtas_ibm_query_pe_dma_window(PowerPCCPU *cpu,
+ sPAPREnvironment *spapr,
+ uint32_t token, uint32_t nargs,
+ target_ulong args,
+ uint32_t nret, target_ulong rets)
+{
+ CPUPPCState *env = &cpu->env;
+ sPAPRPHBState *sphb;
+ uint64_t buid;
+ uint32_t avail, addr, pgmask = 0;
+ unsigned current;
+
+ if ((nargs != 3) || (nret != 5)) {
+ goto param_error_exit;
+ }
+
+ buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
+ addr = rtas_ld(args, 0);
+ sphb = spapr_pci_find_phb(spapr, buid);
+ if (!sphb || !sphb->ddw_enabled) {
+ goto param_error_exit;
+ }
+
+ current = spapr_phb_get_active_win_num(sphb);
+ avail = (sphb->windows_supported > current) ?
+ (sphb->windows_supported - current) : 0;
+
+ /* Work out supported page masks */
+ pgmask = spapr_query_mask(env->sps.sps, sphb->page_size_mask);
+
+ rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+ rtas_st(rets, 1, avail);
+
+ /*
+ * This is "Largest contiguous block of TCEs allocated specifically
+ * for (that is, are reserved for) this PE".
+ * Return the maximum number as all RAM was in 4K pages.
+ */
+ rtas_st(rets, 2, sphb->dma64_window_size >> SPAPR_TCE_PAGE_SHIFT);
+ rtas_st(rets, 3, pgmask);
+ rtas_st(rets, 4, 0); /* DMA migration mask, not supported */
+
+ trace_spapr_iommu_ddw_query(buid, addr, avail, sphb->dma64_window_size,
+ pgmask);
+ return;
+
+param_error_exit:
+ rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+}
+
+static void rtas_ibm_create_pe_dma_window(PowerPCCPU *cpu,
+ sPAPREnvironment *spapr,
+ uint32_t token, uint32_t nargs,
+ target_ulong args,
+ uint32_t nret, target_ulong rets)
+{
+ sPAPRPHBState *sphb;
+ sPAPRPHBClass *spc;
+ sPAPRTCETable *tcet = NULL;
+ uint32_t addr, page_shift, window_shift, liobn;
+ uint64_t buid;
+ long ret;
+
+ if ((nargs != 5) || (nret != 4)) {
+ goto param_error_exit;
+ }
+
+ buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
+ addr = rtas_ld(args, 0);
+ sphb = spapr_pci_find_phb(spapr, buid);
+ if (!sphb || !sphb->ddw_enabled) {
+ goto param_error_exit;
+ }
+
+ spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
+
+ page_shift = rtas_ld(args, 3);
+ window_shift = rtas_ld(args, 4);
+ liobn = spapr_phb_get_free_liobn(sphb);
+
+ if (!liobn || !(sphb->page_size_mask & (1ULL << page_shift))) {
+ goto hw_error_exit;
+ }
+
+ ret = spc->dma_init_window(sphb, liobn, page_shift, 1ULL << window_shift);
+ if (ret) {
+ goto hw_error_exit;
+ }
+ tcet = spapr_tce_find_by_liobn(liobn);
+ trace_spapr_iommu_ddw_create(buid, addr, 1ULL << page_shift,
+ 1ULL << window_shift,
+ tcet ? tcet->bus_offset : 0xbaadf00d,
+ liobn, ret);
+ if (ret || !tcet) {
+ goto hw_error_exit;
+ }
+
+ rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+ rtas_st(rets, 1, liobn);
+ rtas_st(rets, 2, tcet->bus_offset >> 32);
+ rtas_st(rets, 3, tcet->bus_offset & ((uint32_t) -1));
+
+ spapr_tce_table_enable(tcet);
+ return;
+
+hw_error_exit:
+ rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
+ return;
+
+param_error_exit:
+ rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+}
+
+static void rtas_ibm_remove_pe_dma_window(PowerPCCPU *cpu,
+ sPAPREnvironment *spapr,
+ uint32_t token, uint32_t nargs,
+ target_ulong args,
+ uint32_t nret, target_ulong rets)
+{
+ sPAPRPHBState *sphb;
+ sPAPRPHBClass *spc;
+ sPAPRTCETable *tcet;
+ uint32_t liobn;
+ long ret;
+
+ if ((nargs != 1) || (nret != 1)) {
+ goto param_error_exit;
+ }
+
+ liobn = rtas_ld(args, 0);
+ tcet = spapr_tce_find_by_liobn(liobn);
+ if (!tcet) {
+ goto param_error_exit;
+ }
+
+ sphb = SPAPR_PCI_HOST_BRIDGE(OBJECT(tcet)->parent);
+ if (!sphb || !sphb->ddw_enabled) {
+ goto param_error_exit;
+ }
+
+ spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
+ if (!spc->dma_remove_window) {
+ goto hw_error_exit;
+ }
+
+ ret = spc->dma_remove_window(sphb, tcet);
+ trace_spapr_iommu_ddw_remove(liobn, ret);
+ if (ret) {
+ goto hw_error_exit;
+ }
+
+ spapr_tce_table_disable(tcet);
+
+ rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+ return;
+
+hw_error_exit:
+ rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
+ return;
+
+param_error_exit:
+ rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+}
+
+static void rtas_ibm_reset_pe_dma_window(PowerPCCPU *cpu,
+ sPAPREnvironment *spapr,
+ uint32_t token, uint32_t nargs,
+ target_ulong args,
+ uint32_t nret, target_ulong rets)
+{
+ sPAPRPHBState *sphb;
+ uint64_t buid;
+ uint32_t addr;
+ long ret;
+
+ if ((nargs != 3) || (nret != 1)) {
+ goto param_error_exit;
+ }
+
+ buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
+ addr = rtas_ld(args, 0);
+ sphb = spapr_pci_find_phb(spapr, buid);
+ if (!sphb || !sphb->ddw_enabled) {
+ goto param_error_exit;
+ }
+
+ ret = spapr_phb_dma_reset(sphb);
+ trace_spapr_iommu_ddw_reset(buid, addr, ret);
+ if (ret) {
+ goto hw_error_exit;
+ }
+
+ rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+
+ return;
+
+hw_error_exit:
+ rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
+ return;
+
+param_error_exit:
+ rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+}
+
+static void spapr_rtas_ddw_init(void)
+{
+ spapr_rtas_register(RTAS_IBM_QUERY_PE_DMA_WINDOW,
+ "ibm,query-pe-dma-window",
+ rtas_ibm_query_pe_dma_window);
+ spapr_rtas_register(RTAS_IBM_CREATE_PE_DMA_WINDOW,
+ "ibm,create-pe-dma-window",
+ rtas_ibm_create_pe_dma_window);
+ spapr_rtas_register(RTAS_IBM_REMOVE_PE_DMA_WINDOW,
+ "ibm,remove-pe-dma-window",
+ rtas_ibm_remove_pe_dma_window);
+ spapr_rtas_register(RTAS_IBM_RESET_PE_DMA_WINDOW,
+ "ibm,reset-pe-dma-window",
+ rtas_ibm_reset_pe_dma_window);
+}
+
+type_init(spapr_rtas_ddw_init)
diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
index f556a41..e12d22e 100644
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -50,6 +50,8 @@ struct sPAPRPHBClass {
int (*dma_init_window)(sPAPRPHBState *sphb, uint32_t liobn,
uint32_t page_shift, uint64_t window_size_hint);
+ int (*dma_capabilities_update)(sPAPRPHBState *sphb);
+ int (*dma_remove_window)(sPAPRPHBState *sphb, sPAPRTCETable *tcet);
int (*eeh_set_option)(sPAPRPHBState *sphb, unsigned int addr, int option);
int (*eeh_get_state)(sPAPRPHBState *sphb, int *state);
int (*eeh_reset)(sPAPRPHBState *sphb, int option);
@@ -90,6 +92,12 @@ struct sPAPRPHBState {
int32_t msi_devs_num;
spapr_pci_msi_mig *msi_devs;
+ bool ddw_enabled;
+ uint32_t windows_supported;
+ uint64_t page_size_mask;
+ uint32_t dma32_window_size;
+ uint64_t dma64_window_size;
+
QLIST_ENTRY(sPAPRPHBState) list;
};
@@ -97,6 +105,8 @@ struct sPAPRPHBVFIOState {
sPAPRPHBState phb;
int32_t iommugroupid;
+ uint8_t max_levels;
+ uint8_t levels;
};
#define SPAPR_PCI_MAX_INDEX 255
@@ -117,6 +127,12 @@ struct sPAPRPHBVFIOState {
#define SPAPR_PCI_DMA32_SIZE 0x40000000
+/* Default 64bit dynamic window offset */
+#define SPAPR_PCI_DMA64_START 0x8000000000000000ULL
+
+/* Maximum allowed number of DMA windows for emulated PHB */
+#define SPAPR_PCI_DMA_MAX_WINDOWS 2
+
static inline qemu_irq spapr_phb_lsi_qirq(struct sPAPRPHBState *phb, int pin)
{
return xics_get_qirq(spapr->icp, phb->lsi_table[pin].irq);
@@ -135,6 +151,7 @@ void spapr_pci_rtas_init(void);
sPAPRPHBState *spapr_pci_find_phb(sPAPREnvironment *spapr, uint64_t buid);
PCIDevice *spapr_pci_find_dev(sPAPREnvironment *spapr, uint64_t buid,
uint32_t config_addr);
+int spapr_pci_ddw_remove(sPAPRPHBState *sphb, sPAPRTCETable *tcet);
int spapr_phb_dma_reset(sPAPRPHBState *sphb);
#endif /* __HW_SPAPR_PCI_H__ */
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 5a45c3f..2eff8e2 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -381,6 +381,16 @@ int spapr_allocate_irq_block(int num, bool lsi, bool msi);
#define RTAS_OUT_NOT_SUPPORTED -3
#define RTAS_OUT_NOT_AUTHORIZED -9002
+/* DDW pagesize mask values from ibm,query-pe-dma-window */
+#define RTAS_DDW_PGSIZE_4K 0x01
+#define RTAS_DDW_PGSIZE_64K 0x02
+#define RTAS_DDW_PGSIZE_16M 0x04
+#define RTAS_DDW_PGSIZE_32M 0x08
+#define RTAS_DDW_PGSIZE_64M 0x10
+#define RTAS_DDW_PGSIZE_128M 0x20
+#define RTAS_DDW_PGSIZE_256M 0x40
+#define RTAS_DDW_PGSIZE_16G 0x80
+
/* RTAS tokens */
#define RTAS_TOKEN_BASE 0x2000
@@ -422,8 +432,12 @@ int spapr_allocate_irq_block(int num, bool lsi, bool msi);
#define RTAS_IBM_SET_SLOT_RESET (RTAS_TOKEN_BASE + 0x23)
#define RTAS_IBM_CONFIGURE_PE (RTAS_TOKEN_BASE + 0x24)
#define RTAS_IBM_SLOT_ERROR_DETAIL (RTAS_TOKEN_BASE + 0x25)
+#define RTAS_IBM_QUERY_PE_DMA_WINDOW (RTAS_TOKEN_BASE + 0x26)
+#define RTAS_IBM_CREATE_PE_DMA_WINDOW (RTAS_TOKEN_BASE + 0x27)
+#define RTAS_IBM_REMOVE_PE_DMA_WINDOW (RTAS_TOKEN_BASE + 0x28)
+#define RTAS_IBM_RESET_PE_DMA_WINDOW (RTAS_TOKEN_BASE + 0x29)
-#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x26)
+#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x2A)
/* RTAS ibm,get-system-parameter token values */
#define RTAS_SYSPARM_SPLPAR_CHARACTERISTICS 20
@@ -504,6 +518,7 @@ struct sPAPRTCETable {
uint64_t bus_offset;
uint32_t page_shift;
uint64_t *table;
+ uint64_t *migtable;
bool bypass;
bool vfio_accel;
int fd;
diff --git a/trace-events b/trace-events
index 2739140..fd8ea7a 100644
--- a/trace-events
+++ b/trace-events
@@ -1344,6 +1344,10 @@ spapr_iommu_pci_indirect(uint64_t liobn, uint64_t ioba, uint64_t tce, uint64_t i
spapr_iommu_pci_stuff(uint64_t liobn, uint64_t ioba, uint64_t tce_value, uint64_t npages, uint64_t ret) "liobn=%"PRIx64" ioba=0x%"PRIx64" tcevalue=0x%"PRIx64" npages=%"PRId64" ret=%"PRId64
spapr_iommu_xlate(uint64_t liobn, uint64_t ioba, uint64_t tce, unsigned perm, unsigned pgsize) "liobn=%"PRIx64" 0x%"PRIx64" -> 0x%"PRIx64" perm=%u mask=%x"
spapr_iommu_new_table(uint64_t liobn, void *tcet, void *table, int fd) "liobn=%"PRIx64" tcet=%p table=%p fd=%d"
+spapr_iommu_ddw_query(uint64_t buid, uint32_t cfgaddr, unsigned wa, uint64_t win_size, uint32_t pgmask) "buid=%"PRIx64" addr=%"PRIx32", %u windows available, max window size=%"PRIx64", mask=%"PRIx32
+spapr_iommu_ddw_create(uint64_t buid, uint32_t cfgaddr, unsigned long long pg_size, unsigned long long req_size, uint64_t start, uint32_t liobn, long ret) "buid=%"PRIx64" addr=%"PRIx32", page size=0x%llx, requested=0x%llx, start addr=%"PRIx64", liobn=%"PRIx32", ret = %ld"
+spapr_iommu_ddw_remove(uint32_t liobn, long ret) "liobn=%"PRIx32", ret = %ld"
+spapr_iommu_ddw_reset(uint64_t buid, uint32_t cfgaddr, long ret) "buid=%"PRIx64" addr=%"PRIx32", ret = %ld"
# hw/ppc/ppc.c
ppc_tb_adjust(uint64_t offs1, uint64_t offs2, int64_t diff, int64_t seconds) "adjusted from 0x%"PRIx64" to 0x%"PRIx64", diff %"PRId64" (%"PRId64"s)"
--
2.0.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v5 02/12] vmstate: Define VARRAY with VMS_ALLOC
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 02/12] vmstate: Define VARRAY with VMS_ALLOC Alexey Kardashevskiy
@ 2015-04-08 1:55 ` David Gibson
0 siblings, 0 replies; 26+ messages in thread
From: David Gibson @ 2015-04-08 1:55 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Alex Williamson, qemu-ppc, qemu-devel, Alexander Graf
[-- Attachment #1: Type: text/plain, Size: 1971 bytes --]
On Tue, Mar 31, 2015 at 04:28:37PM +1100, Alexey Kardashevskiy wrote:
> This allows dynamic allocation for migrating arrays.
This commit message really needs more information for the benefit of
people who haven't been buried in the savevm code recently.
What existing VMSTATE macros does it extend? How do its semantics
differ from them? What is this useful for that the existing macros are
not?
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> include/migration/vmstate.h | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index bc7616a..73b9d67 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -299,6 +299,16 @@ extern const VMStateInfo vmstate_info_bitmap;
> .offset = vmstate_offset_pointer(_state, _field, _type), \
> }
>
> +#define VMSTATE_VARRAY_UINT32_ALLOC(_field, _state, _field_num, _version, _info, _type) {\
> + .name = (stringify(_field)), \
> + .version_id = (_version), \
> + .num_offset = vmstate_offset_value(_state, _field_num, uint32_t),\
> + .info = &(_info), \
> + .size = sizeof(_type), \
> + .flags = VMS_VARRAY_UINT32|VMS_POINTER|VMS_ALLOC, \
> + .offset = vmstate_offset_pointer(_state, _field, _type), \
> +}
> +
> #define VMSTATE_VARRAY_UINT16_UNSAFE(_field, _state, _field_num, _version, _info, _type) {\
> .name = (stringify(_field)), \
> .version_id = (_version), \
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v5 04/12] spapr_pci_vfio: Enable multiple groups per container
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 04/12] spapr_pci_vfio: Enable multiple groups per container Alexey Kardashevskiy
@ 2015-04-08 2:01 ` David Gibson
2015-04-08 3:45 ` Alexey Kardashevskiy
0 siblings, 1 reply; 26+ messages in thread
From: David Gibson @ 2015-04-08 2:01 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Alex Williamson, qemu-ppc, qemu-devel, Alexander Graf
[-- Attachment #1: Type: text/plain, Size: 3525 bytes --]
On Tue, Mar 31, 2015 at 04:28:39PM +1100, Alexey Kardashevskiy wrote:
> This enables multiple IOMMU groups in one VFIO container which means
> that multiple devices from different groups can share the same IOMMU
> table (or tables if DDW).
>
> This removes a group id from vfio_container_ioctl(). The kernel support
> is required for this; if the host kernel does not have the support,
> it will allow only one group per container. The PHB's "iommuid" property
> is ignored.
>
> This adds a sanity check that there is just one VFIO container per
> PHB address space.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
[snip]
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index b012620..99e1900 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -915,21 +915,23 @@ void vfio_put_base_device(VFIODevice *vbasedev)
> close(vbasedev->fd);
> }
>
> -static int vfio_container_do_ioctl(AddressSpace *as, int32_t groupid,
> +static int vfio_container_do_ioctl(AddressSpace *as,
> int req, void *param)
> {
> - VFIOGroup *group;
> VFIOContainer *container;
> - int ret = -1;
> + int ret;
> + VFIOAddressSpace *space;
>
> - group = vfio_get_group(groupid, as);
> - if (!group) {
> - error_report("vfio: group %d not registered", groupid);
> - return ret;
> - }
> + space = vfio_get_address_space(as);
> + container = QLIST_FIRST(&space->containers);
So getting the container handle from the address space, rather than
the group id certainly makes more sense to me.
> - container = group->container;
> - if (group->container) {
> + if (!container) {
> + error_report("vfio: container is not set");
> + return -1;
> + } else if (QLIST_NEXT(container, next)) {
> + error_report("vfio: multiple containers per PHB are not supported");
> + return -1;
But if only one PHB per address space is possible, why is the
containers field a list in the first place?
> + } else {
> ret = ioctl(container->fd, req, param);
> if (ret < 0) {
> error_report("vfio: failed to ioctl %d to container: ret=%d, %s",
> @@ -937,12 +939,10 @@ static int vfio_container_do_ioctl(AddressSpace *as, int32_t groupid,
> }
> }
>
> - vfio_put_group(group);
> -
> return ret;
> }
>
> -int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
> +int vfio_container_ioctl(AddressSpace *as,
> int req, void *param)
> {
> /* We allow only certain ioctls to the container */
> @@ -957,5 +957,5 @@ int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
> return -1;
> }
>
> - return vfio_container_do_ioctl(as, groupid, req, param);
> + return vfio_container_do_ioctl(as, req, param);
> }
> diff --git a/include/hw/vfio/vfio.h b/include/hw/vfio/vfio.h
> index 0b26cd8..76b5744 100644
> --- a/include/hw/vfio/vfio.h
> +++ b/include/hw/vfio/vfio.h
> @@ -3,7 +3,7 @@
>
> #include "qemu/typedefs.h"
>
> -extern int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
> +extern int vfio_container_ioctl(AddressSpace *as,
> int req, void *param);
>
> #endif
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v5 05/12] vfio: spapr: Move SPAPR-related code to a separate file
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 05/12] vfio: spapr: Move SPAPR-related code to a separate file Alexey Kardashevskiy
@ 2015-04-08 2:05 ` David Gibson
0 siblings, 0 replies; 26+ messages in thread
From: David Gibson @ 2015-04-08 2:05 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Alex Williamson, qemu-ppc, qemu-devel, Alexander Graf
[-- Attachment #1: Type: text/plain, Size: 793 bytes --]
On Tue, Mar 31, 2015 at 04:28:40PM +1100, Alexey Kardashevskiy wrote:
> This moves SPAPR bits to a separate file to avoid pollution of x86 code.
>
> This is a mechanical patch.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Splitting out the iommu mode specific code seems sensible to me. A
bunch of the functions that have moved look as if they're not
inherently spapr specific - it's just that spapr is the only user so
far. But I guess they can be moved back to common.c when and if
another user comes along.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v5 06/12] vfio: spapr: Add SPAPR IOMMU v2 support (DMA memory preregistering)
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 06/12] vfio: spapr: Add SPAPR IOMMU v2 support (DMA memory preregistering) Alexey Kardashevskiy
@ 2015-04-08 2:15 ` David Gibson
2015-04-08 4:05 ` Alexey Kardashevskiy
0 siblings, 1 reply; 26+ messages in thread
From: David Gibson @ 2015-04-08 2:15 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Alex Williamson, qemu-ppc, qemu-devel, Alexander Graf
[-- Attachment #1: Type: text/plain, Size: 10243 bytes --]
On Tue, Mar 31, 2015 at 04:28:41PM +1100, Alexey Kardashevskiy wrote:
> This makes use of the new "memory registering" feature. The idea is
> to provide the guest ability to notify the host kernel about pages which
AFAICT it's not really the guest informing the host, just qemu
informing the host. If I'm reading the code correctly, qemu registers
all RAM regionns, without guest intervention.
> are going to be used for DMA. Having this information, the host kernel
> can pin them all once per user process, do locked pages accounting (once)
> and not spent time on doing that in real time with possible failures
> which cannot be handled nicely in some cases.
>
> This adds a guest RAM memory listener which notifies a VFIO container
> about memory which needs to be pinned/unpinned. VFIO MMIO regions
> (i.e. "skip dump" regions) are skipped.
>
> The feature is only enabled for SPAPR IOMMU v2. The host kernel changes
> are required. Since v2 does not need/support VFIO_IOMMU_ENABLE, this does
> not call it when v2 is detected and enabled.
So I'm clear, the v2 just represents a new userspace<->host kernel
interface for controlling the IOMMU, doesn't it? It doesn't change
the guest visible IOMMU interface and doesn't represent different
IOMMU hardware?
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> Changes:
> v5:
> * simplified the patch
> * added trace points
> * added round_up() for the size
> * SPAPR IOMMU v2 used
> ---
> hw/vfio/common.c | 26 +++++++++-----
> hw/vfio/spapr.c | 79 +++++++++++++++++++++++++++++++++++++++++--
> include/hw/vfio/vfio-common.h | 5 ++-
> trace-events | 1 +
> 4 files changed, 100 insertions(+), 11 deletions(-)
>
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index a71f881..e35e478 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -577,14 +577,18 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as)
>
> container->iommu_data.type1.initialized = true;
>
> - } else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU)) {
> + } else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU) ||
> + ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_v2_IOMMU)) {
> + bool v2 = !!ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_v2_IOMMU);
> +
> ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, &fd);
> if (ret) {
> error_report("vfio: failed to set group container: %m");
> ret = -errno;
> goto free_container_exit;
> }
> - ret = ioctl(fd, VFIO_SET_IOMMU, VFIO_SPAPR_TCE_IOMMU);
> + ret = ioctl(fd, VFIO_SET_IOMMU,
> + v2 ? VFIO_SPAPR_TCE_v2_IOMMU : VFIO_SPAPR_TCE_IOMMU);
> if (ret) {
> error_report("vfio: failed to set iommu for container: %m");
> ret = -errno;
> @@ -596,14 +600,20 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as)
> * when container fd is closed so we do not call it explicitly
> * in this file.
> */
> - ret = ioctl(fd, VFIO_IOMMU_ENABLE);
> - if (ret) {
> - error_report("vfio: failed to enable container: %m");
> - ret = -errno;
> - goto free_container_exit;
> + if (!v2) {
> + ret = ioctl(fd, VFIO_IOMMU_ENABLE);
> + if (ret) {
> + error_report("vfio: failed to enable container: %m");
> + ret = -errno;
> + goto free_container_exit;
> + }
> }
>
> - spapr_memory_listener_register(container);
> + ret = spapr_memory_listener_register(container, v2 ? 2 : 1);
> + if (ret) {
> + error_report("vfio: RAM memory listener initialization failed for container");
> + goto listener_release_exit;
> + }
>
> } else {
> error_report("vfio: No available IOMMU models");
> diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
> index 5f79194..a670907 100644
> --- a/hw/vfio/spapr.c
> +++ b/hw/vfio/spapr.c
> @@ -17,6 +17,9 @@
> * along with this program; if not, see <http://www.gnu.org/licenses/>.
> */
>
> +#include <sys/ioctl.h>
> +#include <linux/vfio.h>
> +
> #include "hw/vfio/vfio-common.h"
> #include "qemu/error-report.h"
> #include "trace.h"
> @@ -211,16 +214,88 @@ static const MemoryListener vfio_spapr_memory_listener = {
> .region_del = vfio_spapr_listener_region_del,
> };
>
> +static void vfio_ram_do_region(VFIOContainer *container,
> + MemoryRegionSection *section, unsigned long req)
> +{
> + int ret;
> + struct vfio_iommu_spapr_register_memory reg = { .argsz = sizeof(reg) };
> +
> + if (!memory_region_is_ram(section->mr) ||
> + memory_region_is_skip_dump(section->mr)) {
> + return;
> + }
> +
> + reg.vaddr = (__u64) memory_region_get_ram_ptr(section->mr) +
> + section->offset_within_region;
Is section->offset_within_region always page aligned?
> + reg.size = ROUND_UP(int128_get64(section->size), TARGET_PAGE_SIZE);
> +
> + ret = ioctl(container->fd, req, ®);
> + trace_vfio_ram_register(_IOC_NR(req) - VFIO_BASE, reg.vaddr, reg.size, ret);
> +
> + /*
> + * On the initfn path, store the first error in the container so we
> + * can gracefully fail. Runtime, there's not much we can do other
> + * than throw a hardware error.
> + */
> + if (!container->iommu_data.spapr.ram_reg_initialized) {
> + if (!container->iommu_data.spapr.ram_reg_error) {
> + container->iommu_data.spapr.ram_reg_error = ret;
> + }
This is pretty clunky, but I don't immediately see a better way.
Also.. won't the return value of ioctl() just be -1 on error, which
won't tell you much. Do you want to store errno, instead?
> + } else {
> + hw_error("vfio: RAM registering failed, unable to continue");
> + }
> +}
> +
> +static void vfio_spapr_ram_listener_region_add(MemoryListener *listener,
> + MemoryRegionSection *section)
> +{
> + VFIOContainer *container = container_of(listener, VFIOContainer,
> + iommu_data.spapr.ramlistener);
> + memory_region_ref(section->mr);
> + vfio_ram_do_region(container, section, VFIO_IOMMU_SPAPR_REGISTER_MEMORY);
> +}
> +
> +static void vfio_spapr_ram_listener_region_del(MemoryListener *listener,
> + MemoryRegionSection *section)
> +{
> + VFIOContainer *container = container_of(listener, VFIOContainer,
> + iommu_data.spapr.ramlistener);
> + memory_region_unref(section->mr);
> + vfio_ram_do_region(container, section, VFIO_IOMMU_SPAPR_UNREGISTER_MEMORY);
> +}
> +
> +static const MemoryListener vfio_spapr_ram_memory_listener = {
> + .region_add = vfio_spapr_ram_listener_region_add,
> + .region_del = vfio_spapr_ram_listener_region_del,
> +};
> +
> static void vfio_spapr_listener_release(VFIOContainer *container)
> {
> memory_listener_unregister(&container->iommu_data.spapr.listener);
> }
>
> -void spapr_memory_listener_register(VFIOContainer *container)
> +static void vfio_spapr_listener_release_v2(VFIOContainer *container)
> +{
> + memory_listener_unregister(&container->iommu_data.spapr.listener);
> + vfio_spapr_listener_release(container);
> +}
> +
> +int spapr_memory_listener_register(VFIOContainer *container, int ver)
> {
> container->iommu_data.spapr.listener = vfio_spapr_memory_listener;
> container->iommu_data.release = vfio_spapr_listener_release;
> -
> memory_listener_register(&container->iommu_data.spapr.listener,
> container->space->as);
> + if (ver < 2) {
> + return 0;
> + }
> +
> + container->iommu_data.spapr.ramlistener = vfio_spapr_ram_memory_listener;
> + container->iommu_data.release = vfio_spapr_listener_release_v2;
> + memory_listener_register(&container->iommu_data.spapr.ramlistener,
> + &address_space_memory);
> +
> + container->iommu_data.spapr.ram_reg_initialized = true;
> +
> + return container->iommu_data.spapr.ram_reg_error;
> }
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index d0b831c..b5ef446 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -71,6 +71,9 @@ typedef struct VFIOType1 {
>
> typedef struct VFIOSPAPR {
> MemoryListener listener;
> + MemoryListener ramlistener;
> + int ram_reg_error;
> + bool ram_reg_initialized;
> } VFIOSPAPR;
>
> typedef struct VFIOContainer {
> @@ -156,6 +159,6 @@ extern int vfio_dma_unmap(VFIOContainer *container,
> hwaddr iova, ram_addr_t size);
> bool vfio_listener_skipped_section(MemoryRegionSection *section);
>
> -extern void spapr_memory_listener_register(VFIOContainer *container);
> +extern int spapr_memory_listener_register(VFIOContainer *container, int ver);
>
> #endif /* !HW_VFIO_VFIO_COMMON_H */
> diff --git a/trace-events b/trace-events
> index 1231ba4..2739140 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -1563,6 +1563,7 @@ vfio_disconnect_container(int fd) "close container->fd=%d"
> vfio_put_group(int fd) "close group->fd=%d"
> vfio_get_device(const char * name, unsigned int flags, unsigned int num_regions, unsigned int num_irqs) "Device %s flags: %u, regions: %u, irqs: %u"
> vfio_put_base_device(int fd) "close vdev->fd=%d"
> +vfio_ram_register(int req, uint64_t va, uint64_t size, int ret) "req=%d va=%"PRIx64" size=%"PRIx64" ret=%d"
>
> #hw/acpi/memory_hotplug.c
> mhp_acpi_invalid_slot_selected(uint32_t slot) "0x%"PRIx32
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v5 07/12] spapr_iommu: Rework TCE table initialization
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 07/12] spapr_iommu: Rework TCE table initialization Alexey Kardashevskiy
@ 2015-04-08 2:35 ` David Gibson
0 siblings, 0 replies; 26+ messages in thread
From: David Gibson @ 2015-04-08 2:35 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Alex Williamson, qemu-ppc, qemu-devel, Alexander Graf
[-- Attachment #1: Type: text/plain, Size: 12950 bytes --]
On Tue, Mar 31, 2015 at 04:28:42PM +1100, Alexey Kardashevskiy wrote:
> Currently TCE tables are created once at start and their size never
> changes. We are going to change that by introducing a Dynamic DMA windows
> support where DMA configuration may change during the guest execution.
>
> This changes spapr_tce_new_table() to create an empty stub object. Only
> LIOBN is assigned by the time of creation. It still will be called once
> at the owner object (VIO or PHB) creation.
>
> This introduces spapr_tce_set_props() to set the table size, start and
> page size. It only assigns the properties. It will be called at the owner
> object creation OR later from the "ibm,create-pe-dma-window" RTAS handler
> so the table's parameters can change.
>
> This introduces an "enabled" state for TCE table objects with two
> helper functions - spapr_tce_table_enable()/spapr_tce_table_disable().
> spapr_tce_table_enable() allocates the guest view of the TCE table
> (in the user space or KVM). spapr_tce_table_disable() disposes the table.
>
> Follow up patches will disable+enable tables on reset (system reset
> or DDW reset).
>
> No visible change in behaviour is expected except the actual table
> will be reallocated every reset. We might optimize this later.
>
> The other way to implement this would be dynamically create/remove
> the TCE table QOM objects but this would make migration impossible
> as migration expects all QOM objects to exist at the receiver
> so we have to have TCE table objects created when migration begins.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> hw/ppc/spapr_iommu.c | 98 +++++++++++++++++++++++++++++++------------------
> hw/ppc/spapr_pci.c | 8 ++--
> hw/ppc/spapr_pci_vfio.c | 11 ++++--
> hw/ppc/spapr_vio.c | 10 ++---
> include/hw/ppc/spapr.h | 12 +++---
> 5 files changed, 87 insertions(+), 52 deletions(-)
>
> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
> index a14cdc4..a015357 100644
> --- a/hw/ppc/spapr_iommu.c
> +++ b/hw/ppc/spapr_iommu.c
> @@ -126,25 +126,6 @@ static MemoryRegionIOMMUOps spapr_iommu_ops = {
> static int spapr_tce_table_realize(DeviceState *dev)
> {
> sPAPRTCETable *tcet = SPAPR_TCE_TABLE(dev);
> - uint64_t window_size = (uint64_t)tcet->nb_table << tcet->page_shift;
> -
> - if (kvm_enabled() && !(window_size >> 32)) {
> - tcet->table = kvmppc_create_spapr_tce(tcet->liobn,
> - window_size,
> - &tcet->fd,
> - tcet->vfio_accel);
> - }
> -
> - if (!tcet->table) {
> - size_t table_size = tcet->nb_table * sizeof(uint64_t);
> - tcet->table = g_malloc0(table_size);
> - }
> -
> - trace_spapr_iommu_new_table(tcet->liobn, tcet, tcet->table, tcet->fd);
> -
> - memory_region_init_iommu(&tcet->iommu, OBJECT(dev), &spapr_iommu_ops,
> - "iommu-spapr",
> - (uint64_t)tcet->nb_table << tcet->page_shift);
>
> QLIST_INSERT_HEAD(&spapr_tce_tables, tcet, list);
>
> @@ -154,11 +135,7 @@ static int spapr_tce_table_realize(DeviceState *dev)
> return 0;
> }
>
> -sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn,
> - uint64_t bus_offset,
> - uint32_t page_shift,
> - uint32_t nb_table,
> - bool vfio_accel)
> +sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn)
> {
> sPAPRTCETable *tcet;
> char tmp[64];
> @@ -169,36 +146,87 @@ sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn,
> return NULL;
> }
>
> - if (!nb_table) {
> - return NULL;
> - }
> -
> tcet = SPAPR_TCE_TABLE(object_new(TYPE_SPAPR_TCE_TABLE));
> tcet->liobn = liobn;
> - tcet->bus_offset = bus_offset;
> - tcet->page_shift = page_shift;
> - tcet->nb_table = nb_table;
> - tcet->vfio_accel = vfio_accel;
>
> snprintf(tmp, sizeof(tmp), "tce-table-%x", liobn);
> object_property_add_child(OBJECT(owner), tmp, OBJECT(tcet), NULL);
>
> object_property_set_bool(OBJECT(tcet), true, "realized", NULL);
>
> + trace_spapr_iommu_new_table(tcet->liobn, tcet, tcet->table, tcet->fd);
> +
> return tcet;
> }
>
> -static void spapr_tce_table_unrealize(DeviceState *dev, Error **errp)
> +void spapr_tce_set_props(sPAPRTCETable *tcet, uint64_t bus_offset,
> + uint32_t page_shift, uint32_t nb_table,
> + bool vfio_accel)
> {
> - sPAPRTCETable *tcet = SPAPR_TCE_TABLE(dev);
> + if (tcet->enabled) {
> + return;
> + }
Since you can't change the properties while the table is enabled, why
not just make these parameters to spapr_tce_table_enable().
It seems to me what this is really about is making a distinction
between two objects: (1) is the TCE table as an abstract concept - it
knows its liobn and its owner, and that's about it (2) the TCE table
as a specific instantiated table - it has a specific size and current
entries.
(2) can't be a QOM object or migration breaks, but you can still think
of it as a distinct entity at the C level.
> + tcet->bus_offset = bus_offset;
> + tcet->page_shift = page_shift;
> + tcet->nb_table = nb_table;
> + tcet->vfio_accel = vfio_accel;
> +}
>
> - QLIST_REMOVE(tcet, list);
> +void spapr_tce_table_enable(sPAPRTCETable *tcet)
> +{
> + uint64_t window_size = (uint64_t)tcet->nb_table << tcet->page_shift;
> +
> + if (tcet->enabled) {
> + return;
> + }
> +
> + if (!tcet->nb_table) {
> + return;
> + }
> +
> + if (kvm_enabled() && !(window_size >> 32)) {
> + tcet->table = kvmppc_create_spapr_tce(tcet->liobn,
> + window_size,
> + &tcet->fd,
> + tcet->vfio_accel);
> + }
> +
> + if (!tcet->table) {
> + size_t table_size = tcet->nb_table * sizeof(uint64_t);
> + tcet->table = g_malloc0(table_size);
> + }
> +
> + memory_region_init_iommu(&tcet->iommu, OBJECT(tcet), &spapr_iommu_ops,
> + "iommu-spapr",
> + (uint64_t)tcet->nb_table << tcet->page_shift);
> +
> + tcet->enabled = true;
> +}
> +
> +void spapr_tce_table_disable(sPAPRTCETable *tcet)
> +{
> + if (!tcet->enabled) {
> + return;
> + }
>
> if (!kvm_enabled() ||
> (kvmppc_remove_spapr_tce(tcet->table, tcet->fd,
> tcet->nb_table) != 0)) {
> + tcet->fd = -1;
> g_free(tcet->table);
> }
> + tcet->table = NULL;
> + tcet->enabled = false;
> + spapr_tce_set_props(tcet, 0, 0, 0, false);
> +}
> +
> +static void spapr_tce_table_unrealize(DeviceState *dev, Error **errp)
> +{
> + sPAPRTCETable *tcet = SPAPR_TCE_TABLE(dev);
> +
> + QLIST_REMOVE(tcet, list);
> +
> + spapr_tce_table_disable(tcet);
> }
>
> MemoryRegion *spapr_tce_get_iommu(sPAPRTCETable *tcet)
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 52c5c73..acfdbe5 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -895,15 +895,17 @@ static void spapr_phb_finish_realize(sPAPRPHBState *sphb, Error **errp)
> sPAPRTCETable *tcet;
> uint32_t nb_table;
>
> - nb_table = SPAPR_PCI_DMA32_SIZE >> SPAPR_TCE_PAGE_SHIFT;
> - tcet = spapr_tce_new_table(DEVICE(sphb), sphb->dma_liobn,
> - 0, SPAPR_TCE_PAGE_SHIFT, nb_table, false);
> + tcet = spapr_tce_new_table(DEVICE(sphb), sphb->dma_liobn);
> if (!tcet) {
> error_setg(errp, "Unable to create TCE table for %s",
> sphb->dtbusname);
> return ;
> }
>
> + nb_table = SPAPR_PCI_DMA32_SIZE >> SPAPR_TCE_PAGE_SHIFT;
> + spapr_tce_set_props(tcet, 0, SPAPR_TCE_PAGE_SHIFT, nb_table, false);
> + spapr_tce_table_enable(tcet);
> +
> /* Register default 32bit DMA window */
> memory_region_add_subregion(&sphb->iommu_root, 0,
> spapr_tce_get_iommu(tcet));
> diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
> index f8b503e..6c9adb5 100644
> --- a/hw/ppc/spapr_pci_vfio.c
> +++ b/hw/ppc/spapr_pci_vfio.c
> @@ -34,6 +34,7 @@ static void spapr_phb_vfio_finish_realize(sPAPRPHBState *sphb, Error **errp)
> int ret;
> sPAPRTCETable *tcet;
> uint32_t liobn = svphb->phb.dma_liobn;
> + uint32_t nb_table;
>
> ret = vfio_container_ioctl(&svphb->phb.iommu_as,
> VFIO_CHECK_EXTENSION,
> @@ -52,16 +53,18 @@ static void spapr_phb_vfio_finish_realize(sPAPRPHBState *sphb, Error **errp)
> return;
> }
>
> - tcet = spapr_tce_new_table(DEVICE(sphb), liobn, info.dma32_window_start,
> - SPAPR_TCE_PAGE_SHIFT,
> - info.dma32_window_size >> SPAPR_TCE_PAGE_SHIFT,
> - true);
> + tcet = spapr_tce_new_table(DEVICE(sphb), liobn);
> if (!tcet) {
> error_setg(errp, "spapr-vfio: failed to create VFIO TCE table");
> return;
> }
>
> /* Register default 32bit DMA window */
> + nb_table = info.dma32_window_size >> SPAPR_TCE_PAGE_SHIFT;
> + spapr_tce_set_props(tcet, info.dma32_window_start, SPAPR_TCE_PAGE_SHIFT,
> + nb_table, true);
> + spapr_tce_table_enable(tcet);
> +
> memory_region_add_subregion(&sphb->iommu_root, tcet->bus_offset,
> spapr_tce_get_iommu(tcet));
> }
> diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
> index 174033d..6394527 100644
> --- a/hw/ppc/spapr_vio.c
> +++ b/hw/ppc/spapr_vio.c
> @@ -479,11 +479,11 @@ static void spapr_vio_busdev_realize(DeviceState *qdev, Error **errp)
> memory_region_add_subregion_overlap(&dev->mrroot, 0, &dev->mrbypass, 1);
> address_space_init(&dev->as, &dev->mrroot, qdev->id);
>
> - dev->tcet = spapr_tce_new_table(qdev, liobn,
> - 0,
> - SPAPR_TCE_PAGE_SHIFT,
> - pc->rtce_window_size >>
> - SPAPR_TCE_PAGE_SHIFT, false);
> + dev->tcet = spapr_tce_new_table(qdev, liobn);
> + spapr_tce_set_props(dev->tcet, 0, SPAPR_TCE_PAGE_SHIFT,
> + pc->rtce_window_size >> SPAPR_TCE_PAGE_SHIFT,
> + false);
> + spapr_tce_table_enable(dev->tcet);
> dev->tcet->vdev = dev;
> memory_region_add_subregion_overlap(&dev->mrroot, 0,
> spapr_tce_get_iommu(dev->tcet), 2);
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 7d9ab9d..6e33b9b 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -498,6 +498,7 @@ typedef struct sPAPRTCETable sPAPRTCETable;
>
> struct sPAPRTCETable {
> DeviceState parent;
> + bool enabled;
> uint32_t liobn;
> uint32_t nb_table;
> uint64_t bus_offset;
> @@ -515,11 +516,12 @@ sPAPRTCETable *spapr_tce_find_by_liobn(uint32_t liobn);
> void spapr_events_init(sPAPREnvironment *spapr);
> void spapr_events_fdt_skel(void *fdt, uint32_t epow_irq);
> int spapr_h_cas_compose_response(target_ulong addr, target_ulong size);
> -sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn,
> - uint64_t bus_offset,
> - uint32_t page_shift,
> - uint32_t nb_table,
> - bool vfio_accel);
> +sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn);
> +void spapr_tce_set_props(sPAPRTCETable *tcet, uint64_t bus_offset,
> + uint32_t page_shift, uint32_t nb_table,
> + bool vfio_accel);
> +void spapr_tce_table_enable(sPAPRTCETable *tcet);
> +void spapr_tce_table_disable(sPAPRTCETable *tcet);
> MemoryRegion *spapr_tce_get_iommu(sPAPRTCETable *tcet);
> int spapr_dma_dt(void *fdt, int node_off, const char *propname,
> uint32_t liobn, uint64_t window, uint32_t size);
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v5 08/12] spapr_pci: Rework reset to reset DMA configuration
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 08/12] spapr_pci: Rework reset to reset DMA configuration Alexey Kardashevskiy
@ 2015-04-08 2:42 ` David Gibson
0 siblings, 0 replies; 26+ messages in thread
From: David Gibson @ 2015-04-08 2:42 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Alex Williamson, qemu-ppc, qemu-devel, Alexander Graf
[-- Attachment #1: Type: text/plain, Size: 7276 bytes --]
On Tue, Mar 31, 2015 at 04:28:43PM +1100, Alexey Kardashevskiy wrote:
> On a system reset, DMA configuration has to reset too. At the moment
> it clears the table content. This is enough for the single table case
> but with DDW, we will also have to disable all DMA windows except
> the default one. Furthermore according to sPAPR, if the guest removed
> the default window and created a huge one at the same zero offset on
> a PCI bus, the reset handler has to recreate the default window with
> the default properties (2GB big, 4K pages).
>
> This reworks SPAPR PHB code to disable the existing DMA window on reset
> and then configure and enable the default window.
> Without DDW that means that the same window will be disabled and then
> enabled with no other change in behaviour.
>
> This changes the table creation to do it in one place in PHB (VFIO PHB
> just inherits the behaviour from PHB). The actual table allocation is
> done from the reset handler and this is where finish_realize() is called.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> hw/ppc/spapr_pci.c | 42 ++++++++++++++++++++++++------------------
> hw/ppc/spapr_pci_vfio.c | 17 +----------------
> include/hw/pci-host/spapr.h | 1 +
> 3 files changed, 26 insertions(+), 34 deletions(-)
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index acfdbe5..57bbc82 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -722,6 +722,22 @@ static const MemoryRegionOps spapr_msi_ops = {
> };
>
> /*
> + * DMA windows
> + */
> +int spapr_phb_dma_reset(sPAPRPHBState *sphb)
> +{
> + const uint32_t liobn = SPAPR_PCI_LIOBN(sphb->index, 0);
> + sPAPRTCETable *tcet = spapr_tce_find_by_liobn(liobn);
> + sPAPRPHBClass *spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
> + Error *err = NULL;
> +
> + spapr_tce_table_disable(tcet);
> + spc->finish_realize(sphb, &err);
Looks like "finish_realize" is no longer the right name for this hook.
> +
> + return 0;
> +}
> +
> +/*
> * PHB PCI device
> */
> static AddressSpace *spapr_pci_dma_iommu(PCIBus *bus, void *opaque, int devfn)
> @@ -736,11 +752,11 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
> SysBusDevice *s = SYS_BUS_DEVICE(dev);
> sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(s);
> PCIHostState *phb = PCI_HOST_BRIDGE(s);
> - sPAPRPHBClass *info = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(s);
> char *namebuf;
> int i;
> PCIBus *bus;
> uint64_t msi_window_size = 4096;
> + sPAPRTCETable *tcet;
>
> if (sphb->index != (uint32_t)-1) {
> hwaddr windows_base;
> @@ -880,12 +896,10 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
> sphb->lsi_table[i].irq = irq;
> }
>
> - if (!info->finish_realize) {
> - error_setg(errp, "finish_realize not defined");
> - return;
> - }
> -
> - info->finish_realize(sphb, errp);
> + /* Create default DMA window */
> + tcet = spapr_tce_new_table(DEVICE(sphb), sphb->dma_liobn);
> + memory_region_add_subregion_overlap(&sphb->iommu_root, 0,
> + spapr_tce_get_iommu(tcet), 0);
Why is this done explicitly, rather than using the set_props and
_enable() functions from the last patch?
Also, you shouldn't need this here - even on the first boot, the reset
hook will be called between realize and actually starting the guest,
so you should be able to delay the table allocation to there.
The reason for changing from add_subregion() to
add_subregion_overlap() also isn't clear.
> sphb->msi = g_hash_table_new_full(g_int_hash, g_int_equal, g_free, g_free);
> }
> @@ -895,20 +909,10 @@ static void spapr_phb_finish_realize(sPAPRPHBState *sphb, Error **errp)
> sPAPRTCETable *tcet;
> uint32_t nb_table;
>
> - tcet = spapr_tce_new_table(DEVICE(sphb), sphb->dma_liobn);
> - if (!tcet) {
> - error_setg(errp, "Unable to create TCE table for %s",
> - sphb->dtbusname);
> - return ;
> - }
> -
> + tcet = spapr_tce_find_by_liobn(sphb->dma_liobn);
> nb_table = SPAPR_PCI_DMA32_SIZE >> SPAPR_TCE_PAGE_SHIFT;
> spapr_tce_set_props(tcet, 0, SPAPR_TCE_PAGE_SHIFT, nb_table, false);
> spapr_tce_table_enable(tcet);
> -
> - /* Register default 32bit DMA window */
> - memory_region_add_subregion(&sphb->iommu_root, 0,
> - spapr_tce_get_iommu(tcet));
> }
>
> static int spapr_phb_children_reset(Object *child, void *opaque)
> @@ -924,6 +928,8 @@ static int spapr_phb_children_reset(Object *child, void *opaque)
>
> static void spapr_phb_reset(DeviceState *qdev)
> {
> + spapr_phb_dma_reset(SPAPR_PCI_HOST_BRIDGE(qdev));
> +
> /* Reset the IOMMU state */
> object_child_foreach(OBJECT(qdev), spapr_phb_children_reset, NULL);
> }
> diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
> index 6c9adb5..1657f6b 100644
> --- a/hw/ppc/spapr_pci_vfio.c
> +++ b/hw/ppc/spapr_pci_vfio.c
> @@ -53,25 +53,11 @@ static void spapr_phb_vfio_finish_realize(sPAPRPHBState *sphb, Error **errp)
> return;
> }
>
> - tcet = spapr_tce_new_table(DEVICE(sphb), liobn);
> - if (!tcet) {
> - error_setg(errp, "spapr-vfio: failed to create VFIO TCE table");
> - return;
> - }
> -
> - /* Register default 32bit DMA window */
> + tcet = spapr_tce_find_by_liobn(liobn);
> nb_table = info.dma32_window_size >> SPAPR_TCE_PAGE_SHIFT;
> spapr_tce_set_props(tcet, info.dma32_window_start, SPAPR_TCE_PAGE_SHIFT,
> nb_table, true);
> spapr_tce_table_enable(tcet);
> -
> - memory_region_add_subregion(&sphb->iommu_root, tcet->bus_offset,
> - spapr_tce_get_iommu(tcet));
> -}
> -
> -static void spapr_phb_vfio_reset(DeviceState *qdev)
> -{
> - /* Do nothing */
> }
>
> static int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
> @@ -191,7 +177,6 @@ static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data)
> sPAPRPHBClass *spc = SPAPR_PCI_HOST_BRIDGE_CLASS(klass);
>
> dc->props = spapr_phb_vfio_properties;
> - dc->reset = spapr_phb_vfio_reset;
> spc->finish_realize = spapr_phb_vfio_finish_realize;
> spc->eeh_set_option = spapr_phb_vfio_eeh_set_option;
> spc->eeh_get_state = spapr_phb_vfio_eeh_get_state;
> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> index 5b497ce..f592276 100644
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -134,5 +134,6 @@ void spapr_pci_rtas_init(void);
> sPAPRPHBState *spapr_pci_find_phb(sPAPREnvironment *spapr, uint64_t buid);
> PCIDevice *spapr_pci_find_dev(sPAPREnvironment *spapr, uint64_t buid,
> uint32_t config_addr);
> +int spapr_phb_dma_reset(sPAPRPHBState *sphb);
>
> #endif /* __HW_SPAPR_PCI_H__ */
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v5 04/12] spapr_pci_vfio: Enable multiple groups per container
2015-04-08 2:01 ` David Gibson
@ 2015-04-08 3:45 ` Alexey Kardashevskiy
2015-04-09 6:43 ` David Gibson
0 siblings, 1 reply; 26+ messages in thread
From: Alexey Kardashevskiy @ 2015-04-08 3:45 UTC (permalink / raw)
To: David Gibson; +Cc: Alex Williamson, qemu-ppc, qemu-devel, Alexander Graf
On 04/08/2015 12:01 PM, David Gibson wrote:
> On Tue, Mar 31, 2015 at 04:28:39PM +1100, Alexey Kardashevskiy wrote:
>> This enables multiple IOMMU groups in one VFIO container which means
>> that multiple devices from different groups can share the same IOMMU
>> table (or tables if DDW).
>>
>> This removes a group id from vfio_container_ioctl(). The kernel support
>> is required for this; if the host kernel does not have the support,
>> it will allow only one group per container. The PHB's "iommuid" property
>> is ignored.
>>
>> This adds a sanity check that there is just one VFIO container per
>> PHB address space.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>
> [snip]
>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>> index b012620..99e1900 100644
>> --- a/hw/vfio/common.c
>> +++ b/hw/vfio/common.c
>> @@ -915,21 +915,23 @@ void vfio_put_base_device(VFIODevice *vbasedev)
>> close(vbasedev->fd);
>> }
>>
>> -static int vfio_container_do_ioctl(AddressSpace *as, int32_t groupid,
>> +static int vfio_container_do_ioctl(AddressSpace *as,
>> int req, void *param)
>> {
>> - VFIOGroup *group;
>> VFIOContainer *container;
>> - int ret = -1;
>> + int ret;
>> + VFIOAddressSpace *space;
>>
>> - group = vfio_get_group(groupid, as);
>> - if (!group) {
>> - error_report("vfio: group %d not registered", groupid);
>> - return ret;
>> - }
>> + space = vfio_get_address_space(as);
>> + container = QLIST_FIRST(&space->containers);
>
> So getting the container handle from the address space, rather than
> the group id certainly makes more sense to me.
>
>> - container = group->container;
>> - if (group->container) {
>> + if (!container) {
>> + error_report("vfio: container is not set");
>> + return -1;
>> + } else if (QLIST_NEXT(container, next)) {
>> + error_report("vfio: multiple containers per PHB are not supported");
>> + return -1;
>
> But if only one PHB per address space is possible, why is the
> containers field a list in the first place?
Historically the list was added in 3df3e0a5872 (the patch of yours :) ).
In theory we could implement spapr-pci-bridge (derived from pci-bridge)
with isolation capability (i.e. its own LIOBN/DMA window), in this case
there could be multiple containers per PHB address space. Other archs could
want multiple containers for some other reason. It would help me a lot if
you remembered why you kept the list at the first place :)
For now I guess I'll move the next patch ("vfio: spapr: Move SPAPR-related
code to a separate file") before this one, do s/vfio_container_do_ioctl/
vfio_spapr_container_do_ioctl/ and move it to hw/vfio/spapr.c. Makes sense?
>> + } else {
>> ret = ioctl(container->fd, req, param);
>> if (ret < 0) {
>> error_report("vfio: failed to ioctl %d to container: ret=%d, %s",
>> @@ -937,12 +939,10 @@ static int vfio_container_do_ioctl(AddressSpace *as, int32_t groupid,
>> }
>> }
>>
>> - vfio_put_group(group);
>> -
>> return ret;
>> }
>>
>> -int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
>> +int vfio_container_ioctl(AddressSpace *as,
>> int req, void *param)
>> {
>> /* We allow only certain ioctls to the container */
>> @@ -957,5 +957,5 @@ int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
>> return -1;
>> }
>>
>> - return vfio_container_do_ioctl(as, groupid, req, param);
>> + return vfio_container_do_ioctl(as, req, param);
>> }
>> diff --git a/include/hw/vfio/vfio.h b/include/hw/vfio/vfio.h
>> index 0b26cd8..76b5744 100644
>> --- a/include/hw/vfio/vfio.h
>> +++ b/include/hw/vfio/vfio.h
>> @@ -3,7 +3,7 @@
>>
>> #include "qemu/typedefs.h"
>>
>> -extern int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
>> +extern int vfio_container_ioctl(AddressSpace *as,
>> int req, void *param);
>>
>> #endif
>
--
Alexey
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v5 06/12] vfio: spapr: Add SPAPR IOMMU v2 support (DMA memory preregistering)
2015-04-08 2:15 ` David Gibson
@ 2015-04-08 4:05 ` Alexey Kardashevskiy
2015-04-08 5:11 ` David Gibson
0 siblings, 1 reply; 26+ messages in thread
From: Alexey Kardashevskiy @ 2015-04-08 4:05 UTC (permalink / raw)
To: David Gibson; +Cc: Alex Williamson, qemu-ppc, qemu-devel, Alexander Graf
On 04/08/2015 12:15 PM, David Gibson wrote:
> On Tue, Mar 31, 2015 at 04:28:41PM +1100, Alexey Kardashevskiy wrote:
>> This makes use of the new "memory registering" feature. The idea is
>> to provide the guest ability to notify the host kernel about pages which
>
> AFAICT it's not really the guest informing the host, just qemu
> informing the host. If I'm reading the code correctly, qemu registers
> all RAM regionns, without guest intervention.
Right. Wrong commit log, I'll fix it.
>> are going to be used for DMA. Having this information, the host kernel
>> can pin them all once per user process, do locked pages accounting (once)
>> and not spent time on doing that in real time with possible failures
>> which cannot be handled nicely in some cases.
>>
>> This adds a guest RAM memory listener which notifies a VFIO container
>> about memory which needs to be pinned/unpinned. VFIO MMIO regions
>> (i.e. "skip dump" regions) are skipped.
>>
>> The feature is only enabled for SPAPR IOMMU v2. The host kernel changes
>> are required. Since v2 does not need/support VFIO_IOMMU_ENABLE, this does
>> not call it when v2 is detected and enabled.
>
> So I'm clear, the v2 just represents a new userspace<->host kernel
> interface for controlling the IOMMU, doesn't it? It doesn't change
> the guest visible IOMMU interface and doesn't represent different
> IOMMU hardware?
Exactly true.
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>> Changes:
>> v5:
>> * simplified the patch
>> * added trace points
>> * added round_up() for the size
>> * SPAPR IOMMU v2 used
>> ---
>> hw/vfio/common.c | 26 +++++++++-----
>> hw/vfio/spapr.c | 79 +++++++++++++++++++++++++++++++++++++++++--
>> include/hw/vfio/vfio-common.h | 5 ++-
>> trace-events | 1 +
>> 4 files changed, 100 insertions(+), 11 deletions(-)
>>
>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>> index a71f881..e35e478 100644
>> --- a/hw/vfio/common.c
>> +++ b/hw/vfio/common.c
>> @@ -577,14 +577,18 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as)
>>
>> container->iommu_data.type1.initialized = true;
>>
>> - } else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU)) {
>> + } else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU) ||
>> + ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_v2_IOMMU)) {
>> + bool v2 = !!ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_v2_IOMMU);
>> +
>> ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, &fd);
>> if (ret) {
>> error_report("vfio: failed to set group container: %m");
>> ret = -errno;
>> goto free_container_exit;
>> }
>> - ret = ioctl(fd, VFIO_SET_IOMMU, VFIO_SPAPR_TCE_IOMMU);
>> + ret = ioctl(fd, VFIO_SET_IOMMU,
>> + v2 ? VFIO_SPAPR_TCE_v2_IOMMU : VFIO_SPAPR_TCE_IOMMU);
>> if (ret) {
>> error_report("vfio: failed to set iommu for container: %m");
>> ret = -errno;
>> @@ -596,14 +600,20 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as)
>> * when container fd is closed so we do not call it explicitly
>> * in this file.
>> */
>> - ret = ioctl(fd, VFIO_IOMMU_ENABLE);
>> - if (ret) {
>> - error_report("vfio: failed to enable container: %m");
>> - ret = -errno;
>> - goto free_container_exit;
>> + if (!v2) {
>> + ret = ioctl(fd, VFIO_IOMMU_ENABLE);
>> + if (ret) {
>> + error_report("vfio: failed to enable container: %m");
>> + ret = -errno;
>> + goto free_container_exit;
>> + }
>> }
>>
>> - spapr_memory_listener_register(container);
>> + ret = spapr_memory_listener_register(container, v2 ? 2 : 1);
>> + if (ret) {
>> + error_report("vfio: RAM memory listener initialization failed for container");
>> + goto listener_release_exit;
>> + }
>>
>> } else {
>> error_report("vfio: No available IOMMU models");
>> diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
>> index 5f79194..a670907 100644
>> --- a/hw/vfio/spapr.c
>> +++ b/hw/vfio/spapr.c
>> @@ -17,6 +17,9 @@
>> * along with this program; if not, see <http://www.gnu.org/licenses/>.
>> */
>>
>> +#include <sys/ioctl.h>
>> +#include <linux/vfio.h>
>> +
>> #include "hw/vfio/vfio-common.h"
>> #include "qemu/error-report.h"
>> #include "trace.h"
>> @@ -211,16 +214,88 @@ static const MemoryListener vfio_spapr_memory_listener = {
>> .region_del = vfio_spapr_listener_region_del,
>> };
>>
>> +static void vfio_ram_do_region(VFIOContainer *container,
>> + MemoryRegionSection *section, unsigned long req)
>> +{
>> + int ret;
>> + struct vfio_iommu_spapr_register_memory reg = { .argsz = sizeof(reg) };
>> +
>> + if (!memory_region_is_ram(section->mr) ||
>> + memory_region_is_skip_dump(section->mr)) {
>> + return;
>> + }
>> +
>> + reg.vaddr = (__u64) memory_region_get_ram_ptr(section->mr) +
>> + section->offset_within_region;
>
> Is section->offset_within_region always page aligned?
I think so as it is RAM. qemu_ram_alloc_from_file() and
qemu_ram_alloc_internal() (called from memory_region_init_ram()) align.
>> + reg.size = ROUND_UP(int128_get64(section->size), TARGET_PAGE_SIZE);
>> +
>> + ret = ioctl(container->fd, req, ®);
>> + trace_vfio_ram_register(_IOC_NR(req) - VFIO_BASE, reg.vaddr, reg.size, ret);
>> +
>> + /*
>> + * On the initfn path, store the first error in the container so we
>> + * can gracefully fail. Runtime, there's not much we can do other
>> + * than throw a hardware error.
>> + */
>> + if (!container->iommu_data.spapr.ram_reg_initialized) {
>> + if (!container->iommu_data.spapr.ram_reg_error) {
>> + container->iommu_data.spapr.ram_reg_error = ret;
>> + }
>
> This is pretty clunky, but I don't immediately see a better way.
>
> Also.. won't the return value of ioctl() just be -1 on error, which
> won't tell you much. Do you want to store errno, instead?
Yes, I do, I'll fix it. Thanks.
>> + } else {
>> + hw_error("vfio: RAM registering failed, unable to continue");
>> + }
>> +}
>> +
>> +static void vfio_spapr_ram_listener_region_add(MemoryListener *listener,
>> + MemoryRegionSection *section)
>> +{
>> + VFIOContainer *container = container_of(listener, VFIOContainer,
>> + iommu_data.spapr.ramlistener);
>> + memory_region_ref(section->mr);
>> + vfio_ram_do_region(container, section, VFIO_IOMMU_SPAPR_REGISTER_MEMORY);
>> +}
>> +
>> +static void vfio_spapr_ram_listener_region_del(MemoryListener *listener,
>> + MemoryRegionSection *section)
>> +{
>> + VFIOContainer *container = container_of(listener, VFIOContainer,
>> + iommu_data.spapr.ramlistener);
>> + memory_region_unref(section->mr);
>> + vfio_ram_do_region(container, section, VFIO_IOMMU_SPAPR_UNREGISTER_MEMORY);
>> +}
>> +
>> +static const MemoryListener vfio_spapr_ram_memory_listener = {
>> + .region_add = vfio_spapr_ram_listener_region_add,
>> + .region_del = vfio_spapr_ram_listener_region_del,
>> +};
>> +
>> static void vfio_spapr_listener_release(VFIOContainer *container)
>> {
>> memory_listener_unregister(&container->iommu_data.spapr.listener);
>> }
>>
>> -void spapr_memory_listener_register(VFIOContainer *container)
>> +static void vfio_spapr_listener_release_v2(VFIOContainer *container)
>> +{
>> + memory_listener_unregister(&container->iommu_data.spapr.listener);
>> + vfio_spapr_listener_release(container);
>> +}
>> +
>> +int spapr_memory_listener_register(VFIOContainer *container, int ver)
>> {
>> container->iommu_data.spapr.listener = vfio_spapr_memory_listener;
>> container->iommu_data.release = vfio_spapr_listener_release;
>> -
>> memory_listener_register(&container->iommu_data.spapr.listener,
>> container->space->as);
>> + if (ver < 2) {
>> + return 0;
>> + }
>> +
>> + container->iommu_data.spapr.ramlistener = vfio_spapr_ram_memory_listener;
>> + container->iommu_data.release = vfio_spapr_listener_release_v2;
>> + memory_listener_register(&container->iommu_data.spapr.ramlistener,
>> + &address_space_memory);
>> +
>> + container->iommu_data.spapr.ram_reg_initialized = true;
>> +
>> + return container->iommu_data.spapr.ram_reg_error;
>> }
>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
>> index d0b831c..b5ef446 100644
>> --- a/include/hw/vfio/vfio-common.h
>> +++ b/include/hw/vfio/vfio-common.h
>> @@ -71,6 +71,9 @@ typedef struct VFIOType1 {
>>
>> typedef struct VFIOSPAPR {
>> MemoryListener listener;
>> + MemoryListener ramlistener;
>> + int ram_reg_error;
>> + bool ram_reg_initialized;
>> } VFIOSPAPR;
>>
>> typedef struct VFIOContainer {
>> @@ -156,6 +159,6 @@ extern int vfio_dma_unmap(VFIOContainer *container,
>> hwaddr iova, ram_addr_t size);
>> bool vfio_listener_skipped_section(MemoryRegionSection *section);
>>
>> -extern void spapr_memory_listener_register(VFIOContainer *container);
>> +extern int spapr_memory_listener_register(VFIOContainer *container, int ver);
>>
>> #endif /* !HW_VFIO_VFIO_COMMON_H */
>> diff --git a/trace-events b/trace-events
>> index 1231ba4..2739140 100644
>> --- a/trace-events
>> +++ b/trace-events
>> @@ -1563,6 +1563,7 @@ vfio_disconnect_container(int fd) "close container->fd=%d"
>> vfio_put_group(int fd) "close group->fd=%d"
>> vfio_get_device(const char * name, unsigned int flags, unsigned int num_regions, unsigned int num_irqs) "Device %s flags: %u, regions: %u, irqs: %u"
>> vfio_put_base_device(int fd) "close vdev->fd=%d"
>> +vfio_ram_register(int req, uint64_t va, uint64_t size, int ret) "req=%d va=%"PRIx64" size=%"PRIx64" ret=%d"
>>
>> #hw/acpi/memory_hotplug.c
>> mhp_acpi_invalid_slot_selected(uint32_t slot) "0x%"PRIx32
>
--
Alexey
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v5 10/12] spapr_pci: Rework finish_realize()
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 10/12] spapr_pci: Rework finish_realize() Alexey Kardashevskiy
@ 2015-04-08 5:08 ` David Gibson
0 siblings, 0 replies; 26+ messages in thread
From: David Gibson @ 2015-04-08 5:08 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Alex Williamson, qemu-ppc, qemu-devel, Alexander Graf
[-- Attachment #1: Type: text/plain, Size: 6827 bytes --]
On Tue, Mar 31, 2015 at 04:28:45PM +1100, Alexey Kardashevskiy wrote:
> This renames and reworks finish_realize() which used to finalize DMA
> setup with an assumption that it will not change later.
>
> The new callback supports multiple windows and supports various
> parameters such as page and windows sizes.
>
> This is a mechanical change so no change in behaviour is expected.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
In terms of the basic concept anyway. I'd prefer to see it moved
earlier in the series though, so we don't have the now confusingly
named "finish_realize" callbacks floating about.
> ---
> hw/ppc/spapr_pci.c | 14 +++++++++-----
> hw/ppc/spapr_pci_vfio.c | 24 +++++++++++++-----------
> include/hw/pci-host/spapr.h | 3 ++-
> 3 files changed, 24 insertions(+), 17 deletions(-)
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 57bbc82..b8c2488 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -729,10 +729,9 @@ int spapr_phb_dma_reset(sPAPRPHBState *sphb)
> const uint32_t liobn = SPAPR_PCI_LIOBN(sphb->index, 0);
> sPAPRTCETable *tcet = spapr_tce_find_by_liobn(liobn);
> sPAPRPHBClass *spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
> - Error *err = NULL;
>
> spapr_tce_table_disable(tcet);
> - spc->finish_realize(sphb, &err);
> + spc->dma_init_window(sphb, liobn, SPAPR_TCE_PAGE_SHIFT, 0);
>
> return 0;
> }
> @@ -904,15 +903,20 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
> sphb->msi = g_hash_table_new_full(g_int_hash, g_int_equal, g_free, g_free);
> }
>
> -static void spapr_phb_finish_realize(sPAPRPHBState *sphb, Error **errp)
> +static int spapr_phb_dma_init_window(sPAPRPHBState *sphb, uint32_t liobn,
> + uint32_t page_shift,
> + uint64_t window_size_hint)
> {
> sPAPRTCETable *tcet;
> uint32_t nb_table;
> + uint64_t bus_offset = 0;
>
> tcet = spapr_tce_find_by_liobn(sphb->dma_liobn);
> nb_table = SPAPR_PCI_DMA32_SIZE >> SPAPR_TCE_PAGE_SHIFT;
> - spapr_tce_set_props(tcet, 0, SPAPR_TCE_PAGE_SHIFT, nb_table, false);
> + spapr_tce_set_props(tcet, bus_offset, page_shift, nb_table, false);
> spapr_tce_table_enable(tcet);
> +
> + return 0;
> }
>
> static int spapr_phb_children_reset(Object *child, void *opaque)
> @@ -1065,7 +1069,7 @@ static void spapr_phb_class_init(ObjectClass *klass, void *data)
> dc->vmsd = &vmstate_spapr_pci;
> set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> dc->cannot_instantiate_with_device_add_yet = false;
> - spc->finish_realize = spapr_phb_finish_realize;
> + spc->dma_init_window = spapr_phb_dma_init_window;
> }
>
> static const TypeInfo spapr_phb_info = {
> diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
> index 1657f6b..a7e32f6 100644
> --- a/hw/ppc/spapr_pci_vfio.c
> +++ b/hw/ppc/spapr_pci_vfio.c
> @@ -21,43 +21,45 @@
> #include "hw/pci-host/spapr.h"
> #include "linux/vfio.h"
> #include "hw/vfio/vfio.h"
> +#include "qemu/error-report.h"
>
> static Property spapr_phb_vfio_properties[] = {
> DEFINE_PROP_INT32("iommu", sPAPRPHBVFIOState, iommugroupid, -1),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> -static void spapr_phb_vfio_finish_realize(sPAPRPHBState *sphb, Error **errp)
> +static int spapr_phb_vfio_dma_init_window(sPAPRPHBState *sphb, uint32_t liobn,
> + uint32_t page_shift,
> + uint64_t window_size_hint)
> {
> sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
> struct vfio_iommu_spapr_tce_info info = { .argsz = sizeof(info) };
> int ret;
> - sPAPRTCETable *tcet;
> - uint32_t liobn = svphb->phb.dma_liobn;
> uint32_t nb_table;
> + sPAPRTCETable *tcet = spapr_tce_find_by_liobn(liobn);
>
> ret = vfio_container_ioctl(&svphb->phb.iommu_as,
> VFIO_CHECK_EXTENSION,
> (void *) VFIO_SPAPR_TCE_IOMMU);
> if (ret != 1) {
> - error_setg_errno(errp, -ret,
> - "spapr-vfio: SPAPR extension is not supported");
> - return;
> + error_report("spapr-vfio: SPAPR extension is not supported: %m");
> + return ret;
> }
>
> ret = vfio_container_ioctl(&sphb->iommu_as,
> VFIO_IOMMU_SPAPR_TCE_GET_INFO, &info);
> if (ret) {
> - error_setg_errno(errp, -ret,
> - "spapr-vfio: get info from container failed");
> - return;
> + error_report("spapr-vfio: get info from container failed: %m");
> + return ret;
> }
>
> tcet = spapr_tce_find_by_liobn(liobn);
> nb_table = info.dma32_window_size >> SPAPR_TCE_PAGE_SHIFT;
> - spapr_tce_set_props(tcet, info.dma32_window_start, SPAPR_TCE_PAGE_SHIFT,
> + spapr_tce_set_props(tcet, info.dma32_window_start, page_shift,
> nb_table, true);
> spapr_tce_table_enable(tcet);
> +
> + return ret;
> }
>
> static int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
> @@ -177,7 +179,7 @@ static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data)
> sPAPRPHBClass *spc = SPAPR_PCI_HOST_BRIDGE_CLASS(klass);
>
> dc->props = spapr_phb_vfio_properties;
> - spc->finish_realize = spapr_phb_vfio_finish_realize;
> + spc->dma_init_window = spapr_phb_vfio_dma_init_window;
> spc->eeh_set_option = spapr_phb_vfio_eeh_set_option;
> spc->eeh_get_state = spapr_phb_vfio_eeh_get_state;
> spc->eeh_reset = spapr_phb_vfio_eeh_reset;
> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> index f592276..f556a41 100644
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -48,7 +48,8 @@ typedef struct sPAPRPHBVFIOState sPAPRPHBVFIOState;
> struct sPAPRPHBClass {
> PCIHostBridgeClass parent_class;
>
> - void (*finish_realize)(sPAPRPHBState *sphb, Error **errp);
> + int (*dma_init_window)(sPAPRPHBState *sphb, uint32_t liobn,
> + uint32_t page_shift, uint64_t window_size_hint);
> int (*eeh_set_option)(sPAPRPHBState *sphb, unsigned int addr, int option);
> int (*eeh_get_state)(sPAPRPHBState *sphb, int *state);
> int (*eeh_reset)(sPAPRPHBState *sphb, int option);
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v5 11/12] spapr_pci: Disable all DMA windows on reset
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 11/12] spapr_pci: Disable all DMA windows on reset Alexey Kardashevskiy
@ 2015-04-08 5:09 ` David Gibson
0 siblings, 0 replies; 26+ messages in thread
From: David Gibson @ 2015-04-08 5:09 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Alex Williamson, qemu-ppc, qemu-devel, Alexander Graf
[-- Attachment #1: Type: text/plain, Size: 717 bytes --]
On Tue, Mar 31, 2015 at 04:28:46PM +1100, Alexey Kardashevskiy wrote:
> This disables all DMA windows on a PHB reset. It does not make any
> difference now as there is just one DMA window but it will later with DDW
> patches.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Although I think I'd prefer to see this folded in with the earlier
reset rework patch. I don't think there's enough here that splitting
it out makes anything clearer.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v5 06/12] vfio: spapr: Add SPAPR IOMMU v2 support (DMA memory preregistering)
2015-04-08 4:05 ` Alexey Kardashevskiy
@ 2015-04-08 5:11 ` David Gibson
0 siblings, 0 replies; 26+ messages in thread
From: David Gibson @ 2015-04-08 5:11 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Alex Williamson, qemu-ppc, qemu-devel, Alexander Graf
[-- Attachment #1: Type: text/plain, Size: 11339 bytes --]
On Wed, Apr 08, 2015 at 02:05:28PM +1000, Alexey Kardashevskiy wrote:
> On 04/08/2015 12:15 PM, David Gibson wrote:
> >On Tue, Mar 31, 2015 at 04:28:41PM +1100, Alexey Kardashevskiy wrote:
> >>This makes use of the new "memory registering" feature. The idea is
> >>to provide the guest ability to notify the host kernel about pages which
> >
> >AFAICT it's not really the guest informing the host, just qemu
> >informing the host. If I'm reading the code correctly, qemu registers
> >all RAM regionns, without guest intervention.
>
> Right. Wrong commit log, I'll fix it.
>
>
> >>are going to be used for DMA. Having this information, the host kernel
> >>can pin them all once per user process, do locked pages accounting (once)
> >>and not spent time on doing that in real time with possible failures
> >>which cannot be handled nicely in some cases.
> >>
> >>This adds a guest RAM memory listener which notifies a VFIO container
> >>about memory which needs to be pinned/unpinned. VFIO MMIO regions
> >>(i.e. "skip dump" regions) are skipped.
> >>
> >>The feature is only enabled for SPAPR IOMMU v2. The host kernel changes
> >>are required. Since v2 does not need/support VFIO_IOMMU_ENABLE, this does
> >>not call it when v2 is detected and enabled.
> >
> >So I'm clear, the v2 just represents a new userspace<->host kernel
> >interface for controlling the IOMMU, doesn't it? It doesn't change
> >the guest visible IOMMU interface and doesn't represent different
> >IOMMU hardware?
>
>
> Exactly true.
>
>
> >>Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> >>---
> >>Changes:
> >>v5:
> >>* simplified the patch
> >>* added trace points
> >>* added round_up() for the size
> >>* SPAPR IOMMU v2 used
> >>---
> >> hw/vfio/common.c | 26 +++++++++-----
> >> hw/vfio/spapr.c | 79 +++++++++++++++++++++++++++++++++++++++++--
> >> include/hw/vfio/vfio-common.h | 5 ++-
> >> trace-events | 1 +
> >> 4 files changed, 100 insertions(+), 11 deletions(-)
> >>
> >>diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> >>index a71f881..e35e478 100644
> >>--- a/hw/vfio/common.c
> >>+++ b/hw/vfio/common.c
> >>@@ -577,14 +577,18 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as)
> >>
> >> container->iommu_data.type1.initialized = true;
> >>
> >>- } else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU)) {
> >>+ } else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU) ||
> >>+ ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_v2_IOMMU)) {
> >>+ bool v2 = !!ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_v2_IOMMU);
> >>+
> >> ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, &fd);
> >> if (ret) {
> >> error_report("vfio: failed to set group container: %m");
> >> ret = -errno;
> >> goto free_container_exit;
> >> }
> >>- ret = ioctl(fd, VFIO_SET_IOMMU, VFIO_SPAPR_TCE_IOMMU);
> >>+ ret = ioctl(fd, VFIO_SET_IOMMU,
> >>+ v2 ? VFIO_SPAPR_TCE_v2_IOMMU : VFIO_SPAPR_TCE_IOMMU);
> >> if (ret) {
> >> error_report("vfio: failed to set iommu for container: %m");
> >> ret = -errno;
> >>@@ -596,14 +600,20 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as)
> >> * when container fd is closed so we do not call it explicitly
> >> * in this file.
> >> */
> >>- ret = ioctl(fd, VFIO_IOMMU_ENABLE);
> >>- if (ret) {
> >>- error_report("vfio: failed to enable container: %m");
> >>- ret = -errno;
> >>- goto free_container_exit;
> >>+ if (!v2) {
> >>+ ret = ioctl(fd, VFIO_IOMMU_ENABLE);
> >>+ if (ret) {
> >>+ error_report("vfio: failed to enable container: %m");
> >>+ ret = -errno;
> >>+ goto free_container_exit;
> >>+ }
> >> }
> >>
> >>- spapr_memory_listener_register(container);
> >>+ ret = spapr_memory_listener_register(container, v2 ? 2 : 1);
> >>+ if (ret) {
> >>+ error_report("vfio: RAM memory listener initialization failed for container");
> >>+ goto listener_release_exit;
> >>+ }
> >>
> >> } else {
> >> error_report("vfio: No available IOMMU models");
> >>diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
> >>index 5f79194..a670907 100644
> >>--- a/hw/vfio/spapr.c
> >>+++ b/hw/vfio/spapr.c
> >>@@ -17,6 +17,9 @@
> >> * along with this program; if not, see <http://www.gnu.org/licenses/>.
> >> */
> >>
> >>+#include <sys/ioctl.h>
> >>+#include <linux/vfio.h>
> >>+
> >> #include "hw/vfio/vfio-common.h"
> >> #include "qemu/error-report.h"
> >> #include "trace.h"
> >>@@ -211,16 +214,88 @@ static const MemoryListener vfio_spapr_memory_listener = {
> >> .region_del = vfio_spapr_listener_region_del,
> >> };
> >>
> >>+static void vfio_ram_do_region(VFIOContainer *container,
> >>+ MemoryRegionSection *section, unsigned long req)
> >>+{
> >>+ int ret;
> >>+ struct vfio_iommu_spapr_register_memory reg = { .argsz = sizeof(reg) };
> >>+
> >>+ if (!memory_region_is_ram(section->mr) ||
> >>+ memory_region_is_skip_dump(section->mr)) {
> >>+ return;
> >>+ }
> >>+
> >>+ reg.vaddr = (__u64) memory_region_get_ram_ptr(section->mr) +
> >>+ section->offset_within_region;
> >
> >Is section->offset_within_region always page aligned?
>
>
> I think so as it is RAM. qemu_ram_alloc_from_file() and
> qemu_ram_alloc_internal() (called from memory_region_init_ram()) align.
Ok. It just seems a but odd to me that offset is guaranteed page
aligned, but size is not. An assert here might help to document the
assumptions you're making.
> >>+ reg.size = ROUND_UP(int128_get64(section->size), TARGET_PAGE_SIZE);
> >>+
> >>+ ret = ioctl(container->fd, req, ®);
> >>+ trace_vfio_ram_register(_IOC_NR(req) - VFIO_BASE, reg.vaddr, reg.size, ret);
> >>+
> >>+ /*
> >>+ * On the initfn path, store the first error in the container so we
> >>+ * can gracefully fail. Runtime, there's not much we can do other
> >>+ * than throw a hardware error.
> >>+ */
> >>+ if (!container->iommu_data.spapr.ram_reg_initialized) {
> >>+ if (!container->iommu_data.spapr.ram_reg_error) {
> >>+ container->iommu_data.spapr.ram_reg_error = ret;
> >>+ }
> >
> >This is pretty clunky, but I don't immediately see a better way.
> >
> >Also.. won't the return value of ioctl() just be -1 on error, which
> >won't tell you much. Do you want to store errno, instead?
>
> Yes, I do, I'll fix it. Thanks.
>
>
> >>+ } else {
> >>+ hw_error("vfio: RAM registering failed, unable to continue");
> >>+ }
> >>+}
> >>+
> >>+static void vfio_spapr_ram_listener_region_add(MemoryListener *listener,
> >>+ MemoryRegionSection *section)
> >>+{
> >>+ VFIOContainer *container = container_of(listener, VFIOContainer,
> >>+ iommu_data.spapr.ramlistener);
> >>+ memory_region_ref(section->mr);
> >>+ vfio_ram_do_region(container, section, VFIO_IOMMU_SPAPR_REGISTER_MEMORY);
> >>+}
> >>+
> >>+static void vfio_spapr_ram_listener_region_del(MemoryListener *listener,
> >>+ MemoryRegionSection *section)
> >>+{
> >>+ VFIOContainer *container = container_of(listener, VFIOContainer,
> >>+ iommu_data.spapr.ramlistener);
> >>+ memory_region_unref(section->mr);
> >>+ vfio_ram_do_region(container, section, VFIO_IOMMU_SPAPR_UNREGISTER_MEMORY);
> >>+}
> >>+
> >>+static const MemoryListener vfio_spapr_ram_memory_listener = {
> >>+ .region_add = vfio_spapr_ram_listener_region_add,
> >>+ .region_del = vfio_spapr_ram_listener_region_del,
> >>+};
> >>+
> >> static void vfio_spapr_listener_release(VFIOContainer *container)
> >> {
> >> memory_listener_unregister(&container->iommu_data.spapr.listener);
> >> }
> >>
> >>-void spapr_memory_listener_register(VFIOContainer *container)
> >>+static void vfio_spapr_listener_release_v2(VFIOContainer *container)
> >>+{
> >>+ memory_listener_unregister(&container->iommu_data.spapr.listener);
> >>+ vfio_spapr_listener_release(container);
> >>+}
> >>+
> >>+int spapr_memory_listener_register(VFIOContainer *container, int ver)
> >> {
> >> container->iommu_data.spapr.listener = vfio_spapr_memory_listener;
> >> container->iommu_data.release = vfio_spapr_listener_release;
> >>-
> >> memory_listener_register(&container->iommu_data.spapr.listener,
> >> container->space->as);
> >>+ if (ver < 2) {
> >>+ return 0;
> >>+ }
> >>+
> >>+ container->iommu_data.spapr.ramlistener = vfio_spapr_ram_memory_listener;
> >>+ container->iommu_data.release = vfio_spapr_listener_release_v2;
> >>+ memory_listener_register(&container->iommu_data.spapr.ramlistener,
> >>+ &address_space_memory);
> >>+
> >>+ container->iommu_data.spapr.ram_reg_initialized = true;
> >>+
> >>+ return container->iommu_data.spapr.ram_reg_error;
> >> }
> >>diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> >>index d0b831c..b5ef446 100644
> >>--- a/include/hw/vfio/vfio-common.h
> >>+++ b/include/hw/vfio/vfio-common.h
> >>@@ -71,6 +71,9 @@ typedef struct VFIOType1 {
> >>
> >> typedef struct VFIOSPAPR {
> >> MemoryListener listener;
> >>+ MemoryListener ramlistener;
> >>+ int ram_reg_error;
> >>+ bool ram_reg_initialized;
> >> } VFIOSPAPR;
> >>
> >> typedef struct VFIOContainer {
> >>@@ -156,6 +159,6 @@ extern int vfio_dma_unmap(VFIOContainer *container,
> >> hwaddr iova, ram_addr_t size);
> >> bool vfio_listener_skipped_section(MemoryRegionSection *section);
> >>
> >>-extern void spapr_memory_listener_register(VFIOContainer *container);
> >>+extern int spapr_memory_listener_register(VFIOContainer *container, int ver);
> >>
> >> #endif /* !HW_VFIO_VFIO_COMMON_H */
> >>diff --git a/trace-events b/trace-events
> >>index 1231ba4..2739140 100644
> >>--- a/trace-events
> >>+++ b/trace-events
> >>@@ -1563,6 +1563,7 @@ vfio_disconnect_container(int fd) "close container->fd=%d"
> >> vfio_put_group(int fd) "close group->fd=%d"
> >> vfio_get_device(const char * name, unsigned int flags, unsigned int num_regions, unsigned int num_irqs) "Device %s flags: %u, regions: %u, irqs: %u"
> >> vfio_put_base_device(int fd) "close vdev->fd=%d"
> >>+vfio_ram_register(int req, uint64_t va, uint64_t size, int ret) "req=%d va=%"PRIx64" size=%"PRIx64" ret=%d"
> >>
> >> #hw/acpi/memory_hotplug.c
> >> mhp_acpi_invalid_slot_selected(uint32_t slot) "0x%"PRIx32
> >
>
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v5 04/12] spapr_pci_vfio: Enable multiple groups per container
2015-04-08 3:45 ` Alexey Kardashevskiy
@ 2015-04-09 6:43 ` David Gibson
2015-04-09 7:13 ` Alexey Kardashevskiy
0 siblings, 1 reply; 26+ messages in thread
From: David Gibson @ 2015-04-09 6:43 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Alex Williamson, qemu-ppc, qemu-devel, Alexander Graf
[-- Attachment #1: Type: text/plain, Size: 5326 bytes --]
On Wed, Apr 08, 2015 at 01:45:19PM +1000, Alexey Kardashevskiy wrote:
> On 04/08/2015 12:01 PM, David Gibson wrote:
> >On Tue, Mar 31, 2015 at 04:28:39PM +1100, Alexey Kardashevskiy wrote:
> >>This enables multiple IOMMU groups in one VFIO container which means
> >>that multiple devices from different groups can share the same IOMMU
> >>table (or tables if DDW).
> >>
> >>This removes a group id from vfio_container_ioctl(). The kernel support
> >>is required for this; if the host kernel does not have the support,
> >>it will allow only one group per container. The PHB's "iommuid" property
> >>is ignored.
> >>
> >>This adds a sanity check that there is just one VFIO container per
> >>PHB address space.
> >>
> >>Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> >
> >[snip]
> >>diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> >>index b012620..99e1900 100644
> >>--- a/hw/vfio/common.c
> >>+++ b/hw/vfio/common.c
> >>@@ -915,21 +915,23 @@ void vfio_put_base_device(VFIODevice *vbasedev)
> >> close(vbasedev->fd);
> >> }
> >>
> >>-static int vfio_container_do_ioctl(AddressSpace *as, int32_t groupid,
> >>+static int vfio_container_do_ioctl(AddressSpace *as,
> >> int req, void *param)
> >> {
> >>- VFIOGroup *group;
> >> VFIOContainer *container;
> >>- int ret = -1;
> >>+ int ret;
> >>+ VFIOAddressSpace *space;
> >>
> >>- group = vfio_get_group(groupid, as);
> >>- if (!group) {
> >>- error_report("vfio: group %d not registered", groupid);
> >>- return ret;
> >>- }
> >>+ space = vfio_get_address_space(as);
> >>+ container = QLIST_FIRST(&space->containers);
> >
> >So getting the container handle from the address space, rather than
> >the group id certainly makes more sense to me.
> >
> >>- container = group->container;
> >>- if (group->container) {
> >>+ if (!container) {
> >>+ error_report("vfio: container is not set");
> >>+ return -1;
> >>+ } else if (QLIST_NEXT(container, next)) {
> >>+ error_report("vfio: multiple containers per PHB are not supported");
> >>+ return -1;
> >
> >But if only one PHB per address space is possible, why is the
> >containers field a list in the first place?
>
>
> Historically the list was added in 3df3e0a5872 (the patch of yours
> :) ).
Heh.
> In theory we could implement spapr-pci-bridge (derived from pci-bridge) with
> isolation capability (i.e. its own LIOBN/DMA window), in this case there
> could be multiple containers per PHB address space. Other archs could want
> multiple containers for some other reason. It would help me a lot if you
> remembered why you kept the list at the first place :)
Ok, I've looked over the patch and it has jogged my memory a bit. So
the dumb answer is that it's because the per address-space list was
replacing a global list of containers
The more useful answer is that I think it was because I was
anticipating the possibility of working around the
one-group-per-container limit by allowing a single VFIOAddressSpace in
qemu to be backed by several containers, whose mappings would be kept
in sync from the userspace side by duplicating all mappings.
Anyway, I think that means the right way to implement this is by
duplicating the ioctl() across all the attached containers, rather
than picking just one.
> For now I guess I'll move the next patch ("vfio: spapr: Move SPAPR-related
> code to a separate file") before this one, do s/vfio_container_do_ioctl/
> vfio_spapr_container_do_ioctl/ and move it to hw/vfio/spapr.c. Makes
> sense?
That sounds fine, though I don't see that it really addresses the
question here.
>
>
> >>+ } else {
> >> ret = ioctl(container->fd, req, param);
> >> if (ret < 0) {
> >> error_report("vfio: failed to ioctl %d to container: ret=%d, %s",
> >>@@ -937,12 +939,10 @@ static int vfio_container_do_ioctl(AddressSpace *as, int32_t groupid,
> >> }
> >> }
> >>
> >>- vfio_put_group(group);
> >>-
> >> return ret;
> >> }
> >>
> >>-int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
> >>+int vfio_container_ioctl(AddressSpace *as,
> >> int req, void *param)
> >> {
> >> /* We allow only certain ioctls to the container */
> >>@@ -957,5 +957,5 @@ int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
> >> return -1;
> >> }
> >>
> >>- return vfio_container_do_ioctl(as, groupid, req, param);
> >>+ return vfio_container_do_ioctl(as, req, param);
> >> }
> >>diff --git a/include/hw/vfio/vfio.h b/include/hw/vfio/vfio.h
> >>index 0b26cd8..76b5744 100644
> >>--- a/include/hw/vfio/vfio.h
> >>+++ b/include/hw/vfio/vfio.h
> >>@@ -3,7 +3,7 @@
> >>
> >> #include "qemu/typedefs.h"
> >>
> >>-extern int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
> >>+extern int vfio_container_ioctl(AddressSpace *as,
> >> int req, void *param);
> >>
> >> #endif
> >
>
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v5 04/12] spapr_pci_vfio: Enable multiple groups per container
2015-04-09 6:43 ` David Gibson
@ 2015-04-09 7:13 ` Alexey Kardashevskiy
0 siblings, 0 replies; 26+ messages in thread
From: Alexey Kardashevskiy @ 2015-04-09 7:13 UTC (permalink / raw)
To: David Gibson; +Cc: Alex Williamson, qemu-ppc, qemu-devel, Alexander Graf
On 04/09/2015 04:43 PM, David Gibson wrote:
> On Wed, Apr 08, 2015 at 01:45:19PM +1000, Alexey Kardashevskiy wrote:
>> On 04/08/2015 12:01 PM, David Gibson wrote:
>>> On Tue, Mar 31, 2015 at 04:28:39PM +1100, Alexey Kardashevskiy wrote:
>>>> This enables multiple IOMMU groups in one VFIO container which means
>>>> that multiple devices from different groups can share the same IOMMU
>>>> table (or tables if DDW).
>>>>
>>>> This removes a group id from vfio_container_ioctl(). The kernel support
>>>> is required for this; if the host kernel does not have the support,
>>>> it will allow only one group per container. The PHB's "iommuid" property
>>>> is ignored.
>>>>
>>>> This adds a sanity check that there is just one VFIO container per
>>>> PHB address space.
>>>>
>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>
>>> [snip]
>>>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>>>> index b012620..99e1900 100644
>>>> --- a/hw/vfio/common.c
>>>> +++ b/hw/vfio/common.c
>>>> @@ -915,21 +915,23 @@ void vfio_put_base_device(VFIODevice *vbasedev)
>>>> close(vbasedev->fd);
>>>> }
>>>>
>>>> -static int vfio_container_do_ioctl(AddressSpace *as, int32_t groupid,
>>>> +static int vfio_container_do_ioctl(AddressSpace *as,
>>>> int req, void *param)
>>>> {
>>>> - VFIOGroup *group;
>>>> VFIOContainer *container;
>>>> - int ret = -1;
>>>> + int ret;
>>>> + VFIOAddressSpace *space;
>>>>
>>>> - group = vfio_get_group(groupid, as);
>>>> - if (!group) {
>>>> - error_report("vfio: group %d not registered", groupid);
>>>> - return ret;
>>>> - }
>>>> + space = vfio_get_address_space(as);
>>>> + container = QLIST_FIRST(&space->containers);
>>>
>>> So getting the container handle from the address space, rather than
>>> the group id certainly makes more sense to me.
>>>
>>>> - container = group->container;
>>>> - if (group->container) {
>>>> + if (!container) {
>>>> + error_report("vfio: container is not set");
>>>> + return -1;
>>>> + } else if (QLIST_NEXT(container, next)) {
>>>> + error_report("vfio: multiple containers per PHB are not supported");
>>>> + return -1;
>>>
>>> But if only one PHB per address space is possible, why is the
>>> containers field a list in the first place?
>>
>>
>> Historically the list was added in 3df3e0a5872 (the patch of yours
>> :) ).
>
> Heh.
>
>> In theory we could implement spapr-pci-bridge (derived from pci-bridge) with
>> isolation capability (i.e. its own LIOBN/DMA window), in this case there
>> could be multiple containers per PHB address space. Other archs could want
>> multiple containers for some other reason. It would help me a lot if you
>> remembered why you kept the list at the first place :)
>
> Ok, I've looked over the patch and it has jogged my memory a bit. So
> the dumb answer is that it's because the per address-space list was
> replacing a global list of containers
>
> The more useful answer is that I think it was because I was
> anticipating the possibility of working around the
> one-group-per-container limit by allowing a single VFIOAddressSpace in
> qemu to be backed by several containers, whose mappings would be kept
> in sync from the userspace side by duplicating all mappings.
>
> Anyway, I think that means the right way to implement this is by
> duplicating the ioctl() across all the attached containers, rather
> than picking just one.
Right. I will do that.
>> For now I guess I'll move the next patch ("vfio: spapr: Move SPAPR-related
>> code to a separate file") before this one, do s/vfio_container_do_ioctl/
>> vfio_spapr_container_do_ioctl/ and move it to hw/vfio/spapr.c. Makes
>> sense?
>
> That sounds fine, though I don't see that it really addresses the
> question here.
You are right, it does not. I won't do it in this patchset then. Thanks.
>
>
>>
>>
>>>> + } else {
>>>> ret = ioctl(container->fd, req, param);
>>>> if (ret < 0) {
>>>> error_report("vfio: failed to ioctl %d to container: ret=%d, %s",
>>>> @@ -937,12 +939,10 @@ static int vfio_container_do_ioctl(AddressSpace *as, int32_t groupid,
>>>> }
>>>> }
>>>>
>>>> - vfio_put_group(group);
>>>> -
>>>> return ret;
>>>> }
>>>>
>>>> -int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
>>>> +int vfio_container_ioctl(AddressSpace *as,
>>>> int req, void *param)
>>>> {
>>>> /* We allow only certain ioctls to the container */
>>>> @@ -957,5 +957,5 @@ int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
>>>> return -1;
>>>> }
>>>>
>>>> - return vfio_container_do_ioctl(as, groupid, req, param);
>>>> + return vfio_container_do_ioctl(as, req, param);
>>>> }
>>>> diff --git a/include/hw/vfio/vfio.h b/include/hw/vfio/vfio.h
>>>> index 0b26cd8..76b5744 100644
>>>> --- a/include/hw/vfio/vfio.h
>>>> +++ b/include/hw/vfio/vfio.h
>>>> @@ -3,7 +3,7 @@
>>>>
>>>> #include "qemu/typedefs.h"
>>>>
>>>> -extern int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
>>>> +extern int vfio_container_ioctl(AddressSpace *as,
>>>> int req, void *param);
>>>>
>>>> #endif
>>>
>>
>>
>
--
Alexey
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2015-04-09 7:13 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-31 5:28 [Qemu-devel] [PATCH qemu v5 00/12] spapr: vfio: Enable Dynamic DMA windows (DDW) Alexey Kardashevskiy
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 01/12] linux headers update for DDW on SPAPR Alexey Kardashevskiy
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 02/12] vmstate: Define VARRAY with VMS_ALLOC Alexey Kardashevskiy
2015-04-08 1:55 ` David Gibson
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 03/12] spapr_pci: Make find_phb()/find_dev() public Alexey Kardashevskiy
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 04/12] spapr_pci_vfio: Enable multiple groups per container Alexey Kardashevskiy
2015-04-08 2:01 ` David Gibson
2015-04-08 3:45 ` Alexey Kardashevskiy
2015-04-09 6:43 ` David Gibson
2015-04-09 7:13 ` Alexey Kardashevskiy
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 05/12] vfio: spapr: Move SPAPR-related code to a separate file Alexey Kardashevskiy
2015-04-08 2:05 ` David Gibson
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 06/12] vfio: spapr: Add SPAPR IOMMU v2 support (DMA memory preregistering) Alexey Kardashevskiy
2015-04-08 2:15 ` David Gibson
2015-04-08 4:05 ` Alexey Kardashevskiy
2015-04-08 5:11 ` David Gibson
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 07/12] spapr_iommu: Rework TCE table initialization Alexey Kardashevskiy
2015-04-08 2:35 ` David Gibson
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 08/12] spapr_pci: Rework reset to reset DMA configuration Alexey Kardashevskiy
2015-04-08 2:42 ` David Gibson
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 09/12] spapr_iommu: Add root memory region Alexey Kardashevskiy
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 10/12] spapr_pci: Rework finish_realize() Alexey Kardashevskiy
2015-04-08 5:08 ` David Gibson
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 11/12] spapr_pci: Disable all DMA windows on reset Alexey Kardashevskiy
2015-04-08 5:09 ` David Gibson
2015-03-31 5:28 ` [Qemu-devel] [PATCH qemu v5 12/12] spapr_pci/spapr_pci_vfio: Support Dynamic DMA Windows (DDW) Alexey Kardashevskiy
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.