RE: [PATCH 5/6] iommu: remove the share_p2m operation

["Durrant, Paul" <pdurrant@xxxxxxxxxxxx>]

> -----Original Message-----
> From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Sent: 24 July 2020 20:01
> To: Paul Durrant <paul@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Durrant, Paul <pdurrant@xxxxxxxxxxxx>; Jan Beulich <jbeulich@xxxxxxxx>; 
> George Dunlap
> <george.dunlap@xxxxxxxxxx>; Wei Liu <wl@xxxxxxx>; Roger Pau Monné 
> <roger.pau@xxxxxxxxxx>; Kevin Tian
> <kevin.tian@xxxxxxxxx>
> Subject: RE: [EXTERNAL] [PATCH 5/6] iommu: remove the share_p2m operation
> 
> 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 17:46, Paul Durrant wrote:
> > From: Paul Durrant <pdurrant@xxxxxxxxxx>
> >
> > Sharing of HAP tables is VT-d specific so the operation is never defined for
> > AMD IOMMU.
> 
> It's not VT-d specific, and Xen really did used to share on AMD.
> 

Oh, I never thought that ever worked.

> In fact, a remnant of this logic is still present in the form of the
> comment for p2m_type_t explaining why p2m_ram_rw needs to be 0.
> 
> That said, I agree it shouldn't be a hook, because it is statically
> known at domain create time based on flags and hardware properties.
> 

Well, for VT-d that may well change in future ;-)

> >  There's also no need to pro-actively set vtd.pgd_maddr when using
> > shared EPT as it is straightforward to simply define a helper function to
> > return the appropriate value in the shared and non-shared cases.
> 
> It occurs to me that vtd.pgd_maddr and amd.root_table really are the
> same thing, and furthermore are common to all IOMMU implementations on
> any architecture.  Is it worth trying to make them common, rather than
> continuing to let each implementation handle them differently?

I looked at this. The problem really lies in terminology. The 'root table' in 
the VT-d code refers to the single root table which IIRC is called the device 
table in the AMD IOMMU code so, whilst I could convert both to use a single 
common field, I think it's not really that valuable to do so since it's likely 
to make the code slightly more confusing to read (for someone expecting the 
names to tally with the spec).

  Paul

> 
> ~Andrew