Re: [Qemu-ppc] [PATCH 05/12] spapr_pci: Fold spapr_phb_vfio_eeh_reset() into spapr_pci code

[Alexey Kardashevskiy]

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 <address@hidden>

Reviewed-by: Alexey Kardashevskiy <address@hidden>

> ---
>   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