Re: [PATCH 3/4] x86/iommu: reorder conditions used in the inclusive iommu mappings

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
> From: Xen-devel [mailto:xen-devel-bounces@lists.xenproject.org] On Behalf
> Of Roger Pau Monne
> Sent: 27 July 2018 16:32
> To: xen-devel@lists.xenproject.org
> Cc: Jan Beulich <jbeulich@suse.com>; Roger Pau Monne
> <roger.pau@citrix.com>
> Subject: [Xen-devel] [PATCH 3/4] x86/iommu: reorder conditions used in the
> inclusive iommu mappings
> 
> In order to place all the map conditions in a single if ... else
> conditional.
> 
> No functional change.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Cc: Jan Beulich <jbeulich@suse.com>
> ---
>  xen/drivers/passthrough/x86/iommu.c | 23 ++++++++++-------------
>  1 file changed, 10 insertions(+), 13 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/x86/iommu.c
> b/xen/drivers/passthrough/x86/iommu.c
> index ba0bbd9a15..24cc591aa5 100644
> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -158,19 +158,9 @@ void __hwdom_init arch_iommu_hwdom_init(struct
> domain *d)
>           * 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_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) )
> +        if ( (pfn > max_pfn && !mfn_valid(_mfn(pfn))) ||
> +             /* Exclude Xen bits */
> +             xen_in_range(pfn) )
>              continue;
> 
>          /*
> @@ -179,6 +169,13 @@ void __hwdom_init arch_iommu_hwdom_init(struct
> domain *d)
>           */
>          if ( iommu_dom0_strict &&
>               page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL) )
> +            map = false;
> +        else if ( iommu_inclusive && pfn <= max_pfn )
> +            map = !page_is_ram_type(pfn, RAM_TYPE_UNUSABLE);
> +        else
> +            map = page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL);
> +

Maybe better as...

If ( page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL) )
  map = !iommu_dom0_strict;
else if ( !page_is_ram_type(pfn, RAM_TYPE_UNUSABLE) )
  map = (iommu_inclusive && pfn <= max_pfn);
else
  map = false; 

(I think that logic is correct).

  Paul

> +        if ( !map )
>              continue;
> 
>          tmp = 1 << (PAGE_SHIFT - PAGE_SHIFT_4K);
> --
> 2.18.0
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel