[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] amd/iommu: remove hidden AMD inclusive mappings


I'm still catching up on the map-inclusive and map-reserved stuff.
I have a couple questions below.

On 9/21/18 10:20 PM, Roger Pau Monne wrote:
And just rely on arch_iommu_hwdom_init to setup the correct inclusive
mappings as it's done for Intel.

AMD has code in amd_iommu_hwdom_init to setup inclusive mappings up to
max_pdx, remove this since it's now a duplication of
arch_iommu_hwdom_init. Note that AMD mapped every page with a valid
mfn up to max_pdx, arch_iommu_hwdom_init will only do so for memory
below 4GB, so this is a functional change for AMD.

Is there any reasons why limit to only below 4GB?

Move the default setting of iommu_hwdom_{inclusive/reserved} to
arch_iommu_hwdom_init since the defaults are now the same for both
Intel and AMD.

Reported-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
Cc: Brian Woods <brian.woods@xxxxxxx>
Cc: Kevin Tian <kevin.tian@xxxxxxxxx>
Cc: Jan Beulich <jbeulich@xxxxxxxx>
Cc: Paul Durrant <paul.durrant@xxxxxxxxxx>
  xen/drivers/passthrough/amd/pci_amd_iommu.c | 39 ---------------------
  xen/drivers/passthrough/vtd/iommu.c         |  7 ----
  xen/drivers/passthrough/x86/iommu.c         |  8 ++++-
  3 files changed, 7 insertions(+), 47 deletions(-)

diff --git a/xen/drivers/passthrough/x86/iommu.c 
index b7c8b5be41..2de8822c59 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -210,7 +210,13 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
BUG_ON(!is_hardware_domain(d)); - ASSERT(iommu_hwdom_inclusive != -1 && iommu_hwdom_inclusive != -1);

Not sure if this was a typo. The logic looks strange. Anyhow, it is removed.

However, I notice in the xen/drivers/passthrough/iommu.c that
the parsing logic for the map-reserved option is setting the 
instead of the iommu_hwdom_reserved. Is that intentional?

Also, what's the difference b/w the option map-inclusive parameter in 
parse_dom0_iommu_param() and the option iommu_inclusive_mapping in 

+    /* Inclusive mappings are enabled by default for PV. */
+    if ( iommu_hwdom_inclusive == -1 )
+        iommu_hwdom_inclusive = is_pv_domain(d);
+    /* Reserved IOMMU mappings are enabled by default. */
+    if ( iommu_hwdom_reserved == -1 )
+        iommu_hwdom_reserved = 1;
      if ( iommu_hwdom_inclusive && !is_pv_domain(d) )


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.