On 15/07/2025 11:48, Hari Limaye wrote:
> Hi Michal,
> 
>> On 15 Jul 2025, at 09:45, Orzel, Michal <michal.or...@amd.com> wrote:
>>
>>
>>
>> On 15/07/2025 10:36, Hari Limaye wrote:
>>> Hi Michal,
>>>
>>>>> +int mpumap_contains_region(pr_t *table, uint8_t nr_regions, paddr_t base,
>>>>> +                           paddr_t limit, uint8_t *index)
>>>>> +{
>>>>> +    ASSERT(index);
>>>>> +    *index = INVALID_REGION_IDX;
>>>>> +
>>>>> +    /*
>>>>> +     * The caller supplies a half-open interval [base, limit), i.e. 
>>>>> limit is the
>>>>> +     * first byte *after* the region. Require limit strictly greater 
>>>>> than base,
>>>>> +     * which is necessarily a non-empty region.
>>>>> +     */
>>>>> +    ASSERT(base < limit);
>>>> Well, that does not guarantee a non-empty region.
>>>> Consider passing [x, x+1). The assert will pass, even though the region is 
>>>> empty.
>>>>
>>>> ~Michal
>>>>
>>>
>>> Apologies, I may well be missing something here! Please could you suggest a 
>>> code snippet to understand your expectation here / what you would prefer 
>>> the assert to be?
>>>
>>> As I understand it, with a half-open interval [base, limit) as is passed to 
>>> this function, the size is  `limit - base` and so the region [x, x+1) will 
>>> have size 1. The empty region starting at the same address would be [x, x). 
>>> But perhaps I am making the off-by-one error here.
>> Hmm, I think I made a mistake here. Region of size 1B would have base == 
>> limit
>> in registers. All good then.
>>
>> ~Michal
>>
> 
> Thanks for double checking. I notice you did not add your tag here, I wanted 
> to check if you think this patch is reviewed from your perspective?
Yes.

Reviewed-by: Michal Orzel <michal.or...@amd.com>

~Michal


Reply via email to