Re: [PATCH v3 3/5] xen/ppc: Define minimal stub headers required for full build

[Jan Beulich <jbeulich@xxxxxxxx>]

On 09.09.2023 00:05, Shawn Anastasio wrote:
> On 9/5/23 10:52 AM, Jan Beulich wrote:
>> On 01.09.2023 20:25, Shawn Anastasio wrote:
>>> @@ -13,6 +28,237 @@
>>>  #define __pa(x)             (virt_to_maddr(x))
>>>  #define __va(x)             (maddr_to_virt(x))
>>>
>>> -void setup_initial_pagetables(void);
>>> +/* Convert between Xen-heap virtual addresses and machine frame numbers. */
>>> +#define __virt_to_mfn(va) (virt_to_maddr(va) >> PAGE_SHIFT)
>>> +#define __mfn_to_virt(mfn) (maddr_to_virt((paddr_t)(mfn) << PAGE_SHIFT))
>>> +
>>> +/* Convert between Xen-heap virtual addresses and page-info structures. */
>>> +static inline struct page_info *virt_to_page(const void *v)
>>> +{
>>> +    BUG_ON("unimplemented");
>>> +    return NULL;
>>> +}
>>> +
>>> +/*
>>> + * We define non-underscored wrappers for above conversion functions.
>>> + * These are overriden in various source files while underscored version
>>> + * remain intact.
>>> + */
>>> +#define virt_to_mfn(va)     __virt_to_mfn(va)
>>> +#define mfn_to_virt(mfn)    __mfn_to_virt(mfn)
>>
>> Is the comment really applicable? The only non-arch-specific file still
>> doing so is xenoprof.c, and I'm not sure you mean to support xenoprof
>> on PPC. PPC-specific files would better be introduced in a type-safe way
>> right from the beginning.
>>
> 
> To be clear, you're suggesting that I define virt_to_mfn and mfn_to_virt
> to operate on the type-safe mfn_t wrapper? When doing this,
> xen/include/xen/domain_page.h:63 fails to build, since it seems to
> assume that these macros do not use the type-safe mfn_t:
> 
>   static inline void *map_domain_page_global(mfn_t mfn)
>   {
>       return mfn_to_virt(mfn_x(mfn));
>   }

Oh, I see. That's a result of (at least) 41c48004d1d8 ("xen/mm: Use
__virt_to_mfn in map_domain_page instead of virt_to_mfn") not going far
enough and f46b6197344f ("xen: Convert page_to_mfn and mfn_to_page to
use typesafe MFN") not properly doing the intended conversion. Let me
(try to) make a patch, once again ...

> If the comment is no longer accurate I can drop it, but there definitely
> seems to be an assumption in the codebase that these macros operate on
> raw unsigned longs rather than mfn_t.

Well, the comment is new (in PPC), so it cannot really be "no longer
accurate". It (sadly) still is accurate in Arm, but I was hoping we
would be able to do better in new ports.

Jan