Re: [Xen-devel] [PATCH v7 1/6] iommu: introduce the concept of DFN...

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Sep 12, 2018 at 12:30:23PM +0100, Paul Durrant wrote:
> diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
> index e35d941f3c..19d5d55d79 100644
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -23,11 +23,37 @@
>  #include <xen/page-defs.h>
>  #include <xen/spinlock.h>
>  #include <xen/pci.h>
> +#include <xen/typesafe.h>
>  #include <public/hvm/ioreq.h>
>  #include <public/domctl.h>
>  #include <asm/device.h>
>  #include <asm/iommu.h>
>  
> +TYPE_SAFE(uint64_t, dfn);

Shouldn't this be unsigned long instead of uint64_t? (like gfn or mfn)

> +#define PRI_dfn     PRIx64
> +#define INVALID_DFN _dfn(~0UL)

Or if there's a reason for the above to be uint64_t, then INVALID_DFN
should use ULL.

> +
> +#ifndef dfn_t
> +#define dfn_t /* Grep fodder: dfn_t, _dfn() and dfn_x() are defined above */
> +#define _dfn
> +#define dfn_x
> +#undef dfn_t
> +#undef _dfn
> +#undef dfn_x
> +#endif
> +
> +#define IOMMU_PAGE_SHIFT 12
> +#define IOMMU_PAGE_SIZE  (_AC(1,L) << IOMMU_PAGE_SHIFT)

There's no need to use _AC?

> +#define IOMMU_PAGE_MASK  (~(IOMMU_PAGE_SIZE - 1))
> +
> +typedef uint64_t daddr_t;

Use paddr_t instead of uint64_t directly?

> +
> +#define __dfn_to_daddr(dfn) ((daddr_t)(dfn) << IOMMU_PAGE_SHIFT)
> +#define __daddr_to_dfn(daddr) ((uint64_t)(daddr >> IOMMU_PAGE_SHIFT))
> +
> +#define dfn_to_daddr(dfn) __dfn_to_daddr(dfn_x(dfn))
> +#define daddr_to_dfn(daddr) _dfn(__daddr_to_dfn(daddr))
> +
>  extern bool_t iommu_enable, iommu_enabled;
>  extern bool_t force_iommu, iommu_dom0_strict, iommu_verbose;
>  extern bool_t iommu_workaround_bios_bug, iommu_igfx, iommu_passthrough;
> @@ -60,9 +86,9 @@ void iommu_teardown(struct domain *d);
>  #define IOMMUF_readable  (1u<<_IOMMUF_readable)
>  #define _IOMMUF_writable 1
>  #define IOMMUF_writable  (1u<<_IOMMUF_writable)
> -int __must_check iommu_map_page(struct domain *d, unsigned long gfn,
> +int __must_check iommu_map_page(struct domain *d, unsigned long dfn,
>                                  unsigned long mfn, unsigned int flags);
> -int __must_check iommu_unmap_page(struct domain *d, unsigned long gfn);
> +int __must_check iommu_unmap_page(struct domain *d, unsigned long dfn);
>  
>  enum iommu_feature
>  {
> @@ -150,9 +176,9 @@ struct iommu_ops {
>  #endif /* HAS_PCI */
>  
>      void (*teardown)(struct domain *d);
> -    int __must_check (*map_page)(struct domain *d, unsigned long gfn,
> +    int __must_check (*map_page)(struct domain *d, unsigned long dfn,
>                                   unsigned long mfn, unsigned int flags);
> -    int __must_check (*unmap_page)(struct domain *d, unsigned long gfn);
> +    int __must_check (*unmap_page)(struct domain *d, unsigned long dfn);
>      void (*free_page_table)(struct page_info *);
>  #ifdef CONFIG_X86
>      void (*update_ire_from_apic)(unsigned int apic, unsigned int reg, 
> unsigned int value);
> @@ -163,7 +189,7 @@ struct iommu_ops {
>      void (*resume)(void);
>      void (*share_p2m)(struct domain *d);
>      void (*crash_shutdown)(void);
> -    int __must_check (*iotlb_flush)(struct domain *d, unsigned long gfn,
> +    int __must_check (*iotlb_flush)(struct domain *d, unsigned long dfn,
>                                      unsigned int page_count);
>      int __must_check (*iotlb_flush_all)(struct domain *d);
>      int (*get_reserved_device_memory)(iommu_grdm_t *, void *);
> @@ -185,7 +211,7 @@ int iommu_do_pci_domctl(struct xen_domctl *, struct 
> domain *d,
>  int iommu_do_domctl(struct xen_domctl *, struct domain *d,
>                      XEN_GUEST_HANDLE_PARAM(xen_domctl_t));
>  
> -int __must_check iommu_iotlb_flush(struct domain *d, unsigned long gfn,
> +int __must_check iommu_iotlb_flush(struct domain *d, unsigned long dfn,

While doing this wholesale gfn to dfn replacement, it might be a good
idea to also change unsigned long for dfn at the same time?

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel