Re: [Xen-devel] [PATCH 2/6] x86/vtd: Rename struct iommu to vtd_iommu

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@xxxxxxxxxx]
> Sent: 22 February 2019 19:13
> To: Xen-devel <xen-devel@xxxxxxxxxxxxx>
> Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Jan Beulich
> <JBeulich@xxxxxxxx>; Paul Durrant <Paul.Durrant@xxxxxxxxxx>; Jun Nakajima
> <jun.nakajima@xxxxxxxxx>; Kevin Tian <kevin.tian@xxxxxxxxx>
> Subject: [PATCH 2/6] x86/vtd: Rename struct iommu to vtd_iommu
> 
> VT-d's local struct iommu is an overly-generic name, for a structure which
> in
> practice maps 1-to-1 with the real IOMMUs in the system.
> 
> Additionally, address style issues on impacted lines.  This is mostly
> positioning of * for pointers and unnecessay casts with void pointers.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Paul Durrant <paul.durrant@xxxxxxxxxx>
> CC: Jun Nakajima <jun.nakajima@xxxxxxxxx>
> CC: Kevin Tian <kevin.tian@xxxxxxxxx>
> ---
>  xen/drivers/passthrough/vtd/dmar.c     |  6 +--
>  xen/drivers/passthrough/vtd/dmar.h     |  4 +-
>  xen/drivers/passthrough/vtd/extern.h   | 36 ++++++++---------
>  xen/drivers/passthrough/vtd/intremap.c | 26 ++++++------
>  xen/drivers/passthrough/vtd/iommu.c    | 74 +++++++++++++++++------------
> -----
>  xen/drivers/passthrough/vtd/iommu.h    |  8 ++--
>  xen/drivers/passthrough/vtd/qinval.c   | 34 ++++++++--------
>  xen/drivers/passthrough/vtd/quirks.c   | 10 ++---
>  xen/drivers/passthrough/vtd/utils.c    |  8 ++--
>  xen/drivers/passthrough/vtd/x86/ats.c  |  6 +--
>  10 files changed, 106 insertions(+), 106 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/vtd/dmar.c
> b/xen/drivers/passthrough/vtd/dmar.c
> index 81afa54..ce1b8ce 100644
> --- a/xen/drivers/passthrough/vtd/dmar.c
> +++ b/xen/drivers/passthrough/vtd/dmar.c
> @@ -137,7 +137,7 @@ struct acpi_drhd_unit * ioapic_to_drhd(unsigned int
> apic_id)
>      return NULL;
>  }
> 
> -struct acpi_drhd_unit * iommu_to_drhd(struct iommu *iommu)
> +struct acpi_drhd_unit *iommu_to_drhd(struct vtd_iommu *iommu)
>  {
>      struct acpi_drhd_unit *drhd;
> 
> @@ -151,7 +151,7 @@ struct acpi_drhd_unit * iommu_to_drhd(struct iommu
> *iommu)
>      return NULL;
>  }
> 
> -struct iommu * ioapic_to_iommu(unsigned int apic_id)
> +struct vtd_iommu *ioapic_to_iommu(unsigned int apic_id)
>  {
>      struct acpi_drhd_unit *drhd;
> 
> @@ -182,7 +182,7 @@ struct acpi_drhd_unit *hpet_to_drhd(unsigned int
> hpet_id)
>      return NULL;
>  }
> 
> -struct iommu *hpet_to_iommu(unsigned int hpet_id)
> +struct vtd_iommu *hpet_to_iommu(unsigned int hpet_id)
>  {
>      struct acpi_drhd_unit *drhd = hpet_to_drhd(hpet_id);
> 
> diff --git a/xen/drivers/passthrough/vtd/dmar.h
> b/xen/drivers/passthrough/vtd/dmar.h
> index 95bb132..1a9c965 100644
> --- a/xen/drivers/passthrough/vtd/dmar.h
> +++ b/xen/drivers/passthrough/vtd/dmar.h
> @@ -63,7 +63,7 @@ struct acpi_drhd_unit {
>      u64    address;                     /* register base address of the
> unit */
>      u16    segment;
>      u8     include_all:1;
> -    struct iommu *iommu;
> +    struct vtd_iommu *iommu;
>      struct list_head ioapic_list;
>      struct list_head hpet_list;
>  };
> @@ -128,7 +128,7 @@ do {                                                \
>  } while (0)
> 
>  int vtd_hw_check(void);
> -void disable_pmr(struct iommu *iommu);
> +void disable_pmr(struct vtd_iommu *iommu);
>  int is_igd_drhd(struct acpi_drhd_unit *drhd);
> 
>  #endif /* _DMAR_H_ */
> diff --git a/xen/drivers/passthrough/vtd/extern.h
> b/xen/drivers/passthrough/vtd/extern.h
> index 16eada9..cfef9a3 100644
> --- a/xen/drivers/passthrough/vtd/extern.h
> +++ b/xen/drivers/passthrough/vtd/extern.h
> @@ -30,38 +30,38 @@ extern bool_t rwbf_quirk;
>  extern const struct iommu_ops intel_iommu_ops;
> 
>  void print_iommu_regs(struct acpi_drhd_unit *drhd);
> -void print_vtd_entries(struct iommu *iommu, int bus, int devfn, u64
> gmfn);
> +void print_vtd_entries(struct vtd_iommu *iommu, int bus, int devfn, u64
> gmfn);

Should clean up the u64 here.

>  keyhandler_fn_t vtd_dump_iommu_info;
> 
> -int enable_qinval(struct iommu *iommu);
> -void disable_qinval(struct iommu *iommu);
> -int enable_intremap(struct iommu *iommu, int eim);
> -void disable_intremap(struct iommu *iommu);
> +int enable_qinval(struct vtd_iommu *iommu);
> +void disable_qinval(struct vtd_iommu *iommu);
> +int enable_intremap(struct vtd_iommu *iommu, bool eim);
> +void disable_intremap(struct vtd_iommu *iommu);
> 
>  void iommu_flush_cache_entry(void *addr, unsigned int size);
>  void iommu_flush_cache_page(void *addr, unsigned long npages);
>  int iommu_alloc(struct acpi_drhd_unit *drhd);
>  void iommu_free(struct acpi_drhd_unit *drhd);
> 
> -int iommu_flush_iec_global(struct iommu *iommu);
> -int iommu_flush_iec_index(struct iommu *iommu, u8 im, u16 iidx);
> -void clear_fault_bits(struct iommu *iommu);
> +int iommu_flush_iec_global(struct vtd_iommu *iommu);
> +int iommu_flush_iec_index(struct vtd_iommu *iommu, u8 im, u16 iidx);

u8 and u16.

> +void clear_fault_bits(struct vtd_iommu *iommu);
> 
> -struct iommu * ioapic_to_iommu(unsigned int apic_id);
> -struct iommu * hpet_to_iommu(unsigned int hpet_id);
> +struct vtd_iommu *ioapic_to_iommu(unsigned int apic_id);
> +struct vtd_iommu *hpet_to_iommu(unsigned int hpet_id);
>  struct acpi_drhd_unit * ioapic_to_drhd(unsigned int apic_id);
>  struct acpi_drhd_unit * hpet_to_drhd(unsigned int hpet_id);
> -struct acpi_drhd_unit * iommu_to_drhd(struct iommu *iommu);
> +struct acpi_drhd_unit *iommu_to_drhd(struct vtd_iommu *iommu);
>  struct acpi_rhsa_unit * drhd_to_rhsa(struct acpi_drhd_unit *drhd);
>

I know this sort of thing tends to snowball, but would it not be better to fix 
the style for the whole block (for consistency)?
 
> -struct acpi_drhd_unit * find_ats_dev_drhd(struct iommu *iommu);
> +struct acpi_drhd_unit *find_ats_dev_drhd(struct vtd_iommu *iommu);
> 
>  int ats_device(const struct pci_dev *, const struct acpi_drhd_unit *);
> 
> -int dev_invalidate_iotlb(struct iommu *iommu, u16 did,
> +int dev_invalidate_iotlb(struct vtd_iommu *iommu, u16 did,
>                           u64 addr, unsigned int size_order, u64 type);

u16 and u64.

> 
> -int __must_check qinval_device_iotlb_sync(struct iommu *iommu,
> +int __must_check qinval_device_iotlb_sync(struct vtd_iommu *iommu,
>                                            struct pci_dev *pdev,
>                                            u16 did, u16 size, u64 addr);

Likewise.

> 
> @@ -73,9 +73,9 @@ u64 alloc_pgtable_maddr(struct acpi_drhd_unit *drhd,
> unsigned long npages);
>  void free_pgtable_maddr(u64 maddr);
>  void *map_vtd_domain_page(u64 maddr);
>  void unmap_vtd_domain_page(void *va);
> -int domain_context_mapping_one(struct domain *domain, struct iommu
> *iommu,
> +int domain_context_mapping_one(struct domain *domain, struct vtd_iommu
> *iommu,
>                                 u8 bus, u8 devfn, const struct pci_dev *);

u8.

> -int domain_context_unmap_one(struct domain *domain, struct iommu *iommu,
> +int domain_context_unmap_one(struct domain *domain, struct vtd_iommu
> *iommu,
>                               u8 bus, u8 devfn);

And again. There are more but I won't comment individually any more. I don't 
see any other issues with the rest of the patch.

  Paul


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