Re: [Qemu-ppc] [RFC PATCH v2 11/13] spapr_pci_vfio: Enable DDW

[David Gibson]

On Fri, Aug 15, 2014 at 08:12:33PM +1000, Alexey Kardashevskiy wrote:
> This implements DDW for VFIO. Host kernel support is required for this.
> 
> Signed-off-by: Alexey Kardashevskiy <address@hidden>
> ---
> Changes:
> v2:
> * remove()/reset() callbacks use spapr_pci's ones
> ---
>  hw/ppc/spapr_pci_vfio.c | 86 
> +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 86 insertions(+)
> 
> diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
> index 11b4272..79df716 100644
> --- a/hw/ppc/spapr_pci_vfio.c
> +++ b/hw/ppc/spapr_pci_vfio.c
> @@ -71,6 +71,88 @@ static void spapr_phb_vfio_finish_realize(sPAPRPHBState 
> *sphb, Error **errp)
>                                  spapr_tce_get_iommu(tcet));
>  
>      object_unref(OBJECT(tcet));
> +
> +    if (sphb->ddw_enabled) {
> +        sphb->ddw_enabled = !!(info.flags & VFIO_IOMMU_SPAPR_TCE_FLAG_DDW);

This overrides an explicit ddw= set by the user, which is a bit
counter-intuitive.

> +    }
> +}
> +
> +static int spapr_pci_vfio_ddw_query(sPAPRPHBState *sphb,
> +                                    uint32_t *windows_available,
> +                                    uint32_t *page_size_mask)
> +{
> +    sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
> +    struct vfio_iommu_spapr_tce_query query = { .argsz = sizeof(query) };
> +    int ret;
> +
> +    ret = vfio_container_ioctl(&sphb->iommu_as, svphb->iommugroupid,
> +                               VFIO_IOMMU_SPAPR_TCE_QUERY, &query);
> +    if (ret) {
> +        return ret;
> +    }
> +
> +    *windows_available = query.windows_available;
> +    *page_size_mask = query.page_size_mask;
> +
> +    return ret;
> +}
> +
> +static int spapr_pci_vfio_ddw_create(sPAPRPHBState *sphb, uint32_t 
> page_shift,
> +                                     uint32_t window_shift, uint32_t liobn,
> +                                     sPAPRTCETable **ptcet)
> +{
> +    sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
> +    struct vfio_iommu_spapr_tce_create create = {
> +        .argsz = sizeof(create),
> +        .page_shift = page_shift,
> +        .window_shift = window_shift,
> +        .start_addr = 0
> +    };
> +    int ret;
> +
> +    ret = vfio_container_ioctl(&sphb->iommu_as, svphb->iommugroupid,
> +                               VFIO_IOMMU_SPAPR_TCE_CREATE, &create);
> +    if (ret) {
> +        return ret;
> +    }
> +
> +    *ptcet = spapr_tce_new_table(DEVICE(sphb), liobn,
> +                                 create.start_addr, page_shift,
> +                                 1ULL << (window_shift - page_shift),
> +                                 true);
> +    memory_region_add_subregion(&sphb->iommu_root, (*ptcet)->bus_offset,
> +                                spapr_tce_get_iommu(*ptcet));
> +
> +    return ret;
> +}
> +
> +static int spapr_pci_vfio_ddw_remove(sPAPRPHBState *sphb, sPAPRTCETable 
> *tcet)
> +{
> +    sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
> +    struct vfio_iommu_spapr_tce_remove remove = {
> +        .argsz = sizeof(remove),
> +        .start_addr = tcet->bus_offset
> +    };
> +    int ret;
> +
> +    spapr_pci_ddw_remove(sphb, tcet);
> +    ret = vfio_container_ioctl(&sphb->iommu_as, svphb->iommugroupid,
> +                               VFIO_IOMMU_SPAPR_TCE_REMOVE, &remove);
> +
> +    return ret;
> +}
> +
> +static int spapr_pci_vfio_ddw_reset(sPAPRPHBState *sphb)
> +{
> +    sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
> +    struct vfio_iommu_spapr_tce_reset reset = { .argsz = sizeof(reset) };
> +    int ret;
> +
> +    spapr_pci_ddw_reset(sphb);
> +    ret = vfio_container_ioctl(&sphb->iommu_as, svphb->iommugroupid,
> +                               VFIO_IOMMU_SPAPR_TCE_RESET, &reset);

Unlike the non-VFIO version, this doesn't appear to reset ddw_num.

Also, there isn't call to explicitly call DDW reset on system reset.
Is that handled in kernel by the overall VFIO reset?

> +    return ret;
>  }
>  
>  static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data)
> @@ -80,6 +162,10 @@ 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->ddw_query = spapr_pci_vfio_ddw_query;
> +    spc->ddw_create = spapr_pci_vfio_ddw_create;
> +    spc->ddw_remove = spapr_pci_vfio_ddw_remove;
> +    spc->ddw_reset = spapr_pci_vfio_ddw_reset;
>  }
>  
>  static const TypeInfo spapr_phb_vfio_info = {

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

pgpIlvvMZcqsP.pgp
Description: PGP signature