[PATCH v2 0/2] grant table and add-to-physmap adjustments on top of XSAs 379 and 384

[PATCH v2 0/2] grant table and add-to-physmap adjustments on top of XSAs 379 and 384

[Jan Beulich]

The original 1st patch has meanwhile gone in, but the adjustment requested
for patch 2 required a new prereq patch.

1: mm: introduce INVALID_{G,M}FN_RAW
2: memory: XENMEM_add_to_physmap (almost) wrapping checks

Jan

[PATCH v2 1/2] mm: introduce INVALID_{G,M}FN_RAW

[Jan Beulich]

This allows properly tying together INVALID_{G,M}FN and
INVALID_{G,M}FN_INITIALIZER as well as using the actual values in
compile time constant expressions (or even preprocessor dirctives).

Since INVALID_PFN is unused, and with x86'es paging_mark_pfn_dirty()
being the only user of pfn_t it also doesn't seem likely that new uses
would appear, remove that one at this same occasion.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: New.

--- a/xen/include/xen/mm-frame.h
+++ b/xen/include/xen/mm-frame.h
@@ -6,12 +6,13 @@
 
 TYPE_SAFE(unsigned long, mfn);
 #define PRI_mfn          "05lx"
-#define INVALID_MFN      _mfn(~0UL)
+#define INVALID_MFN_RAW  (~0UL)
+#define INVALID_MFN      _mfn(INVALID_MFN_RAW)
 /*
  * To be used for global variable initialization. This workaround a bug
  * in GCC < 5.0.
  */
-#define INVALID_MFN_INITIALIZER { ~0UL }
+#define INVALID_MFN_INITIALIZER { INVALID_MFN_RAW }
 
 #ifndef mfn_t
 #define mfn_t /* Grep fodder: mfn_t, _mfn() and mfn_x() are defined above */
@@ -44,12 +45,13 @@ static inline bool_t mfn_eq(mfn_t x, mfn
 
 TYPE_SAFE(unsigned long, gfn);
 #define PRI_gfn          "05lx"
-#define INVALID_GFN      _gfn(~0UL)
+#define INVALID_GFN_RAW  (~0UL)
+#define INVALID_GFN      _gfn(INVALID_GFN_RAW)
 /*
  * To be used for global variable initialization. This workaround a bug
  * in GCC < 5.0 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64856
  */
-#define INVALID_GFN_INITIALIZER { ~0UL }
+#define INVALID_GFN_INITIALIZER { INVALID_GFN_RAW }
 
 #ifndef gfn_t
 #define gfn_t /* Grep fodder: gfn_t, _gfn() and gfn_x() are defined above */
@@ -82,7 +84,6 @@ static inline bool_t gfn_eq(gfn_t x, gfn
 
 TYPE_SAFE(unsigned long, pfn);
 #define PRI_pfn          "05lx"
-#define INVALID_PFN      (~0UL)
 
 #ifndef pfn_t
 #define pfn_t /* Grep fodder: pfn_t, _pfn() and pfn_x() are defined above */

Re: [PATCH v2 1/2] mm: introduce INVALID_{G,M}FN_RAW

[Oleksandr Andrushchenko]

Hi, Jan!

On 10.12.21 11:39, Jan Beulich wrote:
> This allows properly tying together INVALID_{G,M}FN and
> INVALID_{G,M}FN_INITIALIZER as well as using the actual values in
> compile time constant expressions (or even preprocessor dirctives).
s/dirctives/directives
>
> Since INVALID_PFN is unused, and with x86'es paging_mark_pfn_dirty()
> being the only user of pfn_t it also doesn't seem likely that new uses
> would appear, remove that one at this same occasion.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
With the above nit fixed:
Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> ---
> v2: New.
>
> --- a/xen/include/xen/mm-frame.h
> +++ b/xen/include/xen/mm-frame.h
> @@ -6,12 +6,13 @@
>   
>   TYPE_SAFE(unsigned long, mfn);
>   #define PRI_mfn          "05lx"
> -#define INVALID_MFN      _mfn(~0UL)
> +#define INVALID_MFN_RAW  (~0UL)
> +#define INVALID_MFN      _mfn(INVALID_MFN_RAW)
>   /*
>    * To be used for global variable initialization. This workaround a bug
>    * in GCC < 5.0.
>    */
> -#define INVALID_MFN_INITIALIZER { ~0UL }
> +#define INVALID_MFN_INITIALIZER { INVALID_MFN_RAW }
>   
>   #ifndef mfn_t
>   #define mfn_t /* Grep fodder: mfn_t, _mfn() and mfn_x() are defined above */
> @@ -44,12 +45,13 @@ static inline bool_t mfn_eq(mfn_t x, mfn
>   
>   TYPE_SAFE(unsigned long, gfn);
>   #define PRI_gfn          "05lx"
> -#define INVALID_GFN      _gfn(~0UL)
> +#define INVALID_GFN_RAW  (~0UL)
> +#define INVALID_GFN      _gfn(INVALID_GFN_RAW)
>   /*
>    * To be used for global variable initialization. This workaround a bug
>    * in GCC < 5.0 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64856
>    */
> -#define INVALID_GFN_INITIALIZER { ~0UL }
> +#define INVALID_GFN_INITIALIZER { INVALID_GFN_RAW }
>   
>   #ifndef gfn_t
>   #define gfn_t /* Grep fodder: gfn_t, _gfn() and gfn_x() are defined above */
> @@ -82,7 +84,6 @@ static inline bool_t gfn_eq(gfn_t x, gfn
>   
>   TYPE_SAFE(unsigned long, pfn);
>   #define PRI_pfn          "05lx"
> -#define INVALID_PFN      (~0UL)
>   
>   #ifndef pfn_t
>   #define pfn_t /* Grep fodder: pfn_t, _pfn() and pfn_x() are defined above */
>
>

Re: [PATCH v2 1/2] mm: introduce INVALID_{G,M}FN_RAW

[Julien Grall]

Hi Jan,

On 10/12/2021 09:39, Jan Beulich wrote:
> This allows properly tying together INVALID_{G,M}FN and
> INVALID_{G,M}FN_INITIALIZER as well as using the actual values in
> compile time constant expressions (or even preprocessor dirctives).
> 
> Since INVALID_PFN is unused, and with x86'es paging_mark_pfn_dirty()
> being the only user of pfn_t it also doesn't seem likely that new uses
> would appear, remove that one at this same occasion.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

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

Cheers,

-- 
Julien Grall

[PATCH v2 2/2] memory: XENMEM_add_to_physmap (almost) wrapping checks

[Jan Beulich]

Determining that behavior is correct (i.e. results in failure) for a
passed in GFN equaling INVALID_GFN is non-trivial. Make this quite a bit
more obvious by checking input in generic code - both for singular
requests to not match the value and for range ones to not pass / wrap
through it.

For Arm similarly make more obvious that no wrapping of MFNs passed
for XENMAPSPACE_dev_mmio and thus to map_dev_mmio_region() can occur:
Drop the "nr" parameter of the function to avoid future callers
appearing which might not themselves check for wrapping. Otherwise
the respective ASSERT() in rangeset_contains_range() could trigger.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Add comment to BUILD_BUG_ON(). Avoid transiently #define-ing _gfn()
    (by way of new prereq patch).

--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1479,7 +1479,7 @@ int xenmem_add_to_physmap_one(
         break;
     }
     case XENMAPSPACE_dev_mmio:
-        rc = map_dev_mmio_region(d, gfn, 1, _mfn(idx));
+        rc = map_dev_mmio_region(d, gfn, _mfn(idx));
         return rc;
 
     default:
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1357,19 +1357,18 @@ int unmap_mmio_regions(struct domain *d,
 
 int map_dev_mmio_region(struct domain *d,
                         gfn_t gfn,
-                        unsigned long nr,
                         mfn_t mfn)
 {
     int res;
 
-    if ( !(nr && iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn) + nr - 1)) )
+    if ( !iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn)) )
         return 0;
 
-    res = p2m_insert_mapping(d, gfn, nr, mfn, p2m_mmio_direct_c);
+    res = p2m_insert_mapping(d, gfn, 1, mfn, p2m_mmio_direct_c);
     if ( res < 0 )
     {
-        printk(XENLOG_G_ERR "Unable to map MFNs [%#"PRI_mfn" - %#"PRI_mfn" in Dom%d\n",
-               mfn_x(mfn), mfn_x(mfn) + nr - 1, d->domain_id);
+        printk(XENLOG_G_ERR "Unable to map MFN %#"PRI_mfn" in %pd\n",
+               mfn_x(mfn), d);
         return res;
     }
 
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -4150,7 +4150,10 @@ int gnttab_map_frame(struct domain *d, u
     bool status = false;
 
     if ( gfn_eq(gfn, INVALID_GFN) )
+    {
+        ASSERT_UNREACHABLE();
         return -EINVAL;
+    }
 
     grant_write_lock(gt);
 
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -832,6 +832,9 @@ int xenmem_add_to_physmap(struct domain
         return -EACCES;
     }
 
+    if ( gfn_eq(_gfn(xatp->gpfn), INVALID_GFN) )
+        return -EINVAL;
+
     if ( xatp->space == XENMAPSPACE_gmfn_foreign )
         extra.foreign_domid = DOMID_INVALID;
 
@@ -842,6 +845,18 @@ int xenmem_add_to_physmap(struct domain
     if ( xatp->size < start )
         return -EILSEQ;
 
+    if ( xatp->gpfn + xatp->size < xatp->gpfn ||
+         xatp->idx + xatp->size < xatp->idx )
+    {
+        /*
+         * Make sure INVALID_GFN is the highest representable value, i.e.
+         * guaranteeing that it won't fall in the middle of the
+         * [xatp->gpfn, xatp->gpfn + xatp->size) range checked above.
+         */
+        BUILD_BUG_ON(INVALID_GFN_RAW + 1);
+        return -EOVERFLOW;
+    }
+
     xatp->idx += start;
     xatp->gpfn += start;
     xatp->size -= start;
@@ -962,6 +977,9 @@ static int xenmem_add_to_physmap_batch(s
                                                extent, 1)) )
             return -EFAULT;
 
+        if ( gfn_eq(_gfn(gpfn), INVALID_GFN) )
+            return -EINVAL;
+
         rc = xenmem_add_to_physmap_one(d, xatpb->space, extra,
                                        idx, _gfn(gpfn));
 
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -297,7 +297,6 @@ int unmap_regions_p2mt(struct domain *d,
 
 int map_dev_mmio_region(struct domain *d,
                         gfn_t gfn,
-                        unsigned long nr,
                         mfn_t mfn);
 
 int p2m_insert_mapping(struct domain *d, gfn_t start_gfn, unsigned long nr,

Re: [PATCH v2 2/2] memory: XENMEM_add_to_physmap (almost) wrapping checks

[Oleksandr Andrushchenko]

Hi, Jan!

On 10.12.21 11:40, Jan Beulich wrote:
> Determining that behavior is correct (i.e. results in failure) for a
> passed in GFN equaling INVALID_GFN is non-trivial. Make this quite a bit
> more obvious by checking input in generic code - both for singular
> requests to not match the value and for range ones to not pass / wrap
> through it.
>
> For Arm similarly make more obvious that no wrapping of MFNs passed
> for XENMAPSPACE_dev_mmio and thus to map_dev_mmio_region() can occur:
> Drop the "nr" parameter of the function to avoid future callers
> appearing which might not themselves check for wrapping. Otherwise
> the respective ASSERT() in rangeset_contains_range() could trigger.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> v2: Add comment to BUILD_BUG_ON(). Avoid transiently #define-ing _gfn()
>      (by way of new prereq patch).
>
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -1479,7 +1479,7 @@ int xenmem_add_to_physmap_one(
>           break;
>       }
>       case XENMAPSPACE_dev_mmio:
> -        rc = map_dev_mmio_region(d, gfn, 1, _mfn(idx));
> +        rc = map_dev_mmio_region(d, gfn, _mfn(idx));
Technically this is ok, but reads odd now: the function maps a single
page, but its name has "region" in it (which might also be ok, e.g.
for a region of a single page).

I think it is worth either implementing full mfn range check inside
map_dev_mmio_region or renaming it to something else:
with mfn check map_dev_mmio_region will indeed be able to map
a region consisting of multiple pages and perform required validation.

Thank you,
Oleksandr
>           return rc;
>   
>       default:
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -1357,19 +1357,18 @@ int unmap_mmio_regions(struct domain *d,
>   
>   int map_dev_mmio_region(struct domain *d,
>                           gfn_t gfn,
> -                        unsigned long nr,
>                           mfn_t mfn)
>   {
>       int res;
>   
> -    if ( !(nr && iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn) + nr - 1)) )
> +    if ( !iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn)) )
>           return 0;
>   
> -    res = p2m_insert_mapping(d, gfn, nr, mfn, p2m_mmio_direct_c);
> +    res = p2m_insert_mapping(d, gfn, 1, mfn, p2m_mmio_direct_c);
>       if ( res < 0 )
>       {
> -        printk(XENLOG_G_ERR "Unable to map MFNs [%#"PRI_mfn" - %#"PRI_mfn" in Dom%d\n",
> -               mfn_x(mfn), mfn_x(mfn) + nr - 1, d->domain_id);
> +        printk(XENLOG_G_ERR "Unable to map MFN %#"PRI_mfn" in %pd\n",
> +               mfn_x(mfn), d);
>           return res;
>       }
>   
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -4150,7 +4150,10 @@ int gnttab_map_frame(struct domain *d, u
>       bool status = false;
>   
>       if ( gfn_eq(gfn, INVALID_GFN) )
> +    {
> +        ASSERT_UNREACHABLE();
>           return -EINVAL;
> +    }
>   
>       grant_write_lock(gt);
>   
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -832,6 +832,9 @@ int xenmem_add_to_physmap(struct domain
>           return -EACCES;
>       }
>   
> +    if ( gfn_eq(_gfn(xatp->gpfn), INVALID_GFN) )
> +        return -EINVAL;
> +
>       if ( xatp->space == XENMAPSPACE_gmfn_foreign )
>           extra.foreign_domid = DOMID_INVALID;
>   
> @@ -842,6 +845,18 @@ int xenmem_add_to_physmap(struct domain
>       if ( xatp->size < start )
>           return -EILSEQ;
>   
> +    if ( xatp->gpfn + xatp->size < xatp->gpfn ||
> +         xatp->idx + xatp->size < xatp->idx )
> +    {
> +        /*
> +         * Make sure INVALID_GFN is the highest representable value, i.e.
> +         * guaranteeing that it won't fall in the middle of the
> +         * [xatp->gpfn, xatp->gpfn + xatp->size) range checked above.
> +         */
> +        BUILD_BUG_ON(INVALID_GFN_RAW + 1);
> +        return -EOVERFLOW;
> +    }
> +
>       xatp->idx += start;
>       xatp->gpfn += start;
>       xatp->size -= start;
> @@ -962,6 +977,9 @@ static int xenmem_add_to_physmap_batch(s
>                                                  extent, 1)) )
>               return -EFAULT;
>   
> +        if ( gfn_eq(_gfn(gpfn), INVALID_GFN) )
> +            return -EINVAL;
> +
>           rc = xenmem_add_to_physmap_one(d, xatpb->space, extra,
>                                          idx, _gfn(gpfn));
>   
> --- a/xen/include/asm-arm/p2m.h
> +++ b/xen/include/asm-arm/p2m.h
> @@ -297,7 +297,6 @@ int unmap_regions_p2mt(struct domain *d,
>   
>   int map_dev_mmio_region(struct domain *d,
>                           gfn_t gfn,
> -                        unsigned long nr,
>                           mfn_t mfn);
>   
>   int p2m_insert_mapping(struct domain *d, gfn_t start_gfn, unsigned long nr,
>
>

Re: [PATCH v2 2/2] memory: XENMEM_add_to_physmap (almost) wrapping checks

[Jan Beulich]

On 10.12.2021 15:10, Oleksandr Andrushchenko wrote:
> On 10.12.21 11:40, Jan Beulich wrote:
>> --- a/xen/arch/arm/mm.c
>> +++ b/xen/arch/arm/mm.c
>> @@ -1479,7 +1479,7 @@ int xenmem_add_to_physmap_one(
>>           break;
>>       }
>>       case XENMAPSPACE_dev_mmio:
>> -        rc = map_dev_mmio_region(d, gfn, 1, _mfn(idx));
>> +        rc = map_dev_mmio_region(d, gfn, _mfn(idx));
> Technically this is ok, but reads odd now: the function maps a single
> page, but its name has "region" in it (which might also be ok, e.g.
> for a region of a single page).
> 
> I think it is worth either implementing full mfn range check inside
> map_dev_mmio_region or renaming it to something else:
> with mfn check map_dev_mmio_region will indeed be able to map
> a region consisting of multiple pages and perform required validation.

Well, I had no maintainer comments on v1 regarding the name. I'd be
happy to rename to e.g. map_dev_mmio_page(), so long as there can be
an agreed upon name before I submit a possible v3. Julien, Stefano?

Jan

Re: [PATCH v2 2/2] memory: XENMEM_add_to_physmap (almost) wrapping checks

[Stefano Stabellini]

On Mon, 13 Dec 2021, Jan Beulich wrote:
> On 10.12.2021 15:10, Oleksandr Andrushchenko wrote:
> > On 10.12.21 11:40, Jan Beulich wrote:
> >> --- a/xen/arch/arm/mm.c
> >> +++ b/xen/arch/arm/mm.c
> >> @@ -1479,7 +1479,7 @@ int xenmem_add_to_physmap_one(
> >>           break;
> >>       }
> >>       case XENMAPSPACE_dev_mmio:
> >> -        rc = map_dev_mmio_region(d, gfn, 1, _mfn(idx));
> >> +        rc = map_dev_mmio_region(d, gfn, _mfn(idx));
> > Technically this is ok, but reads odd now: the function maps a single
> > page, but its name has "region" in it (which might also be ok, e.g.
> > for a region of a single page).
> > 
> > I think it is worth either implementing full mfn range check inside
> > map_dev_mmio_region or renaming it to something else:
> > with mfn check map_dev_mmio_region will indeed be able to map
> > a region consisting of multiple pages and perform required validation.
> 
> Well, I had no maintainer comments on v1 regarding the name. I'd be
> happy to rename to e.g. map_dev_mmio_page(), so long as there can be
> an agreed upon name before I submit a possible v3. Julien, Stefano?

I like map_dev_mmio_page

[PATCH v3] memory: XENMEM_add_to_physmap (almost) wrapping checks

[Jan Beulich]

Determining that behavior is correct (i.e. results in failure) for a
passed in GFN equaling INVALID_GFN is non-trivial. Make this quite a bit
more obvious by checking input in generic code - both for singular
requests to not match the value and for range ones to not pass / wrap
through it.

For Arm similarly make more obvious that no wrapping of MFNs passed
for XENMAPSPACE_dev_mmio and thus to map_dev_mmio_region() can occur:
Drop the "nr" parameter of the function to avoid future callers
appearing which might not themselves check for wrapping. Otherwise
the respective ASSERT() in rangeset_contains_range() could trigger.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v3: Rename function to map_dev_mmio_page().
v2: Add comment to BUILD_BUG_ON(). Avoid transiently #define-ing _gfn()
    (by way of new prereq patch).

--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1479,7 +1479,7 @@ int xenmem_add_to_physmap_one(
         break;
     }
     case XENMAPSPACE_dev_mmio:
-        rc = map_dev_mmio_region(d, gfn, 1, _mfn(idx));
+        rc = map_dev_mmio_page(d, gfn, _mfn(idx));
         return rc;
 
     default:
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1355,21 +1355,18 @@ int unmap_mmio_regions(struct domain *d,
     return p2m_remove_mapping(d, start_gfn, nr, mfn);
 }
 
-int map_dev_mmio_region(struct domain *d,
-                        gfn_t gfn,
-                        unsigned long nr,
-                        mfn_t mfn)
+int map_dev_mmio_page(struct domain *d, gfn_t gfn, mfn_t mfn)
 {
     int res;
 
-    if ( !(nr && iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn) + nr - 1)) )
+    if ( !iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn)) )
         return 0;
 
-    res = p2m_insert_mapping(d, gfn, nr, mfn, p2m_mmio_direct_c);
+    res = p2m_insert_mapping(d, gfn, 1, mfn, p2m_mmio_direct_c);
     if ( res < 0 )
     {
-        printk(XENLOG_G_ERR "Unable to map MFNs [%#"PRI_mfn" - %#"PRI_mfn" in Dom%d\n",
-               mfn_x(mfn), mfn_x(mfn) + nr - 1, d->domain_id);
+        printk(XENLOG_G_ERR "Unable to map MFN %#"PRI_mfn" in %pd\n",
+               mfn_x(mfn), d);
         return res;
     }
 
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -4157,7 +4157,10 @@ int gnttab_map_frame(struct domain *d, u
     bool status = false;
 
     if ( gfn_eq(gfn, INVALID_GFN) )
+    {
+        ASSERT_UNREACHABLE();
         return -EINVAL;
+    }
 
     grant_write_lock(gt);
 
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -832,6 +832,9 @@ int xenmem_add_to_physmap(struct domain
         return -EACCES;
     }
 
+    if ( gfn_eq(_gfn(xatp->gpfn), INVALID_GFN) )
+        return -EINVAL;
+
     if ( xatp->space == XENMAPSPACE_gmfn_foreign )
         extra.foreign_domid = DOMID_INVALID;
 
@@ -842,6 +845,18 @@ int xenmem_add_to_physmap(struct domain
     if ( xatp->size < start )
         return -EILSEQ;
 
+    if ( xatp->gpfn + xatp->size < xatp->gpfn ||
+         xatp->idx + xatp->size < xatp->idx )
+    {
+        /*
+         * Make sure INVALID_GFN is the highest representable value, i.e.
+         * guaranteeing that it won't fall in the middle of the
+         * [xatp->gpfn, xatp->gpfn + xatp->size) range checked above.
+         */
+        BUILD_BUG_ON(INVALID_GFN_RAW + 1);
+        return -EOVERFLOW;
+    }
+
     xatp->idx += start;
     xatp->gpfn += start;
     xatp->size -= start;
@@ -962,6 +977,9 @@ static int xenmem_add_to_physmap_batch(s
                                                extent, 1)) )
             return -EFAULT;
 
+        if ( gfn_eq(_gfn(gpfn), INVALID_GFN) )
+            return -EINVAL;
+
         rc = xenmem_add_to_physmap_one(d, xatpb->space, extra,
                                        idx, _gfn(gpfn));
 
--- a/xen/arch/arm/include/asm/p2m.h
+++ b/xen/arch/arm/include/asm/p2m.h
@@ -295,10 +295,7 @@ int unmap_regions_p2mt(struct domain *d,
                        unsigned long nr,
                        mfn_t mfn);
 
-int map_dev_mmio_region(struct domain *d,
-                        gfn_t gfn,
-                        unsigned long nr,
-                        mfn_t mfn);
+int map_dev_mmio_page(struct domain *d, gfn_t gfn, mfn_t mfn);
 
 int p2m_insert_mapping(struct domain *d, gfn_t start_gfn, unsigned long nr,
                        mfn_t mfn, p2m_type_t t);

Re: [PATCH v3] memory: XENMEM_add_to_physmap (almost) wrapping checks

[Julien Grall]

Hi,

On 27/01/2022 14:53, Jan Beulich wrote:
> Determining that behavior is correct (i.e. results in failure) for a
> passed in GFN equaling INVALID_GFN is non-trivial. Make this quite a bit
> more obvious by checking input in generic code - both for singular
> requests to not match the value and for range ones to not pass / wrap
> through it.
> 
> For Arm similarly make more obvious that no wrapping of MFNs passed
> for XENMAPSPACE_dev_mmio and thus to map_dev_mmio_region() can occur:
> Drop the "nr" parameter of the function to avoid future callers
> appearing which might not themselves check for wrapping. Otherwise
> the respective ASSERT() in rangeset_contains_range() could trigger.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

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

Cheers,

-- 
Julien Grall