Re: [PATCH v3 8/8] xen/dom0less: store xenstore event channel in page

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.08.2025 15:19, Jason Andryuk wrote:
> On 2025-08-27 03:58, Jan Beulich wrote:
>> On 26.08.2025 23:08, Jason Andryuk wrote:
>>> --- a/xen/common/device-tree/dom0less-build.c
>>> +++ b/xen/common/device-tree/dom0less-build.c
>>> @@ -26,6 +26,7 @@
>>>   #include <public/event_channel.h>
>>>   #include <public/io/xs_wire.h>
>>>   
>>> +#include <asm/guest_access.h>
>>>   #include <asm/setup.h>
>>>   
>>>   #include <xen/static-memory.h>
>>> @@ -120,8 +121,14 @@ static void __init initialize_domU_xenstore(void)
>>>   
>>>           if ( gfn != XENSTORE_PFN_LATE_ALLOC && 
>>> IS_ENABLED(CONFIG_GRANT_TABLE) )
>>>           {
>>> +            evtchn_port_t port = 
>>> d->arch.hvm.params[HVM_PARAM_STORE_EVTCHN];
>>> +            paddr_t evtchn_gaddr = gfn_to_gaddr(_gfn(gfn)) +
>>> +                offsetof(struct xenstore_domain_interface, evtchn_port);
>>> +
>>>               ASSERT(gfn < UINT32_MAX);
>>>               gnttab_seed_entry(d, GNTTAB_RESERVED_XENSTORE, xs_domid, gfn);
>>> +            access_guest_memory_by_gpa(d, evtchn_gaddr, &port, 
>>> sizeof(port),
>>> +                                       true /* is_write */);
>>
>> Isn't the use of an arch-specific function going to pose yet another issue
>> for making this code usable on x86? Can't you use copy_to_guest_phys() here?
>> Which may in turn need to be passed in by the caller, see e.g. dtb_load()
>> and initrd_load() (i.e. cache flushing may also be necessary for Arm).
> 
> Yes, that could be done, but it's not my preferred approach.  Using a 
> function pointer to pass a compile time constant seems to me like a 
> misuse of a function pointer.
> 
> I'd rather each arch using dom0less define:
> unsigned long copy_to_guest_phys(struct domain *d,
>                                   paddr_t gpa,
>                                   void *buf,
>                                   unsigned int len);
> 
> Which does the correct thing for the arch.

That would be even better, just that I don't know whether Arm folks would
like it.

Jan

> Alejandro was able to re-work things to re-use the dom0less parsing code 
> (dom0less-bindings.c), but he has so far kept the x86 domain 
> construction separate such that it does not use dom0less-build.c.  So I 
> don't know how that will shake out.
> 
> But, yeah, I can just pass in a function pointer if that is what is 
> agreed upon.
> 
> Regards,
> Jason