Hi Michal,

>> 
>>> 
>>>> +}
>>>> +
>>>> +/* Set limit address of MPU protection region(pr_t). */
>>>> +static inline void pr_set_limit(pr_t *pr, paddr_t limit)
>>>> +{
>>>> +    pr->prlar.reg.limit = ((limit - 1) >> MPU_REGION_SHIFT);
>>> Why -1? AFAIR these registers take inclusive addresses, so is it because you
>>> want caller to pass limit as exclusive and you convert it to inclusive? I 
>>> think
>>> it's quite error prone.
>> 
>> Yes it’s meant to be used with exclusive range, shall we document it or use
>> Inclusive range instead?
> What's the expected behavior of callers? Are we going to set up protection
> region based on regular start+size pair (like with MMU) or start+end? If the
> latter for some reason (with size there are less issues), then end usually is
> inclusive and you would not need conversion.

I think we have a mix because for example destroy_xen_mappings and 
modify_xen_mappings
are start and end, map_pages_to_xen instead is kind of start+size?

I moved the -1 inside pr_set_limit because it was open-coded in all the places, 
for example when
referencing linker symbols or output of mfn_to_maddr(mfn_add(smfn, nr_mfn)), 
for this reason I
thought: let’s call this one with exclusive ranges and modify internally for 
inclusive.


Reply via email to