Write the associated event channel into the xenstore page so xenstored
can read it. xenstored can map the grant by the reserved grant table
entry, and then read out the event channel and bind it. This eliminates
the need for an additional mechanism to discover the event channel.
Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
v2:
No change
This should go in after the init-dom0less changes so init-dom0less is
ready for xenstored automatically introducing domains.
I'm looking for feedback. This is ARM-only for the time being, but that
is the only in-tree user of this code. From the perspective, it is okay
to go in.
If we want a cross-arch approach, a common function to write to guest
physical addresses would be needed for additional arches, but they
aren't available yet.
Oleksii added a function pointer to dtb_load() and initrd_load() when
moving dom0less to common, but I think that isn't necessary. Just
having a common helper would be sufficient.
copy_to_guest_phys() or something_copy_to_guest_phys() could be defined
or a wrapper for ARM's copy_to_guest_phys_flush_dcache(). Other arches
could need to implement it when using dom0less.
I'm not an ARM expert, but Stefano said
copy_to_guest_phys_flush_dcache() is not necessary since this xenstore
page isn't expected to be accessed without caches enabled.
---
xen/common/device-tree/dom0less-build.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
index badc227031..1a40f68837 100644
--- 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 */);
}
}
}
--
2.50.1On 26/08/2025 23:08, Jason Andryuk wrote: > Write the associated event channel into the xenstore page so xenstored > can read it. xenstored can map the grant by the reserved grant table > entry, and then read out the event channel and bind it. This eliminates > the need for an additional mechanism to discover the event channel. > > Signed-off-by: Jason Andryuk <jason.andryuk@amd.com> > --- > v2: > No change > > This should go in after the init-dom0less changes so init-dom0less is > ready for xenstored automatically introducing domains. > > I'm looking for feedback. This is ARM-only for the time being, but that > is the only in-tree user of this code. From the perspective, it is okay > to go in. > > If we want a cross-arch approach, a common function to write to guest > physical addresses would be needed for additional arches, but they > aren't available yet. > > Oleksii added a function pointer to dtb_load() and initrd_load() when > moving dom0less to common, but I think that isn't necessary. Just > having a common helper would be sufficient. > > copy_to_guest_phys() or something_copy_to_guest_phys() could be defined > or a wrapper for ARM's copy_to_guest_phys_flush_dcache(). Other arches > could need to implement it when using dom0less. > > I'm not an ARM expert, but Stefano said > copy_to_guest_phys_flush_dcache() is not necessary since this xenstore > page isn't expected to be accessed without caches enabled. I'm not sure I understand this point. When copying data *to* the guest, cleaning is about Xen's cache, not guest's... ~Michal
On 2025-08-27 04:03, Orzel, Michal wrote: > > > On 26/08/2025 23:08, Jason Andryuk wrote: >> Write the associated event channel into the xenstore page so xenstored >> can read it. xenstored can map the grant by the reserved grant table >> entry, and then read out the event channel and bind it. This eliminates >> the need for an additional mechanism to discover the event channel. >> >> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com> >> --- >> v2: >> No change >> >> This should go in after the init-dom0less changes so init-dom0less is >> ready for xenstored automatically introducing domains. >> >> I'm looking for feedback. This is ARM-only for the time being, but that >> is the only in-tree user of this code. From the perspective, it is okay >> to go in. >> >> If we want a cross-arch approach, a common function to write to guest >> physical addresses would be needed for additional arches, but they >> aren't available yet. >> >> Oleksii added a function pointer to dtb_load() and initrd_load() when >> moving dom0less to common, but I think that isn't necessary. Just >> having a common helper would be sufficient. >> >> copy_to_guest_phys() or something_copy_to_guest_phys() could be defined >> or a wrapper for ARM's copy_to_guest_phys_flush_dcache(). Other arches >> could need to implement it when using dom0less. >> >> I'm not an ARM expert, but Stefano said >> copy_to_guest_phys_flush_dcache() is not necessary since this xenstore >> page isn't expected to be accessed without caches enabled. > I'm not sure I understand this point. When copying data *to* the guest, cleaning > is about Xen's cache, not guest's... I was trying to highlight that the patch is using access_guest_memory_by_gpa(), but dtb_load() and initrd_load() use copy_to_guest_phys_flush_dcache(). I assumed from the name, and Stefano's comment, that copy_to_guest_phys_flush_dcache() was about ensuring the CPU's cache is flushed to RAM. That way a guest starting with cache disabled would see the correct contents. But I don't really know how it works and may be wrong. Thanks, Jason
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).
Jan
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.
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
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
© 2016 - 2025 Red Hat, Inc.