* [Qemu-devel] [PATCH 00/12] Allow EEH on spapr-pci-host-bridge devices
@ 2016-02-26 11:31 David Gibson
2016-02-26 11:31 ` [Qemu-devel] [PATCH 01/12] vfio: Start improving VFIO/EEH interface David Gibson
` (12 more replies)
0 siblings, 13 replies; 33+ messages in thread
From: David Gibson @ 2016-02-26 11:31 UTC (permalink / raw)
To: aik, benh
Cc: agraf, qemu-devel, gwshan, mdroth, alex.williamson, qemu-ppc,
David Gibson
For historical reasons, the spapr machine type has two PCI host bridge
implementations: spapr-pci-host-bridge, and
spapr-vfio-pci-host-bridge. The latter was (duh) designed for VFIO
devices, but later reworks mean it's not necessary for that, and VFIO
can be used on the regular host bridge.
The only remaining difference is that EEH (Enhanced Error Handling -
IBM's interface with similar purpose to AER) is only supported on the
spapr-vfio-pci-host-bridge device.
This series corrects this, allowing EEH operations on the regular host
bridge. That allows the special VFIO host bridge (we leave a stub,
for backwards compatibility).
EEH is only supported for VFIO devices, for the time being, and due to
bugs in the kernel implementation, it is only usable when the host
bridge only has devices from a single (host-side) IOMMU group
("Partitionable Endpoint", in IBM terminology). That's an annoying
limitation which we hope to lift someday, but it's no worse than now,
since the spapr-vfio-pci-host-bridge only permits devices from a
single IOMMU group in any case (although it doesn't properly enforce
that).
I wrote these a while back, and I'm not sure why they got sidelined -
I suspect I was looking for testing from Gavin Shan, not realising
then that he was no longer working on EEH. In any case, I'm hoping we
can squeeze this change into 2.6, so we don't need to carry the broken
spapr-vfio-pci-host-bridge device any longer.
David Gibson (12):
vfio: Start improving VFIO/EEH interface
spapr_pci: Switch to vfio_eeh_as_op() interface
spapr_pci: Eliminate class callbacks
spapr_pci: Fold spapr_phb_vfio_eeh_configure() into spapr_pci code
spapr_pci: Fold spapr_phb_vfio_eeh_reset() into spapr_pci code
spapr_pci: Fold spapr_phb_vfio_eeh_get_state() into spapr_pci code
spapr_pci: Fold spapr_phb_vfio_eeh_set_option() into spapr_pci code
spapr_pci: Fold spapr_phb_vfio_reset() into spapr_pci code
spapr_pci: Allow EEH on spapr-pci-host-bridge
spapr_pci: (Mostly) remove spapr-pci-vfio-host-bridge
spapr_pci: Remove finish_realize hook
vfio: Eliminate vfio_container_ioctl()
hw/ppc/spapr_pci.c | 203 +++++++++++++++++++++++++++---------
hw/ppc/spapr_pci_vfio.c | 245 +++-----------------------------------------
hw/vfio/common.c | 94 +++++++++++------
include/hw/pci-host/spapr.h | 28 +----
include/hw/vfio/vfio.h | 4 +-
5 files changed, 235 insertions(+), 339 deletions(-)
--
2.5.0
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 01/12] vfio: Start improving VFIO/EEH interface
2016-02-26 11:31 [Qemu-devel] [PATCH 00/12] Allow EEH on spapr-pci-host-bridge devices David Gibson
@ 2016-02-26 11:31 ` David Gibson
2016-02-29 0:58 ` Alexey Kardashevskiy
2016-02-26 11:31 ` [Qemu-devel] [PATCH 02/12] spapr_pci: Switch to vfio_eeh_as_op() interface David Gibson
` (11 subsequent siblings)
12 siblings, 1 reply; 33+ messages in thread
From: David Gibson @ 2016-02-26 11:31 UTC (permalink / raw)
To: aik, benh
Cc: agraf, qemu-devel, gwshan, mdroth, alex.williamson, qemu-ppc,
David Gibson
At present the code handling IBM's Enhanced Error Handling (EEH) interface
on VFIO devices operates by bypassing the usual VFIO logic with
vfio_container_ioctl(). That's a poorly designed interface with unclear
semantics about exactly what can be operated on.
In particular it operates on a single vfio container internally (hence the
name), but takes an address space and group id, from which it deduces the
container in a rather roundabout way. groupids are something that code
outside vfio shouldn't even be aware of.
This patch creates new interfaces for EEH operations. Internally we
have vfio_eeh_container_op() which takes a VFIOContainer object
directly. For external use we have vfio_eeh_as_ok() which determines
if an AddressSpace is usable for EEH (at present this means it has a
single container and at most a single group attached), and
vfio_eeh_as_op() which will perform an operation on an AddressSpace in
the unambiguous case, and otherwise returns an error.
This interface still isn't great, but it's enough of an improvement to
allow a number of cleanups in other places.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/vfio/common.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++
include/hw/vfio/vfio.h | 2 ++
2 files changed, 79 insertions(+)
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 607ec70..e419241 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1003,3 +1003,80 @@ int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
return vfio_container_do_ioctl(as, groupid, req, param);
}
+
+/*
+ * Interfaces for IBM EEH (Enhanced Error Handling)
+ */
+static bool vfio_eeh_container_ok(VFIOContainer *container)
+{
+ /* A broken kernel implementation means EEH operations won't work
+ * correctly if there are multiple groups in a container */
+
+ if (!QLIST_EMPTY(&container->group_list)
+ && QLIST_NEXT(QLIST_FIRST(&container->group_list), container_next)) {
+ return false;
+ }
+
+ return true;
+}
+
+static int vfio_eeh_container_op(VFIOContainer *container, uint32_t op)
+{
+ struct vfio_eeh_pe_op pe_op = {
+ .argsz = sizeof(pe_op),
+ .op = op,
+ };
+ int ret;
+
+ if (!vfio_eeh_container_ok(container)) {
+ error_report("vfio/eeh: EEH_PE_OP 0x%x called on container"
+ " with multiple groups", op);
+ return -EPERM;
+ }
+
+ ret = ioctl(container->fd, VFIO_EEH_PE_OP, &pe_op);
+ if (ret < 0) {
+ error_report("vfio/eeh: EEH_PE_OP 0x%x failed: %m", op);
+ return -errno;
+ }
+
+ return 0;
+}
+
+static VFIOContainer *vfio_eeh_as_container(AddressSpace *as)
+{
+ VFIOAddressSpace *space = vfio_get_address_space(as);
+ VFIOContainer *container = NULL;
+
+ if (QLIST_EMPTY(&space->containers)) {
+ /* No containers to act on */
+ goto out;
+ }
+
+ container = QLIST_FIRST(&space->containers);
+
+ if (QLIST_NEXT(container, next)) {
+ /* We don't yet have logic to synchronize EEH state across
+ * multiple containers */
+ container = NULL;
+ goto out;
+ }
+
+out:
+ vfio_put_address_space(space);
+ return container;
+}
+
+bool vfio_eeh_as_ok(AddressSpace *as)
+{
+ VFIOContainer *container = vfio_eeh_as_container(as);
+
+ return (container != NULL) && vfio_eeh_container_ok(container);
+}
+
+int vfio_eeh_as_op(AddressSpace *as, uint32_t op)
+{
+ VFIOContainer *container = vfio_eeh_as_container(as);
+
+ return vfio_eeh_container_op(container, op);
+}
diff --git a/include/hw/vfio/vfio.h b/include/hw/vfio/vfio.h
index 0b26cd8..fd3933b 100644
--- a/include/hw/vfio/vfio.h
+++ b/include/hw/vfio/vfio.h
@@ -5,5 +5,7 @@
extern int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
int req, void *param);
+bool vfio_eeh_as_ok(AddressSpace *as);
+int vfio_eeh_as_op(AddressSpace *as, uint32_t op);
#endif
--
2.5.0
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 02/12] spapr_pci: Switch to vfio_eeh_as_op() interface
2016-02-26 11:31 [Qemu-devel] [PATCH 00/12] Allow EEH on spapr-pci-host-bridge devices David Gibson
2016-02-26 11:31 ` [Qemu-devel] [PATCH 01/12] vfio: Start improving VFIO/EEH interface David Gibson
@ 2016-02-26 11:31 ` David Gibson
2016-02-29 1:43 ` Alexey Kardashevskiy
2016-02-26 11:31 ` [Qemu-devel] [PATCH 03/12] spapr_pci: Eliminate class callbacks David Gibson
` (10 subsequent siblings)
12 siblings, 1 reply; 33+ messages in thread
From: David Gibson @ 2016-02-26 11:31 UTC (permalink / raw)
To: aik, benh
Cc: agraf, qemu-devel, gwshan, mdroth, alex.williamson, qemu-ppc,
David Gibson
This switches all EEH on VFIO operations in spapr_pci_vfio.c from the
broken vfio_container_ioctl() interface to the new vfio_as_eeh_op()
interface.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>mak
---
hw/ppc/spapr_pci_vfio.c | 50 ++++++++++++++++---------------------------------
1 file changed, 16 insertions(+), 34 deletions(-)
diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
index 2f3752e..b1e8e8e 100644
--- a/hw/ppc/spapr_pci_vfio.c
+++ b/hw/ppc/spapr_pci_vfio.c
@@ -73,15 +73,9 @@ static void spapr_phb_vfio_finish_realize(sPAPRPHBState *sphb, Error **errp)
spapr_tce_get_iommu(tcet));
}
-static void spapr_phb_vfio_eeh_reenable(sPAPRPHBVFIOState *svphb)
+static void spapr_phb_vfio_eeh_reenable(sPAPRPHBState *sphb)
{
- struct vfio_eeh_pe_op op = {
- .argsz = sizeof(op),
- .op = VFIO_EEH_PE_ENABLE
- };
-
- vfio_container_ioctl(&svphb->phb.iommu_as,
- svphb->iommugroupid, VFIO_EEH_PE_OP, &op);
+ vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_ENABLE);
}
static void spapr_phb_vfio_reset(DeviceState *qdev)
@@ -92,19 +86,18 @@ static void spapr_phb_vfio_reset(DeviceState *qdev)
* ensures that the contained PCI devices will work properly
* after reboot.
*/
- spapr_phb_vfio_eeh_reenable(SPAPR_PCI_VFIO_HOST_BRIDGE(qdev));
+ spapr_phb_vfio_eeh_reenable(SPAPR_PCI_HOST_BRIDGE(qdev));
}
static int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
unsigned int addr, int option)
{
- sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
- struct vfio_eeh_pe_op op = { .argsz = sizeof(op) };
+ uint32_t op;
int ret;
switch (option) {
case RTAS_EEH_DISABLE:
- op.op = VFIO_EEH_PE_DISABLE;
+ op = VFIO_EEH_PE_DISABLE;
break;
case RTAS_EEH_ENABLE: {
PCIHostState *phb;
@@ -122,21 +115,20 @@ static int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
return RTAS_OUT_PARAM_ERROR;
}
- op.op = VFIO_EEH_PE_ENABLE;
+ op = VFIO_EEH_PE_ENABLE;
break;
}
case RTAS_EEH_THAW_IO:
- op.op = VFIO_EEH_PE_UNFREEZE_IO;
+ op = VFIO_EEH_PE_UNFREEZE_IO;
break;
case RTAS_EEH_THAW_DMA:
- op.op = VFIO_EEH_PE_UNFREEZE_DMA;
+ op = VFIO_EEH_PE_UNFREEZE_DMA;
break;
default:
return RTAS_OUT_PARAM_ERROR;
}
- ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
- VFIO_EEH_PE_OP, &op);
+ ret = vfio_eeh_as_op(&sphb->iommu_as, op);
if (ret < 0) {
return RTAS_OUT_HW_ERROR;
}
@@ -146,13 +138,9 @@ static int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
static int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state)
{
- sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
- struct vfio_eeh_pe_op op = { .argsz = sizeof(op) };
int ret;
- op.op = VFIO_EEH_PE_GET_STATE;
- ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
- VFIO_EEH_PE_OP, &op);
+ ret = vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_GET_STATE);
if (ret < 0) {
return RTAS_OUT_PARAM_ERROR;
}
@@ -206,28 +194,26 @@ static void spapr_phb_vfio_eeh_pre_reset(sPAPRPHBState *sphb)
static int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option)
{
- sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
- struct vfio_eeh_pe_op op = { .argsz = sizeof(op) };
+ uint32_t op;
int ret;
switch (option) {
case RTAS_SLOT_RESET_DEACTIVATE:
- op.op = VFIO_EEH_PE_RESET_DEACTIVATE;
+ op = VFIO_EEH_PE_RESET_DEACTIVATE;
break;
case RTAS_SLOT_RESET_HOT:
spapr_phb_vfio_eeh_pre_reset(sphb);
- op.op = VFIO_EEH_PE_RESET_HOT;
+ op = VFIO_EEH_PE_RESET_HOT;
break;
case RTAS_SLOT_RESET_FUNDAMENTAL:
spapr_phb_vfio_eeh_pre_reset(sphb);
- op.op = VFIO_EEH_PE_RESET_FUNDAMENTAL;
+ op = VFIO_EEH_PE_RESET_FUNDAMENTAL;
break;
default:
return RTAS_OUT_PARAM_ERROR;
}
- ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
- VFIO_EEH_PE_OP, &op);
+ ret = vfio_eeh_as_op(&sphb->iommu_as, op);
if (ret < 0) {
return RTAS_OUT_HW_ERROR;
}
@@ -237,13 +223,9 @@ static int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option)
static int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb)
{
- sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
- struct vfio_eeh_pe_op op = { .argsz = sizeof(op) };
int ret;
- op.op = VFIO_EEH_PE_CONFIGURE;
- ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
- VFIO_EEH_PE_OP, &op);
+ ret = vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_CONFIGURE);
if (ret < 0) {
return RTAS_OUT_PARAM_ERROR;
}
--
2.5.0
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 03/12] spapr_pci: Eliminate class callbacks
2016-02-26 11:31 [Qemu-devel] [PATCH 00/12] Allow EEH on spapr-pci-host-bridge devices David Gibson
2016-02-26 11:31 ` [Qemu-devel] [PATCH 01/12] vfio: Start improving VFIO/EEH interface David Gibson
2016-02-26 11:31 ` [Qemu-devel] [PATCH 02/12] spapr_pci: Switch to vfio_eeh_as_op() interface David Gibson
@ 2016-02-26 11:31 ` David Gibson
2016-02-29 1:43 ` Alexey Kardashevskiy
2016-02-26 11:31 ` [Qemu-devel] [PATCH 04/12] spapr_pci: Fold spapr_phb_vfio_eeh_configure() into spapr_pci code David Gibson
` (9 subsequent siblings)
12 siblings, 1 reply; 33+ messages in thread
From: David Gibson @ 2016-02-26 11:31 UTC (permalink / raw)
To: aik, benh
Cc: agraf, qemu-devel, gwshan, mdroth, alex.williamson, qemu-ppc,
David Gibson
The EEH operations in the spapr-vfio-pci-host-bridge no longer rely on the
special groupid field in sPAPRPHBVFIOState. So we can simplify, removing
the class specific callbacks with direct calls based on a simple
spapr_phb_eeh_enabled() helper. For now we implement that in terms of
a boolean in the class, but we'll continue to clean that up later.
On its own this is a rather strange way of doing things, but it's a useful
intermediate step to further cleanups.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/ppc/spapr_pci.c | 44 ++++++++++++++++++++++----------------------
hw/ppc/spapr_pci_vfio.c | 18 +++++++-----------
include/hw/pci-host/spapr.h | 13 +++++++++----
3 files changed, 38 insertions(+), 37 deletions(-)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 9b2b546..d1e5222 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -92,6 +92,13 @@ PCIDevice *spapr_pci_find_dev(sPAPRMachineState *spapr, uint64_t buid,
return pci_find_device(phb->bus, bus_num, devfn);
}
+static bool spapr_phb_eeh_available(sPAPRPHBState *sphb)
+{
+ sPAPRPHBClass *spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
+
+ return spc->eeh_available;
+}
+
static uint32_t rtas_pci_cfgaddr(uint32_t arg)
{
/* This handles the encoding of extended config space addresses */
@@ -438,7 +445,6 @@ static void rtas_ibm_set_eeh_option(PowerPCCPU *cpu,
target_ulong rets)
{
sPAPRPHBState *sphb;
- sPAPRPHBClass *spc;
uint32_t addr, option;
uint64_t buid;
int ret;
@@ -456,12 +462,11 @@ static void rtas_ibm_set_eeh_option(PowerPCCPU *cpu,
goto param_error_exit;
}
- spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
- if (!spc->eeh_set_option) {
+ if (!spapr_phb_eeh_available(sphb)) {
goto param_error_exit;
}
- ret = spc->eeh_set_option(sphb, addr, option);
+ ret = spapr_phb_vfio_eeh_set_option(sphb, addr, option);
rtas_st(rets, 0, ret);
return;
@@ -476,7 +481,6 @@ static void rtas_ibm_get_config_addr_info2(PowerPCCPU *cpu,
target_ulong rets)
{
sPAPRPHBState *sphb;
- sPAPRPHBClass *spc;
PCIDevice *pdev;
uint32_t addr, option;
uint64_t buid;
@@ -491,8 +495,7 @@ static void rtas_ibm_get_config_addr_info2(PowerPCCPU *cpu,
goto param_error_exit;
}
- spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
- if (!spc->eeh_set_option) {
+ if (!spapr_phb_eeh_available(sphb)) {
goto param_error_exit;
}
@@ -532,7 +535,6 @@ static void rtas_ibm_read_slot_reset_state2(PowerPCCPU *cpu,
target_ulong rets)
{
sPAPRPHBState *sphb;
- sPAPRPHBClass *spc;
uint64_t buid;
int state, ret;
@@ -546,12 +548,11 @@ static void rtas_ibm_read_slot_reset_state2(PowerPCCPU *cpu,
goto param_error_exit;
}
- spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
- if (!spc->eeh_get_state) {
+ if (!spapr_phb_eeh_available(sphb)) {
goto param_error_exit;
}
- ret = spc->eeh_get_state(sphb, &state);
+ ret = spapr_phb_vfio_eeh_get_state(sphb, &state);
rtas_st(rets, 0, ret);
if (ret != RTAS_OUT_SUCCESS) {
return;
@@ -576,7 +577,6 @@ static void rtas_ibm_set_slot_reset(PowerPCCPU *cpu,
target_ulong rets)
{
sPAPRPHBState *sphb;
- sPAPRPHBClass *spc;
uint32_t option;
uint64_t buid;
int ret;
@@ -592,12 +592,11 @@ static void rtas_ibm_set_slot_reset(PowerPCCPU *cpu,
goto param_error_exit;
}
- spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
- if (!spc->eeh_reset) {
+ if (!spapr_phb_eeh_available(sphb)) {
goto param_error_exit;
}
- ret = spc->eeh_reset(sphb, option);
+ ret = spapr_phb_vfio_eeh_reset(sphb, option);
rtas_st(rets, 0, ret);
return;
@@ -612,7 +611,6 @@ static void rtas_ibm_configure_pe(PowerPCCPU *cpu,
target_ulong rets)
{
sPAPRPHBState *sphb;
- sPAPRPHBClass *spc;
uint64_t buid;
int ret;
@@ -626,12 +624,11 @@ static void rtas_ibm_configure_pe(PowerPCCPU *cpu,
goto param_error_exit;
}
- spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
- if (!spc->eeh_configure) {
+ if (!spapr_phb_eeh_available(sphb)) {
goto param_error_exit;
}
- ret = spc->eeh_configure(sphb);
+ ret = spapr_phb_vfio_eeh_configure(sphb);
rtas_st(rets, 0, ret);
return;
@@ -647,7 +644,6 @@ static void rtas_ibm_slot_error_detail(PowerPCCPU *cpu,
target_ulong rets)
{
sPAPRPHBState *sphb;
- sPAPRPHBClass *spc;
int option;
uint64_t buid;
@@ -661,8 +657,7 @@ static void rtas_ibm_slot_error_detail(PowerPCCPU *cpu,
goto param_error_exit;
}
- spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
- if (!spc->eeh_set_option) {
+ if (!spapr_phb_eeh_available(sphb)) {
goto param_error_exit;
}
@@ -1430,6 +1425,10 @@ static void spapr_phb_reset(DeviceState *qdev)
{
/* Reset the IOMMU state */
object_child_foreach(OBJECT(qdev), spapr_phb_children_reset, NULL);
+
+ if (spapr_phb_eeh_available(SPAPR_PCI_HOST_BRIDGE(qdev))) {
+ spapr_phb_vfio_reset(qdev);
+ }
}
static Property spapr_phb_properties[] = {
@@ -1560,6 +1559,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->finish_realize = spapr_phb_finish_realize;
+ spc->eeh_available = false;
hp->plug = spapr_phb_hot_plug_child;
hp->unplug = spapr_phb_hot_unplug_child;
}
diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
index b1e8e8e..10fa88a 100644
--- a/hw/ppc/spapr_pci_vfio.c
+++ b/hw/ppc/spapr_pci_vfio.c
@@ -78,7 +78,7 @@ static void spapr_phb_vfio_eeh_reenable(sPAPRPHBState *sphb)
vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_ENABLE);
}
-static void spapr_phb_vfio_reset(DeviceState *qdev)
+void spapr_phb_vfio_reset(DeviceState *qdev)
{
/*
* The PE might be in frozen state. To reenable the EEH
@@ -89,8 +89,8 @@ static void spapr_phb_vfio_reset(DeviceState *qdev)
spapr_phb_vfio_eeh_reenable(SPAPR_PCI_HOST_BRIDGE(qdev));
}
-static int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
- unsigned int addr, int option)
+int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
+ unsigned int addr, int option)
{
uint32_t op;
int ret;
@@ -136,7 +136,7 @@ static int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
return RTAS_OUT_SUCCESS;
}
-static int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state)
+int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state)
{
int ret;
@@ -192,7 +192,7 @@ static void spapr_phb_vfio_eeh_pre_reset(sPAPRPHBState *sphb)
pci_for_each_bus(phb->bus, spapr_phb_vfio_eeh_clear_bus_msix, NULL);
}
-static int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option)
+int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option)
{
uint32_t op;
int ret;
@@ -221,7 +221,7 @@ static int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option)
return RTAS_OUT_SUCCESS;
}
-static int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb)
+int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb)
{
int ret;
@@ -239,12 +239,8 @@ 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;
- spc->eeh_reset = spapr_phb_vfio_eeh_reset;
- spc->eeh_configure = spapr_phb_vfio_eeh_configure;
+ spc->eeh_available = true;
}
static const TypeInfo spapr_phb_vfio_info = {
diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
index 7de5e02..cc1b82c 100644
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -49,10 +49,7 @@ struct sPAPRPHBClass {
PCIHostBridgeClass parent_class;
void (*finish_realize)(sPAPRPHBState *sphb, Error **errp);
- 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);
- int (*eeh_configure)(sPAPRPHBState *sphb);
+ bool eeh_available;
};
typedef struct spapr_pci_msi {
@@ -136,5 +133,13 @@ void spapr_pci_rtas_init(void);
sPAPRPHBState *spapr_pci_find_phb(sPAPRMachineState *spapr, uint64_t buid);
PCIDevice *spapr_pci_find_dev(sPAPRMachineState *spapr, uint64_t buid,
uint32_t config_addr);
+void spapr_phb_vfio_reset(DeviceState *qdev);
+
+/* VFIO EEH hooks */
+int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
+ unsigned int addr, int option);
+int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state);
+int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option);
+int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb);
#endif /* __HW_SPAPR_PCI_H__ */
--
2.5.0
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 04/12] spapr_pci: Fold spapr_phb_vfio_eeh_configure() into spapr_pci code
2016-02-26 11:31 [Qemu-devel] [PATCH 00/12] Allow EEH on spapr-pci-host-bridge devices David Gibson
` (2 preceding siblings ...)
2016-02-26 11:31 ` [Qemu-devel] [PATCH 03/12] spapr_pci: Eliminate class callbacks David Gibson
@ 2016-02-26 11:31 ` David Gibson
2016-02-29 1:43 ` Alexey Kardashevskiy
2016-02-26 11:31 ` [Qemu-devel] [PATCH 05/12] spapr_pci: Fold spapr_phb_vfio_eeh_reset() " David Gibson
` (8 subsequent siblings)
12 siblings, 1 reply; 33+ messages in thread
From: David Gibson @ 2016-02-26 11:31 UTC (permalink / raw)
To: aik, benh
Cc: agraf, qemu-devel, gwshan, mdroth, alex.williamson, qemu-ppc,
David Gibson
Simplify the sPAPR PCI code by folding spapr_phb_vfio_eeh_configure()
into rtas_ibm_configure_pe().
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/ppc/spapr_pci.c | 11 +++++++++--
hw/ppc/spapr_pci_vfio.c | 12 ------------
include/hw/pci-host/spapr.h | 1 -
3 files changed, 9 insertions(+), 15 deletions(-)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index d1e5222..fa633a9 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -42,6 +42,9 @@
#include "hw/ppc/spapr_drc.h"
#include "sysemu/device_tree.h"
+#include "hw/vfio/vfio.h"
+#include <linux/vfio.h>
+
/* Copied from the kernel arch/powerpc/platforms/pseries/msi.c */
#define RTAS_QUERY_FN 0
#define RTAS_CHANGE_FN 1
@@ -628,8 +631,12 @@ static void rtas_ibm_configure_pe(PowerPCCPU *cpu,
goto param_error_exit;
}
- ret = spapr_phb_vfio_eeh_configure(sphb);
- rtas_st(rets, 0, ret);
+ ret = vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_CONFIGURE);
+ if (ret < 0) {
+ goto param_error_exit;
+ }
+
+ rtas_st(rets, 0, RTAS_OUT_SUCCESS);
return;
param_error_exit:
diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
index 10fa88a..4ac5736 100644
--- a/hw/ppc/spapr_pci_vfio.c
+++ b/hw/ppc/spapr_pci_vfio.c
@@ -221,18 +221,6 @@ int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option)
return RTAS_OUT_SUCCESS;
}
-int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb)
-{
- int ret;
-
- ret = vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_CONFIGURE);
- if (ret < 0) {
- return RTAS_OUT_PARAM_ERROR;
- }
-
- return RTAS_OUT_SUCCESS;
-}
-
static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
index cc1b82c..f02ef51 100644
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -140,6 +140,5 @@ int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
unsigned int addr, int option);
int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state);
int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option);
-int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb);
#endif /* __HW_SPAPR_PCI_H__ */
--
2.5.0
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 05/12] spapr_pci: Fold spapr_phb_vfio_eeh_reset() into spapr_pci code
2016-02-26 11:31 [Qemu-devel] [PATCH 00/12] Allow EEH on spapr-pci-host-bridge devices David Gibson
` (3 preceding siblings ...)
2016-02-26 11:31 ` [Qemu-devel] [PATCH 04/12] spapr_pci: Fold spapr_phb_vfio_eeh_configure() into spapr_pci code David Gibson
@ 2016-02-26 11:31 ` David Gibson
2016-02-29 1:43 ` Alexey Kardashevskiy
2016-02-26 11:31 ` [Qemu-devel] [PATCH 06/12] spapr_pci: Fold spapr_phb_vfio_eeh_get_state() " David Gibson
` (7 subsequent siblings)
12 siblings, 1 reply; 33+ messages in thread
From: David Gibson @ 2016-02-26 11:31 UTC (permalink / raw)
To: aik, benh
Cc: agraf, qemu-devel, gwshan, mdroth, alex.williamson, qemu-ppc,
David Gibson
Simplify the sPAPR PCI code by folding spapr_phb_vfio_eeh_reset() into
rtas_ibm_set_slot_reset(). We move several functions of which it was
the only caller (spapr_phb_eeh_clear_dev_msix(),
spapr_phb_eeh_clear_bus_msix() and spapr_phb_eeh_pre_reset()) into
spapr_pci.c along with it.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/ppc/spapr_pci.c | 68 ++++++++++++++++++++++++++++++++++++++++--
hw/ppc/spapr_pci_vfio.c | 72 ---------------------------------------------
include/hw/pci-host/spapr.h | 1 -
3 files changed, 66 insertions(+), 75 deletions(-)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index fa633a9..96cdca2 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -573,6 +573,48 @@ param_error_exit:
rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
}
+static void spapr_phb_eeh_clear_dev_msix(PCIBus *bus, PCIDevice *pdev,
+ void *opaque)
+{
+ /* Check if the device is VFIO PCI device */
+ if (!object_dynamic_cast(OBJECT(pdev), "vfio-pci")) {
+ return;
+ }
+
+ /*
+ * The MSIx table will be cleaned out by reset. We need
+ * disable it so that it can be reenabled properly. Also,
+ * the cached MSIx table should be cleared as it's not
+ * reflecting the contents in hardware.
+ */
+ if (msix_enabled(pdev)) {
+ uint16_t flags;
+
+ flags = pci_host_config_read_common(pdev,
+ pdev->msix_cap + PCI_MSIX_FLAGS,
+ pci_config_size(pdev), 2);
+ flags &= ~PCI_MSIX_FLAGS_ENABLE;
+ pci_host_config_write_common(pdev,
+ pdev->msix_cap + PCI_MSIX_FLAGS,
+ pci_config_size(pdev), flags, 2);
+ }
+
+ msix_reset(pdev);
+}
+
+static void spapr_phb_eeh_clear_bus_msix(PCIBus *bus, void *opaque)
+{
+ pci_for_each_device(bus, pci_bus_num(bus),
+ spapr_phb_eeh_clear_dev_msix, NULL);
+}
+
+static void spapr_phb_eeh_pre_reset(sPAPRPHBState *sphb)
+{
+ PCIHostState *phb = PCI_HOST_BRIDGE(sphb);
+
+ pci_for_each_bus(phb->bus, spapr_phb_eeh_clear_bus_msix, NULL);
+}
+
static void rtas_ibm_set_slot_reset(PowerPCCPU *cpu,
sPAPRMachineState *spapr,
uint32_t token, uint32_t nargs,
@@ -582,6 +624,7 @@ static void rtas_ibm_set_slot_reset(PowerPCCPU *cpu,
sPAPRPHBState *sphb;
uint32_t option;
uint64_t buid;
+ uint32_t op;
int ret;
if ((nargs != 4) || (nret != 1)) {
@@ -599,8 +642,29 @@ static void rtas_ibm_set_slot_reset(PowerPCCPU *cpu,
goto param_error_exit;
}
- ret = spapr_phb_vfio_eeh_reset(sphb, option);
- rtas_st(rets, 0, ret);
+ switch (option) {
+ case RTAS_SLOT_RESET_DEACTIVATE:
+ op = VFIO_EEH_PE_RESET_DEACTIVATE;
+ break;
+ case RTAS_SLOT_RESET_HOT:
+ spapr_phb_eeh_pre_reset(sphb);
+ op = VFIO_EEH_PE_RESET_HOT;
+ break;
+ case RTAS_SLOT_RESET_FUNDAMENTAL:
+ spapr_phb_eeh_pre_reset(sphb);
+ op = VFIO_EEH_PE_RESET_FUNDAMENTAL;
+ break;
+ default:
+ goto param_error_exit;
+ }
+
+ ret = vfio_eeh_as_op(&sphb->iommu_as, op);
+ if (ret < 0) {
+ rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
+ return;
+ }
+
+ rtas_st(rets, 0, RTAS_OUT_SUCCESS);
return;
param_error_exit:
diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
index 4ac5736..54075e0 100644
--- a/hw/ppc/spapr_pci_vfio.c
+++ b/hw/ppc/spapr_pci_vfio.c
@@ -149,78 +149,6 @@ int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state)
return RTAS_OUT_SUCCESS;
}
-static void spapr_phb_vfio_eeh_clear_dev_msix(PCIBus *bus,
- PCIDevice *pdev,
- void *opaque)
-{
- /* Check if the device is VFIO PCI device */
- if (!object_dynamic_cast(OBJECT(pdev), "vfio-pci")) {
- return;
- }
-
- /*
- * The MSIx table will be cleaned out by reset. We need
- * disable it so that it can be reenabled properly. Also,
- * the cached MSIx table should be cleared as it's not
- * reflecting the contents in hardware.
- */
- if (msix_enabled(pdev)) {
- uint16_t flags;
-
- flags = pci_host_config_read_common(pdev,
- pdev->msix_cap + PCI_MSIX_FLAGS,
- pci_config_size(pdev), 2);
- flags &= ~PCI_MSIX_FLAGS_ENABLE;
- pci_host_config_write_common(pdev,
- pdev->msix_cap + PCI_MSIX_FLAGS,
- pci_config_size(pdev), flags, 2);
- }
-
- msix_reset(pdev);
-}
-
-static void spapr_phb_vfio_eeh_clear_bus_msix(PCIBus *bus, void *opaque)
-{
- pci_for_each_device(bus, pci_bus_num(bus),
- spapr_phb_vfio_eeh_clear_dev_msix, NULL);
-}
-
-static void spapr_phb_vfio_eeh_pre_reset(sPAPRPHBState *sphb)
-{
- PCIHostState *phb = PCI_HOST_BRIDGE(sphb);
-
- pci_for_each_bus(phb->bus, spapr_phb_vfio_eeh_clear_bus_msix, NULL);
-}
-
-int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option)
-{
- uint32_t op;
- int ret;
-
- switch (option) {
- case RTAS_SLOT_RESET_DEACTIVATE:
- op = VFIO_EEH_PE_RESET_DEACTIVATE;
- break;
- case RTAS_SLOT_RESET_HOT:
- spapr_phb_vfio_eeh_pre_reset(sphb);
- op = VFIO_EEH_PE_RESET_HOT;
- break;
- case RTAS_SLOT_RESET_FUNDAMENTAL:
- spapr_phb_vfio_eeh_pre_reset(sphb);
- op = VFIO_EEH_PE_RESET_FUNDAMENTAL;
- break;
- default:
- return RTAS_OUT_PARAM_ERROR;
- }
-
- ret = vfio_eeh_as_op(&sphb->iommu_as, op);
- if (ret < 0) {
- return RTAS_OUT_HW_ERROR;
- }
-
- return RTAS_OUT_SUCCESS;
-}
-
static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
index f02ef51..d31162b 100644
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -139,6 +139,5 @@ void spapr_phb_vfio_reset(DeviceState *qdev);
int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
unsigned int addr, int option);
int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state);
-int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option);
#endif /* __HW_SPAPR_PCI_H__ */
--
2.5.0
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 06/12] spapr_pci: Fold spapr_phb_vfio_eeh_get_state() into spapr_pci code
2016-02-26 11:31 [Qemu-devel] [PATCH 00/12] Allow EEH on spapr-pci-host-bridge devices David Gibson
` (4 preceding siblings ...)
2016-02-26 11:31 ` [Qemu-devel] [PATCH 05/12] spapr_pci: Fold spapr_phb_vfio_eeh_reset() " David Gibson
@ 2016-02-26 11:31 ` David Gibson
2016-02-29 1:43 ` Alexey Kardashevskiy
2016-02-26 11:31 ` [Qemu-devel] [PATCH 07/12] spapr_pci: Fold spapr_phb_vfio_eeh_set_option() " David Gibson
` (6 subsequent siblings)
12 siblings, 1 reply; 33+ messages in thread
From: David Gibson @ 2016-02-26 11:31 UTC (permalink / raw)
To: aik, benh
Cc: agraf, qemu-devel, gwshan, mdroth, alex.williamson, qemu-ppc,
David Gibson
Simplify the sPAPR PCI code by folding spapr_phb_vfio_eeh_get_state9)
into rtas_ibm_read_slot_reset_state2().
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/ppc/spapr_pci.c | 12 ++++++------
hw/ppc/spapr_pci_vfio.c | 13 -------------
include/hw/pci-host/spapr.h | 1 -
3 files changed, 6 insertions(+), 20 deletions(-)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 96cdca2..eaae7e2 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -539,7 +539,7 @@ static void rtas_ibm_read_slot_reset_state2(PowerPCCPU *cpu,
{
sPAPRPHBState *sphb;
uint64_t buid;
- int state, ret;
+ int ret;
if ((nargs != 3) || (nret != 4 && nret != 5)) {
goto param_error_exit;
@@ -555,13 +555,13 @@ static void rtas_ibm_read_slot_reset_state2(PowerPCCPU *cpu,
goto param_error_exit;
}
- ret = spapr_phb_vfio_eeh_get_state(sphb, &state);
- rtas_st(rets, 0, ret);
- if (ret != RTAS_OUT_SUCCESS) {
- return;
+ ret = vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_GET_STATE);
+ if (ret < 0) {
+ goto param_error_exit;
}
- rtas_st(rets, 1, state);
+ rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+ rtas_st(rets, 1, ret);
rtas_st(rets, 2, RTAS_EEH_SUPPORT);
rtas_st(rets, 3, RTAS_EEH_PE_UNAVAIL_INFO);
if (nret >= 5) {
diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
index 54075e0..c87f2e4 100644
--- a/hw/ppc/spapr_pci_vfio.c
+++ b/hw/ppc/spapr_pci_vfio.c
@@ -136,19 +136,6 @@ int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
return RTAS_OUT_SUCCESS;
}
-int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state)
-{
- int ret;
-
- ret = vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_GET_STATE);
- if (ret < 0) {
- return RTAS_OUT_PARAM_ERROR;
- }
-
- *state = ret;
- return RTAS_OUT_SUCCESS;
-}
-
static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
index d31162b..55237fc 100644
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -138,6 +138,5 @@ void spapr_phb_vfio_reset(DeviceState *qdev);
/* VFIO EEH hooks */
int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
unsigned int addr, int option);
-int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state);
#endif /* __HW_SPAPR_PCI_H__ */
--
2.5.0
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 07/12] spapr_pci: Fold spapr_phb_vfio_eeh_set_option() into spapr_pci code
2016-02-26 11:31 [Qemu-devel] [PATCH 00/12] Allow EEH on spapr-pci-host-bridge devices David Gibson
` (5 preceding siblings ...)
2016-02-26 11:31 ` [Qemu-devel] [PATCH 06/12] spapr_pci: Fold spapr_phb_vfio_eeh_get_state() " David Gibson
@ 2016-02-26 11:31 ` David Gibson
2016-02-29 1:44 ` Alexey Kardashevskiy
2016-02-26 11:31 ` [Qemu-devel] [PATCH 08/12] spapr_pci: Fold spapr_phb_vfio_reset() " David Gibson
` (5 subsequent siblings)
12 siblings, 1 reply; 33+ messages in thread
From: David Gibson @ 2016-02-26 11:31 UTC (permalink / raw)
To: aik, benh
Cc: agraf, qemu-devel, gwshan, mdroth, alex.williamson, qemu-ppc,
David Gibson
Simplify the sPAPR PCI code by folding spapr_phb_eeh_set_option() into
rtas_ibm_set_eeh_option().
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/ppc/spapr_pci.c | 43 +++++++++++++++++++++++++++++++++++++++--
hw/ppc/spapr_pci_vfio.c | 47 ---------------------------------------------
include/hw/pci-host/spapr.h | 4 ----
3 files changed, 41 insertions(+), 53 deletions(-)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index eaae7e2..26d08ad 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -450,6 +450,7 @@ static void rtas_ibm_set_eeh_option(PowerPCCPU *cpu,
sPAPRPHBState *sphb;
uint32_t addr, option;
uint64_t buid;
+ uint32_t op;
int ret;
if ((nargs != 4) || (nret != 1)) {
@@ -469,8 +470,46 @@ static void rtas_ibm_set_eeh_option(PowerPCCPU *cpu,
goto param_error_exit;
}
- ret = spapr_phb_vfio_eeh_set_option(sphb, addr, option);
- rtas_st(rets, 0, ret);
+ switch (option) {
+ case RTAS_EEH_DISABLE:
+ op = VFIO_EEH_PE_DISABLE;
+ break;
+ case RTAS_EEH_ENABLE: {
+ PCIHostState *phb;
+ PCIDevice *pdev;
+
+ /*
+ * The EEH functionality is enabled on basis of PCI device,
+ * instead of PE. We need check the validity of the PCI
+ * device address.
+ */
+ phb = PCI_HOST_BRIDGE(sphb);
+ pdev = pci_find_device(phb->bus,
+ (addr >> 16) & 0xFF, (addr >> 8) & 0xFF);
+ if (!pdev || !object_dynamic_cast(OBJECT(pdev), "vfio-pci")) {
+ goto param_error_exit;
+ }
+
+ op = VFIO_EEH_PE_ENABLE;
+ break;
+ }
+ case RTAS_EEH_THAW_IO:
+ op = VFIO_EEH_PE_UNFREEZE_IO;
+ break;
+ case RTAS_EEH_THAW_DMA:
+ op = VFIO_EEH_PE_UNFREEZE_DMA;
+ break;
+ default:
+ goto param_error_exit;
+ }
+
+ ret = vfio_eeh_as_op(&sphb->iommu_as, op);
+ if (ret < 0) {
+ rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
+ return;
+ }
+
+ rtas_st(rets, 0, RTAS_OUT_SUCCESS);
return;
param_error_exit:
diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
index c87f2e4..cccd444 100644
--- a/hw/ppc/spapr_pci_vfio.c
+++ b/hw/ppc/spapr_pci_vfio.c
@@ -89,53 +89,6 @@ void spapr_phb_vfio_reset(DeviceState *qdev)
spapr_phb_vfio_eeh_reenable(SPAPR_PCI_HOST_BRIDGE(qdev));
}
-int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
- unsigned int addr, int option)
-{
- uint32_t op;
- int ret;
-
- switch (option) {
- case RTAS_EEH_DISABLE:
- op = VFIO_EEH_PE_DISABLE;
- break;
- case RTAS_EEH_ENABLE: {
- PCIHostState *phb;
- PCIDevice *pdev;
-
- /*
- * The EEH functionality is enabled on basis of PCI device,
- * instead of PE. We need check the validity of the PCI
- * device address.
- */
- phb = PCI_HOST_BRIDGE(sphb);
- pdev = pci_find_device(phb->bus,
- (addr >> 16) & 0xFF, (addr >> 8) & 0xFF);
- if (!pdev || !object_dynamic_cast(OBJECT(pdev), "vfio-pci")) {
- return RTAS_OUT_PARAM_ERROR;
- }
-
- op = VFIO_EEH_PE_ENABLE;
- break;
- }
- case RTAS_EEH_THAW_IO:
- op = VFIO_EEH_PE_UNFREEZE_IO;
- break;
- case RTAS_EEH_THAW_DMA:
- op = VFIO_EEH_PE_UNFREEZE_DMA;
- break;
- default:
- return RTAS_OUT_PARAM_ERROR;
- }
-
- ret = vfio_eeh_as_op(&sphb->iommu_as, op);
- if (ret < 0) {
- return RTAS_OUT_HW_ERROR;
- }
-
- return RTAS_OUT_SUCCESS;
-}
-
static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
index 55237fc..d32750e 100644
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -135,8 +135,4 @@ PCIDevice *spapr_pci_find_dev(sPAPRMachineState *spapr, uint64_t buid,
uint32_t config_addr);
void spapr_phb_vfio_reset(DeviceState *qdev);
-/* VFIO EEH hooks */
-int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
- unsigned int addr, int option);
-
#endif /* __HW_SPAPR_PCI_H__ */
--
2.5.0
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 08/12] spapr_pci: Fold spapr_phb_vfio_reset() into spapr_pci code
2016-02-26 11:31 [Qemu-devel] [PATCH 00/12] Allow EEH on spapr-pci-host-bridge devices David Gibson
` (6 preceding siblings ...)
2016-02-26 11:31 ` [Qemu-devel] [PATCH 07/12] spapr_pci: Fold spapr_phb_vfio_eeh_set_option() " David Gibson
@ 2016-02-26 11:31 ` David Gibson
2016-02-29 1:44 ` Alexey Kardashevskiy
2016-02-26 11:32 ` [Qemu-devel] [PATCH 09/12] spapr_pci: Allow EEH on spapr-pci-host-bridge David Gibson
` (4 subsequent siblings)
12 siblings, 1 reply; 33+ messages in thread
From: David Gibson @ 2016-02-26 11:31 UTC (permalink / raw)
To: aik, benh
Cc: agraf, qemu-devel, gwshan, mdroth, alex.williamson, qemu-ppc,
David Gibson
Simplify the sPAPR PCI code by folding spapr_phb_vfio_reset() into
spapr_phb_reset().
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/ppc/spapr_pci.c | 13 ++++++++++++-
hw/ppc/spapr_pci_vfio.c | 16 ----------------
2 files changed, 12 insertions(+), 17 deletions(-)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 26d08ad..be18bf6 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1531,13 +1531,24 @@ static int spapr_phb_children_reset(Object *child, void *opaque)
return 0;
}
+static void spapr_phb_eeh_reenable(sPAPRPHBState *sphb)
+{
+ vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_ENABLE);
+}
+
static void spapr_phb_reset(DeviceState *qdev)
{
/* Reset the IOMMU state */
object_child_foreach(OBJECT(qdev), spapr_phb_children_reset, NULL);
if (spapr_phb_eeh_available(SPAPR_PCI_HOST_BRIDGE(qdev))) {
- spapr_phb_vfio_reset(qdev);
+ /*
+ * The PE might be in frozen state. To reenable the EEH
+ * functionality on it will clean the frozen state, which
+ * ensures that the contained PCI devices will work properly
+ * after reboot.
+ */
+ spapr_phb_eeh_reenable(SPAPR_PCI_HOST_BRIDGE(qdev));
}
}
diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
index cccd444..0a0f31a 100644
--- a/hw/ppc/spapr_pci_vfio.c
+++ b/hw/ppc/spapr_pci_vfio.c
@@ -73,22 +73,6 @@ static void spapr_phb_vfio_finish_realize(sPAPRPHBState *sphb, Error **errp)
spapr_tce_get_iommu(tcet));
}
-static void spapr_phb_vfio_eeh_reenable(sPAPRPHBState *sphb)
-{
- vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_ENABLE);
-}
-
-void spapr_phb_vfio_reset(DeviceState *qdev)
-{
- /*
- * The PE might be in frozen state. To reenable the EEH
- * functionality on it will clean the frozen state, which
- * ensures that the contained PCI devices will work properly
- * after reboot.
- */
- spapr_phb_vfio_eeh_reenable(SPAPR_PCI_HOST_BRIDGE(qdev));
-}
-
static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
--
2.5.0
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 09/12] spapr_pci: Allow EEH on spapr-pci-host-bridge
2016-02-26 11:31 [Qemu-devel] [PATCH 00/12] Allow EEH on spapr-pci-host-bridge devices David Gibson
` (7 preceding siblings ...)
2016-02-26 11:31 ` [Qemu-devel] [PATCH 08/12] spapr_pci: Fold spapr_phb_vfio_reset() " David Gibson
@ 2016-02-26 11:32 ` David Gibson
2016-02-29 1:44 ` Alexey Kardashevskiy
2016-02-26 11:32 ` [Qemu-devel] [PATCH 10/12] spapr_pci: (Mostly) remove spapr-pci-vfio-host-bridge David Gibson
` (3 subsequent siblings)
12 siblings, 1 reply; 33+ messages in thread
From: David Gibson @ 2016-02-26 11:32 UTC (permalink / raw)
To: aik, benh
Cc: agraf, qemu-devel, gwshan, mdroth, alex.williamson, qemu-ppc,
David Gibson
Now that the EEH code is independent of the special
spapr-vfio-pci-host-bridge device, we can allow it on all spapr PCI
host bridges instead. We do this by changing spapr_phb_eeh_available()
to be based on the vfio_eeh_as_ok() call instead of the host bridge class.
Because the value of vfio_eeh_as_ok() can change with devices being
hotplugged or unplugged, this can potentially lead to some strange edge
cases where the guest starts using EEH, then it starts failing because
of a change in status.
However, it's not really any worse than the current situation. Cases that
would have worked previously will still work (i.e. VFIO devices from at
most one VFIO IOMMU group per vPHB), it's just that it's no longer
necessary to use spapr-vfio-pci-host-bridge with the groupid pre-specified.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/ppc/spapr_pci.c | 5 +----
hw/ppc/spapr_pci_vfio.c | 1 -
include/hw/pci-host/spapr.h | 1 -
3 files changed, 1 insertion(+), 6 deletions(-)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index be18bf6..336ffd2 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -97,9 +97,7 @@ PCIDevice *spapr_pci_find_dev(sPAPRMachineState *spapr, uint64_t buid,
static bool spapr_phb_eeh_available(sPAPRPHBState *sphb)
{
- sPAPRPHBClass *spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
-
- return spc->eeh_available;
+ return vfio_eeh_as_ok(&sphb->iommu_as);
}
static uint32_t rtas_pci_cfgaddr(uint32_t arg)
@@ -1680,7 +1678,6 @@ 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->finish_realize = spapr_phb_finish_realize;
- spc->eeh_available = false;
hp->plug = spapr_phb_hot_plug_child;
hp->unplug = spapr_phb_hot_unplug_child;
}
diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
index 0a0f31a..ecbcc5c 100644
--- a/hw/ppc/spapr_pci_vfio.c
+++ b/hw/ppc/spapr_pci_vfio.c
@@ -80,7 +80,6 @@ static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data)
dc->props = spapr_phb_vfio_properties;
spc->finish_realize = spapr_phb_vfio_finish_realize;
- spc->eeh_available = true;
}
static const TypeInfo spapr_phb_vfio_info = {
diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
index d32750e..9313209 100644
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -49,7 +49,6 @@ struct sPAPRPHBClass {
PCIHostBridgeClass parent_class;
void (*finish_realize)(sPAPRPHBState *sphb, Error **errp);
- bool eeh_available;
};
typedef struct spapr_pci_msi {
--
2.5.0
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 10/12] spapr_pci: (Mostly) remove spapr-pci-vfio-host-bridge
2016-02-26 11:31 [Qemu-devel] [PATCH 00/12] Allow EEH on spapr-pci-host-bridge devices David Gibson
` (8 preceding siblings ...)
2016-02-26 11:32 ` [Qemu-devel] [PATCH 09/12] spapr_pci: Allow EEH on spapr-pci-host-bridge David Gibson
@ 2016-02-26 11:32 ` David Gibson
2016-02-29 1:42 ` Alexey Kardashevskiy
2016-02-26 11:32 ` [Qemu-devel] [PATCH 11/12] spapr_pci: Remove finish_realize hook David Gibson
` (2 subsequent siblings)
12 siblings, 1 reply; 33+ messages in thread
From: David Gibson @ 2016-02-26 11:32 UTC (permalink / raw)
To: aik, benh
Cc: agraf, qemu-devel, gwshan, mdroth, alex.williamson, qemu-ppc,
David Gibson
Now that the regular spapr-pci-host-bridge can handle EEH, there are only
two things that spapr-pci-vfio-host-bridge does differently:
1. automatically sizes its DMA window to match the host IOMMU
2. checks if the attached VFIO container is backed by the
VFIO_SPAPR_TCE_IOMMU type on the host
(1) is not particularly useful, since the default window used by the
regular host bridge will work with the host IOMMU configuration on all
current systems anyway.
Plus, automatically changing guest visible configuration (such as the DMA
window) based on host settings is generally a bad idea. It's not
definitively broken, since spapr-pci-vfio-host-bridge is only supposed to
support VFIO devices which can't be migrated anyway, but still.
(2) is not really useful, because if a guest tries to configure EEH on a
different host IOMMU, the first call will fail and that will be that.
It's possible there are scripts or tools out there which expect
spapr-pci-vfio-host-bridge, so we don't remove it entirely. This patch
reduces it to just a stub for backwards compatibility.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/ppc/spapr_pci_vfio.c | 64 ++++++++++++---------------------------------
include/hw/pci-host/spapr.h | 11 --------
2 files changed, 17 insertions(+), 58 deletions(-)
diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
index ecbcc5c..41d9e5a 100644
--- a/hw/ppc/spapr_pci_vfio.c
+++ b/hw/ppc/spapr_pci_vfio.c
@@ -20,74 +20,44 @@
#include "qemu/osdep.h"
#include "hw/ppc/spapr.h"
#include "hw/pci-host/spapr.h"
-#include "hw/pci/msix.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(),
-};
+#define TYPE_SPAPR_PCI_VFIO_HOST_BRIDGE "spapr-pci-vfio-host-bridge"
-static void spapr_phb_vfio_finish_realize(sPAPRPHBState *sphb, Error **errp)
-{
- 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;
+#define SPAPR_PCI_VFIO_HOST_BRIDGE(obj) \
+ OBJECT_CHECK(sPAPRPHBVFIOState, (obj), TYPE_SPAPR_PCI_VFIO_HOST_BRIDGE)
- if (svphb->iommugroupid == -1) {
- error_setg(errp, "Wrong IOMMU group ID %d", svphb->iommugroupid);
- return;
- }
+typedef struct sPAPRPHBVFIOState sPAPRPHBVFIOState;
- ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
- VFIO_CHECK_EXTENSION,
- (void *) VFIO_SPAPR_TCE_IOMMU);
- if (ret != 1) {
- error_setg_errno(errp, -ret,
- "spapr-vfio: SPAPR extension is not supported");
- return;
- }
+struct sPAPRPHBVFIOState {
+ sPAPRPHBState phb;
- ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
- VFIO_IOMMU_SPAPR_TCE_GET_INFO, &info);
- if (ret) {
- error_setg_errno(errp, -ret,
- "spapr-vfio: get info from container failed");
- return;
- }
+ int32_t iommugroupid;
+};
- 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);
- if (!tcet) {
- error_setg(errp, "spapr-vfio: failed to create VFIO TCE table");
- return;
- }
+static Property spapr_phb_vfio_properties[] = {
+ DEFINE_PROP_INT32("iommu", sPAPRPHBVFIOState, iommugroupid, -1),
+ DEFINE_PROP_END_OF_LIST(),
+};
- /* Register default 32bit DMA window */
- memory_region_add_subregion(&sphb->iommu_root, tcet->bus_offset,
- spapr_tce_get_iommu(tcet));
+static void spapr_phb_vfio_instance_init(Object *obj)
+{
+ error_report("spapr-pci-vfio-host-bridge is deprecated");
}
static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
- sPAPRPHBClass *spc = SPAPR_PCI_HOST_BRIDGE_CLASS(klass);
dc->props = spapr_phb_vfio_properties;
- spc->finish_realize = spapr_phb_vfio_finish_realize;
}
static const TypeInfo spapr_phb_vfio_info = {
.name = TYPE_SPAPR_PCI_VFIO_HOST_BRIDGE,
.parent = TYPE_SPAPR_PCI_HOST_BRIDGE,
.instance_size = sizeof(sPAPRPHBVFIOState),
+ .instance_init = spapr_phb_vfio_instance_init,
.class_init = spapr_phb_vfio_class_init,
- .class_size = sizeof(sPAPRPHBClass),
};
static void spapr_pci_vfio_register_types(void)
diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
index 9313209..7110222 100644
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -28,14 +28,10 @@
#include "hw/ppc/xics.h"
#define TYPE_SPAPR_PCI_HOST_BRIDGE "spapr-pci-host-bridge"
-#define TYPE_SPAPR_PCI_VFIO_HOST_BRIDGE "spapr-pci-vfio-host-bridge"
#define SPAPR_PCI_HOST_BRIDGE(obj) \
OBJECT_CHECK(sPAPRPHBState, (obj), TYPE_SPAPR_PCI_HOST_BRIDGE)
-#define SPAPR_PCI_VFIO_HOST_BRIDGE(obj) \
- OBJECT_CHECK(sPAPRPHBVFIOState, (obj), TYPE_SPAPR_PCI_VFIO_HOST_BRIDGE)
-
#define SPAPR_PCI_HOST_BRIDGE_CLASS(klass) \
OBJECT_CLASS_CHECK(sPAPRPHBClass, (klass), TYPE_SPAPR_PCI_HOST_BRIDGE)
#define SPAPR_PCI_HOST_BRIDGE_GET_CLASS(obj) \
@@ -43,7 +39,6 @@
typedef struct sPAPRPHBClass sPAPRPHBClass;
typedef struct sPAPRPHBState sPAPRPHBState;
-typedef struct sPAPRPHBVFIOState sPAPRPHBVFIOState;
struct sPAPRPHBClass {
PCIHostBridgeClass parent_class;
@@ -90,12 +85,6 @@ struct sPAPRPHBState {
QLIST_ENTRY(sPAPRPHBState) list;
};
-struct sPAPRPHBVFIOState {
- sPAPRPHBState phb;
-
- int32_t iommugroupid;
-};
-
#define SPAPR_PCI_MAX_INDEX 255
#define SPAPR_PCI_BASE_BUID 0x800000020000000ULL
--
2.5.0
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 11/12] spapr_pci: Remove finish_realize hook
2016-02-26 11:31 [Qemu-devel] [PATCH 00/12] Allow EEH on spapr-pci-host-bridge devices David Gibson
` (9 preceding siblings ...)
2016-02-26 11:32 ` [Qemu-devel] [PATCH 10/12] spapr_pci: (Mostly) remove spapr-pci-vfio-host-bridge David Gibson
@ 2016-02-26 11:32 ` David Gibson
2016-02-29 1:44 ` Alexey Kardashevskiy
2016-02-26 11:32 ` [Qemu-devel] [PATCH 12/12] vfio: Eliminate vfio_container_ioctl() David Gibson
2016-02-26 11:33 ` [Qemu-devel] [PATCH 00/12] Allow EEH on spapr-pci-host-bridge devices David Gibson
12 siblings, 1 reply; 33+ messages in thread
From: David Gibson @ 2016-02-26 11:32 UTC (permalink / raw)
To: aik, benh
Cc: agraf, qemu-devel, gwshan, mdroth, alex.williamson, qemu-ppc,
David Gibson
Now that spapr-pci-vfio-host-bridge is reduced to just a stub, there is
only one implementation of the finish_realize hook in sPAPRPHBClass. So,
we can fold that implementation into its (single) caller, and remove the
hook. That's the last thing left in sPAPRPHBClass, so that can go away as
well.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/ppc/spapr_pci.c | 25 +++++--------------------
include/hw/pci-host/spapr.h | 12 ------------
2 files changed, 5 insertions(+), 32 deletions(-)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 336ffd2..2faaa03 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1335,11 +1335,12 @@ 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;
+ uint32_t nb_table;
if (sphb->index != (uint32_t)-1) {
hwaddr windows_base;
@@ -1489,33 +1490,20 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
}
}
- if (!info->finish_realize) {
- error_setg(errp, "finish_realize not defined");
- return;
- }
-
- info->finish_realize(sphb, 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)
-{
- sPAPRTCETable *tcet;
- uint32_t nb_table;
-
nb_table = sphb->dma_win_size >> SPAPR_TCE_PAGE_SHIFT;
tcet = spapr_tce_new_table(DEVICE(sphb), sphb->dma_liobn,
0, SPAPR_TCE_PAGE_SHIFT, nb_table, false);
if (!tcet) {
error_setg(errp, "Unable to create TCE table for %s",
sphb->dtbusname);
- return ;
+ return;
}
/* Register default 32bit DMA window */
memory_region_add_subregion(&sphb->iommu_root, sphb->dma_win_addr,
spapr_tce_get_iommu(tcet));
+
+ sphb->msi = g_hash_table_new_full(g_int_hash, g_int_equal, g_free, g_free);
}
static int spapr_phb_children_reset(Object *child, void *opaque)
@@ -1667,7 +1655,6 @@ static void spapr_phb_class_init(ObjectClass *klass, void *data)
{
PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(klass);
DeviceClass *dc = DEVICE_CLASS(klass);
- sPAPRPHBClass *spc = SPAPR_PCI_HOST_BRIDGE_CLASS(klass);
HotplugHandlerClass *hp = HOTPLUG_HANDLER_CLASS(klass);
hc->root_bus_path = spapr_phb_root_bus_path;
@@ -1677,7 +1664,6 @@ 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;
hp->plug = spapr_phb_hot_plug_child;
hp->unplug = spapr_phb_hot_unplug_child;
}
@@ -1687,7 +1673,6 @@ static const TypeInfo spapr_phb_info = {
.parent = TYPE_PCI_HOST_BRIDGE,
.instance_size = sizeof(sPAPRPHBState),
.class_init = spapr_phb_class_init,
- .class_size = sizeof(sPAPRPHBClass),
.interfaces = (InterfaceInfo[]) {
{ TYPE_HOTPLUG_HANDLER },
{ }
diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
index 7110222..a603feb 100644
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -32,20 +32,8 @@
#define SPAPR_PCI_HOST_BRIDGE(obj) \
OBJECT_CHECK(sPAPRPHBState, (obj), TYPE_SPAPR_PCI_HOST_BRIDGE)
-#define SPAPR_PCI_HOST_BRIDGE_CLASS(klass) \
- OBJECT_CLASS_CHECK(sPAPRPHBClass, (klass), TYPE_SPAPR_PCI_HOST_BRIDGE)
-#define SPAPR_PCI_HOST_BRIDGE_GET_CLASS(obj) \
- OBJECT_GET_CLASS(sPAPRPHBClass, (obj), TYPE_SPAPR_PCI_HOST_BRIDGE)
-
-typedef struct sPAPRPHBClass sPAPRPHBClass;
typedef struct sPAPRPHBState sPAPRPHBState;
-struct sPAPRPHBClass {
- PCIHostBridgeClass parent_class;
-
- void (*finish_realize)(sPAPRPHBState *sphb, Error **errp);
-};
-
typedef struct spapr_pci_msi {
uint32_t first_irq;
uint32_t num;
--
2.5.0
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 12/12] vfio: Eliminate vfio_container_ioctl()
2016-02-26 11:31 [Qemu-devel] [PATCH 00/12] Allow EEH on spapr-pci-host-bridge devices David Gibson
` (10 preceding siblings ...)
2016-02-26 11:32 ` [Qemu-devel] [PATCH 11/12] spapr_pci: Remove finish_realize hook David Gibson
@ 2016-02-26 11:32 ` David Gibson
2016-02-29 1:44 ` Alexey Kardashevskiy
2016-02-26 11:33 ` [Qemu-devel] [PATCH 00/12] Allow EEH on spapr-pci-host-bridge devices David Gibson
12 siblings, 1 reply; 33+ messages in thread
From: David Gibson @ 2016-02-26 11:32 UTC (permalink / raw)
To: aik, benh
Cc: agraf, qemu-devel, gwshan, mdroth, alex.williamson, qemu-ppc,
David Gibson
vfio_container_ioctl() was a bad interface that bypassed abstraction
boundaries, had semantics that sat uneasily with its name, and was unsafe
in many realistic circumstances. Now that spapr-pci-vfio-host-bridge has
been folded into spapr-pci-host-bridge, there are no more users, so remove
it.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/vfio/common.c | 45 ---------------------------------------------
include/hw/vfio/vfio.h | 2 --
2 files changed, 47 deletions(-)
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index e419241..26e91ad 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -959,51 +959,6 @@ void vfio_put_base_device(VFIODevice *vbasedev)
close(vbasedev->fd);
}
-static int vfio_container_do_ioctl(AddressSpace *as, int32_t groupid,
- int req, void *param)
-{
- VFIOGroup *group;
- VFIOContainer *container;
- int ret = -1;
-
- group = vfio_get_group(groupid, as);
- if (!group) {
- error_report("vfio: group %d not registered", groupid);
- return ret;
- }
-
- container = group->container;
- if (group->container) {
- ret = ioctl(container->fd, req, param);
- if (ret < 0) {
- error_report("vfio: failed to ioctl %d to container: ret=%d, %s",
- _IOC_NR(req) - VFIO_BASE, ret, strerror(errno));
- }
- }
-
- vfio_put_group(group);
-
- return ret;
-}
-
-int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
- int req, void *param)
-{
- /* We allow only certain ioctls to the container */
- switch (req) {
- case VFIO_CHECK_EXTENSION:
- case VFIO_IOMMU_SPAPR_TCE_GET_INFO:
- case VFIO_EEH_PE_OP:
- break;
- default:
- /* Return an error on unknown requests */
- error_report("vfio: unsupported ioctl %X", req);
- return -1;
- }
-
- return vfio_container_do_ioctl(as, groupid, req, param);
-}
-
/*
* Interfaces for IBM EEH (Enhanced Error Handling)
*/
diff --git a/include/hw/vfio/vfio.h b/include/hw/vfio/vfio.h
index fd3933b..7153604 100644
--- a/include/hw/vfio/vfio.h
+++ b/include/hw/vfio/vfio.h
@@ -3,8 +3,6 @@
#include "qemu/typedefs.h"
-extern int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
- int req, void *param);
bool vfio_eeh_as_ok(AddressSpace *as);
int vfio_eeh_as_op(AddressSpace *as, uint32_t op);
--
2.5.0
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 00/12] Allow EEH on spapr-pci-host-bridge devices
2016-02-26 11:31 [Qemu-devel] [PATCH 00/12] Allow EEH on spapr-pci-host-bridge devices David Gibson
` (11 preceding siblings ...)
2016-02-26 11:32 ` [Qemu-devel] [PATCH 12/12] vfio: Eliminate vfio_container_ioctl() David Gibson
@ 2016-02-26 11:33 ` David Gibson
12 siblings, 0 replies; 33+ messages in thread
From: David Gibson @ 2016-02-26 11:33 UTC (permalink / raw)
To: aik, benh; +Cc: agraf, qemu-devel, gwshan, mdroth, alex.williamson, qemu-ppc
[-- Attachment #1: Type: text/plain, Size: 2047 bytes --]
On Fri, Feb 26, 2016 at 10:31:51PM +1100, David Gibson wrote:
> For historical reasons, the spapr machine type has two PCI host bridge
> implementations: spapr-pci-host-bridge, and
> spapr-vfio-pci-host-bridge. The latter was (duh) designed for VFIO
> devices, but later reworks mean it's not necessary for that, and VFIO
> can be used on the regular host bridge.
>
> The only remaining difference is that EEH (Enhanced Error Handling -
> IBM's interface with similar purpose to AER) is only supported on the
> spapr-vfio-pci-host-bridge device.
>
> This series corrects this, allowing EEH operations on the regular host
> bridge. That allows the special VFIO host bridge (we leave a stub,
> for backwards compatibility).
>
> EEH is only supported for VFIO devices, for the time being, and due to
> bugs in the kernel implementation, it is only usable when the host
> bridge only has devices from a single (host-side) IOMMU group
> ("Partitionable Endpoint", in IBM terminology). That's an annoying
> limitation which we hope to lift someday, but it's no worse than now,
> since the spapr-vfio-pci-host-bridge only permits devices from a
> single IOMMU group in any case (although it doesn't properly enforce
> that).
>
> I wrote these a while back, and I'm not sure why they got sidelined -
> I suspect I was looking for testing from Gavin Shan, not realising
> then that he was no longer working on EEH. In any case, I'm hoping we
> can squeeze this change into 2.6, so we don't need to carry the broken
> spapr-vfio-pci-host-bridge device any longer.
Btw, Alexey. Sorry, I know this will conflict with your vfio-ddw
patches. However, from my brief glance at those, it looked like you
removed spapr-vfio-pci-host-bridge without properly fixing up EEH on
the regular host bridge, which I'd like to avoid.
--
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: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 01/12] vfio: Start improving VFIO/EEH interface
2016-02-26 11:31 ` [Qemu-devel] [PATCH 01/12] vfio: Start improving VFIO/EEH interface David Gibson
@ 2016-02-29 0:58 ` Alexey Kardashevskiy
2016-02-29 3:13 ` David Gibson
0 siblings, 1 reply; 33+ messages in thread
From: Alexey Kardashevskiy @ 2016-02-29 0:58 UTC (permalink / raw)
To: David Gibson, benh
Cc: agraf, qemu-devel, gwshan, mdroth, alex.williamson, qemu-ppc
On 02/26/2016 10:31 PM, David Gibson wrote:
> At present the code handling IBM's Enhanced Error Handling (EEH) interface
> on VFIO devices operates by bypassing the usual VFIO logic with
> vfio_container_ioctl(). That's a poorly designed interface with unclear
> semantics about exactly what can be operated on.
>
> In particular it operates on a single vfio container internally (hence the
> name), but takes an address space and group id, from which it deduces the
> container in a rather roundabout way. groupids are something that code
> outside vfio shouldn't even be aware of.
>
> This patch creates new interfaces for EEH operations. Internally we
> have vfio_eeh_container_op() which takes a VFIOContainer object
> directly. For external use we have vfio_eeh_as_ok() which determines
> if an AddressSpace is usable for EEH (at present this means it has a
> single container and at most a single group attached), and
> vfio_eeh_as_op() which will perform an operation on an AddressSpace in
> the unambiguous case, and otherwise returns an error.
>
> This interface still isn't great, but it's enough of an improvement to
> allow a number of cleanups in other places.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> hw/vfio/common.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++
> include/hw/vfio/vfio.h | 2 ++
> 2 files changed, 79 insertions(+)
>
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 607ec70..e419241 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -1003,3 +1003,80 @@ int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
>
> return vfio_container_do_ioctl(as, groupid, req, param);
> }
> +
> +/*
> + * Interfaces for IBM EEH (Enhanced Error Handling)
> + */
> +static bool vfio_eeh_container_ok(VFIOContainer *container)
> +{
> + /* A broken kernel implementation means EEH operations won't work
> + * correctly if there are multiple groups in a container */
> +
> + if (!QLIST_EMPTY(&container->group_list)
> + && QLIST_NEXT(QLIST_FIRST(&container->group_list), container_next)) {
> + return false;
> + }
> +
> + return true;
If &container->group_list is empty, this helper returns "true". Does not
look right, does it?...
> +}
> +
> +static int vfio_eeh_container_op(VFIOContainer *container, uint32_t op)
> +{
> + struct vfio_eeh_pe_op pe_op = {
> + .argsz = sizeof(pe_op),
> + .op = op,
> + };
> + int ret;
> +
> + if (!vfio_eeh_container_ok(container)) {
> + error_report("vfio/eeh: EEH_PE_OP 0x%x called on container"
> + " with multiple groups", op);
> + return -EPERM;
> + }
> +
> + ret = ioctl(container->fd, VFIO_EEH_PE_OP, &pe_op);
> + if (ret < 0) {
> + error_report("vfio/eeh: EEH_PE_OP 0x%x failed: %m", op);
> + return -errno;
> + }
> +
> + return 0;
> +}
> +
> +static VFIOContainer *vfio_eeh_as_container(AddressSpace *as)
> +{
> + VFIOAddressSpace *space = vfio_get_address_space(as);
> + VFIOContainer *container = NULL;
> +
> + if (QLIST_EMPTY(&space->containers)) {
> + /* No containers to act on */
> + goto out;
> + }
> +
> + container = QLIST_FIRST(&space->containers);
> +
> + if (QLIST_NEXT(container, next)) {
> + /* We don't yet have logic to synchronize EEH state across
> + * multiple containers */
> + container = NULL;
> + goto out;
> + }
> +
> +out:
> + vfio_put_address_space(space);
> + return container;
> +}
> +
> +bool vfio_eeh_as_ok(AddressSpace *as)
> +{
> + VFIOContainer *container = vfio_eeh_as_container(as);
> +
> + return (container != NULL) && vfio_eeh_container_ok(container);
> +}
> +
> +int vfio_eeh_as_op(AddressSpace *as, uint32_t op)
> +{
> + VFIOContainer *container = vfio_eeh_as_container(as);
> +
> + return vfio_eeh_container_op(container, op);
vfio_eeh_as_ok() checks for (container != NULL) but this one does not,
should not it?
> +}
> diff --git a/include/hw/vfio/vfio.h b/include/hw/vfio/vfio.h
> index 0b26cd8..fd3933b 100644
> --- a/include/hw/vfio/vfio.h
> +++ b/include/hw/vfio/vfio.h
> @@ -5,5 +5,7 @@
>
> extern int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
> int req, void *param);
> +bool vfio_eeh_as_ok(AddressSpace *as);
> +int vfio_eeh_as_op(AddressSpace *as, uint32_t op);
>
> #endif
>
--
Alexey
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 10/12] spapr_pci: (Mostly) remove spapr-pci-vfio-host-bridge
2016-02-26 11:32 ` [Qemu-devel] [PATCH 10/12] spapr_pci: (Mostly) remove spapr-pci-vfio-host-bridge David Gibson
@ 2016-02-29 1:42 ` Alexey Kardashevskiy
2016-02-29 3:06 ` David Gibson
0 siblings, 1 reply; 33+ messages in thread
From: Alexey Kardashevskiy @ 2016-02-29 1:42 UTC (permalink / raw)
To: David Gibson, benh
Cc: agraf, qemu-devel, gwshan, mdroth, alex.williamson, qemu-ppc
On 02/26/2016 10:32 PM, David Gibson wrote:
> Now that the regular spapr-pci-host-bridge can handle EEH, there are only
> two things that spapr-pci-vfio-host-bridge does differently:
> 1. automatically sizes its DMA window to match the host IOMMU
> 2. checks if the attached VFIO container is backed by the
> VFIO_SPAPR_TCE_IOMMU type on the host
>
> (1) is not particularly useful, since the default window used by the
> regular host bridge will work with the host IOMMU configuration on all
> current systems anyway.
>
> Plus, automatically changing guest visible configuration (such as the DMA
> window) based on host settings is generally a bad idea.
btw why exactly is it a bad idea?
Anyway,
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
--
Alexey
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 02/12] spapr_pci: Switch to vfio_eeh_as_op() interface
2016-02-26 11:31 ` [Qemu-devel] [PATCH 02/12] spapr_pci: Switch to vfio_eeh_as_op() interface David Gibson
@ 2016-02-29 1:43 ` Alexey Kardashevskiy
2016-02-29 3:00 ` David Gibson
0 siblings, 1 reply; 33+ messages in thread
From: Alexey Kardashevskiy @ 2016-02-29 1:43 UTC (permalink / raw)
To: David Gibson, benh
Cc: agraf, qemu-devel, gwshan, mdroth, alex.williamson, qemu-ppc
On 02/26/2016 10:31 PM, David Gibson wrote:
> This switches all EEH on VFIO operations in spapr_pci_vfio.c from the
> broken vfio_container_ioctl() interface to the new vfio_as_eeh_op()
> interface.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>mak
Where is that "mak" from? :)
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> hw/ppc/spapr_pci_vfio.c | 50 ++++++++++++++++---------------------------------
> 1 file changed, 16 insertions(+), 34 deletions(-)
>
> diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
> index 2f3752e..b1e8e8e 100644
> --- a/hw/ppc/spapr_pci_vfio.c
> +++ b/hw/ppc/spapr_pci_vfio.c
> @@ -73,15 +73,9 @@ static void spapr_phb_vfio_finish_realize(sPAPRPHBState *sphb, Error **errp)
> spapr_tce_get_iommu(tcet));
> }
>
> -static void spapr_phb_vfio_eeh_reenable(sPAPRPHBVFIOState *svphb)
> +static void spapr_phb_vfio_eeh_reenable(sPAPRPHBState *sphb)
> {
> - struct vfio_eeh_pe_op op = {
> - .argsz = sizeof(op),
> - .op = VFIO_EEH_PE_ENABLE
> - };
> -
> - vfio_container_ioctl(&svphb->phb.iommu_as,
> - svphb->iommugroupid, VFIO_EEH_PE_OP, &op);
> + vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_ENABLE);
> }
>
> static void spapr_phb_vfio_reset(DeviceState *qdev)
> @@ -92,19 +86,18 @@ static void spapr_phb_vfio_reset(DeviceState *qdev)
> * ensures that the contained PCI devices will work properly
> * after reboot.
> */
> - spapr_phb_vfio_eeh_reenable(SPAPR_PCI_VFIO_HOST_BRIDGE(qdev));
> + spapr_phb_vfio_eeh_reenable(SPAPR_PCI_HOST_BRIDGE(qdev));
> }
>
> static int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
> unsigned int addr, int option)
> {
> - sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
> - struct vfio_eeh_pe_op op = { .argsz = sizeof(op) };
> + uint32_t op;
> int ret;
>
> switch (option) {
> case RTAS_EEH_DISABLE:
> - op.op = VFIO_EEH_PE_DISABLE;
> + op = VFIO_EEH_PE_DISABLE;
> break;
> case RTAS_EEH_ENABLE: {
> PCIHostState *phb;
> @@ -122,21 +115,20 @@ static int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
> return RTAS_OUT_PARAM_ERROR;
> }
>
> - op.op = VFIO_EEH_PE_ENABLE;
> + op = VFIO_EEH_PE_ENABLE;
> break;
> }
> case RTAS_EEH_THAW_IO:
> - op.op = VFIO_EEH_PE_UNFREEZE_IO;
> + op = VFIO_EEH_PE_UNFREEZE_IO;
> break;
> case RTAS_EEH_THAW_DMA:
> - op.op = VFIO_EEH_PE_UNFREEZE_DMA;
> + op = VFIO_EEH_PE_UNFREEZE_DMA;
> break;
> default:
> return RTAS_OUT_PARAM_ERROR;
> }
>
> - ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
> - VFIO_EEH_PE_OP, &op);
> + ret = vfio_eeh_as_op(&sphb->iommu_as, op);
> if (ret < 0) {
> return RTAS_OUT_HW_ERROR;
> }
> @@ -146,13 +138,9 @@ static int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
>
> static int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state)
> {
> - sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
> - struct vfio_eeh_pe_op op = { .argsz = sizeof(op) };
> int ret;
>
> - op.op = VFIO_EEH_PE_GET_STATE;
> - ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
> - VFIO_EEH_PE_OP, &op);
> + ret = vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_GET_STATE);
> if (ret < 0) {
> return RTAS_OUT_PARAM_ERROR;
> }
> @@ -206,28 +194,26 @@ static void spapr_phb_vfio_eeh_pre_reset(sPAPRPHBState *sphb)
>
> static int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option)
> {
> - sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
> - struct vfio_eeh_pe_op op = { .argsz = sizeof(op) };
> + uint32_t op;
> int ret;
>
> switch (option) {
> case RTAS_SLOT_RESET_DEACTIVATE:
> - op.op = VFIO_EEH_PE_RESET_DEACTIVATE;
> + op = VFIO_EEH_PE_RESET_DEACTIVATE;
> break;
> case RTAS_SLOT_RESET_HOT:
> spapr_phb_vfio_eeh_pre_reset(sphb);
> - op.op = VFIO_EEH_PE_RESET_HOT;
> + op = VFIO_EEH_PE_RESET_HOT;
> break;
> case RTAS_SLOT_RESET_FUNDAMENTAL:
> spapr_phb_vfio_eeh_pre_reset(sphb);
> - op.op = VFIO_EEH_PE_RESET_FUNDAMENTAL;
> + op = VFIO_EEH_PE_RESET_FUNDAMENTAL;
> break;
> default:
> return RTAS_OUT_PARAM_ERROR;
> }
>
> - ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
> - VFIO_EEH_PE_OP, &op);
> + ret = vfio_eeh_as_op(&sphb->iommu_as, op);
> if (ret < 0) {
> return RTAS_OUT_HW_ERROR;
> }
> @@ -237,13 +223,9 @@ static int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option)
>
> static int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb)
> {
> - sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
> - struct vfio_eeh_pe_op op = { .argsz = sizeof(op) };
> int ret;
>
> - op.op = VFIO_EEH_PE_CONFIGURE;
> - ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
> - VFIO_EEH_PE_OP, &op);
> + ret = vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_CONFIGURE);
> if (ret < 0) {
> return RTAS_OUT_PARAM_ERROR;
> }
>
--
Alexey
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 03/12] spapr_pci: Eliminate class callbacks
2016-02-26 11:31 ` [Qemu-devel] [PATCH 03/12] spapr_pci: Eliminate class callbacks David Gibson
@ 2016-02-29 1:43 ` Alexey Kardashevskiy
2016-02-29 3:42 ` Alexey Kardashevskiy
0 siblings, 1 reply; 33+ messages in thread
From: Alexey Kardashevskiy @ 2016-02-29 1:43 UTC (permalink / raw)
To: David Gibson, benh
Cc: agraf, qemu-devel, gwshan, mdroth, alex.williamson, qemu-ppc
On 02/26/2016 10:31 PM, David Gibson wrote:
> The EEH operations in the spapr-vfio-pci-host-bridge no longer rely on the
> special groupid field in sPAPRPHBVFIOState. So we can simplify, removing
> the class specific callbacks with direct calls based on a simple
> spapr_phb_eeh_enabled() helper. For now we implement that in terms of
> a boolean in the class, but we'll continue to clean that up later.
>
> On its own this is a rather strange way of doing things, but it's a useful
> intermediate step to further cleanups.
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> hw/ppc/spapr_pci.c | 44 ++++++++++++++++++++++----------------------
> hw/ppc/spapr_pci_vfio.c | 18 +++++++-----------
> include/hw/pci-host/spapr.h | 13 +++++++++----
> 3 files changed, 38 insertions(+), 37 deletions(-)
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 9b2b546..d1e5222 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -92,6 +92,13 @@ PCIDevice *spapr_pci_find_dev(sPAPRMachineState *spapr, uint64_t buid,
> return pci_find_device(phb->bus, bus_num, devfn);
> }
>
> +static bool spapr_phb_eeh_available(sPAPRPHBState *sphb)
> +{
> + sPAPRPHBClass *spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
> +
> + return spc->eeh_available;
> +}
> +
> static uint32_t rtas_pci_cfgaddr(uint32_t arg)
> {
> /* This handles the encoding of extended config space addresses */
> @@ -438,7 +445,6 @@ static void rtas_ibm_set_eeh_option(PowerPCCPU *cpu,
> target_ulong rets)
> {
> sPAPRPHBState *sphb;
> - sPAPRPHBClass *spc;
> uint32_t addr, option;
> uint64_t buid;
> int ret;
> @@ -456,12 +462,11 @@ static void rtas_ibm_set_eeh_option(PowerPCCPU *cpu,
> goto param_error_exit;
> }
>
> - spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
> - if (!spc->eeh_set_option) {
> + if (!spapr_phb_eeh_available(sphb)) {
> goto param_error_exit;
> }
>
> - ret = spc->eeh_set_option(sphb, addr, option);
> + ret = spapr_phb_vfio_eeh_set_option(sphb, addr, option);
> rtas_st(rets, 0, ret);
> return;
>
> @@ -476,7 +481,6 @@ static void rtas_ibm_get_config_addr_info2(PowerPCCPU *cpu,
> target_ulong rets)
> {
> sPAPRPHBState *sphb;
> - sPAPRPHBClass *spc;
> PCIDevice *pdev;
> uint32_t addr, option;
> uint64_t buid;
> @@ -491,8 +495,7 @@ static void rtas_ibm_get_config_addr_info2(PowerPCCPU *cpu,
> goto param_error_exit;
> }
>
> - spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
> - if (!spc->eeh_set_option) {
> + if (!spapr_phb_eeh_available(sphb)) {
> goto param_error_exit;
> }
>
> @@ -532,7 +535,6 @@ static void rtas_ibm_read_slot_reset_state2(PowerPCCPU *cpu,
> target_ulong rets)
> {
> sPAPRPHBState *sphb;
> - sPAPRPHBClass *spc;
> uint64_t buid;
> int state, ret;
>
> @@ -546,12 +548,11 @@ static void rtas_ibm_read_slot_reset_state2(PowerPCCPU *cpu,
> goto param_error_exit;
> }
>
> - spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
> - if (!spc->eeh_get_state) {
> + if (!spapr_phb_eeh_available(sphb)) {
> goto param_error_exit;
> }
>
> - ret = spc->eeh_get_state(sphb, &state);
> + ret = spapr_phb_vfio_eeh_get_state(sphb, &state);
> rtas_st(rets, 0, ret);
> if (ret != RTAS_OUT_SUCCESS) {
> return;
> @@ -576,7 +577,6 @@ static void rtas_ibm_set_slot_reset(PowerPCCPU *cpu,
> target_ulong rets)
> {
> sPAPRPHBState *sphb;
> - sPAPRPHBClass *spc;
> uint32_t option;
> uint64_t buid;
> int ret;
> @@ -592,12 +592,11 @@ static void rtas_ibm_set_slot_reset(PowerPCCPU *cpu,
> goto param_error_exit;
> }
>
> - spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
> - if (!spc->eeh_reset) {
> + if (!spapr_phb_eeh_available(sphb)) {
> goto param_error_exit;
> }
>
> - ret = spc->eeh_reset(sphb, option);
> + ret = spapr_phb_vfio_eeh_reset(sphb, option);
> rtas_st(rets, 0, ret);
> return;
>
> @@ -612,7 +611,6 @@ static void rtas_ibm_configure_pe(PowerPCCPU *cpu,
> target_ulong rets)
> {
> sPAPRPHBState *sphb;
> - sPAPRPHBClass *spc;
> uint64_t buid;
> int ret;
>
> @@ -626,12 +624,11 @@ static void rtas_ibm_configure_pe(PowerPCCPU *cpu,
> goto param_error_exit;
> }
>
> - spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
> - if (!spc->eeh_configure) {
> + if (!spapr_phb_eeh_available(sphb)) {
> goto param_error_exit;
> }
>
> - ret = spc->eeh_configure(sphb);
> + ret = spapr_phb_vfio_eeh_configure(sphb);
> rtas_st(rets, 0, ret);
> return;
>
> @@ -647,7 +644,6 @@ static void rtas_ibm_slot_error_detail(PowerPCCPU *cpu,
> target_ulong rets)
> {
> sPAPRPHBState *sphb;
> - sPAPRPHBClass *spc;
> int option;
> uint64_t buid;
>
> @@ -661,8 +657,7 @@ static void rtas_ibm_slot_error_detail(PowerPCCPU *cpu,
> goto param_error_exit;
> }
>
> - spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
> - if (!spc->eeh_set_option) {
> + if (!spapr_phb_eeh_available(sphb)) {
> goto param_error_exit;
> }
>
> @@ -1430,6 +1425,10 @@ static void spapr_phb_reset(DeviceState *qdev)
> {
> /* Reset the IOMMU state */
> object_child_foreach(OBJECT(qdev), spapr_phb_children_reset, NULL);
> +
> + if (spapr_phb_eeh_available(SPAPR_PCI_HOST_BRIDGE(qdev))) {
> + spapr_phb_vfio_reset(qdev);
> + }
> }
>
> static Property spapr_phb_properties[] = {
> @@ -1560,6 +1559,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->finish_realize = spapr_phb_finish_realize;
> + spc->eeh_available = false;
> hp->plug = spapr_phb_hot_plug_child;
> hp->unplug = spapr_phb_hot_unplug_child;
> }
> diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
> index b1e8e8e..10fa88a 100644
> --- a/hw/ppc/spapr_pci_vfio.c
> +++ b/hw/ppc/spapr_pci_vfio.c
> @@ -78,7 +78,7 @@ static void spapr_phb_vfio_eeh_reenable(sPAPRPHBState *sphb)
> vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_ENABLE);
> }
>
> -static void spapr_phb_vfio_reset(DeviceState *qdev)
> +void spapr_phb_vfio_reset(DeviceState *qdev)
> {
> /*
> * The PE might be in frozen state. To reenable the EEH
> @@ -89,8 +89,8 @@ static void spapr_phb_vfio_reset(DeviceState *qdev)
> spapr_phb_vfio_eeh_reenable(SPAPR_PCI_HOST_BRIDGE(qdev));
> }
>
> -static int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
> - unsigned int addr, int option)
> +int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
> + unsigned int addr, int option)
> {
> uint32_t op;
> int ret;
> @@ -136,7 +136,7 @@ static int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
> return RTAS_OUT_SUCCESS;
> }
>
> -static int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state)
> +int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state)
> {
> int ret;
>
> @@ -192,7 +192,7 @@ static void spapr_phb_vfio_eeh_pre_reset(sPAPRPHBState *sphb)
> pci_for_each_bus(phb->bus, spapr_phb_vfio_eeh_clear_bus_msix, NULL);
> }
>
> -static int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option)
> +int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option)
> {
> uint32_t op;
> int ret;
> @@ -221,7 +221,7 @@ static int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option)
> return RTAS_OUT_SUCCESS;
> }
>
> -static int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb)
> +int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb)
> {
> int ret;
>
> @@ -239,12 +239,8 @@ 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;
> - spc->eeh_reset = spapr_phb_vfio_eeh_reset;
> - spc->eeh_configure = spapr_phb_vfio_eeh_configure;
> + spc->eeh_available = true;
> }
>
> static const TypeInfo spapr_phb_vfio_info = {
> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> index 7de5e02..cc1b82c 100644
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -49,10 +49,7 @@ struct sPAPRPHBClass {
> PCIHostBridgeClass parent_class;
>
> void (*finish_realize)(sPAPRPHBState *sphb, Error **errp);
> - 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);
> - int (*eeh_configure)(sPAPRPHBState *sphb);
> + bool eeh_available;
> };
>
> typedef struct spapr_pci_msi {
> @@ -136,5 +133,13 @@ void spapr_pci_rtas_init(void);
> sPAPRPHBState *spapr_pci_find_phb(sPAPRMachineState *spapr, uint64_t buid);
> PCIDevice *spapr_pci_find_dev(sPAPRMachineState *spapr, uint64_t buid,
> uint32_t config_addr);
> +void spapr_phb_vfio_reset(DeviceState *qdev);
> +
> +/* VFIO EEH hooks */
> +int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
> + unsigned int addr, int option);
> +int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state);
> +int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option);
> +int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb);
>
> #endif /* __HW_SPAPR_PCI_H__ */
>
--
Alexey
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 04/12] spapr_pci: Fold spapr_phb_vfio_eeh_configure() into spapr_pci code
2016-02-26 11:31 ` [Qemu-devel] [PATCH 04/12] spapr_pci: Fold spapr_phb_vfio_eeh_configure() into spapr_pci code David Gibson
@ 2016-02-29 1:43 ` Alexey Kardashevskiy
2016-02-29 3:45 ` Alexey Kardashevskiy
0 siblings, 1 reply; 33+ messages in thread
From: Alexey Kardashevskiy @ 2016-02-29 1:43 UTC (permalink / raw)
To: David Gibson, benh
Cc: agraf, qemu-devel, gwshan, mdroth, alex.williamson, qemu-ppc
On 02/26/2016 10:31 PM, David Gibson wrote:
> Simplify the sPAPR PCI code by folding spapr_phb_vfio_eeh_configure()
> into rtas_ibm_configure_pe().
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> hw/ppc/spapr_pci.c | 11 +++++++++--
> hw/ppc/spapr_pci_vfio.c | 12 ------------
> include/hw/pci-host/spapr.h | 1 -
> 3 files changed, 9 insertions(+), 15 deletions(-)
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index d1e5222..fa633a9 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -42,6 +42,9 @@
> #include "hw/ppc/spapr_drc.h"
> #include "sysemu/device_tree.h"
>
> +#include "hw/vfio/vfio.h"
> +#include <linux/vfio.h>
> +
> /* Copied from the kernel arch/powerpc/platforms/pseries/msi.c */
> #define RTAS_QUERY_FN 0
> #define RTAS_CHANGE_FN 1
> @@ -628,8 +631,12 @@ static void rtas_ibm_configure_pe(PowerPCCPU *cpu,
> goto param_error_exit;
> }
>
> - ret = spapr_phb_vfio_eeh_configure(sphb);
> - rtas_st(rets, 0, ret);
> + ret = vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_CONFIGURE);
> + if (ret < 0) {
> + goto param_error_exit;
> + }
> +
> + rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> return;
>
> param_error_exit:
> diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
> index 10fa88a..4ac5736 100644
> --- a/hw/ppc/spapr_pci_vfio.c
> +++ b/hw/ppc/spapr_pci_vfio.c
> @@ -221,18 +221,6 @@ int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option)
> return RTAS_OUT_SUCCESS;
> }
>
> -int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb)
> -{
> - int ret;
> -
> - ret = vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_CONFIGURE);
> - if (ret < 0) {
> - return RTAS_OUT_PARAM_ERROR;
> - }
> -
> - return RTAS_OUT_SUCCESS;
> -}
> -
> static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> index cc1b82c..f02ef51 100644
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -140,6 +140,5 @@ int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
> unsigned int addr, int option);
> int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state);
> int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option);
> -int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb);
>
> #endif /* __HW_SPAPR_PCI_H__ */
>
--
Alexey
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 05/12] spapr_pci: Fold spapr_phb_vfio_eeh_reset() into spapr_pci code
2016-02-26 11:31 ` [Qemu-devel] [PATCH 05/12] spapr_pci: Fold spapr_phb_vfio_eeh_reset() " David Gibson
@ 2016-02-29 1:43 ` Alexey Kardashevskiy
0 siblings, 0 replies; 33+ messages in thread
From: Alexey Kardashevskiy @ 2016-02-29 1:43 UTC (permalink / raw)
To: David Gibson, benh
Cc: agraf, qemu-devel, gwshan, mdroth, alex.williamson, qemu-ppc
On 02/26/2016 10:31 PM, David Gibson wrote:
> Simplify the sPAPR PCI code by folding spapr_phb_vfio_eeh_reset() into
> rtas_ibm_set_slot_reset(). We move several functions of which it was
> the only caller (spapr_phb_eeh_clear_dev_msix(),
> spapr_phb_eeh_clear_bus_msix() and spapr_phb_eeh_pre_reset()) into
> spapr_pci.c along with it.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> hw/ppc/spapr_pci.c | 68 ++++++++++++++++++++++++++++++++++++++++--
> hw/ppc/spapr_pci_vfio.c | 72 ---------------------------------------------
> include/hw/pci-host/spapr.h | 1 -
> 3 files changed, 66 insertions(+), 75 deletions(-)
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index fa633a9..96cdca2 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -573,6 +573,48 @@ param_error_exit:
> rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> }
>
> +static void spapr_phb_eeh_clear_dev_msix(PCIBus *bus, PCIDevice *pdev,
> + void *opaque)
> +{
> + /* Check if the device is VFIO PCI device */
> + if (!object_dynamic_cast(OBJECT(pdev), "vfio-pci")) {
> + return;
> + }
> +
> + /*
> + * The MSIx table will be cleaned out by reset. We need
> + * disable it so that it can be reenabled properly. Also,
> + * the cached MSIx table should be cleared as it's not
> + * reflecting the contents in hardware.
> + */
> + if (msix_enabled(pdev)) {
> + uint16_t flags;
> +
> + flags = pci_host_config_read_common(pdev,
> + pdev->msix_cap + PCI_MSIX_FLAGS,
> + pci_config_size(pdev), 2);
> + flags &= ~PCI_MSIX_FLAGS_ENABLE;
> + pci_host_config_write_common(pdev,
> + pdev->msix_cap + PCI_MSIX_FLAGS,
> + pci_config_size(pdev), flags, 2);
> + }
> +
> + msix_reset(pdev);
> +}
> +
> +static void spapr_phb_eeh_clear_bus_msix(PCIBus *bus, void *opaque)
> +{
> + pci_for_each_device(bus, pci_bus_num(bus),
> + spapr_phb_eeh_clear_dev_msix, NULL);
> +}
> +
> +static void spapr_phb_eeh_pre_reset(sPAPRPHBState *sphb)
> +{
> + PCIHostState *phb = PCI_HOST_BRIDGE(sphb);
> +
> + pci_for_each_bus(phb->bus, spapr_phb_eeh_clear_bus_msix, NULL);
> +}
> +
> static void rtas_ibm_set_slot_reset(PowerPCCPU *cpu,
> sPAPRMachineState *spapr,
> uint32_t token, uint32_t nargs,
> @@ -582,6 +624,7 @@ static void rtas_ibm_set_slot_reset(PowerPCCPU *cpu,
> sPAPRPHBState *sphb;
> uint32_t option;
> uint64_t buid;
> + uint32_t op;
> int ret;
>
> if ((nargs != 4) || (nret != 1)) {
> @@ -599,8 +642,29 @@ static void rtas_ibm_set_slot_reset(PowerPCCPU *cpu,
> goto param_error_exit;
> }
>
> - ret = spapr_phb_vfio_eeh_reset(sphb, option);
> - rtas_st(rets, 0, ret);
> + switch (option) {
> + case RTAS_SLOT_RESET_DEACTIVATE:
> + op = VFIO_EEH_PE_RESET_DEACTIVATE;
> + break;
> + case RTAS_SLOT_RESET_HOT:
> + spapr_phb_eeh_pre_reset(sphb);
> + op = VFIO_EEH_PE_RESET_HOT;
> + break;
> + case RTAS_SLOT_RESET_FUNDAMENTAL:
> + spapr_phb_eeh_pre_reset(sphb);
> + op = VFIO_EEH_PE_RESET_FUNDAMENTAL;
> + break;
> + default:
> + goto param_error_exit;
> + }
> +
> + ret = vfio_eeh_as_op(&sphb->iommu_as, op);
> + if (ret < 0) {
> + rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
> + return;
> + }
> +
> + rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> return;
>
> param_error_exit:
> diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
> index 4ac5736..54075e0 100644
> --- a/hw/ppc/spapr_pci_vfio.c
> +++ b/hw/ppc/spapr_pci_vfio.c
> @@ -149,78 +149,6 @@ int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state)
> return RTAS_OUT_SUCCESS;
> }
>
> -static void spapr_phb_vfio_eeh_clear_dev_msix(PCIBus *bus,
> - PCIDevice *pdev,
> - void *opaque)
> -{
> - /* Check if the device is VFIO PCI device */
> - if (!object_dynamic_cast(OBJECT(pdev), "vfio-pci")) {
> - return;
> - }
> -
> - /*
> - * The MSIx table will be cleaned out by reset. We need
> - * disable it so that it can be reenabled properly. Also,
> - * the cached MSIx table should be cleared as it's not
> - * reflecting the contents in hardware.
> - */
> - if (msix_enabled(pdev)) {
> - uint16_t flags;
> -
> - flags = pci_host_config_read_common(pdev,
> - pdev->msix_cap + PCI_MSIX_FLAGS,
> - pci_config_size(pdev), 2);
> - flags &= ~PCI_MSIX_FLAGS_ENABLE;
> - pci_host_config_write_common(pdev,
> - pdev->msix_cap + PCI_MSIX_FLAGS,
> - pci_config_size(pdev), flags, 2);
> - }
> -
> - msix_reset(pdev);
> -}
> -
> -static void spapr_phb_vfio_eeh_clear_bus_msix(PCIBus *bus, void *opaque)
> -{
> - pci_for_each_device(bus, pci_bus_num(bus),
> - spapr_phb_vfio_eeh_clear_dev_msix, NULL);
> -}
> -
> -static void spapr_phb_vfio_eeh_pre_reset(sPAPRPHBState *sphb)
> -{
> - PCIHostState *phb = PCI_HOST_BRIDGE(sphb);
> -
> - pci_for_each_bus(phb->bus, spapr_phb_vfio_eeh_clear_bus_msix, NULL);
> -}
> -
> -int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option)
> -{
> - uint32_t op;
> - int ret;
> -
> - switch (option) {
> - case RTAS_SLOT_RESET_DEACTIVATE:
> - op = VFIO_EEH_PE_RESET_DEACTIVATE;
> - break;
> - case RTAS_SLOT_RESET_HOT:
> - spapr_phb_vfio_eeh_pre_reset(sphb);
> - op = VFIO_EEH_PE_RESET_HOT;
> - break;
> - case RTAS_SLOT_RESET_FUNDAMENTAL:
> - spapr_phb_vfio_eeh_pre_reset(sphb);
> - op = VFIO_EEH_PE_RESET_FUNDAMENTAL;
> - break;
> - default:
> - return RTAS_OUT_PARAM_ERROR;
> - }
> -
> - ret = vfio_eeh_as_op(&sphb->iommu_as, op);
> - if (ret < 0) {
> - return RTAS_OUT_HW_ERROR;
> - }
> -
> - return RTAS_OUT_SUCCESS;
> -}
> -
> static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> index f02ef51..d31162b 100644
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -139,6 +139,5 @@ void spapr_phb_vfio_reset(DeviceState *qdev);
> int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
> unsigned int addr, int option);
> int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state);
> -int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option);
>
> #endif /* __HW_SPAPR_PCI_H__ */
>
--
Alexey
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 06/12] spapr_pci: Fold spapr_phb_vfio_eeh_get_state() into spapr_pci code
2016-02-26 11:31 ` [Qemu-devel] [PATCH 06/12] spapr_pci: Fold spapr_phb_vfio_eeh_get_state() " David Gibson
@ 2016-02-29 1:43 ` Alexey Kardashevskiy
0 siblings, 0 replies; 33+ messages in thread
From: Alexey Kardashevskiy @ 2016-02-29 1:43 UTC (permalink / raw)
To: David Gibson, benh
Cc: agraf, qemu-devel, gwshan, mdroth, alex.williamson, qemu-ppc
On 02/26/2016 10:31 PM, David Gibson wrote:
> Simplify the sPAPR PCI code by folding spapr_phb_vfio_eeh_get_state9)
> into rtas_ibm_read_slot_reset_state2().
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> hw/ppc/spapr_pci.c | 12 ++++++------
> hw/ppc/spapr_pci_vfio.c | 13 -------------
> include/hw/pci-host/spapr.h | 1 -
> 3 files changed, 6 insertions(+), 20 deletions(-)
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 96cdca2..eaae7e2 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -539,7 +539,7 @@ static void rtas_ibm_read_slot_reset_state2(PowerPCCPU *cpu,
> {
> sPAPRPHBState *sphb;
> uint64_t buid;
> - int state, ret;
> + int ret;
>
> if ((nargs != 3) || (nret != 4 && nret != 5)) {
> goto param_error_exit;
> @@ -555,13 +555,13 @@ static void rtas_ibm_read_slot_reset_state2(PowerPCCPU *cpu,
> goto param_error_exit;
> }
>
> - ret = spapr_phb_vfio_eeh_get_state(sphb, &state);
> - rtas_st(rets, 0, ret);
> - if (ret != RTAS_OUT_SUCCESS) {
> - return;
> + ret = vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_GET_STATE);
> + if (ret < 0) {
> + goto param_error_exit;
> }
>
> - rtas_st(rets, 1, state);
> + rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> + rtas_st(rets, 1, ret);
> rtas_st(rets, 2, RTAS_EEH_SUPPORT);
> rtas_st(rets, 3, RTAS_EEH_PE_UNAVAIL_INFO);
> if (nret >= 5) {
> diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
> index 54075e0..c87f2e4 100644
> --- a/hw/ppc/spapr_pci_vfio.c
> +++ b/hw/ppc/spapr_pci_vfio.c
> @@ -136,19 +136,6 @@ int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
> return RTAS_OUT_SUCCESS;
> }
>
> -int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state)
> -{
> - int ret;
> -
> - ret = vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_GET_STATE);
> - if (ret < 0) {
> - return RTAS_OUT_PARAM_ERROR;
> - }
> -
> - *state = ret;
> - return RTAS_OUT_SUCCESS;
> -}
> -
> static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> index d31162b..55237fc 100644
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -138,6 +138,5 @@ void spapr_phb_vfio_reset(DeviceState *qdev);
> /* VFIO EEH hooks */
> int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
> unsigned int addr, int option);
> -int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state);
>
> #endif /* __HW_SPAPR_PCI_H__ */
>
--
Alexey
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 07/12] spapr_pci: Fold spapr_phb_vfio_eeh_set_option() into spapr_pci code
2016-02-26 11:31 ` [Qemu-devel] [PATCH 07/12] spapr_pci: Fold spapr_phb_vfio_eeh_set_option() " David Gibson
@ 2016-02-29 1:44 ` Alexey Kardashevskiy
0 siblings, 0 replies; 33+ messages in thread
From: Alexey Kardashevskiy @ 2016-02-29 1:44 UTC (permalink / raw)
To: David Gibson, benh
Cc: agraf, qemu-devel, gwshan, mdroth, alex.williamson, qemu-ppc
On 02/26/2016 10:31 PM, David Gibson wrote:
> Simplify the sPAPR PCI code by folding spapr_phb_eeh_set_option() into
> rtas_ibm_set_eeh_option().
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> hw/ppc/spapr_pci.c | 43 +++++++++++++++++++++++++++++++++++++++--
> hw/ppc/spapr_pci_vfio.c | 47 ---------------------------------------------
> include/hw/pci-host/spapr.h | 4 ----
> 3 files changed, 41 insertions(+), 53 deletions(-)
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index eaae7e2..26d08ad 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -450,6 +450,7 @@ static void rtas_ibm_set_eeh_option(PowerPCCPU *cpu,
> sPAPRPHBState *sphb;
> uint32_t addr, option;
> uint64_t buid;
> + uint32_t op;
> int ret;
>
> if ((nargs != 4) || (nret != 1)) {
> @@ -469,8 +470,46 @@ static void rtas_ibm_set_eeh_option(PowerPCCPU *cpu,
> goto param_error_exit;
> }
>
> - ret = spapr_phb_vfio_eeh_set_option(sphb, addr, option);
> - rtas_st(rets, 0, ret);
> + switch (option) {
> + case RTAS_EEH_DISABLE:
> + op = VFIO_EEH_PE_DISABLE;
> + break;
> + case RTAS_EEH_ENABLE: {
> + PCIHostState *phb;
> + PCIDevice *pdev;
> +
> + /*
> + * The EEH functionality is enabled on basis of PCI device,
> + * instead of PE. We need check the validity of the PCI
> + * device address.
> + */
> + phb = PCI_HOST_BRIDGE(sphb);
> + pdev = pci_find_device(phb->bus,
> + (addr >> 16) & 0xFF, (addr >> 8) & 0xFF);
> + if (!pdev || !object_dynamic_cast(OBJECT(pdev), "vfio-pci")) {
> + goto param_error_exit;
> + }
> +
> + op = VFIO_EEH_PE_ENABLE;
> + break;
> + }
> + case RTAS_EEH_THAW_IO:
> + op = VFIO_EEH_PE_UNFREEZE_IO;
> + break;
> + case RTAS_EEH_THAW_DMA:
> + op = VFIO_EEH_PE_UNFREEZE_DMA;
> + break;
> + default:
> + goto param_error_exit;
> + }
> +
> + ret = vfio_eeh_as_op(&sphb->iommu_as, op);
> + if (ret < 0) {
> + rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
> + return;
> + }
> +
> + rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> return;
>
> param_error_exit:
> diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
> index c87f2e4..cccd444 100644
> --- a/hw/ppc/spapr_pci_vfio.c
> +++ b/hw/ppc/spapr_pci_vfio.c
> @@ -89,53 +89,6 @@ void spapr_phb_vfio_reset(DeviceState *qdev)
> spapr_phb_vfio_eeh_reenable(SPAPR_PCI_HOST_BRIDGE(qdev));
> }
>
> -int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
> - unsigned int addr, int option)
> -{
> - uint32_t op;
> - int ret;
> -
> - switch (option) {
> - case RTAS_EEH_DISABLE:
> - op = VFIO_EEH_PE_DISABLE;
> - break;
> - case RTAS_EEH_ENABLE: {
> - PCIHostState *phb;
> - PCIDevice *pdev;
> -
> - /*
> - * The EEH functionality is enabled on basis of PCI device,
> - * instead of PE. We need check the validity of the PCI
> - * device address.
> - */
> - phb = PCI_HOST_BRIDGE(sphb);
> - pdev = pci_find_device(phb->bus,
> - (addr >> 16) & 0xFF, (addr >> 8) & 0xFF);
> - if (!pdev || !object_dynamic_cast(OBJECT(pdev), "vfio-pci")) {
> - return RTAS_OUT_PARAM_ERROR;
> - }
> -
> - op = VFIO_EEH_PE_ENABLE;
> - break;
> - }
> - case RTAS_EEH_THAW_IO:
> - op = VFIO_EEH_PE_UNFREEZE_IO;
> - break;
> - case RTAS_EEH_THAW_DMA:
> - op = VFIO_EEH_PE_UNFREEZE_DMA;
> - break;
> - default:
> - return RTAS_OUT_PARAM_ERROR;
> - }
> -
> - ret = vfio_eeh_as_op(&sphb->iommu_as, op);
> - if (ret < 0) {
> - return RTAS_OUT_HW_ERROR;
> - }
> -
> - return RTAS_OUT_SUCCESS;
> -}
> -
> static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> index 55237fc..d32750e 100644
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -135,8 +135,4 @@ PCIDevice *spapr_pci_find_dev(sPAPRMachineState *spapr, uint64_t buid,
> uint32_t config_addr);
> void spapr_phb_vfio_reset(DeviceState *qdev);
>
> -/* VFIO EEH hooks */
> -int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
> - unsigned int addr, int option);
> -
> #endif /* __HW_SPAPR_PCI_H__ */
>
--
Alexey
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 08/12] spapr_pci: Fold spapr_phb_vfio_reset() into spapr_pci code
2016-02-26 11:31 ` [Qemu-devel] [PATCH 08/12] spapr_pci: Fold spapr_phb_vfio_reset() " David Gibson
@ 2016-02-29 1:44 ` Alexey Kardashevskiy
0 siblings, 0 replies; 33+ messages in thread
From: Alexey Kardashevskiy @ 2016-02-29 1:44 UTC (permalink / raw)
To: David Gibson, benh
Cc: agraf, qemu-devel, gwshan, mdroth, alex.williamson, qemu-ppc
On 02/26/2016 10:31 PM, David Gibson wrote:
> Simplify the sPAPR PCI code by folding spapr_phb_vfio_reset() into
> spapr_phb_reset().
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> hw/ppc/spapr_pci.c | 13 ++++++++++++-
> hw/ppc/spapr_pci_vfio.c | 16 ----------------
> 2 files changed, 12 insertions(+), 17 deletions(-)
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 26d08ad..be18bf6 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1531,13 +1531,24 @@ static int spapr_phb_children_reset(Object *child, void *opaque)
> return 0;
> }
>
> +static void spapr_phb_eeh_reenable(sPAPRPHBState *sphb)
> +{
> + vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_ENABLE);
> +}
> +
> static void spapr_phb_reset(DeviceState *qdev)
> {
> /* Reset the IOMMU state */
> object_child_foreach(OBJECT(qdev), spapr_phb_children_reset, NULL);
>
> if (spapr_phb_eeh_available(SPAPR_PCI_HOST_BRIDGE(qdev))) {
> - spapr_phb_vfio_reset(qdev);
> + /*
> + * The PE might be in frozen state. To reenable the EEH
> + * functionality on it will clean the frozen state, which
> + * ensures that the contained PCI devices will work properly
> + * after reboot.
> + */
> + spapr_phb_eeh_reenable(SPAPR_PCI_HOST_BRIDGE(qdev));
> }
> }
>
> diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
> index cccd444..0a0f31a 100644
> --- a/hw/ppc/spapr_pci_vfio.c
> +++ b/hw/ppc/spapr_pci_vfio.c
> @@ -73,22 +73,6 @@ static void spapr_phb_vfio_finish_realize(sPAPRPHBState *sphb, Error **errp)
> spapr_tce_get_iommu(tcet));
> }
>
> -static void spapr_phb_vfio_eeh_reenable(sPAPRPHBState *sphb)
> -{
> - vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_ENABLE);
> -}
> -
> -void spapr_phb_vfio_reset(DeviceState *qdev)
> -{
> - /*
> - * The PE might be in frozen state. To reenable the EEH
> - * functionality on it will clean the frozen state, which
> - * ensures that the contained PCI devices will work properly
> - * after reboot.
> - */
> - spapr_phb_vfio_eeh_reenable(SPAPR_PCI_HOST_BRIDGE(qdev));
> -}
> -
> static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
>
--
Alexey
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 09/12] spapr_pci: Allow EEH on spapr-pci-host-bridge
2016-02-26 11:32 ` [Qemu-devel] [PATCH 09/12] spapr_pci: Allow EEH on spapr-pci-host-bridge David Gibson
@ 2016-02-29 1:44 ` Alexey Kardashevskiy
0 siblings, 0 replies; 33+ messages in thread
From: Alexey Kardashevskiy @ 2016-02-29 1:44 UTC (permalink / raw)
To: David Gibson, benh
Cc: agraf, qemu-devel, gwshan, mdroth, alex.williamson, qemu-ppc
On 02/26/2016 10:32 PM, David Gibson wrote:
> Now that the EEH code is independent of the special
> spapr-vfio-pci-host-bridge device, we can allow it on all spapr PCI
> host bridges instead. We do this by changing spapr_phb_eeh_available()
> to be based on the vfio_eeh_as_ok() call instead of the host bridge class.
>
> Because the value of vfio_eeh_as_ok() can change with devices being
> hotplugged or unplugged, this can potentially lead to some strange edge
> cases where the guest starts using EEH, then it starts failing because
> of a change in status.
>
> However, it's not really any worse than the current situation. Cases that
> would have worked previously will still work (i.e. VFIO devices from at
> most one VFIO IOMMU group per vPHB), it's just that it's no longer
> necessary to use spapr-vfio-pci-host-bridge with the groupid pre-specified.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> hw/ppc/spapr_pci.c | 5 +----
> hw/ppc/spapr_pci_vfio.c | 1 -
> include/hw/pci-host/spapr.h | 1 -
> 3 files changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index be18bf6..336ffd2 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -97,9 +97,7 @@ PCIDevice *spapr_pci_find_dev(sPAPRMachineState *spapr, uint64_t buid,
>
> static bool spapr_phb_eeh_available(sPAPRPHBState *sphb)
> {
> - sPAPRPHBClass *spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
> -
> - return spc->eeh_available;
> + return vfio_eeh_as_ok(&sphb->iommu_as);
> }
>
> static uint32_t rtas_pci_cfgaddr(uint32_t arg)
> @@ -1680,7 +1678,6 @@ 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->finish_realize = spapr_phb_finish_realize;
> - spc->eeh_available = false;
> hp->plug = spapr_phb_hot_plug_child;
> hp->unplug = spapr_phb_hot_unplug_child;
> }
> diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
> index 0a0f31a..ecbcc5c 100644
> --- a/hw/ppc/spapr_pci_vfio.c
> +++ b/hw/ppc/spapr_pci_vfio.c
> @@ -80,7 +80,6 @@ static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data)
>
> dc->props = spapr_phb_vfio_properties;
> spc->finish_realize = spapr_phb_vfio_finish_realize;
> - spc->eeh_available = true;
> }
>
> static const TypeInfo spapr_phb_vfio_info = {
> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> index d32750e..9313209 100644
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -49,7 +49,6 @@ struct sPAPRPHBClass {
> PCIHostBridgeClass parent_class;
>
> void (*finish_realize)(sPAPRPHBState *sphb, Error **errp);
> - bool eeh_available;
> };
>
> typedef struct spapr_pci_msi {
>
--
Alexey
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 11/12] spapr_pci: Remove finish_realize hook
2016-02-26 11:32 ` [Qemu-devel] [PATCH 11/12] spapr_pci: Remove finish_realize hook David Gibson
@ 2016-02-29 1:44 ` Alexey Kardashevskiy
0 siblings, 0 replies; 33+ messages in thread
From: Alexey Kardashevskiy @ 2016-02-29 1:44 UTC (permalink / raw)
To: David Gibson, benh
Cc: agraf, qemu-devel, gwshan, mdroth, alex.williamson, qemu-ppc
On 02/26/2016 10:32 PM, David Gibson wrote:
> Now that spapr-pci-vfio-host-bridge is reduced to just a stub, there is
> only one implementation of the finish_realize hook in sPAPRPHBClass. So,
> we can fold that implementation into its (single) caller, and remove the
> hook. That's the last thing left in sPAPRPHBClass, so that can go away as
> well.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> hw/ppc/spapr_pci.c | 25 +++++--------------------
> include/hw/pci-host/spapr.h | 12 ------------
> 2 files changed, 5 insertions(+), 32 deletions(-)
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 336ffd2..2faaa03 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1335,11 +1335,12 @@ 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;
> + uint32_t nb_table;
>
> if (sphb->index != (uint32_t)-1) {
> hwaddr windows_base;
> @@ -1489,33 +1490,20 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
> }
> }
>
> - if (!info->finish_realize) {
> - error_setg(errp, "finish_realize not defined");
> - return;
> - }
> -
> - info->finish_realize(sphb, 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)
> -{
> - sPAPRTCETable *tcet;
> - uint32_t nb_table;
> -
> nb_table = sphb->dma_win_size >> SPAPR_TCE_PAGE_SHIFT;
> tcet = spapr_tce_new_table(DEVICE(sphb), sphb->dma_liobn,
> 0, SPAPR_TCE_PAGE_SHIFT, nb_table, false);
> if (!tcet) {
> error_setg(errp, "Unable to create TCE table for %s",
> sphb->dtbusname);
> - return ;
> + return;
> }
>
> /* Register default 32bit DMA window */
> memory_region_add_subregion(&sphb->iommu_root, sphb->dma_win_addr,
> spapr_tce_get_iommu(tcet));
> +
> + sphb->msi = g_hash_table_new_full(g_int_hash, g_int_equal, g_free, g_free);
> }
>
> static int spapr_phb_children_reset(Object *child, void *opaque)
> @@ -1667,7 +1655,6 @@ static void spapr_phb_class_init(ObjectClass *klass, void *data)
> {
> PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(klass);
> DeviceClass *dc = DEVICE_CLASS(klass);
> - sPAPRPHBClass *spc = SPAPR_PCI_HOST_BRIDGE_CLASS(klass);
> HotplugHandlerClass *hp = HOTPLUG_HANDLER_CLASS(klass);
>
> hc->root_bus_path = spapr_phb_root_bus_path;
> @@ -1677,7 +1664,6 @@ 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;
> hp->plug = spapr_phb_hot_plug_child;
> hp->unplug = spapr_phb_hot_unplug_child;
> }
> @@ -1687,7 +1673,6 @@ static const TypeInfo spapr_phb_info = {
> .parent = TYPE_PCI_HOST_BRIDGE,
> .instance_size = sizeof(sPAPRPHBState),
> .class_init = spapr_phb_class_init,
> - .class_size = sizeof(sPAPRPHBClass),
> .interfaces = (InterfaceInfo[]) {
> { TYPE_HOTPLUG_HANDLER },
> { }
> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> index 7110222..a603feb 100644
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -32,20 +32,8 @@
> #define SPAPR_PCI_HOST_BRIDGE(obj) \
> OBJECT_CHECK(sPAPRPHBState, (obj), TYPE_SPAPR_PCI_HOST_BRIDGE)
>
> -#define SPAPR_PCI_HOST_BRIDGE_CLASS(klass) \
> - OBJECT_CLASS_CHECK(sPAPRPHBClass, (klass), TYPE_SPAPR_PCI_HOST_BRIDGE)
> -#define SPAPR_PCI_HOST_BRIDGE_GET_CLASS(obj) \
> - OBJECT_GET_CLASS(sPAPRPHBClass, (obj), TYPE_SPAPR_PCI_HOST_BRIDGE)
> -
> -typedef struct sPAPRPHBClass sPAPRPHBClass;
> typedef struct sPAPRPHBState sPAPRPHBState;
>
> -struct sPAPRPHBClass {
> - PCIHostBridgeClass parent_class;
> -
> - void (*finish_realize)(sPAPRPHBState *sphb, Error **errp);
> -};
> -
> typedef struct spapr_pci_msi {
> uint32_t first_irq;
> uint32_t num;
>
--
Alexey
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 12/12] vfio: Eliminate vfio_container_ioctl()
2016-02-26 11:32 ` [Qemu-devel] [PATCH 12/12] vfio: Eliminate vfio_container_ioctl() David Gibson
@ 2016-02-29 1:44 ` Alexey Kardashevskiy
0 siblings, 0 replies; 33+ messages in thread
From: Alexey Kardashevskiy @ 2016-02-29 1:44 UTC (permalink / raw)
To: David Gibson, benh
Cc: agraf, qemu-devel, gwshan, mdroth, alex.williamson, qemu-ppc
On 02/26/2016 10:32 PM, David Gibson wrote:
> vfio_container_ioctl() was a bad interface that bypassed abstraction
> boundaries, had semantics that sat uneasily with its name, and was unsafe
> in many realistic circumstances. Now that spapr-pci-vfio-host-bridge has
> been folded into spapr-pci-host-bridge, there are no more users, so remove
> it.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> hw/vfio/common.c | 45 ---------------------------------------------
> include/hw/vfio/vfio.h | 2 --
> 2 files changed, 47 deletions(-)
>
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index e419241..26e91ad 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -959,51 +959,6 @@ void vfio_put_base_device(VFIODevice *vbasedev)
> close(vbasedev->fd);
> }
>
> -static int vfio_container_do_ioctl(AddressSpace *as, int32_t groupid,
> - int req, void *param)
> -{
> - VFIOGroup *group;
> - VFIOContainer *container;
> - int ret = -1;
> -
> - group = vfio_get_group(groupid, as);
> - if (!group) {
> - error_report("vfio: group %d not registered", groupid);
> - return ret;
> - }
> -
> - container = group->container;
> - if (group->container) {
> - ret = ioctl(container->fd, req, param);
> - if (ret < 0) {
> - error_report("vfio: failed to ioctl %d to container: ret=%d, %s",
> - _IOC_NR(req) - VFIO_BASE, ret, strerror(errno));
> - }
> - }
> -
> - vfio_put_group(group);
> -
> - return ret;
> -}
> -
> -int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
> - int req, void *param)
> -{
> - /* We allow only certain ioctls to the container */
> - switch (req) {
> - case VFIO_CHECK_EXTENSION:
> - case VFIO_IOMMU_SPAPR_TCE_GET_INFO:
> - case VFIO_EEH_PE_OP:
> - break;
> - default:
> - /* Return an error on unknown requests */
> - error_report("vfio: unsupported ioctl %X", req);
> - return -1;
> - }
> -
> - return vfio_container_do_ioctl(as, groupid, req, param);
> -}
> -
> /*
> * Interfaces for IBM EEH (Enhanced Error Handling)
> */
> diff --git a/include/hw/vfio/vfio.h b/include/hw/vfio/vfio.h
> index fd3933b..7153604 100644
> --- a/include/hw/vfio/vfio.h
> +++ b/include/hw/vfio/vfio.h
> @@ -3,8 +3,6 @@
>
> #include "qemu/typedefs.h"
>
> -extern int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
> - int req, void *param);
> bool vfio_eeh_as_ok(AddressSpace *as);
> int vfio_eeh_as_op(AddressSpace *as, uint32_t op);
>
>
--
Alexey
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 02/12] spapr_pci: Switch to vfio_eeh_as_op() interface
2016-02-29 1:43 ` Alexey Kardashevskiy
@ 2016-02-29 3:00 ` David Gibson
0 siblings, 0 replies; 33+ messages in thread
From: David Gibson @ 2016-02-29 3:00 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: qemu-devel, mdroth, gwshan, agraf, alex.williamson, qemu-ppc
[-- Attachment #1: Type: text/plain, Size: 6139 bytes --]
On Mon, Feb 29, 2016 at 12:43:05PM +1100, Alexey Kardashevskiy wrote:
> On 02/26/2016 10:31 PM, David Gibson wrote:
> >This switches all EEH on VFIO operations in spapr_pci_vfio.c from the
> >broken vfio_container_ioctl() interface to the new vfio_as_eeh_op()
> >interface.
> >
> >Signed-off-by: David Gibson <david@gibson.dropbear.id.au>mak
>
> Where is that "mak" from? :)
Oops, no idea. Fixed.
> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>
>
> >---
> > hw/ppc/spapr_pci_vfio.c | 50 ++++++++++++++++---------------------------------
> > 1 file changed, 16 insertions(+), 34 deletions(-)
> >
> >diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
> >index 2f3752e..b1e8e8e 100644
> >--- a/hw/ppc/spapr_pci_vfio.c
> >+++ b/hw/ppc/spapr_pci_vfio.c
> >@@ -73,15 +73,9 @@ static void spapr_phb_vfio_finish_realize(sPAPRPHBState *sphb, Error **errp)
> > spapr_tce_get_iommu(tcet));
> > }
> >
> >-static void spapr_phb_vfio_eeh_reenable(sPAPRPHBVFIOState *svphb)
> >+static void spapr_phb_vfio_eeh_reenable(sPAPRPHBState *sphb)
> > {
> >- struct vfio_eeh_pe_op op = {
> >- .argsz = sizeof(op),
> >- .op = VFIO_EEH_PE_ENABLE
> >- };
> >-
> >- vfio_container_ioctl(&svphb->phb.iommu_as,
> >- svphb->iommugroupid, VFIO_EEH_PE_OP, &op);
> >+ vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_ENABLE);
> > }
> >
> > static void spapr_phb_vfio_reset(DeviceState *qdev)
> >@@ -92,19 +86,18 @@ static void spapr_phb_vfio_reset(DeviceState *qdev)
> > * ensures that the contained PCI devices will work properly
> > * after reboot.
> > */
> >- spapr_phb_vfio_eeh_reenable(SPAPR_PCI_VFIO_HOST_BRIDGE(qdev));
> >+ spapr_phb_vfio_eeh_reenable(SPAPR_PCI_HOST_BRIDGE(qdev));
> > }
> >
> > static int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
> > unsigned int addr, int option)
> > {
> >- sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
> >- struct vfio_eeh_pe_op op = { .argsz = sizeof(op) };
> >+ uint32_t op;
> > int ret;
> >
> > switch (option) {
> > case RTAS_EEH_DISABLE:
> >- op.op = VFIO_EEH_PE_DISABLE;
> >+ op = VFIO_EEH_PE_DISABLE;
> > break;
> > case RTAS_EEH_ENABLE: {
> > PCIHostState *phb;
> >@@ -122,21 +115,20 @@ static int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
> > return RTAS_OUT_PARAM_ERROR;
> > }
> >
> >- op.op = VFIO_EEH_PE_ENABLE;
> >+ op = VFIO_EEH_PE_ENABLE;
> > break;
> > }
> > case RTAS_EEH_THAW_IO:
> >- op.op = VFIO_EEH_PE_UNFREEZE_IO;
> >+ op = VFIO_EEH_PE_UNFREEZE_IO;
> > break;
> > case RTAS_EEH_THAW_DMA:
> >- op.op = VFIO_EEH_PE_UNFREEZE_DMA;
> >+ op = VFIO_EEH_PE_UNFREEZE_DMA;
> > break;
> > default:
> > return RTAS_OUT_PARAM_ERROR;
> > }
> >
> >- ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
> >- VFIO_EEH_PE_OP, &op);
> >+ ret = vfio_eeh_as_op(&sphb->iommu_as, op);
> > if (ret < 0) {
> > return RTAS_OUT_HW_ERROR;
> > }
> >@@ -146,13 +138,9 @@ static int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
> >
> > static int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state)
> > {
> >- sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
> >- struct vfio_eeh_pe_op op = { .argsz = sizeof(op) };
> > int ret;
> >
> >- op.op = VFIO_EEH_PE_GET_STATE;
> >- ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
> >- VFIO_EEH_PE_OP, &op);
> >+ ret = vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_GET_STATE);
> > if (ret < 0) {
> > return RTAS_OUT_PARAM_ERROR;
> > }
> >@@ -206,28 +194,26 @@ static void spapr_phb_vfio_eeh_pre_reset(sPAPRPHBState *sphb)
> >
> > static int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option)
> > {
> >- sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
> >- struct vfio_eeh_pe_op op = { .argsz = sizeof(op) };
> >+ uint32_t op;
> > int ret;
> >
> > switch (option) {
> > case RTAS_SLOT_RESET_DEACTIVATE:
> >- op.op = VFIO_EEH_PE_RESET_DEACTIVATE;
> >+ op = VFIO_EEH_PE_RESET_DEACTIVATE;
> > break;
> > case RTAS_SLOT_RESET_HOT:
> > spapr_phb_vfio_eeh_pre_reset(sphb);
> >- op.op = VFIO_EEH_PE_RESET_HOT;
> >+ op = VFIO_EEH_PE_RESET_HOT;
> > break;
> > case RTAS_SLOT_RESET_FUNDAMENTAL:
> > spapr_phb_vfio_eeh_pre_reset(sphb);
> >- op.op = VFIO_EEH_PE_RESET_FUNDAMENTAL;
> >+ op = VFIO_EEH_PE_RESET_FUNDAMENTAL;
> > break;
> > default:
> > return RTAS_OUT_PARAM_ERROR;
> > }
> >
> >- ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
> >- VFIO_EEH_PE_OP, &op);
> >+ ret = vfio_eeh_as_op(&sphb->iommu_as, op);
> > if (ret < 0) {
> > return RTAS_OUT_HW_ERROR;
> > }
> >@@ -237,13 +223,9 @@ static int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option)
> >
> > static int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb)
> > {
> >- sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
> >- struct vfio_eeh_pe_op op = { .argsz = sizeof(op) };
> > int ret;
> >
> >- op.op = VFIO_EEH_PE_CONFIGURE;
> >- ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
> >- VFIO_EEH_PE_OP, &op);
> >+ ret = vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_CONFIGURE);
> > if (ret < 0) {
> > return RTAS_OUT_PARAM_ERROR;
> > }
> >
>
>
--
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: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 10/12] spapr_pci: (Mostly) remove spapr-pci-vfio-host-bridge
2016-02-29 1:42 ` Alexey Kardashevskiy
@ 2016-02-29 3:06 ` David Gibson
0 siblings, 0 replies; 33+ messages in thread
From: David Gibson @ 2016-02-29 3:06 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: qemu-devel, mdroth, gwshan, agraf, alex.williamson, qemu-ppc
[-- Attachment #1: Type: text/plain, Size: 1491 bytes --]
On Mon, Feb 29, 2016 at 12:42:28PM +1100, Alexey Kardashevskiy wrote:
> On 02/26/2016 10:32 PM, David Gibson wrote:
> >Now that the regular spapr-pci-host-bridge can handle EEH, there are only
> >two things that spapr-pci-vfio-host-bridge does differently:
> > 1. automatically sizes its DMA window to match the host IOMMU
> > 2. checks if the attached VFIO container is backed by the
> > VFIO_SPAPR_TCE_IOMMU type on the host
> >
> >(1) is not particularly useful, since the default window used by the
> >regular host bridge will work with the host IOMMU configuration on all
> >current systems anyway.
> >
> >Plus, automatically changing guest visible configuration (such as the DMA
> >window) based on host settings is generally a bad idea.
>
> btw why exactly is it a bad idea?
So, as noted, in this case it's probably ok. But in general changing
configuration based on host state will break migration, because the
source and destination VMs, with apparently identical setup won't
actually be the same.
It also makes it more difficult for management layers to know exactly
what they're constructing, which is potentially an issue even for this
non-migratable device.
>
>
> Anyway,
> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>
>
--
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: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 01/12] vfio: Start improving VFIO/EEH interface
2016-02-29 0:58 ` Alexey Kardashevskiy
@ 2016-02-29 3:13 ` David Gibson
0 siblings, 0 replies; 33+ messages in thread
From: David Gibson @ 2016-02-29 3:13 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: qemu-devel, mdroth, gwshan, agraf, alex.williamson, qemu-ppc
[-- Attachment #1: Type: text/plain, Size: 4980 bytes --]
On Mon, Feb 29, 2016 at 11:58:54AM +1100, Alexey Kardashevskiy wrote:
> On 02/26/2016 10:31 PM, David Gibson wrote:
> >At present the code handling IBM's Enhanced Error Handling (EEH) interface
> >on VFIO devices operates by bypassing the usual VFIO logic with
> >vfio_container_ioctl(). That's a poorly designed interface with unclear
> >semantics about exactly what can be operated on.
> >
> >In particular it operates on a single vfio container internally (hence the
> >name), but takes an address space and group id, from which it deduces the
> >container in a rather roundabout way. groupids are something that code
> >outside vfio shouldn't even be aware of.
> >
> >This patch creates new interfaces for EEH operations. Internally we
> >have vfio_eeh_container_op() which takes a VFIOContainer object
> >directly. For external use we have vfio_eeh_as_ok() which determines
> >if an AddressSpace is usable for EEH (at present this means it has a
> >single container and at most a single group attached), and
> >vfio_eeh_as_op() which will perform an operation on an AddressSpace in
> >the unambiguous case, and otherwise returns an error.
> >
> >This interface still isn't great, but it's enough of an improvement to
> >allow a number of cleanups in other places.
> >
> >Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> >---
> > hw/vfio/common.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > include/hw/vfio/vfio.h | 2 ++
> > 2 files changed, 79 insertions(+)
> >
> >diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> >index 607ec70..e419241 100644
> >--- a/hw/vfio/common.c
> >+++ b/hw/vfio/common.c
> >@@ -1003,3 +1003,80 @@ int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
> >
> > return vfio_container_do_ioctl(as, groupid, req, param);
> > }
> >+
> >+/*
> >+ * Interfaces for IBM EEH (Enhanced Error Handling)
> >+ */
> >+static bool vfio_eeh_container_ok(VFIOContainer *container)
> >+{
> >+ /* A broken kernel implementation means EEH operations won't work
> >+ * correctly if there are multiple groups in a container */
> >+
> >+ if (!QLIST_EMPTY(&container->group_list)
> >+ && QLIST_NEXT(QLIST_FIRST(&container->group_list), container_next)) {
> >+ return false;
> >+ }
> >+
> >+ return true;
>
> If &container->group_list is empty, this helper returns "true". Does not
> look right, does it?...
Hmm.. my thinking was that EEH was safe (if a no-op) on a container
with no groups. But, thinking about it again I'm not sure that the
state of previous EEH ops will get transferred to a group added to the
container later. So I think returning false on an empty container
probably is safer.
I'll change it.
> >+}
> >+
> >+static int vfio_eeh_container_op(VFIOContainer *container, uint32_t op)
> >+{
> >+ struct vfio_eeh_pe_op pe_op = {
> >+ .argsz = sizeof(pe_op),
> >+ .op = op,
> >+ };
> >+ int ret;
> >+
> >+ if (!vfio_eeh_container_ok(container)) {
> >+ error_report("vfio/eeh: EEH_PE_OP 0x%x called on container"
> >+ " with multiple groups", op);
> >+ return -EPERM;
> >+ }
> >+
> >+ ret = ioctl(container->fd, VFIO_EEH_PE_OP, &pe_op);
> >+ if (ret < 0) {
> >+ error_report("vfio/eeh: EEH_PE_OP 0x%x failed: %m", op);
> >+ return -errno;
> >+ }
> >+
> >+ return 0;
> >+}
> >+
> >+static VFIOContainer *vfio_eeh_as_container(AddressSpace *as)
> >+{
> >+ VFIOAddressSpace *space = vfio_get_address_space(as);
> >+ VFIOContainer *container = NULL;
> >+
> >+ if (QLIST_EMPTY(&space->containers)) {
> >+ /* No containers to act on */
> >+ goto out;
> >+ }
> >+
> >+ container = QLIST_FIRST(&space->containers);
> >+
> >+ if (QLIST_NEXT(container, next)) {
> >+ /* We don't yet have logic to synchronize EEH state across
> >+ * multiple containers */
> >+ container = NULL;
> >+ goto out;
> >+ }
> >+
> >+out:
> >+ vfio_put_address_space(space);
> >+ return container;
> >+}
> >+
> >+bool vfio_eeh_as_ok(AddressSpace *as)
> >+{
> >+ VFIOContainer *container = vfio_eeh_as_container(as);
> >+
> >+ return (container != NULL) && vfio_eeh_container_ok(container);
> >+}
> >+
> >+int vfio_eeh_as_op(AddressSpace *as, uint32_t op)
> >+{
> >+ VFIOContainer *container = vfio_eeh_as_container(as);
> >+
> >+ return vfio_eeh_container_op(container, op);
>
>
> vfio_eeh_as_ok() checks for (container != NULL) but this one does not,
> should not it?
Well.. you're not supposed to call as_op() if as_ok() returned false,
so it should be safe. I'll add an assert to make this 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: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 03/12] spapr_pci: Eliminate class callbacks
2016-02-29 1:43 ` Alexey Kardashevskiy
@ 2016-02-29 3:42 ` Alexey Kardashevskiy
0 siblings, 0 replies; 33+ messages in thread
From: Alexey Kardashevskiy @ 2016-02-29 3:42 UTC (permalink / raw)
To: David Gibson, benh
Cc: agraf, qemu-devel, gwshan, mdroth, alex.williamson, qemu-ppc
On 02/29/2016 12:43 PM, Alexey Kardashevskiy wrote:
> On 02/26/2016 10:31 PM, David Gibson wrote:
>> The EEH operations in the spapr-vfio-pci-host-bridge no longer rely on the
>> special groupid field in sPAPRPHBVFIOState. So we can simplify, removing
>> the class specific callbacks with direct calls based on a simple
>> spapr_phb_eeh_enabled() helper. For now we implement that in terms of
>> a boolean in the class, but we'll continue to clean that up later.
>>
>> On its own this is a rather strange way of doing things, but it's a useful
>> intermediate step to further cleanups.
>
> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
This actually breaks mingw32 build:
LINK ppc64-softmmu/qemu-system-ppc64.exe
hw/ppc/spapr_pci.o: In function `rtas_ibm_set_eeh_option':
/home/aik/p/qemu-dwg-eeh/hw/ppc/spapr_pci.c:471: undefined reference to
`spapr_phb_vfio_eeh_set_option'
hw/ppc/spapr_pci.o: In function `rtas_ibm_read_slot_reset_state2':
/home/aik/p/qemu-dwg-eeh/hw/ppc/spapr_pci.c:557: undefined reference to
`spapr_phb_vfio_eeh_get_state'
hw/ppc/spapr_pci.o: In function `rtas_ibm_set_slot_reset':
/home/aik/p/qemu-dwg-eeh/hw/ppc/spapr_pci.c:601: undefined reference to
`spapr_phb_vfio_eeh_reset'
hw/ppc/spapr_pci.o: In function `rtas_ibm_configure_pe':
/home/aik/p/qemu-dwg-eeh/hw/ppc/spapr_pci.c:633: undefined reference to
`spapr_phb_vfio_eeh_configure'
hw/ppc/spapr_pci.o: In function `spapr_phb_reset':
/home/aik/p/qemu-dwg-eeh/hw/ppc/spapr_pci.c:1434: undefined reference to
`spapr_phb_vfio_reset'
collect2: error: ld returned 1 exit status
>
>>
>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>> ---
>> hw/ppc/spapr_pci.c | 44
>> ++++++++++++++++++++++----------------------
>> hw/ppc/spapr_pci_vfio.c | 18 +++++++-----------
>> include/hw/pci-host/spapr.h | 13 +++++++++----
>> 3 files changed, 38 insertions(+), 37 deletions(-)
>>
>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>> index 9b2b546..d1e5222 100644
>> --- a/hw/ppc/spapr_pci.c
>> +++ b/hw/ppc/spapr_pci.c
>> @@ -92,6 +92,13 @@ PCIDevice *spapr_pci_find_dev(sPAPRMachineState
>> *spapr, uint64_t buid,
>> return pci_find_device(phb->bus, bus_num, devfn);
>> }
>>
>> +static bool spapr_phb_eeh_available(sPAPRPHBState *sphb)
>> +{
>> + sPAPRPHBClass *spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
>> +
>> + return spc->eeh_available;
>> +}
>> +
>> static uint32_t rtas_pci_cfgaddr(uint32_t arg)
>> {
>> /* This handles the encoding of extended config space addresses */
>> @@ -438,7 +445,6 @@ static void rtas_ibm_set_eeh_option(PowerPCCPU *cpu,
>> target_ulong rets)
>> {
>> sPAPRPHBState *sphb;
>> - sPAPRPHBClass *spc;
>> uint32_t addr, option;
>> uint64_t buid;
>> int ret;
>> @@ -456,12 +462,11 @@ static void rtas_ibm_set_eeh_option(PowerPCCPU *cpu,
>> goto param_error_exit;
>> }
>>
>> - spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
>> - if (!spc->eeh_set_option) {
>> + if (!spapr_phb_eeh_available(sphb)) {
>> goto param_error_exit;
>> }
>>
>> - ret = spc->eeh_set_option(sphb, addr, option);
>> + ret = spapr_phb_vfio_eeh_set_option(sphb, addr, option);
>> rtas_st(rets, 0, ret);
>> return;
>>
>> @@ -476,7 +481,6 @@ static void rtas_ibm_get_config_addr_info2(PowerPCCPU
>> *cpu,
>> target_ulong rets)
>> {
>> sPAPRPHBState *sphb;
>> - sPAPRPHBClass *spc;
>> PCIDevice *pdev;
>> uint32_t addr, option;
>> uint64_t buid;
>> @@ -491,8 +495,7 @@ static void rtas_ibm_get_config_addr_info2(PowerPCCPU
>> *cpu,
>> goto param_error_exit;
>> }
>>
>> - spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
>> - if (!spc->eeh_set_option) {
>> + if (!spapr_phb_eeh_available(sphb)) {
>> goto param_error_exit;
>> }
>>
>> @@ -532,7 +535,6 @@ static void
>> rtas_ibm_read_slot_reset_state2(PowerPCCPU *cpu,
>> target_ulong rets)
>> {
>> sPAPRPHBState *sphb;
>> - sPAPRPHBClass *spc;
>> uint64_t buid;
>> int state, ret;
>>
>> @@ -546,12 +548,11 @@ static void
>> rtas_ibm_read_slot_reset_state2(PowerPCCPU *cpu,
>> goto param_error_exit;
>> }
>>
>> - spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
>> - if (!spc->eeh_get_state) {
>> + if (!spapr_phb_eeh_available(sphb)) {
>> goto param_error_exit;
>> }
>>
>> - ret = spc->eeh_get_state(sphb, &state);
>> + ret = spapr_phb_vfio_eeh_get_state(sphb, &state);
>> rtas_st(rets, 0, ret);
>> if (ret != RTAS_OUT_SUCCESS) {
>> return;
>> @@ -576,7 +577,6 @@ static void rtas_ibm_set_slot_reset(PowerPCCPU *cpu,
>> target_ulong rets)
>> {
>> sPAPRPHBState *sphb;
>> - sPAPRPHBClass *spc;
>> uint32_t option;
>> uint64_t buid;
>> int ret;
>> @@ -592,12 +592,11 @@ static void rtas_ibm_set_slot_reset(PowerPCCPU *cpu,
>> goto param_error_exit;
>> }
>>
>> - spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
>> - if (!spc->eeh_reset) {
>> + if (!spapr_phb_eeh_available(sphb)) {
>> goto param_error_exit;
>> }
>>
>> - ret = spc->eeh_reset(sphb, option);
>> + ret = spapr_phb_vfio_eeh_reset(sphb, option);
>> rtas_st(rets, 0, ret);
>> return;
>>
>> @@ -612,7 +611,6 @@ static void rtas_ibm_configure_pe(PowerPCCPU *cpu,
>> target_ulong rets)
>> {
>> sPAPRPHBState *sphb;
>> - sPAPRPHBClass *spc;
>> uint64_t buid;
>> int ret;
>>
>> @@ -626,12 +624,11 @@ static void rtas_ibm_configure_pe(PowerPCCPU *cpu,
>> goto param_error_exit;
>> }
>>
>> - spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
>> - if (!spc->eeh_configure) {
>> + if (!spapr_phb_eeh_available(sphb)) {
>> goto param_error_exit;
>> }
>>
>> - ret = spc->eeh_configure(sphb);
>> + ret = spapr_phb_vfio_eeh_configure(sphb);
>> rtas_st(rets, 0, ret);
>> return;
>>
>> @@ -647,7 +644,6 @@ static void rtas_ibm_slot_error_detail(PowerPCCPU *cpu,
>> target_ulong rets)
>> {
>> sPAPRPHBState *sphb;
>> - sPAPRPHBClass *spc;
>> int option;
>> uint64_t buid;
>>
>> @@ -661,8 +657,7 @@ static void rtas_ibm_slot_error_detail(PowerPCCPU *cpu,
>> goto param_error_exit;
>> }
>>
>> - spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
>> - if (!spc->eeh_set_option) {
>> + if (!spapr_phb_eeh_available(sphb)) {
>> goto param_error_exit;
>> }
>>
>> @@ -1430,6 +1425,10 @@ static void spapr_phb_reset(DeviceState *qdev)
>> {
>> /* Reset the IOMMU state */
>> object_child_foreach(OBJECT(qdev), spapr_phb_children_reset, NULL);
>> +
>> + if (spapr_phb_eeh_available(SPAPR_PCI_HOST_BRIDGE(qdev))) {
>> + spapr_phb_vfio_reset(qdev);
>> + }
>> }
>>
>> static Property spapr_phb_properties[] = {
>> @@ -1560,6 +1559,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->finish_realize = spapr_phb_finish_realize;
>> + spc->eeh_available = false;
>> hp->plug = spapr_phb_hot_plug_child;
>> hp->unplug = spapr_phb_hot_unplug_child;
>> }
>> diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
>> index b1e8e8e..10fa88a 100644
>> --- a/hw/ppc/spapr_pci_vfio.c
>> +++ b/hw/ppc/spapr_pci_vfio.c
>> @@ -78,7 +78,7 @@ static void spapr_phb_vfio_eeh_reenable(sPAPRPHBState
>> *sphb)
>> vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_ENABLE);
>> }
>>
>> -static void spapr_phb_vfio_reset(DeviceState *qdev)
>> +void spapr_phb_vfio_reset(DeviceState *qdev)
>> {
>> /*
>> * The PE might be in frozen state. To reenable the EEH
>> @@ -89,8 +89,8 @@ static void spapr_phb_vfio_reset(DeviceState *qdev)
>> spapr_phb_vfio_eeh_reenable(SPAPR_PCI_HOST_BRIDGE(qdev));
>> }
>>
>> -static int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
>> - unsigned int addr, int option)
>> +int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
>> + unsigned int addr, int option)
>> {
>> uint32_t op;
>> int ret;
>> @@ -136,7 +136,7 @@ static int
>> spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
>> return RTAS_OUT_SUCCESS;
>> }
>>
>> -static int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state)
>> +int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state)
>> {
>> int ret;
>>
>> @@ -192,7 +192,7 @@ static void
>> spapr_phb_vfio_eeh_pre_reset(sPAPRPHBState *sphb)
>> pci_for_each_bus(phb->bus, spapr_phb_vfio_eeh_clear_bus_msix,
>> NULL);
>> }
>>
>> -static int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option)
>> +int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option)
>> {
>> uint32_t op;
>> int ret;
>> @@ -221,7 +221,7 @@ static int spapr_phb_vfio_eeh_reset(sPAPRPHBState
>> *sphb, int option)
>> return RTAS_OUT_SUCCESS;
>> }
>>
>> -static int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb)
>> +int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb)
>> {
>> int ret;
>>
>> @@ -239,12 +239,8 @@ 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;
>> - spc->eeh_reset = spapr_phb_vfio_eeh_reset;
>> - spc->eeh_configure = spapr_phb_vfio_eeh_configure;
>> + spc->eeh_available = true;
>> }
>>
>> static const TypeInfo spapr_phb_vfio_info = {
>> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
>> index 7de5e02..cc1b82c 100644
>> --- a/include/hw/pci-host/spapr.h
>> +++ b/include/hw/pci-host/spapr.h
>> @@ -49,10 +49,7 @@ struct sPAPRPHBClass {
>> PCIHostBridgeClass parent_class;
>>
>> void (*finish_realize)(sPAPRPHBState *sphb, Error **errp);
>> - 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);
>> - int (*eeh_configure)(sPAPRPHBState *sphb);
>> + bool eeh_available;
>> };
>>
>> typedef struct spapr_pci_msi {
>> @@ -136,5 +133,13 @@ void spapr_pci_rtas_init(void);
>> sPAPRPHBState *spapr_pci_find_phb(sPAPRMachineState *spapr, uint64_t
>> buid);
>> PCIDevice *spapr_pci_find_dev(sPAPRMachineState *spapr, uint64_t buid,
>> uint32_t config_addr);
>> +void spapr_phb_vfio_reset(DeviceState *qdev);
>> +
>> +/* VFIO EEH hooks */
>> +int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
>> + unsigned int addr, int option);
>> +int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state);
>> +int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option);
>> +int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb);
>>
>> #endif /* __HW_SPAPR_PCI_H__ */
>>
>
>
--
Alexey
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 04/12] spapr_pci: Fold spapr_phb_vfio_eeh_configure() into spapr_pci code
2016-02-29 1:43 ` Alexey Kardashevskiy
@ 2016-02-29 3:45 ` Alexey Kardashevskiy
2016-02-29 4:25 ` Alexey Kardashevskiy
0 siblings, 1 reply; 33+ messages in thread
From: Alexey Kardashevskiy @ 2016-02-29 3:45 UTC (permalink / raw)
To: David Gibson, benh
Cc: agraf, qemu-devel, gwshan, mdroth, alex.williamson, qemu-ppc
On 02/29/2016 12:43 PM, Alexey Kardashevskiy wrote:
> On 02/26/2016 10:31 PM, David Gibson wrote:
>> Simplify the sPAPR PCI code by folding spapr_phb_vfio_eeh_configure()
>> into rtas_ibm_configure_pe().
>>
>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>
> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Aaaand this breaks mingw32:
CC ppc64-softmmu/hw/ppc/spapr_pci.o
/home/aik/p/qemu-dwg-eeh/hw/ppc/spapr_pci.c:46:24: fatal error:
linux/vfio.h: No such file or directory
compilation terminated.
/home/aik/p/qemu-dwg-eeh/rules.mak:57: recipe for target
'hw/ppc/spapr_pci.o' failed
make[1]: *** [hw/ppc/spapr_pci.o] Error 1
make[1]: *** Waiting for unfinished jobs....
Makefile:186: recipe for target 'subdir-ppc64-softmmu' failed
make: *** [subdir-ppc64-softmmu] Error 2
make: Leaving directory '/scratch/aik/p/qemu-dwg-eeh--ppc64_mingw32-build'
>
>> ---
>> hw/ppc/spapr_pci.c | 11 +++++++++--
>> hw/ppc/spapr_pci_vfio.c | 12 ------------
>> include/hw/pci-host/spapr.h | 1 -
>> 3 files changed, 9 insertions(+), 15 deletions(-)
>>
>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>> index d1e5222..fa633a9 100644
>> --- a/hw/ppc/spapr_pci.c
>> +++ b/hw/ppc/spapr_pci.c
>> @@ -42,6 +42,9 @@
>> #include "hw/ppc/spapr_drc.h"
>> #include "sysemu/device_tree.h"
>>
>> +#include "hw/vfio/vfio.h"
>> +#include <linux/vfio.h>
>> +
>> /* Copied from the kernel arch/powerpc/platforms/pseries/msi.c */
>> #define RTAS_QUERY_FN 0
>> #define RTAS_CHANGE_FN 1
>> @@ -628,8 +631,12 @@ static void rtas_ibm_configure_pe(PowerPCCPU *cpu,
>> goto param_error_exit;
>> }
>>
>> - ret = spapr_phb_vfio_eeh_configure(sphb);
>> - rtas_st(rets, 0, ret);
>> + ret = vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_CONFIGURE);
>> + if (ret < 0) {
>> + goto param_error_exit;
>> + }
>> +
>> + rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>> return;
>>
>> param_error_exit:
>> diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
>> index 10fa88a..4ac5736 100644
>> --- a/hw/ppc/spapr_pci_vfio.c
>> +++ b/hw/ppc/spapr_pci_vfio.c
>> @@ -221,18 +221,6 @@ int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb,
>> int option)
>> return RTAS_OUT_SUCCESS;
>> }
>>
>> -int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb)
>> -{
>> - int ret;
>> -
>> - ret = vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_CONFIGURE);
>> - if (ret < 0) {
>> - return RTAS_OUT_PARAM_ERROR;
>> - }
>> -
>> - return RTAS_OUT_SUCCESS;
>> -}
>> -
>> static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data)
>> {
>> DeviceClass *dc = DEVICE_CLASS(klass);
>> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
>> index cc1b82c..f02ef51 100644
>> --- a/include/hw/pci-host/spapr.h
>> +++ b/include/hw/pci-host/spapr.h
>> @@ -140,6 +140,5 @@ int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
>> unsigned int addr, int option);
>> int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state);
>> int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option);
>> -int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb);
>>
>> #endif /* __HW_SPAPR_PCI_H__ */
>>
>
>
--
Alexey
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 04/12] spapr_pci: Fold spapr_phb_vfio_eeh_configure() into spapr_pci code
2016-02-29 3:45 ` Alexey Kardashevskiy
@ 2016-02-29 4:25 ` Alexey Kardashevskiy
2016-02-29 6:20 ` David Gibson
0 siblings, 1 reply; 33+ messages in thread
From: Alexey Kardashevskiy @ 2016-02-29 4:25 UTC (permalink / raw)
To: David Gibson, benh
Cc: agraf, qemu-devel, gwshan, mdroth, alex.williamson, qemu-ppc
On 02/29/2016 02:45 PM, Alexey Kardashevskiy wrote:
> On 02/29/2016 12:43 PM, Alexey Kardashevskiy wrote:
>> On 02/26/2016 10:31 PM, David Gibson wrote:
>>> Simplify the sPAPR PCI code by folding spapr_phb_vfio_eeh_configure()
>>> into rtas_ibm_configure_pe().
>>>
>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>>
>> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>
> Aaaand this breaks mingw32:
>
> CC ppc64-softmmu/hw/ppc/spapr_pci.o
> /home/aik/p/qemu-dwg-eeh/hw/ppc/spapr_pci.c:46:24: fatal error:
> linux/vfio.h: No such file or directory
> compilation terminated.
> /home/aik/p/qemu-dwg-eeh/rules.mak:57: recipe for target
> 'hw/ppc/spapr_pci.o' failed
> make[1]: *** [hw/ppc/spapr_pci.o] Error 1
> make[1]: *** Waiting for unfinished jobs....
> Makefile:186: recipe for target 'subdir-ppc64-softmmu' failed
> make: *** [subdir-ppc64-softmmu] Error 2
> make: Leaving directory '/scratch/aik/p/qemu-dwg-eeh--ppc64_mingw32-build'
>
>
>
>>
>>> ---
>>> hw/ppc/spapr_pci.c | 11 +++++++++--
>>> hw/ppc/spapr_pci_vfio.c | 12 ------------
>>> include/hw/pci-host/spapr.h | 1 -
>>> 3 files changed, 9 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>>> index d1e5222..fa633a9 100644
>>> --- a/hw/ppc/spapr_pci.c
>>> +++ b/hw/ppc/spapr_pci.c
>>> @@ -42,6 +42,9 @@
>>> #include "hw/ppc/spapr_drc.h"
>>> #include "sysemu/device_tree.h"
>>>
>>> +#include "hw/vfio/vfio.h"
>>> +#include <linux/vfio.h>
>>> +
This is missing:
#ifdef CONFIG_LINUX
#include <linux/vfio.h>
#endif
and below where you use symbols from linux/vfio.h.
My version of this rework did convert class callbacks to exported helpers
and keep these helpers (plus stubs) in spapr_pci_vfio.c, with one #ifdef
CONFIG_LINUX. Looked quite clean to me...
>>> /* Copied from the kernel arch/powerpc/platforms/pseries/msi.c */
>>> #define RTAS_QUERY_FN 0
>>> #define RTAS_CHANGE_FN 1
>>> @@ -628,8 +631,12 @@ static void rtas_ibm_configure_pe(PowerPCCPU *cpu,
>>> goto param_error_exit;
>>> }
>>>
>>> - ret = spapr_phb_vfio_eeh_configure(sphb);
>>> - rtas_st(rets, 0, ret);
>>> + ret = vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_CONFIGURE);
>>> + if (ret < 0) {
>>> + goto param_error_exit;
>>> + }
>>> +
>>> + rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>>> return;
>>>
>>> param_error_exit:
>>> diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
>>> index 10fa88a..4ac5736 100644
>>> --- a/hw/ppc/spapr_pci_vfio.c
>>> +++ b/hw/ppc/spapr_pci_vfio.c
>>> @@ -221,18 +221,6 @@ int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb,
>>> int option)
>>> return RTAS_OUT_SUCCESS;
>>> }
>>>
>>> -int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb)
>>> -{
>>> - int ret;
>>> -
>>> - ret = vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_CONFIGURE);
>>> - if (ret < 0) {
>>> - return RTAS_OUT_PARAM_ERROR;
>>> - }
>>> -
>>> - return RTAS_OUT_SUCCESS;
>>> -}
>>> -
>>> static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data)
>>> {
>>> DeviceClass *dc = DEVICE_CLASS(klass);
>>> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
>>> index cc1b82c..f02ef51 100644
>>> --- a/include/hw/pci-host/spapr.h
>>> +++ b/include/hw/pci-host/spapr.h
>>> @@ -140,6 +140,5 @@ int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
>>> unsigned int addr, int option);
>>> int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state);
>>> int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option);
>>> -int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb);
>>>
>>> #endif /* __HW_SPAPR_PCI_H__ */
--
Alexey
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 04/12] spapr_pci: Fold spapr_phb_vfio_eeh_configure() into spapr_pci code
2016-02-29 4:25 ` Alexey Kardashevskiy
@ 2016-02-29 6:20 ` David Gibson
0 siblings, 0 replies; 33+ messages in thread
From: David Gibson @ 2016-02-29 6:20 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: qemu-devel, mdroth, gwshan, agraf, alex.williamson, qemu-ppc
[-- Attachment #1: Type: text/plain, Size: 2313 bytes --]
On Mon, Feb 29, 2016 at 03:25:24PM +1100, Alexey Kardashevskiy wrote:
> On 02/29/2016 02:45 PM, Alexey Kardashevskiy wrote:
> >On 02/29/2016 12:43 PM, Alexey Kardashevskiy wrote:
> >>On 02/26/2016 10:31 PM, David Gibson wrote:
> >>>Simplify the sPAPR PCI code by folding spapr_phb_vfio_eeh_configure()
> >>>into rtas_ibm_configure_pe().
> >>>
> >>>Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> >>
> >>Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> >
> >Aaaand this breaks mingw32:
> >
> > CC ppc64-softmmu/hw/ppc/spapr_pci.o
> >/home/aik/p/qemu-dwg-eeh/hw/ppc/spapr_pci.c:46:24: fatal error:
> >linux/vfio.h: No such file or directory
> >compilation terminated.
> >/home/aik/p/qemu-dwg-eeh/rules.mak:57: recipe for target
> >'hw/ppc/spapr_pci.o' failed
> >make[1]: *** [hw/ppc/spapr_pci.o] Error 1
> >make[1]: *** Waiting for unfinished jobs....
> >Makefile:186: recipe for target 'subdir-ppc64-softmmu' failed
> >make: *** [subdir-ppc64-softmmu] Error 2
> >make: Leaving directory '/scratch/aik/p/qemu-dwg-eeh--ppc64_mingw32-build'
> >
> >
> >
> >>
> >>>---
> >>> hw/ppc/spapr_pci.c | 11 +++++++++--
> >>> hw/ppc/spapr_pci_vfio.c | 12 ------------
> >>> include/hw/pci-host/spapr.h | 1 -
> >>> 3 files changed, 9 insertions(+), 15 deletions(-)
> >>>
> >>>diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> >>>index d1e5222..fa633a9 100644
> >>>--- a/hw/ppc/spapr_pci.c
> >>>+++ b/hw/ppc/spapr_pci.c
> >>>@@ -42,6 +42,9 @@
> >>> #include "hw/ppc/spapr_drc.h"
> >>> #include "sysemu/device_tree.h"
> >>>
> >>>+#include "hw/vfio/vfio.h"
> >>>+#include <linux/vfio.h>
> >>>+
>
> This is missing:
>
> #ifdef CONFIG_LINUX
> #include <linux/vfio.h>
> #endif
>
> and below where you use symbols from linux/vfio.h.
>
>
> My version of this rework did convert class callbacks to exported helpers
> and keep these helpers (plus stubs) in spapr_pci_vfio.c, with one #ifdef
> CONFIG_LINUX. Looked quite clean to me...
Yeah, good idea. I'd forgotten the case of non-Linux builds. I'll do
something similar in v2.
--
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: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2016-02-29 6:21 UTC | newest]
Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-26 11:31 [Qemu-devel] [PATCH 00/12] Allow EEH on spapr-pci-host-bridge devices David Gibson
2016-02-26 11:31 ` [Qemu-devel] [PATCH 01/12] vfio: Start improving VFIO/EEH interface David Gibson
2016-02-29 0:58 ` Alexey Kardashevskiy
2016-02-29 3:13 ` David Gibson
2016-02-26 11:31 ` [Qemu-devel] [PATCH 02/12] spapr_pci: Switch to vfio_eeh_as_op() interface David Gibson
2016-02-29 1:43 ` Alexey Kardashevskiy
2016-02-29 3:00 ` David Gibson
2016-02-26 11:31 ` [Qemu-devel] [PATCH 03/12] spapr_pci: Eliminate class callbacks David Gibson
2016-02-29 1:43 ` Alexey Kardashevskiy
2016-02-29 3:42 ` Alexey Kardashevskiy
2016-02-26 11:31 ` [Qemu-devel] [PATCH 04/12] spapr_pci: Fold spapr_phb_vfio_eeh_configure() into spapr_pci code David Gibson
2016-02-29 1:43 ` Alexey Kardashevskiy
2016-02-29 3:45 ` Alexey Kardashevskiy
2016-02-29 4:25 ` Alexey Kardashevskiy
2016-02-29 6:20 ` David Gibson
2016-02-26 11:31 ` [Qemu-devel] [PATCH 05/12] spapr_pci: Fold spapr_phb_vfio_eeh_reset() " David Gibson
2016-02-29 1:43 ` Alexey Kardashevskiy
2016-02-26 11:31 ` [Qemu-devel] [PATCH 06/12] spapr_pci: Fold spapr_phb_vfio_eeh_get_state() " David Gibson
2016-02-29 1:43 ` Alexey Kardashevskiy
2016-02-26 11:31 ` [Qemu-devel] [PATCH 07/12] spapr_pci: Fold spapr_phb_vfio_eeh_set_option() " David Gibson
2016-02-29 1:44 ` Alexey Kardashevskiy
2016-02-26 11:31 ` [Qemu-devel] [PATCH 08/12] spapr_pci: Fold spapr_phb_vfio_reset() " David Gibson
2016-02-29 1:44 ` Alexey Kardashevskiy
2016-02-26 11:32 ` [Qemu-devel] [PATCH 09/12] spapr_pci: Allow EEH on spapr-pci-host-bridge David Gibson
2016-02-29 1:44 ` Alexey Kardashevskiy
2016-02-26 11:32 ` [Qemu-devel] [PATCH 10/12] spapr_pci: (Mostly) remove spapr-pci-vfio-host-bridge David Gibson
2016-02-29 1:42 ` Alexey Kardashevskiy
2016-02-29 3:06 ` David Gibson
2016-02-26 11:32 ` [Qemu-devel] [PATCH 11/12] spapr_pci: Remove finish_realize hook David Gibson
2016-02-29 1:44 ` Alexey Kardashevskiy
2016-02-26 11:32 ` [Qemu-devel] [PATCH 12/12] vfio: Eliminate vfio_container_ioctl() David Gibson
2016-02-29 1:44 ` Alexey Kardashevskiy
2016-02-26 11:33 ` [Qemu-devel] [PATCH 00/12] Allow EEH on spapr-pci-host-bridge devices David Gibson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).