Re: [PATCH v2 1/3] xen/riscv: introduce setup_mm()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 31.10.2024 14:19, oleksii.kurochko@xxxxxxxxx wrote:
>>>>> +void __init setup_mm(void)
>>>>> +{
>>>>> +    const struct membanks *banks = bootinfo_get_mem();
>>>>> +    paddr_t ram_start = INVALID_PADDR;
>>>>> +    paddr_t ram_end = 0;
>>>>> +    paddr_t ram_size = 0;
>>>>> +    unsigned int i;
>>>>> +
>>>>> +    /*
>>>>> +     * We need some memory to allocate the page-tables used
>>>>> for
>>>>> the directmap
>>>>> +     * mappings. But some regions may contain memory already
>>>>> allocated
>>>>> +     * for other uses (e.g. modules, reserved-memory...).
>>>>> +     *
>>>>> +     * For simplicity, add all the free regions in the boot
>>>>> allocator.
>>>>> +     */
>>>>> +    populate_boot_allocator();
>>>>> +
>>>>> +    total_pages = 0;
>>>>> +
>>>>> +    for ( i = 0; i < banks->nr_banks; i++ )
>>>>> +    {
>>>>> +        const struct membank *bank = &banks->bank[i];
>>>>> +        paddr_t bank_end = bank->start + bank->size;
>>>>> +
>>>>> +        ram_size += ROUNDDOWN(bank->size, PAGE_SIZE);
>>>>
>>>> As before - if a bank doesn't cover full pages, this may give the
>>>> impression
>>>> of there being more "total pages" than there are.
>>> Since it rounds down to PAGE_SIZE, if ram_start is 2K and the total
>>> size of a bank is 11K, ram_size will end up being 8K, so the "total
>>> pages" will cover less RAM than the actual size of the RAM bank.
>>
>> ram_start at 2k but bank size being 13k would yield 2 usable pages
>> (first partial page of 2k unusable and last partial page of 3k
>> unusable), yet ram_size of 12k (3 pages). You need to consider the
>> common case; checking things work for a randomly chosen example isn't
>> enough.
> Then I have to check separately the start and end of bank and check if
> ram_size should be reduced in case if the start or end isn't properly
> aligned.

All I think you need to do is align bank start up to a page boundary and
align bank end down to a page boundary.

Jan