We check if the xenstore page is already allocated. If yes, there is
nothing to do. If no, we proceed allocating it.

Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---
Changes in v6:
- remove double blank lines

 tools/helpers/init-dom0less.c | 53 +++++++++++++++++++++++++++++++++--
 1 file changed, 50 insertions(+), 3 deletions(-)

diff --git a/tools/helpers/init-dom0less.c b/tools/helpers/init-dom0less.c
index 2b51965fa7..78c59ec5e7 100644
--- a/tools/helpers/init-dom0less.c
+++ b/tools/helpers/init-dom0less.c
@@ -16,8 +16,34 @@
 
 #include "init-dom-json.h"
 
+#define XENSTORE_PFN_OFFSET 1
 #define STR_MAX_LENGTH 128
 
+static int alloc_xs_page(struct xc_interface_core *xch,
+                         libxl_dominfo *info,
+                         uint64_t *xenstore_pfn)
+{
+    int rc;
+    const xen_pfn_t base = GUEST_MAGIC_BASE >> XC_PAGE_SHIFT;
+    xen_pfn_t p2m = (GUEST_MAGIC_BASE >> XC_PAGE_SHIFT) + XENSTORE_PFN_OFFSET;
+
+    rc = xc_domain_setmaxmem(xch, info->domid,
+                             info->max_memkb + (XC_PAGE_SIZE/1024));
+    if (rc < 0)
+        return rc;
+
+    rc = xc_domain_populate_physmap_exact(xch, info->domid, 1, 0, 0, &p2m);
+    if (rc < 0)
+        return rc;
+
+    *xenstore_pfn = base + XENSTORE_PFN_OFFSET;
+    rc = xc_clear_domain_page(xch, info->domid, *xenstore_pfn);
+    if (rc < 0)
+        return rc;
+
+    return 0;
+}
+
 static int get_xs_page(struct xc_interface_core *xch, libxl_dominfo *info,
                        uint64_t *xenstore_pfn)
 {
@@ -233,9 +259,30 @@ static int init_domain(struct xs_handle *xsh,
         return 0;
 
     /* Get xenstore page */
-    if (get_xs_page(xch, info, &xenstore_pfn) != 0) {
-        printf("Error on getting xenstore page\n");
-        return 1;
+    if (get_xs_page(xch, info, &xenstore_pfn) != 0 || xenstore_pfn == ~0ULL) {
+        struct xenstore_domain_interface *intf;
+
+        rc = alloc_xs_page(xch, 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);
+        if (!intf) {
+            printf("Error mapping xenstore page\n");
+            return 1;
+        }
+
+        intf->connection = XENSTORE_RECONNECT;
+        xenforeignmemory_unmap(xfh, intf, 1);
+
+        /* Now everything is ready: set HVM_PARAM_STORE_PFN */
+        rc = xc_hvm_param_set(xch, info->domid, HVM_PARAM_STORE_PFN,
+                xenstore_pfn);
+        if (rc < 0)
+            return rc;
     }
 
     rc = xc_dom_gnttab_seed(xch, info->domid, true,
-- 
2.25.1

NIT: commit title: s/is/if/

On 07/02/2025 02:53, Stefano Stabellini wrote:
> We check if the xenstore page is already allocated. If yes, there is
> nothing to do. If no, we proceed allocating it.
The commit message lacks justification which is to support old unpatched/unfixed kernels.

> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> ---
> Changes in v6:
> - remove double blank lines
> 
>  tools/helpers/init-dom0less.c | 53 +++++++++++++++++++++++++++++++++--
>  1 file changed, 50 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/helpers/init-dom0less.c b/tools/helpers/init-dom0less.c
> index 2b51965fa7..78c59ec5e7 100644
> --- a/tools/helpers/init-dom0less.c
> +++ b/tools/helpers/init-dom0less.c
> @@ -16,8 +16,34 @@
>  
>  #include "init-dom-json.h"
>  
> +#define XENSTORE_PFN_OFFSET 1
>  #define STR_MAX_LENGTH 128
>  
> +static int alloc_xs_page(struct xc_interface_core *xch,
> +                         libxl_dominfo *info,
> +                         uint64_t *xenstore_pfn)
> +{
> +    int rc;
> +    const xen_pfn_t base = GUEST_MAGIC_BASE >> XC_PAGE_SHIFT;
> +    xen_pfn_t p2m = (GUEST_MAGIC_BASE >> XC_PAGE_SHIFT) + XENSTORE_PFN_OFFSET;
base already contains shifted value so why not use it?

> +
> +    rc = xc_domain_setmaxmem(xch, info->domid,
> +                             info->max_memkb + (XC_PAGE_SIZE/1024));
> +    if (rc < 0)
> +        return rc;
> +
> +    rc = xc_domain_populate_physmap_exact(xch, info->domid, 1, 0, 0, &p2m);
> +    if (rc < 0)
> +        return rc;
> +
> +    *xenstore_pfn = base + XENSTORE_PFN_OFFSET;
> +    rc = xc_clear_domain_page(xch, info->domid, *xenstore_pfn);
> +    if (rc < 0)
> +        return rc;
> +
> +    return 0;
> +}
> +
>  static int get_xs_page(struct xc_interface_core *xch, libxl_dominfo *info,
>                         uint64_t *xenstore_pfn)
>  {
> @@ -233,9 +259,30 @@ static int init_domain(struct xs_handle *xsh,
>          return 0;
>  
>      /* Get xenstore page */
> -    if (get_xs_page(xch, info, &xenstore_pfn) != 0) {
> -        printf("Error on getting xenstore page\n");
> -        return 1;
> +    if (get_xs_page(xch, info, &xenstore_pfn) != 0 || xenstore_pfn == ~0ULL) {
If get_xs_page() returns != 0, then something is wrong and we definitiely should not try
to allocate a page. The only reason the script should allocate a page is if xenstore_pfn is
invalid i.e. ~0ULL or not set i.e. 0. At this point we already validated that guest is xenstore enhanced
so the only possibility is ~0ULL. So the code should be:

if (get_xs_page(xch, info, &xenstore_pfn) != 0) {
    return 1;
}

if (xenstore_pfn == ~0ULL) {
...

Other than that:
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

~Michal

On Fri, 7 Feb 2025, Orzel, Michal wrote:
> NIT: commit title: s/is/if/
> 
> On 07/02/2025 02:53, Stefano Stabellini wrote:
> > We check if the xenstore page is already allocated. If yes, there is
> > nothing to do. If no, we proceed allocating it.
> The commit message lacks justification which is to support old unpatched/unfixed kernels.
> 
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> > ---
> > Changes in v6:
> > - remove double blank lines
> > 
> >  tools/helpers/init-dom0less.c | 53 +++++++++++++++++++++++++++++++++--
> >  1 file changed, 50 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/helpers/init-dom0less.c b/tools/helpers/init-dom0less.c
> > index 2b51965fa7..78c59ec5e7 100644
> > --- a/tools/helpers/init-dom0less.c
> > +++ b/tools/helpers/init-dom0less.c
> > @@ -16,8 +16,34 @@
> >  
> >  #include "init-dom-json.h"
> >  
> > +#define XENSTORE_PFN_OFFSET 1
> >  #define STR_MAX_LENGTH 128
> >  
> > +static int alloc_xs_page(struct xc_interface_core *xch,
> > +                         libxl_dominfo *info,
> > +                         uint64_t *xenstore_pfn)
> > +{
> > +    int rc;
> > +    const xen_pfn_t base = GUEST_MAGIC_BASE >> XC_PAGE_SHIFT;
> > +    xen_pfn_t p2m = (GUEST_MAGIC_BASE >> XC_PAGE_SHIFT) + XENSTORE_PFN_OFFSET;
> base already contains shifted value so why not use it?
> 
> > +
> > +    rc = xc_domain_setmaxmem(xch, info->domid,
> > +                             info->max_memkb + (XC_PAGE_SIZE/1024));
> > +    if (rc < 0)
> > +        return rc;
> > +
> > +    rc = xc_domain_populate_physmap_exact(xch, info->domid, 1, 0, 0, &p2m);
> > +    if (rc < 0)
> > +        return rc;
> > +
> > +    *xenstore_pfn = base + XENSTORE_PFN_OFFSET;
> > +    rc = xc_clear_domain_page(xch, info->domid, *xenstore_pfn);
> > +    if (rc < 0)
> > +        return rc;
> > +
> > +    return 0;
> > +}
> > +
> >  static int get_xs_page(struct xc_interface_core *xch, libxl_dominfo *info,
> >                         uint64_t *xenstore_pfn)
> >  {
> > @@ -233,9 +259,30 @@ static int init_domain(struct xs_handle *xsh,
> >          return 0;
> >  
> >      /* Get xenstore page */
> > -    if (get_xs_page(xch, info, &xenstore_pfn) != 0) {
> > -        printf("Error on getting xenstore page\n");
> > -        return 1;
> > +    if (get_xs_page(xch, info, &xenstore_pfn) != 0 || xenstore_pfn == ~0ULL) {
> If get_xs_page() returns != 0, then something is wrong and we definitiely should not try
> to allocate a page. The only reason the script should allocate a page is if xenstore_pfn is
> invalid i.e. ~0ULL or not set i.e. 0. At this point we already validated that guest is xenstore enhanced
> so the only possibility is ~0ULL. So the code should be:
> 
> if (get_xs_page(xch, info, &xenstore_pfn) != 0) {
>     return 1;
> }
> 
> if (xenstore_pfn == ~0ULL) {
> ...
> 
> Other than that:
> Reviewed-by: Michal Orzel <michal.orzel@amd.com>

Thanks Michal, great catch! I made this change and all the other changes
you suggested and validated with a successful pipeline again. I'll
queue it for 4.21.