[PATCH] Arm: relax iomem_access_permitted() check

[PATCH] Arm: relax iomem_access_permitted() check

[Jan Beulich]

Ranges checked by iomem_access_permitted() are inclusive; to permit a
mapping there's no need for access to also have been granted for the
subsequent page.

Fixes: 80f9c3167084 ("xen/arm: acpi: Map MMIO on fault in stage-2 page table for the hardware domain")
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1893,7 +1893,7 @@ static bool try_map_mmio(gfn_t gfn)
         return false;
 
     /* The hardware domain can only map permitted MMIO regions */
-    if ( !iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn) + 1) )
+    if ( !iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn)) )
         return false;
 
     return !map_regions_p2mt(d, gfn, 1, mfn, p2m_mmio_direct_c);

Re: [PATCH] Arm: relax iomem_access_permitted() check

[Julien Grall]

Hi Jan,

On 18/08/2021 08:52, Jan Beulich wrote:
> Ranges checked by iomem_access_permitted() are inclusive; to permit a
> mapping there's no need for access to also have been granted for the
> subsequent page.

Good catch! OOI, how did you find it?

> 
> Fixes: 80f9c3167084 ("xen/arm: acpi: Map MMIO on fault in stage-2 page table for the hardware domain")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Julien Grall <jgrall@amazon.com>

Cheers,

> 
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -1893,7 +1893,7 @@ static bool try_map_mmio(gfn_t gfn)
>           return false;
>   
>       /* The hardware domain can only map permitted MMIO regions */
> -    if ( !iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn) + 1) )
> +    if ( !iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn)) )
>           return false;
>   
>       return !map_regions_p2mt(d, gfn, 1, mfn, p2m_mmio_direct_c);
> 

-- 
Julien Grall

Re: [PATCH] Arm: relax iomem_access_permitted() check

[Jan Beulich]

On 18.08.2021 19:44, Julien Grall wrote:
> On 18/08/2021 08:52, Jan Beulich wrote:
>> Ranges checked by iomem_access_permitted() are inclusive; to permit a
>> mapping there's no need for access to also have been granted for the
>> subsequent page.
> 
> Good catch! OOI, how did you find it?

In the course of my large-IOMMU-mappings work I ended up grep-ing for
all uses of the function, and this one - while unrelated to that work -
caught my eye.

>> Fixes: 80f9c3167084 ("xen/arm: acpi: Map MMIO on fault in stage-2 page table for the hardware domain")
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Reviewed-by: Julien Grall <jgrall@amazon.com>

Thanks.

Jan