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 <[email protected]> wrote: > > > > On 28/11/2025 10:58, Harry Ramsey wrote: >> From: Luca Fancellu <[email protected]> >> >> 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
