Re: [PATCH v4 3/4] tools/init-dom0less: Avoid hardcoding GUEST_MAGIC_BASE

[Anthony PERARD <anthony.perard@vates.tech>]

On Fri, May 24, 2024 at 03:55:21PM -0700, Stefano Stabellini wrote:
> From: Henry Wang <xin.wang2@amd.com>
> 
> Currently the GUEST_MAGIC_BASE in the init-dom0less application is
> hardcoded, which will lead to failures for 1:1 direct-mapped Dom0less
> DomUs.
> 
> Since the guest magic region allocation from init-dom0less is for
> XenStore, and the XenStore page is now allocated from the hypervisor,
> instead of hardcoding the guest magic pages region, use
> xc_hvm_param_get() to get the XenStore page PFN. Rename alloc_xs_page()
> to get_xs_page() to reflect the changes.
> 
> With this change, some existing code is not needed anymore, including:
> (1) The definition of the XenStore page offset.
> (2) Call to xc_domain_setmaxmem() and xc_clear_domain_page() as we
>     don't need to set the max mem and clear the page anymore.
> (3) Foreign mapping of the XenStore page, setting of XenStore interface
>     status and HVM_PARAM_STORE_PFN from init-dom0less, as they are set
>     by the hypervisor.
> 
> Take the opportunity to do some coding style improvements when possible.
> 
> Reported-by: Alec Kwapis <alec.kwapis@medtronic.com>
> Signed-off-by: Henry Wang <xin.wang2@amd.com>
> Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
> ---
> +static int get_xs_page(struct xc_interface_core *xch, libxl_dominfo *info,
> +                       uint64_t *xenstore_pfn)
>  {

[...]

> +    rc = xc_hvm_param_get(xch, info->domid, HVM_PARAM_STORE_PFN, xenstore_pfn);
> +    if (rc < 0) {
> +        printf("Failed to get HVM_PARAM_STORE_PFN\n");

Shouldn't we print error message on "stderr" instead?

> @@ -245,20 +232,11 @@ static int init_domain(struct xs_handle *xsh,
>      if (!xenstore_evtchn)
>          return 0;
>  
> -    /* Alloc xenstore page */
> -    if (alloc_xs_page(xch, info, &xenstore_pfn) != 0) {
> -        printf("Error on alloc magic pages\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");
> +    /* Get xenstore page */
> +    if (get_xs_page(xch, info, &xenstore_pfn) != 0) {
> +        printf("Error on getting xenstore page\n");

Same here.


In anycase:
Acked-by: Anthony PERARD <anthony.perard@vates.tech>

Thanks,

-- 


Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech