[PATCH v3 0/8] init-dom0less and dom0less xenstore evtchn

[PATCH v3 0/8] init-dom0less and dom0less xenstore evtchn

[Jason Andryuk]

These are the init-dom0less and dom0less xenstore evtchn changes split
from https://lore.kernel.org/xen-devel/20250716211504.291104-1-jason.andryuk@amd.com/

The xenstored changes have already gone in.

init-dom0less needs to be changed to handle auto-introduced domains,
which this series does.  Once that is done, dom0less code can populate
the event channel in the xenstore page (which triggers the
auto-population).  The previous posting's ordering of "xen/dom0less:
store xenstore event channel in page" before init-dom0less changes would
have been broken.

Jason Andryuk (8):
  tools/init-dom0less: Make handles global
  tools/init-dom0less: Factor out xenstore setup
  tools/init-dom0less: Only introduce un-introduced domains
  tools/init-dom0less: Switch domain_exists to check xenstore name
  tools/init-dom0less: Use introduced to determine no-enhanced
  tools/init-dom0less: Remove use of err()
  tools/init-dom0less: Continue on error
  xen/dom0less: store xenstore event channel in page

 tools/helpers/init-dom0less.c           | 127 ++++++++++++++++--------
 xen/common/device-tree/dom0less-build.c |   7 ++
 2 files changed, 95 insertions(+), 39 deletions(-)

-- 
2.50.1

[PATCH v3 1/8] tools/init-dom0less: Make handles global

[Jason Andryuk]

init-dom0less passes the assorted xen library handles from main
downward.  This is a little excessive in places with 4 handles passed
into configure_xenstore().

Replace the handle passing with file-scoped variables.

The xenstore helpers are not changed.  This keeps their implementation
common with the libxenstore functions that take a handle, transation,
and then additional arguments.

Suggested-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
v3:
New
---
 tools/helpers/init-dom0less.c | 37 +++++++++++++++--------------------
 1 file changed, 16 insertions(+), 21 deletions(-)

diff --git a/tools/helpers/init-dom0less.c b/tools/helpers/init-dom0less.c
index a182dce563..0b0be08449 100644
--- a/tools/helpers/init-dom0less.c
+++ b/tools/helpers/init-dom0less.c
@@ -18,8 +18,12 @@
 #define XENSTORE_PFN_OFFSET 1
 #define STR_MAX_LENGTH 128
 
-static int alloc_xs_page(struct xc_interface_core *xch,
-                         libxl_dominfo *info,
+static libxl_ctx *ctx;
+static struct xs_handle *xsh;
+static struct xc_interface_core *xch;
+static xenforeignmemory_handle *xfh;
+
+static int alloc_xs_page(libxl_dominfo *info,
                          uint64_t *xenstore_pfn)
 {
     int rc;
@@ -43,8 +47,7 @@ static int alloc_xs_page(struct xc_interface_core *xch,
     return 0;
 }
 
-static int get_xs_page(struct xc_interface_core *xch, libxl_dominfo *info,
-                       uint64_t *xenstore_pfn)
+static int get_xs_page(libxl_dominfo *info, uint64_t *xenstore_pfn)
 {
     int rc;
 
@@ -111,8 +114,7 @@ static bool do_xs_write_vm(struct xs_handle *xsh, xs_transaction_t t,
  * The list was retrieved by running xenstore-ls on a corresponding
  * domain started by xl/libxl.
  */
-static int create_xenstore(struct xs_handle *xsh,
-                           libxl_dominfo *info, libxl_uuid uuid,
+static int create_xenstore(libxl_dominfo *info, libxl_uuid uuid,
                            uint64_t xenstore_pfn,
                            evtchn_port_t xenstore_port)
 {
@@ -235,10 +237,7 @@ err:
     return rc;
 }
 
-static int init_domain(struct xs_handle *xsh,
-                       struct xc_interface_core *xch,
-                       xenforeignmemory_handle *xfh,
-                       libxl_dominfo *info)
+static int init_domain(libxl_dominfo *info)
 {
     libxl_uuid uuid;
     uint64_t xenstore_evtchn, xenstore_pfn;
@@ -258,13 +257,13 @@ static int init_domain(struct xs_handle *xsh,
         return 0;
 
     /* Get xenstore page */
-    if (get_xs_page(xch, info, &xenstore_pfn) != 0)
+    if (get_xs_page(info, &xenstore_pfn) != 0)
         return 1;
 
     if (xenstore_pfn == ~0ULL) {
         struct xenstore_domain_interface *intf;
 
-        rc = alloc_xs_page(xch, info, &xenstore_pfn);
+        rc = alloc_xs_page(info, &xenstore_pfn);
         if (rc != 0) {
             printf("Error on getting xenstore page\n");
             return 1;
@@ -299,7 +298,7 @@ static int init_domain(struct xs_handle *xsh,
     if (rc)
         err(1, "gen_stub_json_config");
 
-    rc = create_xenstore(xsh, info, uuid, xenstore_pfn, xenstore_evtchn);
+    rc = create_xenstore(info, uuid, xenstore_pfn, xenstore_evtchn);
     if (rc)
         err(1, "writing to xenstore");
 
@@ -310,7 +309,7 @@ static int init_domain(struct xs_handle *xsh,
 }
 
 /* Check if domain has been configured in XS */
-static bool domain_exists(struct xs_handle *xsh, int domid)
+static bool domain_exists(int domid)
 {
     return xs_is_domain_introduced(xsh, domid);
 }
@@ -318,11 +317,7 @@ static bool domain_exists(struct xs_handle *xsh, int domid)
 int main(int argc, char **argv)
 {
     libxl_dominfo *info = NULL;
-    libxl_ctx *ctx;
     int nb_vm = 0, rc = 0, i;
-    struct xs_handle *xsh = NULL;
-    struct xc_interface_core *xch = NULL;
-    xenforeignmemory_handle *xfh = NULL;
 
     /* TODO reuse libxl xsh connection */
     xsh = xs_open(0);
@@ -355,9 +350,9 @@ int main(int argc, char **argv)
             continue;
 
         printf("Checking domid: %u\n", domid);
-        if (!domain_exists(xsh, domid)) {
-            rc = init_domain(xsh, xch, xfh, &info[i]);
-            if (rc < 0) {
+        if (!domain_exists(domid)) {
+            rc = init_domain(&info[i]);
+            if (rc) {
                 fprintf(stderr, "init_domain failed.\n");
                 goto out;
             }
-- 
2.50.1

Re: [PATCH v3 1/8] tools/init-dom0less: Make handles global

[Jürgen Groß]

[-- Attachment #1.1.1: Type: text/plain, Size: 628 bytes --]

On 26.08.25 23:08, Jason Andryuk wrote:
> init-dom0less passes the assorted xen library handles from main
> downward.  This is a little excessive in places with 4 handles passed
> into configure_xenstore().
> 
> Replace the handle passing with file-scoped variables.
> 
> The xenstore helpers are not changed.  This keeps their implementation
> common with the libxenstore functions that take a handle, transation,
> and then additional arguments.
> 
> Suggested-by: Juergen Gross <jgross@suse.com>
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

[PATCH v3 2/8] tools/init-dom0less: Factor out xenstore setup

[Jason Andryuk]

Factor out the xenstore setup code into configure_xenstore().  This is
in preparation for handling already-introduced domains.

Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
v3:
Remove handle passing
---
 tools/helpers/init-dom0less.c | 39 +++++++++++++++++++++++++----------
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/tools/helpers/init-dom0less.c b/tools/helpers/init-dom0less.c
index 0b0be08449..8de8e44ad3 100644
--- a/tools/helpers/init-dom0less.c
+++ b/tools/helpers/init-dom0less.c
@@ -237,40 +237,40 @@ err:
     return rc;
 }
 
-static int init_domain(libxl_dominfo *info)
+static int configure_xenstore(libxl_dominfo *info,
+                              uint64_t *xenstore_evtchn,
+                              uint64_t *xenstore_pfn)
 {
-    libxl_uuid uuid;
-    uint64_t xenstore_evtchn, xenstore_pfn;
     int rc;
 
     printf("Init dom0less domain: %u\n", info->domid);
 
     rc = xc_hvm_param_get(xch, info->domid, HVM_PARAM_STORE_EVTCHN,
-                          &xenstore_evtchn);
+                          xenstore_evtchn);
     if (rc != 0) {
         printf("Failed to get HVM_PARAM_STORE_EVTCHN\n");
         return 1;
     }
 
     /* no xen,enhanced; nothing to do */
-    if (!xenstore_evtchn)
+    if (!*xenstore_evtchn)
         return 0;
 
     /* Get xenstore page */
-    if (get_xs_page(info, &xenstore_pfn) != 0)
+    if (get_xs_page(info, xenstore_pfn) != 0)
         return 1;
 
-    if (xenstore_pfn == ~0ULL) {
+    if (*xenstore_pfn == ~0ULL) {
         struct xenstore_domain_interface *intf;
 
-        rc = alloc_xs_page(info, &xenstore_pfn);
+        rc = alloc_xs_page(info, xenstore_pfn);
         if (rc != 0) {
             printf("Error on getting xenstore page\n");
             return 1;
         }
 
         intf = xenforeignmemory_map(xfh, info->domid, PROT_READ | PROT_WRITE, 1,
-                                    &xenstore_pfn, NULL);
+                                    xenstore_pfn, NULL);
         if (!intf) {
             printf("Error mapping xenstore page\n");
             return 1;
@@ -281,16 +281,33 @@ static int init_domain(libxl_dominfo *info)
 
         /* Now everything is ready: set HVM_PARAM_STORE_PFN */
         rc = xc_hvm_param_set(xch, info->domid, HVM_PARAM_STORE_PFN,
-                xenstore_pfn);
+                              *xenstore_pfn);
         if (rc < 0)
             return rc;
 
         rc = xc_dom_gnttab_seed(xch, info->domid, true,
-                                (xen_pfn_t)-1, xenstore_pfn, 0, 0);
+                                (xen_pfn_t)-1, *xenstore_pfn, 0, 0);
         if (rc)
                err(1, "xc_dom_gnttab_seed");
     }
 
+    return 0;
+}
+
+static int init_domain(libxl_dominfo *info)
+{
+    uint64_t xenstore_evtchn, xenstore_pfn = 0;
+    libxl_uuid uuid;
+    int rc;
+
+    rc = configure_xenstore(info, &xenstore_evtchn, &xenstore_pfn);
+    if (rc)
+        return rc;
+
+    if (xenstore_evtchn == 0) {
+        return 0;
+    }
+
     libxl_uuid_generate(&uuid);
     xc_domain_sethandle(xch, info->domid, libxl_uuid_bytearray(&uuid));
 
-- 
2.50.1

[PATCH v3 3/8] tools/init-dom0less: Only introduce un-introduced domains

[Jason Andryuk]

In preparation for supporting already-introduced domain, only call
xs_introduce_domain() if a domain is not already introduced.  Their
xenstore entries will be written, but the xenstore introduction is
skipped.

Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
v3:
Reordered
---
 tools/helpers/init-dom0less.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/helpers/init-dom0less.c b/tools/helpers/init-dom0less.c
index 8de8e44ad3..3906c4b61a 100644
--- a/tools/helpers/init-dom0less.c
+++ b/tools/helpers/init-dom0less.c
@@ -319,9 +319,13 @@ static int init_domain(libxl_dominfo *info)
     if (rc)
         err(1, "writing to xenstore");
 
-    rc = xs_introduce_domain(xsh, info->domid, xenstore_pfn, xenstore_evtchn);
-    if (!rc)
-        err(1, "xs_introduce_domain");
+    if (!xs_is_domain_introduced(xsh, info->domid)) {
+        rc = xs_introduce_domain(xsh, info->domid, xenstore_pfn,
+                                 xenstore_evtchn);
+        if (!rc)
+            err(1, "xs_introduce_domain");
+    }
+
     return 0;
 }
 
-- 
2.50.1

[PATCH v3 4/8] tools/init-dom0less: Switch domain_exists to check xenstore name

[Jason Andryuk]

Previously, the xenstore "introduced" state was used to avoid running
init-dom0less twice on the same domain.  With xenstored
auto-introduction, that can no longer be used.  Instead check of the
domain's name has been set and use that to determine whether or not to
bail out.

Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
v3:
Move xenstore reading into domain_exists()
Reorder
---
 tools/helpers/init-dom0less.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/tools/helpers/init-dom0less.c b/tools/helpers/init-dom0less.c
index 3906c4b61a..eb793c7aab 100644
--- a/tools/helpers/init-dom0less.c
+++ b/tools/helpers/init-dom0less.c
@@ -60,6 +60,19 @@ static int get_xs_page(libxl_dominfo *info, uint64_t *xenstore_pfn)
     return 0;
 }
 
+static char *do_xs_read_dom(struct xs_handle *xsh, xs_transaction_t t,
+                            domid_t domid, char *path)
+{
+    char full_path[STR_MAX_LENGTH];
+    int rc;
+
+    rc = snprintf(full_path, STR_MAX_LENGTH,
+                  "/local/domain/%u/%s", domid, path);
+    if (rc < 0 || rc >= STR_MAX_LENGTH)
+        return NULL;
+    return xs_read(xsh, t, full_path, NULL);
+}
+
 static bool do_xs_write_dom(struct xs_handle *xsh, xs_transaction_t t,
                             domid_t domid, char *path, char *val)
 {
@@ -332,7 +345,13 @@ static int init_domain(libxl_dominfo *info)
 /* Check if domain has been configured in XS */
 static bool domain_exists(int domid)
 {
-    return xs_is_domain_introduced(xsh, domid);
+    char *name = do_xs_read_dom(xsh, XBT_NULL, domid, "name");
+    if (name) {
+        free(name);
+        return true;
+    }
+
+    return false;
 }
 
 int main(int argc, char **argv)
-- 
2.50.1

[PATCH v3 5/8] tools/init-dom0less: Use introduced to determine no-enhanced

[Jason Andryuk]

A hardware+xenstore domain will not be able to read HVM params, and
init-dom0less reads HVM_PARAM_STORE_EVTCHN to determine whether or not
a domain has xenstore.

xenstored had a similar issue, and it just tries to map a domain's
reserved grant, GNTTAB_RESERVED_XENSTORE, to see if it is accessible.
If successful, xenstored introduces the domain.  Use the existing
introduced state to determine if init-dom0less should try and read the
param.  If already introduced, initialization (and reading the
HVM_PARAM) is skipped.

This allows for running init-dom0less from a xenstored+hardware domain.
It relies on the local socket xenstore connections being considered
privileged.

oxenstored has not been updated, so the exist code remains for backwards
compatibility.

Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
v3:
New
---
 tools/helpers/init-dom0less.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/tools/helpers/init-dom0less.c b/tools/helpers/init-dom0less.c
index eb793c7aab..a4de40aeac 100644
--- a/tools/helpers/init-dom0less.c
+++ b/tools/helpers/init-dom0less.c
@@ -309,16 +309,20 @@ static int configure_xenstore(libxl_dominfo *info,
 
 static int init_domain(libxl_dominfo *info)
 {
-    uint64_t xenstore_evtchn, xenstore_pfn = 0;
+    uint64_t xenstore_evtchn = 0, xenstore_pfn = 0;
+    bool introduced;
     libxl_uuid uuid;
     int rc;
 
-    rc = configure_xenstore(info, &xenstore_evtchn, &xenstore_pfn);
-    if (rc)
-        return rc;
+    introduced = xs_is_domain_introduced(xsh, info->domid);
+    if (!introduced) {
+        rc = configure_xenstore(info, &xenstore_evtchn, &xenstore_pfn);
+        if (rc)
+            return rc;
 
-    if (xenstore_evtchn == 0) {
-        return 0;
+        if (xenstore_evtchn == 0) {
+            return 0;
+        }
     }
 
     libxl_uuid_generate(&uuid);
@@ -332,7 +336,7 @@ static int init_domain(libxl_dominfo *info)
     if (rc)
         err(1, "writing to xenstore");
 
-    if (!xs_is_domain_introduced(xsh, info->domid)) {
+    if (!introduced) {
         rc = xs_introduce_domain(xsh, info->domid, xenstore_pfn,
                                  xenstore_evtchn);
         if (!rc)
-- 
2.50.1

[PATCH v3 6/8] tools/init-dom0less: Remove use of err()

[Jason Andryuk]

err() is fatal - print a message and exit.  We want init-dom0less to
continue on as much as possible, so replace with returning an error.

Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
v3:
New
---
 tools/helpers/init-dom0less.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/tools/helpers/init-dom0less.c b/tools/helpers/init-dom0less.c
index a4de40aeac..d08b0b1362 100644
--- a/tools/helpers/init-dom0less.c
+++ b/tools/helpers/init-dom0less.c
@@ -300,8 +300,10 @@ static int configure_xenstore(libxl_dominfo *info,
 
         rc = xc_dom_gnttab_seed(xch, info->domid, true,
                                 (xen_pfn_t)-1, *xenstore_pfn, 0, 0);
-        if (rc)
-               err(1, "xc_dom_gnttab_seed");
+        if (rc) {
+            printf("xc_dom_gnttab_seed");
+            return 1;
+        }
     }
 
     return 0;
@@ -329,18 +331,24 @@ static int init_domain(libxl_dominfo *info)
     xc_domain_sethandle(xch, info->domid, libxl_uuid_bytearray(&uuid));
 
     rc = gen_stub_json_config(info->domid, &uuid);
-    if (rc)
-        err(1, "gen_stub_json_config");
+    if (rc) {
+        printf("gen_stub_json_config");
+        return 1;
+    }
 
     rc = create_xenstore(info, uuid, xenstore_pfn, xenstore_evtchn);
-    if (rc)
-        err(1, "writing to xenstore");
+    if (rc) {
+        printf("writing to xenstore");
+        return 1;
+    }
 
     if (!introduced) {
         rc = xs_introduce_domain(xsh, info->domid, xenstore_pfn,
                                  xenstore_evtchn);
-        if (!rc)
-            err(1, "xs_introduce_domain");
+        if (!rc) {
+            printf("xs_introduce_domain");
+            return 1;
+        }
     }
 
     return 0;
-- 
2.50.1

[PATCH v3 7/8] tools/init-dom0less: Continue on error

[Jason Andryuk]

An error on one domain doesn't apply to others.  Continue the loop over
all domains if init_domain() fails.

There is a mix of positive and negative errors.  0 is success, so just
check for that.

The last rc value is used as the exit value, so clear it before the end.
This avoids a stale rc value indicating failure.

Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
v3:
Use != 0 since there are both positive and negative errors
---
 tools/helpers/init-dom0less.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/helpers/init-dom0less.c b/tools/helpers/init-dom0less.c
index d08b0b1362..ce70ddee29 100644
--- a/tools/helpers/init-dom0less.c
+++ b/tools/helpers/init-dom0less.c
@@ -406,12 +406,14 @@ int main(int argc, char **argv)
             rc = init_domain(&info[i]);
             if (rc) {
                 fprintf(stderr, "init_domain failed.\n");
-                goto out;
             }
         } else {
             printf("Domain %u has already been initialized\n", domid);
         }
     }
+
+    rc = 0;
+
 out:
     libxl_dominfo_list_free(info, nb_vm);
     return rc;
-- 
2.50.1

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

[Jason Andryuk]

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.1

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

[Jan Beulich]

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

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

[Jason Andryuk]

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

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

[Jan Beulich]

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

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

[Orzel, Michal]

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...

~Michal

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

[Jason Andryuk]

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