Also, two more things here apart from my other remarks.
On 05/01/2026 12:34, Harry Ramsey wrote:
> From: Luca Fancellu <[email protected]>
>
> 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_xen_mapping_containing` to aid with
> unmapping an entire region when only the start address is known.
>
> Signed-off-by: Luca Fancellu <[email protected]>
> Signed-off-by: Harry Ramsey <[email protected]>
> ---
> v2:
> - Rename `destroy_entire_xen_mapping` to `destroy_xen_mapping_containing`
> - Improve code documentation.
> - Refactor nested code.
> - Fix ignored rc error code in `destroy_xen_mapping_containing`.
> ---
...
>
> +int destroy_xen_mapping_containing(paddr_t s)
> +{
> + int rc;
> + uint8_t idx;
> +
> + ASSERT(IS_ALIGNED(s, PAGE_SIZE));
> +
> + spin_lock(&xen_mpumap_lock);
Here you take a 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");
> + return -EINVAL;
and here you would return while still holding the lock i.e. deadlock.
> + }
> +
> + /* As we are unmapping entire region use MPUMAP_REGION_FOUND instead */
> + rc = xen_mpumap_free_entry(idx, MPUMAP_REGION_FOUND);
Why don't you perform context_sync_mpu() here?
~Michal
