Re: [PATCH v3 6/7] xen: introduce Kconfig ARCH_PAGING_MEMPOOL

["Orzel, Michal" <michal.orzel@xxxxxxx>]

On 26/03/2025 10:54, Jan Beulich wrote:
> 
> 
> On 26.03.2025 10:39, Orzel, Michal wrote:
>>
>>
>> On 20/03/2025 08:32, Jan Beulich wrote:
>>>
>>>
>>> On 19.03.2025 17:31, Oleksii Kurochko wrote:
>>>>
>>>> On 3/19/25 12:35 PM, Jan Beulich wrote:
>>>>> On 18.03.2025 14:05, Oleksii Kurochko wrote:
>>>>>> On 3/17/25 9:07 PM, Luca Fancellu wrote:
>>>>>>> From: Penny Zheng<Penny.Zheng@xxxxxxx>
>>>>>>>
>>>>>>> ARM MPU system doesn't need to use paging memory pool, as MPU memory
>>>>>>> mapping table at most takes only one 4KB page, which is enough to
>>>>>>> manage the maximum 255 MPU memory regions, for all EL2 stage 1
>>>>>>> translation and EL1 stage 2 translation.
>>>>>>>
>>>>>>> Introduce ARCH_PAGING_MEMPOOL Kconfig common symbol, selected for Arm
>>>>>>> MMU systems, x86 and RISC-V.
>>>>>>>
>>>>>>> Wrap the code inside 'construct_domU' that deal with p2m paging
>>>>>>> allocation in a new function 'domain_p2m_set_allocation', protected
>>>>>>> by ARCH_PAGING_MEMPOOL, this is done in this way to prevent polluting
>>>>>>> the former function with #ifdefs and improve readability
>>>>>>>
>>>>>>> Introduce arch_{get,set}_paging_mempool_size stubs for architecture
>>>>>>> with !ARCH_PAGING_MEMPOOL.
>>>>>>>
>>>>>>> Remove 'struct paging_domain' from Arm 'struct arch_domain' when the
>>>>>>> field is not required.
>>>>>>>
>>>>>>> Signed-off-by: Penny Zheng<penny.zheng@xxxxxxx>
>>>>>>> Signed-off-by: Wei Chen<wei.chen@xxxxxxx>
>>>>>>> Signed-off-by: Luca Fancellu<luca.fancellu@xxxxxxx>
>>>>>>> ---
>>>>>>> v3 changes:
>>>>>>>    - Introduced ARCH_PAGING_MEMPOOL instead of HAS_PAGING_MEMPOOL
>>>>>>> v2 changes:
>>>>>>>    - make Kconfig HAS_PAGING_MEMPOOL common
>>>>>>>    - protect also "xen,domain-p2m-mem-mb" reading with 
>>>>>>> HAS_PAGING_MEMPOOL
>>>>>>>    - do not define p2m_teardown{_allocation} in this patch
>>>>>>>    - change commit message
>>>>>>> ---
>>>>>>>    xen/arch/arm/Kconfig              |  1 +
>>>>>>>    xen/arch/arm/dom0less-build.c     | 74 
>>>>>>> ++++++++++++++++++++-----------
>>>>>>>    xen/arch/arm/include/asm/domain.h |  2 +
>>>>>>>    xen/arch/riscv/Kconfig            |  1 +
>>>>>>>    xen/arch/x86/Kconfig              |  1 +
>>>>>>>    xen/common/Kconfig                |  3 ++
>>>>>>>    xen/include/xen/domain.h          | 17 +++++++
>>>>>>>    7 files changed, 73 insertions(+), 26 deletions(-)
>>>>>> For RISC-V:
>>>>>>    Reviewed-by: Oleksii Kurochko<oleksii.kurochko@xxxxxxxxx>
>>>>> Mind me asking then why RISC-V needs this at this point? The stubs surely
>>>>> were added to address some build issue, not because they are actively
>>>>> meaningful?
>>>>
>>>> Only because we have stubs and not to have redefinition compilation
>>>> error. And, yes, they are not actively meaningful now, at least. I am
>>>> okay with not enabling of this config for RISC-V but then seems to me we
>>>> have to drop stubs in riscv/stubs.c. ~ Oleksii
>>>
>>> Well, I don't think it's "have to", but I agree that dropping them would
>>> make sense then (and be desirable).
>> @Jan, @Oleksii, is there anything blocking this patch to be committed (Luca
>> question does not seem to be answered)? Other patches in the series are 
>> ready to
>> be merged.
> 
> While I think Oleksii's reply has sufficiently clarified things, I still 
> wonder:
> What question of Luca are you referring to? There's none visible in context 
> here
> afaics. The two questions that are visible were raised by me, and answered by
> Oleksii (also visible in context above). There was an implication to draw from
> that, yes, which Oleksii has now made explicit in his reply to your mail.
Sure thing. I'll clarify. Here Luca asks question to you if it is possible for
RISCV guys to handle it afterwards:
https://lore.kernel.org/xen-devel/D86D58F5-1EDD-4362-B163-5AD25C5DCC51@xxxxxxx/
https://lore.kernel.org/xen-devel/F827A271-0E9B-4240-BB1E-C039E9EE5656@xxxxxxx/
and you did not answer to these e-mails.

> 
> And then, formally this still didn't have a REST ack (you limited your R-b to
> Arm) nor an x86 one.
That's why I asked: "is there anything blocking". Missing REST ack fits into my
query.

~Michal