From: Luca Fancellu <luca.fancellu@arm.com>
HAS_VMAP is not enabled on MPU systems, but the vmap functions are used
in places across common code. In order to keep the existing code and
maintain correct functionality, implement the `vmap_contig` and `vunmap`
functions for MPU systems.
Introduce a helper function `destroy_entire_xen_mapping` to aid with
unmapping an entire region when only the start address is known.
Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Signed-off-by: Harry Ramsey <harry.ramsey@arm.com>
---
xen/arch/arm/include/asm/mpu/mm.h | 11 +++++
xen/arch/arm/mpu/mm.c | 67 +++++++++++++++++++++++++------
xen/arch/arm/mpu/vmap.c | 14 +++++--
3 files changed, 77 insertions(+), 15 deletions(-)
diff --git a/xen/arch/arm/include/asm/mpu/mm.h b/xen/arch/arm/include/asm/mpu/mm.h
index e1ded6521d..83ee0e59ca 100644
--- a/xen/arch/arm/include/asm/mpu/mm.h
+++ b/xen/arch/arm/include/asm/mpu/mm.h
@@ -111,6 +111,17 @@ pr_t pr_of_addr(paddr_t base, paddr_t limit, unsigned int flags);
int mpumap_contains_region(pr_t *table, uint8_t nr_regions, paddr_t base,
paddr_t limit, uint8_t *index);
+
+/*
+ * Destroys and frees (if reference count is 0) an entire xen mapping on MPU
+ * systems where only the start address is known.
+ *
+ * @param s Start address of memory region to be destroyed.
+ *
+ * @return: 0 on success, negative on error.
+ */
+int destroy_entire_xen_mapping(paddr_t s);
+
#endif /* __ARM_MPU_MM_H__ */
/*
diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
index 687dec3bc6..29d8e7ff11 100644
--- a/xen/arch/arm/mpu/mm.c
+++ b/xen/arch/arm/mpu/mm.c
@@ -290,6 +290,35 @@ static void disable_mpu_region_from_index(uint8_t index)
write_protection_region(&xen_mpumap[index], index);
}
+/*
+ * Free a xen_mpumap entry given the index. A mpu region is actually disabled
+ * when the refcount is 0 and the region type is MPUMAP_REGION_FOUND.
+ *
+ * @param idx Index of the mpumap entry.
+ * @param region_found_type Either MPUMAP_REGION_FOUND or MPUMAP_REGION_INCLUSIVE.
+ * @return 0 on success, otherwise negative on error.
+ */
+static int xen_mpumap_free_entry(uint8_t idx, int region_found_type)
+{
+ ASSERT(spin_is_locked(&xen_mpumap_lock));
+ ASSERT(idx != INVALID_REGION_IDX);
+
+ if ( xen_mpumap[idx].refcount == 0 )
+ {
+ if ( MPUMAP_REGION_FOUND == region_found_type )
+ disable_mpu_region_from_index(idx);
+ else
+ {
+ printk(XENLOG_ERR "Cannot remove a partial region\n");
+ return -EINVAL;
+ }
+ }
+ else
+ xen_mpumap[idx].refcount -= 1;
+
+ return 0;
+}
+
/*
* Update the entry in the MPU memory region mapping table (xen_mpumap) for the
* given memory range and flags, creating one if none exists.
@@ -357,18 +386,7 @@ static int xen_mpumap_update_entry(paddr_t base, paddr_t limit,
return -EINVAL;
}
- if ( xen_mpumap[idx].refcount == 0 )
- {
- if ( MPUMAP_REGION_FOUND == rc )
- disable_mpu_region_from_index(idx);
- else
- {
- printk("Cannot remove a partial region\n");
- return -EINVAL;
- }
- }
- else
- xen_mpumap[idx].refcount -= 1;
+ return xen_mpumap_free_entry(idx, rc);
}
return 0;
@@ -418,6 +436,31 @@ int destroy_xen_mappings(unsigned long s, unsigned long e)
return xen_mpumap_update(s, e, 0);
}
+int destroy_entire_xen_mapping(paddr_t s)
+{
+ int rc;
+ uint8_t idx;
+
+ ASSERT(IS_ALIGNED(s, PAGE_SIZE));
+
+ spin_lock(&xen_mpumap_lock);
+
+ rc = mpumap_contains_region(xen_mpumap, max_mpu_regions, s, s + PAGE_SIZE,
+ &idx);
+ if ( rc == MPUMAP_REGION_NOTFOUND )
+ {
+ printk(XENLOG_ERR "Cannot remove entry that does not exist");
+ rc = -EINVAL;
+ }
+
+ /* As we are unmapping entire region use MPUMAP_REGION_FOUND instead */
+ rc = xen_mpumap_free_entry(idx, MPUMAP_REGION_FOUND);
+
+ spin_unlock(&xen_mpumap_lock);
+
+ return rc;
+}
+
int map_pages_to_xen(unsigned long virt, mfn_t mfn, unsigned long nr_mfns,
unsigned int flags)
{
diff --git a/xen/arch/arm/mpu/vmap.c b/xen/arch/arm/mpu/vmap.c
index f977b79cd4..d3037ae98a 100644
--- a/xen/arch/arm/mpu/vmap.c
+++ b/xen/arch/arm/mpu/vmap.c
@@ -1,19 +1,27 @@
/* SPDX-License-Identifier: GPL-2.0-only */
#include <xen/bug.h>
+#include <xen/mm.h>
#include <xen/mm-frame.h>
#include <xen/types.h>
#include <xen/vmap.h>
void *vmap_contig(mfn_t mfn, unsigned int nr)
{
- BUG_ON("unimplemented");
- return NULL;
+ paddr_t base = mfn_to_maddr(mfn);
+
+ if ( map_pages_to_xen(base, mfn, nr, PAGE_HYPERVISOR ) )
+ return NULL;
+
+ return maddr_to_virt(base);
}
void vunmap(const void *va)
{
- BUG_ON("unimplemented");
+ paddr_t base = virt_to_maddr(va);
+
+ if ( destroy_entire_xen_mapping(base) )
+ panic("Failed to vunmap region\n");
}
/*
--
2.43.0
On 28/11/2025 10:58, Harry Ramsey wrote:
> From: Luca Fancellu <luca.fancellu@arm.com>
>
> HAS_VMAP is not enabled on MPU systems, but the vmap functions are used
Just as a reminder, we don't intend to support VMAP on MPU? Asking because it
would otherwise be a duplicate effort to implement only these two helpers.
> in places across common code. In order to keep the existing code and
> maintain correct functionality, implement the `vmap_contig` and `vunmap`
> functions for MPU systems.
>
> Introduce a helper function `destroy_entire_xen_mapping` to aid with
> unmapping an entire region when only the start address is known.
>
> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
> Signed-off-by: Harry Ramsey <harry.ramsey@arm.com>
> ---
> xen/arch/arm/include/asm/mpu/mm.h | 11 +++++
> xen/arch/arm/mpu/mm.c | 67 +++++++++++++++++++++++++------
> xen/arch/arm/mpu/vmap.c | 14 +++++--
> 3 files changed, 77 insertions(+), 15 deletions(-)
>
> diff --git a/xen/arch/arm/include/asm/mpu/mm.h b/xen/arch/arm/include/asm/mpu/mm.h
> index e1ded6521d..83ee0e59ca 100644
> --- a/xen/arch/arm/include/asm/mpu/mm.h
> +++ b/xen/arch/arm/include/asm/mpu/mm.h
> @@ -111,6 +111,17 @@ pr_t pr_of_addr(paddr_t base, paddr_t limit, unsigned int flags);
> int mpumap_contains_region(pr_t *table, uint8_t nr_regions, paddr_t base,
> paddr_t limit, uint8_t *index);
>
> +
> +/*
> + * Destroys and frees (if reference count is 0) an entire xen mapping on MPU
> + * systems where only the start address is known.
> + *
> + * @param s Start address of memory region to be destroyed.
> + *
> + * @return: 0 on success, negative on error.
> + */
> +int destroy_entire_xen_mapping(paddr_t s);
NIT: I read it as all the mappings which is a bit misleading :)
Maybe something like: destroy_mapping_containing or alike?
> +
> #endif /* __ARM_MPU_MM_H__ */
>
> /*
> diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
> index 687dec3bc6..29d8e7ff11 100644
> --- a/xen/arch/arm/mpu/mm.c
> +++ b/xen/arch/arm/mpu/mm.c
> @@ -290,6 +290,35 @@ static void disable_mpu_region_from_index(uint8_t index)
> write_protection_region(&xen_mpumap[index], index);
> }
>
> +/*
> + * Free a xen_mpumap entry given the index. A mpu region is actually disabled
> + * when the refcount is 0 and the region type is MPUMAP_REGION_FOUND.
> + *
> + * @param idx Index of the mpumap entry.
> + * @param region_found_type Either MPUMAP_REGION_FOUND or MPUMAP_REGION_INCLUSIVE.
Both of these are unsigned, so why the parameter is int?
> + * @return 0 on success, otherwise negative on error.
> + */
> +static int xen_mpumap_free_entry(uint8_t idx, int region_found_type)
> +{
> + ASSERT(spin_is_locked(&xen_mpumap_lock));
> + ASSERT(idx != INVALID_REGION_IDX);
> +
> + if ( xen_mpumap[idx].refcount == 0 )
> + {
> + if ( MPUMAP_REGION_FOUND == region_found_type )
> + disable_mpu_region_from_index(idx);
> + else
> + {
> + printk(XENLOG_ERR "Cannot remove a partial region\n");
> + return -EINVAL;
> + }
> + }
> + else
> + xen_mpumap[idx].refcount -= 1;
You could avoid nesting if/else by doing:
if ( xen_mpumap[idx].refcount )
{
xen_mpumap[idx].refcount -= 1;
return 0;
}
if ( MPUMAP_REGION_FOUND == region_found_type )
disable_mpu_region_from_index(idx);
else
{
printk(XENLOG_ERR "Cannot remove a partial region\n");
return -EINVAL;
}
> +
> + return 0;
> +}
> +
> /*
> * Update the entry in the MPU memory region mapping table (xen_mpumap) for the
> * given memory range and flags, creating one if none exists.
> @@ -357,18 +386,7 @@ static int xen_mpumap_update_entry(paddr_t base, paddr_t limit,
> return -EINVAL;
> }
>
> - if ( xen_mpumap[idx].refcount == 0 )
> - {
> - if ( MPUMAP_REGION_FOUND == rc )
> - disable_mpu_region_from_index(idx);
> - else
> - {
> - printk("Cannot remove a partial region\n");
> - return -EINVAL;
> - }
> - }
> - else
> - xen_mpumap[idx].refcount -= 1;
> + return xen_mpumap_free_entry(idx, rc);
> }
>
> return 0;
> @@ -418,6 +436,31 @@ int destroy_xen_mappings(unsigned long s, unsigned long e)
> return xen_mpumap_update(s, e, 0);
> }
>
> +int destroy_entire_xen_mapping(paddr_t s)
> +{
> + int rc;
> + uint8_t idx;
> +
> + ASSERT(IS_ALIGNED(s, PAGE_SIZE));
> +
> + spin_lock(&xen_mpumap_lock);
> +
> + rc = mpumap_contains_region(xen_mpumap, max_mpu_regions, s, s + PAGE_SIZE,
> + &idx);
So here you are searching for a region that includes at least s + PAGE_SIZE.
> + if ( rc == MPUMAP_REGION_NOTFOUND )
> + {
> + printk(XENLOG_ERR "Cannot remove entry that does not exist");
> + rc = -EINVAL;
Here you assing rc but ...
> + }
> +
> + /* As we are unmapping entire region use MPUMAP_REGION_FOUND instead */
> + rc = xen_mpumap_free_entry(idx, MPUMAP_REGION_FOUND);
here you would redefine it.
> +
> + spin_unlock(&xen_mpumap_lock);
> +
> + return rc;
> +}
> +
> int map_pages_to_xen(unsigned long virt, mfn_t mfn, unsigned long nr_mfns,
> unsigned int flags)
> {
> diff --git a/xen/arch/arm/mpu/vmap.c b/xen/arch/arm/mpu/vmap.c
> index f977b79cd4..d3037ae98a 100644
> --- a/xen/arch/arm/mpu/vmap.c
> +++ b/xen/arch/arm/mpu/vmap.c
> @@ -1,19 +1,27 @@
> /* SPDX-License-Identifier: GPL-2.0-only */
>
> #include <xen/bug.h>
> +#include <xen/mm.h>
> #include <xen/mm-frame.h>
> #include <xen/types.h>
> #include <xen/vmap.h>
>
> void *vmap_contig(mfn_t mfn, unsigned int nr)
> {
> - BUG_ON("unimplemented");
> - return NULL;
> + paddr_t base = mfn_to_maddr(mfn);
> +
> + if ( map_pages_to_xen(base, mfn, nr, PAGE_HYPERVISOR ) )
> + return NULL;
> +
> + return maddr_to_virt(base);
> }
>
> void vunmap(const void *va)
> {
> - BUG_ON("unimplemented");
> + paddr_t base = virt_to_maddr(va);
> +
> + if ( destroy_entire_xen_mapping(base) )
> + panic("Failed to vunmap region\n");
Looking at common vunmap() we ignore the return code from
destroy_xen_mappings(). Is it ok to diverge?
~Michal
> }
>
> /*
On 08/12/2025 09:53, Orzel, Michal wrote:
>
> On 28/11/2025 10:58, Harry Ramsey wrote:
>> From: Luca Fancellu <luca.fancellu@arm.com>
>>
>> HAS_VMAP is not enabled on MPU systems, but the vmap functions are used
> Just as a reminder, we don't intend to support VMAP on MPU? Asking because it
> would otherwise be a duplicate effort to implement only these two helpers.
>
>> in places across common code. In order to keep the existing code and
>> maintain correct functionality, implement the `vmap_contig` and `vunmap`
>> functions for MPU systems.
>>
>> Introduce a helper function `destroy_entire_xen_mapping` to aid with
>> unmapping an entire region when only the start address is known.
>>
>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
>> Signed-off-by: Harry Ramsey <harry.ramsey@arm.com>
>> ---
>> xen/arch/arm/include/asm/mpu/mm.h | 11 +++++
>> xen/arch/arm/mpu/mm.c | 67 +++++++++++++++++++++++++------
>> xen/arch/arm/mpu/vmap.c | 14 +++++--
>> 3 files changed, 77 insertions(+), 15 deletions(-)
>>
>> diff --git a/xen/arch/arm/include/asm/mpu/mm.h b/xen/arch/arm/include/asm/mpu/mm.h
>> index e1ded6521d..83ee0e59ca 100644
>> --- a/xen/arch/arm/include/asm/mpu/mm.h
>> +++ b/xen/arch/arm/include/asm/mpu/mm.h
>> @@ -111,6 +111,17 @@ pr_t pr_of_addr(paddr_t base, paddr_t limit, unsigned int flags);
>> int mpumap_contains_region(pr_t *table, uint8_t nr_regions, paddr_t base,
>> paddr_t limit, uint8_t *index);
>>
>> +
>> +/*
>> + * Destroys and frees (if reference count is 0) an entire xen mapping on MPU
>> + * systems where only the start address is known.
>> + *
>> + * @param s Start address of memory region to be destroyed.
>> + *
>> + * @return: 0 on success, negative on error.
>> + */
>> +int destroy_entire_xen_mapping(paddr_t s);
> NIT: I read it as all the mappings which is a bit misleading :)
> Maybe something like: destroy_mapping_containing or alike?
>
>> +
>> #endif /* __ARM_MPU_MM_H__ */
>>
>> /*
>> diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
>> index 687dec3bc6..29d8e7ff11 100644
>> --- a/xen/arch/arm/mpu/mm.c
>> +++ b/xen/arch/arm/mpu/mm.c
>> @@ -290,6 +290,35 @@ static void disable_mpu_region_from_index(uint8_t index)
>> write_protection_region(&xen_mpumap[index], index);
>> }
>>
>> +/*
>> + * Free a xen_mpumap entry given the index. A mpu region is actually disabled
>> + * when the refcount is 0 and the region type is MPUMAP_REGION_FOUND.
>> + *
>> + * @param idx Index of the mpumap entry.
>> + * @param region_found_type Either MPUMAP_REGION_FOUND or MPUMAP_REGION_INCLUSIVE.
> Both of these are unsigned, so why the parameter is int?
>
>> + * @return 0 on success, otherwise negative on error.
>> + */
>> +static int xen_mpumap_free_entry(uint8_t idx, int region_found_type)
>> +{
>> + ASSERT(spin_is_locked(&xen_mpumap_lock));
>> + ASSERT(idx != INVALID_REGION_IDX);
>> +
>> + if ( xen_mpumap[idx].refcount == 0 )
>> + {
>> + if ( MPUMAP_REGION_FOUND == region_found_type )
>> + disable_mpu_region_from_index(idx);
>> + else
>> + {
>> + printk(XENLOG_ERR "Cannot remove a partial region\n");
>> + return -EINVAL;
>> + }
>> + }
>> + else
>> + xen_mpumap[idx].refcount -= 1;
> You could avoid nesting if/else by doing:
> if ( xen_mpumap[idx].refcount )
> {
> xen_mpumap[idx].refcount -= 1;
> return 0;
> }
>
> if ( MPUMAP_REGION_FOUND == region_found_type )
> disable_mpu_region_from_index(idx);
> else
> {
> printk(XENLOG_ERR "Cannot remove a partial region\n");
> return -EINVAL;
> }
>
>> +
>> + return 0;
>> +}
>> +
>> /*
>> * Update the entry in the MPU memory region mapping table (xen_mpumap) for the
>> * given memory range and flags, creating one if none exists.
>> @@ -357,18 +386,7 @@ static int xen_mpumap_update_entry(paddr_t base, paddr_t limit,
>> return -EINVAL;
>> }
>>
>> - if ( xen_mpumap[idx].refcount == 0 )
>> - {
>> - if ( MPUMAP_REGION_FOUND == rc )
>> - disable_mpu_region_from_index(idx);
>> - else
>> - {
>> - printk("Cannot remove a partial region\n");
>> - return -EINVAL;
>> - }
>> - }
>> - else
>> - xen_mpumap[idx].refcount -= 1;
>> + return xen_mpumap_free_entry(idx, rc);
>> }
>>
>> return 0;
>> @@ -418,6 +436,31 @@ int destroy_xen_mappings(unsigned long s, unsigned long e)
>> return xen_mpumap_update(s, e, 0);
>> }
>>
>> +int destroy_entire_xen_mapping(paddr_t s)
>> +{
>> + int rc;
>> + uint8_t idx;
>> +
>> + ASSERT(IS_ALIGNED(s, PAGE_SIZE));
>> +
>> + spin_lock(&xen_mpumap_lock);
>> +
>> + rc = mpumap_contains_region(xen_mpumap, max_mpu_regions, s, s + PAGE_SIZE,
>> + &idx);
> So here you are searching for a region that includes at least s + PAGE_SIZE.
>
>> + if ( rc == MPUMAP_REGION_NOTFOUND )
>> + {
>> + printk(XENLOG_ERR "Cannot remove entry that does not exist");
>> + rc = -EINVAL;
> Here you assing rc but ...
>
>> + }
>> +
>> + /* As we are unmapping entire region use MPUMAP_REGION_FOUND instead */
>> + rc = xen_mpumap_free_entry(idx, MPUMAP_REGION_FOUND);
> here you would redefine it.
Sorry, this is a mistake and we should just return early as the mapping
doesn't exist and therefore will have no references to be decremented.
>> +
>> + spin_unlock(&xen_mpumap_lock);
>> +
>> + return rc;
>> +}
>> +
>> int map_pages_to_xen(unsigned long virt, mfn_t mfn, unsigned long nr_mfns,
>> unsigned int flags)
>> {
>> diff --git a/xen/arch/arm/mpu/vmap.c b/xen/arch/arm/mpu/vmap.c
>> index f977b79cd4..d3037ae98a 100644
>> --- a/xen/arch/arm/mpu/vmap.c
>> +++ b/xen/arch/arm/mpu/vmap.c
>> @@ -1,19 +1,27 @@
>> /* SPDX-License-Identifier: GPL-2.0-only */
>>
>> #include <xen/bug.h>
>> +#include <xen/mm.h>
>> #include <xen/mm-frame.h>
>> #include <xen/types.h>
>> #include <xen/vmap.h>
>>
>> void *vmap_contig(mfn_t mfn, unsigned int nr)
>> {
>> - BUG_ON("unimplemented");
>> - return NULL;
>> + paddr_t base = mfn_to_maddr(mfn);
>> +
>> + if ( map_pages_to_xen(base, mfn, nr, PAGE_HYPERVISOR ) )
>> + return NULL;
>> +
>> + return maddr_to_virt(base);
>> }
>>
>> void vunmap(const void *va)
>> {
>> - BUG_ON("unimplemented");
>> + paddr_t base = virt_to_maddr(va);
>> +
>> + if ( destroy_entire_xen_mapping(base) )
>> + panic("Failed to vunmap region\n");
> Looking at common vunmap() we ignore the return code from
> destroy_xen_mappings(). Is it ok to diverge?
>
> ~Michal
>
>> }
>>
>> /*
I will address the code style comments in the next version of these patches.
Thanks,
Harry Ramsey
Hi Michal,
thanks for your review, I’ll answer only few of your points and I’ll let Harry take the rest.
> On 8 Dec 2025, at 09:53, Orzel, Michal <Michal.Orzel@amd.com> wrote:
>
>
>
> On 28/11/2025 10:58, Harry Ramsey wrote:
>> From: Luca Fancellu <luca.fancellu@arm.com>
>>
>> HAS_VMAP is not enabled on MPU systems, but the vmap functions are used
> Just as a reminder, we don't intend to support VMAP on MPU? Asking because it
> would otherwise be a duplicate effort to implement only these two helpers.
Yes we are not going to support VMAP on MPU, here we want only to provide the
implementation of these two helper so that we don’t touch the common code using these.
Are you suggesting some rewording or was it only a curiosity on your side?
>>
>> +/*
>> + * Free a xen_mpumap entry given the index. A mpu region is actually disabled
>> + * when the refcount is 0 and the region type is MPUMAP_REGION_FOUND.
>> + *
>> + * @param idx Index of the mpumap entry.
>> + * @param region_found_type Either MPUMAP_REGION_FOUND or MPUMAP_REGION_INCLUSIVE.
> Both of these are unsigned, so why the parameter is int?
Uhm, good catch, I think this is a documentation issue because it might be also MPUMAP_REGION_OVERLAP which is
-1, we should not mandate which value might be here, we should only say MPUMAP_REGION_* values.
>>
>>
>> void vunmap(const void *va)
>> {
>> - BUG_ON("unimplemented");
>> + paddr_t base = virt_to_maddr(va);
>> +
>> + if ( destroy_entire_xen_mapping(base) )
>> + panic("Failed to vunmap region\n");
> Looking at common vunmap() we ignore the return code from
> destroy_xen_mappings(). Is it ok to diverge?
In our tests it’s ok, I asked Harry to have this so that we can catch any vunmap that is not balanced
with a prior mapping. To be fair I’m not really sure why in the vmap.c implementation we are not
reading the error codes from destroy_xen_mappings.
Cheers,
Luca
On 08/12/2025 11:20, Luca Fancellu wrote:
> Hi Michal,
>
> thanks for your review, I’ll answer only few of your points and I’ll let Harry take the rest.
>
>> On 8 Dec 2025, at 09:53, Orzel, Michal <Michal.Orzel@amd.com> wrote:
>>
>>
>>
>> On 28/11/2025 10:58, Harry Ramsey wrote:
>>> From: Luca Fancellu <luca.fancellu@arm.com>
>>>
>>> HAS_VMAP is not enabled on MPU systems, but the vmap functions are used
>> Just as a reminder, we don't intend to support VMAP on MPU? Asking because it
>> would otherwise be a duplicate effort to implement only these two helpers.
>
> Yes we are not going to support VMAP on MPU, here we want only to provide the
> implementation of these two helper so that we don’t touch the common code using these.
> Are you suggesting some rewording or was it only a curiosity on your side?
It was a question to check whether we are aligned.
>
>>>
>>> +/*
>>> + * Free a xen_mpumap entry given the index. A mpu region is actually disabled
>>> + * when the refcount is 0 and the region type is MPUMAP_REGION_FOUND.
>>> + *
>>> + * @param idx Index of the mpumap entry.
>>> + * @param region_found_type Either MPUMAP_REGION_FOUND or MPUMAP_REGION_INCLUSIVE.
>> Both of these are unsigned, so why the parameter is int?
>
> Uhm, good catch, I think this is a documentation issue because it might be also MPUMAP_REGION_OVERLAP which is
> -1, we should not mandate which value might be here, we should only say MPUMAP_REGION_* values.
>
>
>>>
>>>
>>> void vunmap(const void *va)
>>> {
>>> - BUG_ON("unimplemented");
>>> + paddr_t base = virt_to_maddr(va);
>>> +
>>> + if ( destroy_entire_xen_mapping(base) )
>>> + panic("Failed to vunmap region\n");
>> Looking at common vunmap() we ignore the return code from
>> destroy_xen_mappings(). Is it ok to diverge?
>
> In our tests it’s ok, I asked Harry to have this so that we can catch any vunmap that is not balanced
> with a prior mapping. To be fair I’m not really sure why in the vmap.c implementation we are not
> reading the error codes from destroy_xen_mappings.
I'm ok with that. I don't know why we don't check the rc there. If you look at
x86 destroy_xen_mappings calls, none seem to be checked against rc. It can be
that it's impossible scenario there.
~Michal
>
> Cheers,
> Luca
>
© 2016 - 2025 Red Hat, Inc.