> -----Original Message-----
> From: Andrew Cooper <andrew.coop...@citrix.com>
> Sent: 24 July 2020 18:29
> To: Paul Durrant <p...@xen.org>; xen-devel@lists.xenproject.org
> Cc: Paul Durrant <pdurr...@amazon.com>; Lukasz Hawrylko
> <lukasz.hawry...@linux.intel.com>; Jan Beulich
> <jbeul...@suse.com>; Wei Liu <w...@xen.org>; Roger Pau Monné
> <roger....@citrix.com>; Kevin Tian
> <kevin.t...@intel.com>
> Subject: Re: [PATCH 1/6] x86/iommu: re-arrange arch_iommu to separate common
> fields...
>
> On 24/07/2020 17:46, Paul Durrant wrote:
> > diff --git a/xen/include/asm-x86/iommu.h b/xen/include/asm-x86/iommu.h
> > index 6c9d5e5632..a7add5208c 100644
> > --- a/xen/include/asm-x86/iommu.h
> > +++ b/xen/include/asm-x86/iommu.h
> > @@ -45,16 +45,23 @@ typedef uint64_t daddr_t;
> >
> > struct arch_iommu
> > {
> > - u64 pgd_maddr; /* io page directory machine address */
> > - spinlock_t mapping_lock; /* io page table lock */
> > - int agaw; /* adjusted guest address width, 0 is level 2 30-bit */
> > - u64 iommu_bitmap; /* bitmap of iommu(s) that the domain
> > uses */
> > - struct list_head mapped_rmrrs;
> > -
> > - /* amd iommu support */
> > - int paging_mode;
> > - struct page_info *root_table;
> > - struct guest_iommu *g_iommu;
> > + spinlock_t mapping_lock; /* io page table lock */
> > +
> > + union {
> > + /* Intel VT-d */
> > + struct {
> > + u64 pgd_maddr; /* io page directory machine address */
> > + int agaw; /* adjusted guest address width, 0 is level 2 30-bit
> > */
> > + u64 iommu_bitmap; /* bitmap of iommu(s) that the domain uses */
> > + struct list_head mapped_rmrrs;
> > + } vtd;
> > + /* AMD IOMMU */
> > + struct {
> > + int paging_mode;
> > + struct page_info *root_table;
> > + struct guest_iommu *g_iommu;
> > + } amd_iommu;
> > + };
>
> The naming split here is weird.
>
> Ideally we'd have struct {vtd,amd}_iommu in appropriate headers, and
> this would be simply
>
> union {
> struct vtd_iommu vtd;
> struct amd_iommu amd;
> };
>
> If this isn't trivial to arrange, can we at least s/amd_iommu/amd/ here ?
I was in two minds. I tried to look for a TLA for the AMD IOMMU and 'amd'
seemed a little too non-descript. I don't really mind though if there's a
strong preference to shorted it.
I can certainly try moving the struct definitions into the implementation
headers.
Paul
>
> ~Andrew
