On Mon, Jan 05, 2015 at 11:26:28AM +1100, Gavin Shan wrote: > The patch implements sPAPRPHBClass::eeh_handler so that the > EEH RTAS requests can be routed to VFIO for further handling. > > Signed-off-by: Gavin Shan <[email protected]> > --- > hw/ppc/spapr_pci_vfio.c | 56 > +++++++++++++++++++++++++++++++++++++++++++++++++ > hw/vfio/common.c | 1 + > 2 files changed, 57 insertions(+) > > diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c > index 144912b..73652a9 100644 > --- a/hw/ppc/spapr_pci_vfio.c > +++ b/hw/ppc/spapr_pci_vfio.c > @@ -71,6 +71,61 @@ static void spapr_phb_vfio_finish_realize(sPAPRPHBState > *sphb, Error **errp) > spapr_tce_get_iommu(tcet)); > } > > +static int spapr_phb_vfio_eeh_handler(sPAPRPHBState *sphb, int req, int opt) > +{ > + sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb); > + struct vfio_eeh_pe_op op = { .argsz = sizeof(op) };
This is a local variable, which means it won't be initialized. You
never memset() it and it's not obvious that all fields get
initialized, which makes it dangerous to pass to an ioctl().
> + int cmd;
> +
> + switch (req) {
> + case RTAS_EEH_REQ_SET_OPTION:
> + switch (opt) {
> + case RTAS_EEH_DISABLE:
> + cmd = VFIO_EEH_PE_DISABLE;
> + break;
> + case RTAS_EEH_ENABLE:
> + cmd = VFIO_EEH_PE_ENABLE;
> + break;
> + case RTAS_EEH_THAW_IO:
> + cmd = VFIO_EEH_PE_UNFREEZE_IO;
> + break;
> + case RTAS_EEH_THAW_DMA:
> + cmd = VFIO_EEH_PE_UNFREEZE_DMA;
> + break;
> + default:
> + return -EINVAL;
> + }
> + break;
> + case RTAS_EEH_REQ_GET_STATE:
> + cmd = VFIO_EEH_PE_GET_STATE;
> + break;
> + case RTAS_EEH_REQ_RESET:
> + switch (opt) {
> + case RTAS_SLOT_RESET_DEACTIVATE:
> + cmd = VFIO_EEH_PE_RESET_DEACTIVATE;
> + break;
> + case RTAS_SLOT_RESET_HOT:
> + cmd = VFIO_EEH_PE_RESET_HOT;
> + break;
> + case RTAS_SLOT_RESET_FUNDAMENTAL:
> + cmd = VFIO_EEH_PE_RESET_FUNDAMENTAL;
> + break;
> + default:
> + return -EINVAL;
> + }
> + break;
> + case RTAS_EEH_REQ_CONFIGURE:
> + cmd = VFIO_EEH_PE_CONFIGURE;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + op.op = cmd;
> + return vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
> + VFIO_EEH_PE_OP, &op);
Don't you need some sort of translation from the errnos the ioctl()
returns into RTAS error codes?
> +}
> +
> static void spapr_phb_vfio_reset(DeviceState *qdev)
> {
> /* Do nothing */
> @@ -84,6 +139,7 @@ static void spapr_phb_vfio_class_init(ObjectClass *klass,
> void *data)
> dc->props = spapr_phb_vfio_properties;
> dc->reset = spapr_phb_vfio_reset;
> spc->finish_realize = spapr_phb_vfio_finish_realize;
> + spc->eeh_handler = spapr_phb_vfio_eeh_handler;
> }
>
> static const TypeInfo spapr_phb_vfio_info = {
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index cf483ff..8a10c8b 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -948,6 +948,7 @@ int vfio_container_ioctl(AddressSpace *as, int32_t
> groupid,
> 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 */
--
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
pgpjGi7d6REGx.pgp
Description: PGP signature
