[Xen-devel] [PATCH v5 5/8] x86/iommu: switch the hwdom mapping function to use page_get_type
This avoids repeated calls to page_is_ram_type which improves
performance and makes the code easier to read.
No functional change.
Signed-off-by: Roger Pau Monné
---
Changes since v4:
- New in this version.
---
Cc: Jan Beulich
---
xen/drivers/passthrough/x86/iommu.c | 60 +++--
1 file changed, 32 insertions(+), 28 deletions(-)
diff --git a/xen/drivers/passthrough/x86/iommu.c
b/xen/drivers/passthrough/x86/iommu.c
index 25e1ebf8b3..36ec5c4f54 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -134,6 +134,37 @@ void arch_iommu_domain_destroy(struct domain *d)
{
}
+static bool __hwdom_init hwdom_iommu_map(const struct domain *d,
+ unsigned long pfn,
+ unsigned long max_pfn)
+{
+/*
+ * Set up 1:1 mapping for dom0. Default to include only conventional RAM
+ * areas and let RMRRs include needed reserved regions. When set, the
+ * inclusive mapping additionally maps in every pfn up to 4GB except those
+ * that fall in unusable ranges.
+ */
+if ( (pfn > max_pfn && !mfn_valid(_mfn(pfn))) || xen_in_range(pfn) )
+return false;
+
+switch ( page_get_type(pfn) )
+{
+case RAM_TYPE_UNUSABLE:
+return false;
+
+case RAM_TYPE_CONVENTIONAL:
+if ( iommu_hwdom_strict )
+return false;
+break;
+
+default:
+if ( !iommu_hwdom_inclusive || pfn > max_pfn )
+return false;
+}
+
+return true;
+}
+
void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
{
unsigned long i, top, max_pfn;
@@ -149,36 +180,9 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
for ( i = 0; i < top; i++ )
{
unsigned long pfn = pdx_to_pfn(i);
-bool map;
int rc;
-/*
- * Set up 1:1 mapping for dom0. Default to include only
- * conventional RAM areas and let RMRRs include needed reserved
- * regions. When set, the inclusive mapping additionally maps in
- * every pfn up to 4GB except those that fall in unusable ranges.
- */
-if ( pfn > max_pfn && !mfn_valid(_mfn(pfn)) )
-continue;
-
-if ( iommu_hwdom_inclusive && pfn <= max_pfn )
-map = !page_is_ram_type(pfn, RAM_TYPE_UNUSABLE);
-else
-map = page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL);
-
-if ( !map )
-continue;
-
-/* Exclude Xen bits */
-if ( xen_in_range(pfn) )
-continue;
-
-/*
- * If dom0-strict mode is enabled then exclude conventional RAM
- * and let the common code map dom0's pages.
- */
-if ( iommu_hwdom_strict &&
- page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL) )
+if ( !hwdom_iommu_map(d, pfn, max_pfn) )
continue;
rc = iommu_map_page(d, pfn, pfn, IOMMUF_readable|IOMMUF_writable);
--
2.18.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 5/8] x86/iommu: switch the hwdom mapping function to use page_get_type
>>> On 14.08.18 at 15:43, wrote:
> +static bool __hwdom_init hwdom_iommu_map(const struct domain *d,
Unused function parameter?
> + unsigned long pfn,
> + unsigned long max_pfn)
> +{
> +/*
> + * Set up 1:1 mapping for dom0. Default to include only conventional RAM
> + * areas and let RMRRs include needed reserved regions. When set, the
> + * inclusive mapping additionally maps in every pfn up to 4GB except
> those
> + * that fall in unusable ranges.
> + */
> +if ( (pfn > max_pfn && !mfn_valid(_mfn(pfn))) || xen_in_range(pfn) )
> +return false;
> +
> +switch ( page_get_type(pfn) )
> +{
> +case RAM_TYPE_UNUSABLE:
> +return false;
> +
> +case RAM_TYPE_CONVENTIONAL:
> +if ( iommu_hwdom_strict )
> +return false;
> +break;
Simply "return !iommu_hwdom_strict", to make it yet easier to follow
what happens in which case? But depending on your thoughts on
my remarks on the previous patch, this may change all over again
anyway.
> +default:
> +if ( !iommu_hwdom_inclusive || pfn > max_pfn )
> +return false;
And then a plain return statement here too?
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 5/8] x86/iommu: switch the hwdom mapping function to use page_get_type
>>> On 14.08.18 at 15:43, wrote:
> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -134,6 +134,37 @@ void arch_iommu_domain_destroy(struct domain *d)
> {
> }
>
> +static bool __hwdom_init hwdom_iommu_map(const struct domain *d,
> + unsigned long pfn,
> + unsigned long max_pfn)
> +{
> +/*
> + * Set up 1:1 mapping for dom0. Default to include only conventional RAM
> + * areas and let RMRRs include needed reserved regions. When set, the
> + * inclusive mapping additionally maps in every pfn up to 4GB except
> those
> + * that fall in unusable ranges.
> + */
> +if ( (pfn > max_pfn && !mfn_valid(_mfn(pfn))) || xen_in_range(pfn) )
> +return false;
> +
> +switch ( page_get_type(pfn) )
> +{
> +case RAM_TYPE_UNUSABLE:
> +return false;
> +
> +case RAM_TYPE_CONVENTIONAL:
> +if ( iommu_hwdom_strict )
> +return false;
> +break;
> +
> +default:
> +if ( !iommu_hwdom_inclusive || pfn > max_pfn )
> +return false;
> +}
Ah, in patch 8 it becomes clear why you want execution to fall out
of the switch() in the "true" case. Please disregard the respective
earlier comments then.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
