Re: [PATCH v2 2/6] arm/mpu: Implement vmap functions for MPU

[Harry Ramsey <harry.ramsey@xxxxxxx>]

On 12/01/2026 10:33, Harry Ramsey wrote:
> On 09/01/2026 14:00, Orzel, Michal wrote:
> > On 05/01/2026 12:34, Harry Ramsey wrote:
> > > +        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 +393,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 +443,31 @@ int destroy_xen_mappings(unsigned long s, 
> > > unsigned long e)
> > >       return xen_mpumap_update(s, e, 0);
> > >   }
> > >   +int destroy_xen_mapping_containing(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");
> > Why do we split sanity checking between this and xen_mpumap_free_entry?
> > What are the possible region types that xen_mpumap_free_entry is 
> > expected to
> > work with? I thought that it should only be MPUMAP_REGION_FOUND.
> I will move the region checks to xen_mpumap_free_entry since we only 
> want xen_mpumap_update_entry and xen_mpumap_free_entry to modify our 
> refcount properties. These functions should then account for all 
> potential values of MPUMAP_REGION_*.
Sorry, after looking back at this code the reason the sanity checking 
happens here is because we require MPUMAP_REGION_FOUND to be used in 
`destroy_xen_mapping_containing` to actually remove the region. In 
`destroy_xen_mapping_containing` we are forcibly setting the region type 
to be MPUMAP_REGION_FOUND to ensure it is removed and thus we require 
the check here too.

Thanks,
Harry Ramsey.