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.stabell...@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.or...@amd.com>
~Michal
