Re: [PATCH v6 11/19] xen/riscv: implement p2m_set_range()

[Oleksii Kurochko <oleksii.kurochko@gmail.com>]

On 12/8/25 5:52 PM, Jan Beulich wrote:
> On 24.11.2025 13:33, Oleksii Kurochko wrote:
>> @@ -28,6 +36,77 @@ unsigned char get_max_supported_mode(void)
>>       return max_gstage_mode.mode;
>>   }
>>   
>> +static inline unsigned int calc_offset(const struct p2m_domain *p2m,
>> +                                       const unsigned int lvl,
>> +                                       const paddr_t gpa)
>> +{
>> +    unsigned int off = (gpa >> P2M_GFN_LEVEL_SHIFT(lvl)) &
>> +                       P2M_TABLE_OFFSET(p2m, lvl);
>> +
>> +    /*
>> +     * For P2M_ROOT_LEVEL, `offset` ranges from 0 to 2047, since the root
>> +     * page table spans 4 consecutive 4KB pages.
>> +     * We want to return an index within one of these 4 pages.
>> +     * The specific page to use is determined by `p2m_get_root_pointer()`.
>> +     *
>> +     * Example: if `offset == 512`:
>> +     *  - A single 4KB page holds 512 entries.
>> +     *  - Therefore, entry 512 corresponds to index 0 of the second page.
>> +     *
>> +     * At all other levels, only one page is allocated, and `offset` is
>> +     * always in the range 0 to 511, since the VPN is 9 bits long.
>> +     */
>> +    return off & (PAGETABLE_ENTRIES - 1);
>> +}
>> +
>> +#define P2M_MAX_ROOT_LEVEL 5
>> +
>> +#define P2M_BUILD_LEVEL_OFFSETS(p2m, var, addr) \
>> +    unsigned int var[P2M_MAX_ROOT_LEVEL] = {-1}; \
> What use is this initializer? Slot 0 ...

I wanted a way to detect if something mistakenly tries to access an uninitialized
array element, and using -1 would help distinguish whether an element was truly
uninitialized or if calc_offset() simply returned 0. However, you’re right, it
doesn’t work because only Slot 0 is initialized with -1. So...

>
>> +    BUG_ON(P2M_ROOT_LEVEL(p2m) >= P2M_MAX_ROOT_LEVEL); \
>> +    for ( unsigned int i = 0; i <= P2M_ROOT_LEVEL(p2m); i++ ) \
>> +        var[i] = calc_offset(p2m, i, addr);
> ... is guaranteed to be written to, afaics. With this simplified (or an
> explanation given for why it is needed)

... I will just drop an initializer and let var[] to be initialized with 0s.

> Acked-by: Jan Beulich <jbeulich@suse.com>

Thanks.

~ Oleksii