Re: [PATCH] xen/x86: Check supported features even for PHV dom0

["Roger Pau Monné" <roger.pau@citrix.com>]

Typo on the subject s/PHV/PVH/.

On Wed, Mar 25, 2026 at 03:55:28PM +0000, Frediano Ziglio wrote:
> The supported features ELF note was tested only if the dom0 was
> PV. Factor out a function to check ELF notes and reuse it even
> for PVH.
> 
> Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com>
> ---
>  xen/arch/x86/dom0_build.c             | 16 ++++++++++++++++
>  xen/arch/x86/hvm/dom0_build.c         |  3 +++
>  xen/arch/x86/include/asm/dom0_build.h |  2 ++
>  xen/arch/x86/pv/dom0_build.c          | 10 ++--------
>  4 files changed, 23 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
> index 864dd9e53e..c6bb2f8067 100644
> --- a/xen/arch/x86/dom0_build.c
> +++ b/xen/arch/x86/dom0_build.c
> @@ -321,6 +321,22 @@ unsigned long __init dom0_paging_pages(const
> struct domain *d,
>  }
> 
> 
> +int __init dom0_check_parms(
> +    const struct elf_dom_parms *parms, bool is_pv_shim)
> +{
> +    if ( parms->elf_notes[XEN_ELFNOTE_SUPPORTED_FEATURES].type !=
> XEN_ENT_NONE )

The patch seems to be mangled here?

And the line is too long otherwise.  You might want to consider
returning early here, to reduce the indentation of the following code
block.

> +    {
> +        if ( !is_pv_shim && !test_bit(XENFEAT_dom0, parms->f_supported) )

I think you want to pass the domain being built to this function, so
you can do a check like:

if ( is_hardware_domain(d) && !test_bit(XENFEAT_dom0, parms->f_supported) )
{
    printk(...

That way you don't need to explicitly check for pv-shim mode, and
would more naturally work with things like
Hyperlaunch/dom0less/late-hwdom.

> +        {
> +            printk("Kernel does not support Dom0 operation\n");
> +            return -EINVAL;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +
>  /*
>   * If allocation isn't specified, reserve 1/16th of available memory for
>   * things like DMA buffers. This reservation is clamped to a maximum of 128MB.
> diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
> index d69a83b089..ca96f32acd 100644
> --- a/xen/arch/x86/hvm/dom0_build.c
> +++ b/xen/arch/x86/hvm/dom0_build.c
> @@ -699,6 +699,9 @@ static int __init pvh_load_kernel(
>      if ( !check_and_adjust_load_address(d, &elf, &parms) )
>          return -ENOSPC;
> 
> +    if ( (rc = dom0_check_parms(&parms, false)) != 0 )
> +        return rc;
> +
>      elf_set_vcpu(&elf, v);
>      rc = elf_load_binary(&elf);
>      if ( rc < 0 )
> diff --git a/xen/arch/x86/include/asm/dom0_build.h
> b/xen/arch/x86/include/asm/dom0_build.h
> index ff021c24af..a322bf455c 100644
> --- a/xen/arch/x86/include/asm/dom0_build.h
> +++ b/xen/arch/x86/include/asm/dom0_build.h
> @@ -8,6 +8,8 @@
> 
>  extern unsigned int dom0_memflags;
> 
> +int dom0_check_parms(const struct elf_dom_parms *parms,
> +                     bool is_pv_shim);
>  unsigned long dom0_compute_nr_pages(struct domain *d,
>                                      struct elf_dom_parms *parms,
>                                      unsigned long initrd_len);
> diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
> index 075a3646c2..9d0310ad91 100644
> --- a/xen/arch/x86/pv/dom0_build.c
> +++ b/xen/arch/x86/pv/dom0_build.c
> @@ -494,14 +494,8 @@ static int __init dom0_construct(const struct
> boot_domain *bd)
>          return -EINVAL;
>      }
> 
> -    if ( parms.elf_notes[XEN_ELFNOTE_SUPPORTED_FEATURES].type != XEN_ENT_NONE )
> -    {
> -        if ( !pv_shim && !test_bit(XENFEAT_dom0, parms.f_supported) )
> -        {
> -            printk("Kernel does not support Dom0 operation\n");
> -            return -EINVAL;
> -        }
> -    }
> +    if ( (rc = dom0_check_parms(&parms, pv_shim)) != 0 )

pv_shim is a global variable, you don't need to pass it around.

Thanks, Roger.