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