> -----Original Message----- > From: Jan Beulich <jbeul...@suse.com> > Sent: 26 July 2020 09:14 > To: p...@xen.org > Cc: 'Andrew Cooper' <andrew.coop...@citrix.com>; > xen-devel@lists.xenproject.org; Durrant, Paul > <pdurr...@amazon.co.uk>; 'Lukasz Hawrylko' <lukasz.hawry...@linux.intel.com>; > 'Wei Liu' <w...@xen.org>; > 'Roger Pau Monné' <roger....@citrix.com>; 'Kevin Tian' <kevin.t...@intel.com> > Subject: RE: [EXTERNAL] [PATCH 1/6] x86/iommu: re-arrange arch_iommu to > separate common fields... > > CAUTION: This email originated from outside of the organization. Do not click > links or open > attachments unless you can confirm the sender and know the content is safe. > > > > On 24.07.2020 20:49, Paul Durrant wrote: > >> From: Andrew Cooper <andrew.coop...@citrix.com> > >> Sent: 24 July 2020 18:29 > >> > >> 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. > > +1 for shortening in some way. Even amd_vi would already be better imo, > albeit I'm with Andrew and would think just amd is fine here (and > matches how things are in the file system structure). >
OK, I'll shorten to 'amd'. > While at it, may I ask that you also switch the plain "int" fields to > "unsigned int" - I think that's doable for both of them. > Sure, I can do that. Paul > Jan