On 05.02.2025 17:55, Oleksii Kurochko wrote:
> On 2/4/25 2:50 PM, Jan Beulich wrote:
>> On 03.02.2025 14:12, Oleksii Kurochko wrote:
>>> --- a/xen/arch/riscv/include/asm/mm.h
>>> +++ b/xen/arch/riscv/include/asm/mm.h
>>> @@ -156,6 +156,10 @@ static inline struct page_info *virt_to_page(const
>>> void *v)
>>> return frametable_virt_start + PFN_DOWN(va - directmap_virt_start);
>>> }
>>>
>>> +#include <asm/page.h>
>> asm/page.h already includes asm/mm.h, so you're introducing a circular
>> dependency here (much of which the patch description is about, so it's
>> unclear why you didn't solve this another way). Afaict ...
>>
>>> +pte_t * pt_walk(vaddr_t va, unsigned int *pte_level);
>> ... it's pte_t that presents a problem here. Why not simply put the
>> declaration in asm/page.h (and drop all the secondary changes that
>> don't really belong in this patch anyway)?
>
> In the patch 2 it is used for implementing vmap_to_mfn():
>
> static inline mfn_t vmap_to_mfn_(vaddr_t va)
> {
> pte_t *entry = pt_walk(va, NULL);
>
> BUG_ON(!pte_is_mapping(*entry));
>
> return mfn_from_pte(*entry);
> }
>
> #define vmap_to_mfn(va) vmap_to_mfn_((vaddr_t)va)mfn_from_pte
>
> what leads to including of <asm/page.h> in <asm/mm.h>.
>
> As an option, if to move the following to <asm/page.h>:
> #define vmap_to_mfn(va) vmap_to_mfn_((vaddr_t)va)
> #define vmap_to_page(va) mfn_to_page(vmap_to_mfn(va))
> the circular dependency could be dropped.
I wouldn't like that, but it's an option, yes. Alternatives I can think of
right away:
- put the helper function an asm/page.h, but keep the macros where they are,
- convert the helper function to a helper macro.
Jan
