Re: [Xen-devel] [RFC for-4.5 08/12] xen/passthrough: iommu: Split generic IOMMU code

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.02.14 at 18:43, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
> The generic IOMMU framework code (xen/drivers/passthrough/iommu.c) contains
> functions specific to x86 and PCI.
> 
> Split the framework in 3 distincts files:
>     - iommu.c: contains generic functions shared between x86 and ARM
>                (when it will be supported)
>     - iommu_pci.c: contains specific functions for PCI passthrough
>     - iommu_x86.c: contains specific functions for x86
> 
> iommu_pci.c will be only compiled when PCI is supported by the architecture
> (eg. HAS_PCI is defined).
> 
> This patch is mostly code movement in new files.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> Cc: Xiantao Zhang <xiantao.zhang@xxxxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> ---
>  xen/drivers/passthrough/Makefile    |    6 +-
>  xen/drivers/passthrough/iommu.c     |  473 
> +----------------------------------
>  xen/drivers/passthrough/iommu_pci.c |  468 ++++++++++++++++++++++++++++++++++

There's xen/drivers/passthrough/pci.c already - any reason not to
move the code there?

>  xen/drivers/passthrough/iommu_x86.c |   65 +++++

Same here for xen/drivers/passthrough/x86/.

> @@ -696,125 +344,6 @@ void iommu_crash_shutdown(void)
>      iommu_enabled = iommu_intremap = 0;
>  }
>  
> -int iommu_do_domctl(
> -    struct xen_domctl *domctl, struct domain *d,
> -    XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)

The function itself should probably not be moved out. Either the
PCI-specific pieces of it should be made conditional, or a
descendant function be created. Since (afaict) you'll need all of
the domctl-s (with different arguments) too for pass-through on
ARM - what's your plan for them?

> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -1784,31 +1784,31 @@ static int intel_iommu_unmap_page(struct domain *d, 
> unsigned long gfn)
>  
>  void iommu_pte_flush(struct domain *d, u64 gfn, u64 *pte,
>                       int order, int present)
> -{
> -    struct acpi_drhd_unit *drhd;
> -    struct iommu *iommu = NULL;
> -    struct hvm_iommu *hd = domain_hvm_iommu(d);
> -    int flush_dev_iotlb;
> -    int iommu_domid;
> +    {
> +        struct acpi_drhd_unit *drhd;
> +        struct iommu *iommu = NULL;
> +        struct hvm_iommu *hd = domain_hvm_iommu(d);
> +        int flush_dev_iotlb;
> +        int iommu_domid;
>  
> -    iommu_flush_cache_entry(pte, sizeof(struct dma_pte));
> +        iommu_flush_cache_entry(pte, sizeof(struct dma_pte));
>  
> -    for_each_drhd_unit ( drhd )
> -    {
> -        iommu = drhd->iommu;
> -        if ( !test_bit(iommu->index, &hd->iommu_bitmap) )
> -            continue;
> +        for_each_drhd_unit ( drhd )
> +        {
> +            iommu = drhd->iommu;
> +            if ( !test_bit(iommu->index, &hd->iommu_bitmap) )
> +                continue;
>  
> -        flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
> -        iommu_domid= domain_iommu_domid(d, iommu);
> -        if ( iommu_domid == -1 )
> -            continue;
> -        if ( iommu_flush_iotlb_psi(iommu, iommu_domid,
> -                                   (paddr_t)gfn << PAGE_SHIFT_4K,
> -                                   order, !present, flush_dev_iotlb) )
> -            iommu_flush_write_buffer(iommu);
> +            flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
> +            iommu_domid= domain_iommu_domid(d, iommu);
> +            if ( iommu_domid == -1 )
> +                continue;
> +            if ( iommu_flush_iotlb_psi(iommu, iommu_domid,
> +                                       (paddr_t)gfn << PAGE_SHIFT_4K,
> +                                       order, !present, flush_dev_iotlb) )
> +                iommu_flush_write_buffer(iommu);
> +        }
>      }
> -}

What are these changes to indentation about? Are you
deliberately breaking common rules here, or is this some sort of
unintentional leftover?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel