[PATCH v2] xen/x86: Check supported features even for PVH dom0

Frediano Ziglio posted 1 patch 1 week, 3 days ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20260402155512.80170-1-frediano.ziglio@cloud.com
There is a newer version of this series
xen/arch/x86/dom0_build.c             | 14 ++++++++++++++
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, 21 insertions(+), 8 deletions(-)
[PATCH v2] xen/x86: Check supported features even for PVH dom0
Posted by Frediano Ziglio 1 week, 3 days ago
The supported features ELF notes 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@cloud.com>
--
Changes since v1:
- fix typo in title;
- fix minor formatting issue;
- use is_hardware_domain instead of checking is_pv_shim;
- reduce indentation returning earlier;
- return error instead of jumping to cleanup code.
---
 xen/arch/x86/dom0_build.c             | 14 ++++++++++++++
 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, 21 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index 864dd9e53e..a33ce77321 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -320,6 +320,20 @@ unsigned long __init dom0_paging_pages(const struct domain *d,
     return DIV_ROUND_UP(memkb, 1024) << (20 - PAGE_SHIFT);
 }
 
+int __init dom0_check_parms(
+    struct domain *d, const struct elf_dom_parms *parms)
+{
+    if ( parms->elf_notes[XEN_ELFNOTE_SUPPORTED_FEATURES].type == XEN_ENT_NONE )
+        return 0;
+
+    if ( is_hardware_domain(d) && !test_bit(XENFEAT_dom0, parms->f_supported) )
+    {
+        printk("Kernel does not support Dom0 operation\n");
+        return -EINVAL;
+    }
+
+    return 0;
+}
 
 /*
  * If allocation isn't specified, reserve 1/16th of available memory for
diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
index d69a83b089..f95a00acfd 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(d, &parms)) != 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..b7bbfcbe6a 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(struct domain *d,
+                     const struct elf_dom_parms *parms);
 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..d50e1b9f78 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(d, &parms)) != 0 )
+        return rc;
 
     nr_pages = dom0_compute_nr_pages(d, &parms, initrd_len);
 
-- 
2.43.0
Re: [PATCH v2] xen/x86: Check supported features even for PVH dom0
Posted by Jan Beulich 5 days, 15 hours ago
On 02.04.2026 17:55, Frediano Ziglio wrote:
> --- a/xen/arch/x86/dom0_build.c
> +++ b/xen/arch/x86/dom0_build.c
> @@ -320,6 +320,20 @@ unsigned long __init dom0_paging_pages(const struct domain *d,
>      return DIV_ROUND_UP(memkb, 1024) << (20 - PAGE_SHIFT);
>  }
>  
> +int __init dom0_check_parms(

I understand the "dom0" in the name is owing to the filename and perhaps
adjacent other similar functions, yet ...

> +    struct domain *d, const struct elf_dom_parms *parms)
> +{
> +    if ( parms->elf_notes[XEN_ELFNOTE_SUPPORTED_FEATURES].type == XEN_ENT_NONE )
> +        return 0;
> +
> +    if ( is_hardware_domain(d) && !test_bit(XENFEAT_dom0, parms->f_supported) )

... if this was about solely Dom0, no is_hardware_domain() should be present
here. Maybe s/dom0/initdom/ ?

Jan
Re: [PATCH v2] xen/x86: Check supported features even for PVH dom0
Posted by Roger Pau Monné 4 days, 12 hours ago
On Tue, Apr 07, 2026 at 08:56:03AM +0200, Jan Beulich wrote:
> On 02.04.2026 17:55, Frediano Ziglio wrote:
> > --- a/xen/arch/x86/dom0_build.c
> > +++ b/xen/arch/x86/dom0_build.c
> > @@ -320,6 +320,20 @@ unsigned long __init dom0_paging_pages(const struct domain *d,
> >      return DIV_ROUND_UP(memkb, 1024) << (20 - PAGE_SHIFT);
> >  }
> >  
> > +int __init dom0_check_parms(
> 
> I understand the "dom0" in the name is owing to the filename and perhaps
> adjacent other similar functions, yet ...
> 
> > +    struct domain *d, const struct elf_dom_parms *parms)
> > +{
> > +    if ( parms->elf_notes[XEN_ELFNOTE_SUPPORTED_FEATURES].type == XEN_ENT_NONE )
> > +        return 0;
> > +
> > +    if ( is_hardware_domain(d) && !test_bit(XENFEAT_dom0, parms->f_supported) )
> 
> ... if this was about solely Dom0, no is_hardware_domain() should be present
> here. Maybe s/dom0/initdom/ ?

I think the naming of the feature flag is not very useful TBH.  What
is the kernel really advertising when setting XENFEAT_dom0?  I've
assumed it was the capability of running as a hardware domain, which
requires a different set of functionality inside of the kernel to deal
with hardware devices.

We might want to take this opportunity to clarify in the headers what
XENFEAT_dom0 means.

Thanks, Roger.
Re: [PATCH v2] xen/x86: Check supported features even for PVH dom0
Posted by Jan Beulich 4 days, 12 hours ago
On 08.04.2026 12:22, Roger Pau Monné wrote:
> On Tue, Apr 07, 2026 at 08:56:03AM +0200, Jan Beulich wrote:
>> On 02.04.2026 17:55, Frediano Ziglio wrote:
>>> --- a/xen/arch/x86/dom0_build.c
>>> +++ b/xen/arch/x86/dom0_build.c
>>> @@ -320,6 +320,20 @@ unsigned long __init dom0_paging_pages(const struct domain *d,
>>>      return DIV_ROUND_UP(memkb, 1024) << (20 - PAGE_SHIFT);
>>>  }
>>>  
>>> +int __init dom0_check_parms(
>>
>> I understand the "dom0" in the name is owing to the filename and perhaps
>> adjacent other similar functions, yet ...
>>
>>> +    struct domain *d, const struct elf_dom_parms *parms)
>>> +{
>>> +    if ( parms->elf_notes[XEN_ELFNOTE_SUPPORTED_FEATURES].type == XEN_ENT_NONE )
>>> +        return 0;
>>> +
>>> +    if ( is_hardware_domain(d) && !test_bit(XENFEAT_dom0, parms->f_supported) )
>>
>> ... if this was about solely Dom0, no is_hardware_domain() should be present
>> here. Maybe s/dom0/initdom/ ?
> 
> I think the naming of the feature flag is not very useful TBH.  What
> is the kernel really advertising when setting XENFEAT_dom0?  I've
> assumed it was the capability of running as a hardware domain, which
> requires a different set of functionality inside of the kernel to deal
> with hardware devices.

Yes, that's my understanding.

> We might want to take this opportunity to clarify in the headers what
> XENFEAT_dom0 means.

I'm not opposed, yet this isn't directly related to giving the function
an appropriate name.

Jan

Re: [PATCH v2] xen/x86: Check supported features even for PVH dom0
Posted by Roger Pau Monné 1 week, 2 days ago
On Thu, Apr 02, 2026 at 04:55:10PM +0100, Frediano Ziglio wrote:
> The supported features ELF notes 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@cloud.com>
> --
> Changes since v1:
> - fix typo in title;
> - fix minor formatting issue;
> - use is_hardware_domain instead of checking is_pv_shim;
> - reduce indentation returning earlier;
> - return error instead of jumping to cleanup code.
> ---
>  xen/arch/x86/dom0_build.c             | 14 ++++++++++++++
>  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, 21 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
> index 864dd9e53e..a33ce77321 100644
> --- a/xen/arch/x86/dom0_build.c
> +++ b/xen/arch/x86/dom0_build.c
> @@ -320,6 +320,20 @@ unsigned long __init dom0_paging_pages(const struct domain *d,
>      return DIV_ROUND_UP(memkb, 1024) << (20 - PAGE_SHIFT);
>  }
>  
> +int __init dom0_check_parms(
> +    struct domain *d, const struct elf_dom_parms *parms)

d should be const also.

> +{
> +    if ( parms->elf_notes[XEN_ELFNOTE_SUPPORTED_FEATURES].type == XEN_ENT_NONE )
> +        return 0;
> +
> +    if ( is_hardware_domain(d) && !test_bit(XENFEAT_dom0, parms->f_supported) )
> +    {
> +        printk("Kernel does not support Dom0 operation\n");
> +        return -EINVAL;
> +    }
> +
> +    return 0;
> +}
>  
>  /*
>   * If allocation isn't specified, reserve 1/16th of available memory for
> diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
> index d69a83b089..f95a00acfd 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(d, &parms)) != 0 )
> +        return rc;

I would do the check ahead of check_and_adjust_load_address(), as then
we could avoid the load address adjustment if we detect earlier than
the dom0 feature is not present.  But that's just my taste.

I can adjust the const-ification of d on commit if there are no
further objections:

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.

Re: [PATCH v2] xen/x86: Check supported features even for PVH dom0
Posted by Jan Beulich 5 days, 15 hours ago
On 03.04.2026 12:11, Roger Pau Monné wrote:
> On Thu, Apr 02, 2026 at 04:55:10PM +0100, Frediano Ziglio wrote:
>> The supported features ELF notes 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@cloud.com>
>> --
>> Changes since v1:
>> - fix typo in title;
>> - fix minor formatting issue;
>> - use is_hardware_domain instead of checking is_pv_shim;
>> - reduce indentation returning earlier;
>> - return error instead of jumping to cleanup code.
>> ---
>>  xen/arch/x86/dom0_build.c             | 14 ++++++++++++++
>>  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, 21 insertions(+), 8 deletions(-)
>>
>> diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
>> index 864dd9e53e..a33ce77321 100644
>> --- a/xen/arch/x86/dom0_build.c
>> +++ b/xen/arch/x86/dom0_build.c
>> @@ -320,6 +320,20 @@ unsigned long __init dom0_paging_pages(const struct domain *d,
>>      return DIV_ROUND_UP(memkb, 1024) << (20 - PAGE_SHIFT);
>>  }
>>  
>> +int __init dom0_check_parms(
>> +    struct domain *d, const struct elf_dom_parms *parms)
> 
> d should be const also.
> 
>> +{
>> +    if ( parms->elf_notes[XEN_ELFNOTE_SUPPORTED_FEATURES].type == XEN_ENT_NONE )
>> +        return 0;
>> +
>> +    if ( is_hardware_domain(d) && !test_bit(XENFEAT_dom0, parms->f_supported) )
>> +    {
>> +        printk("Kernel does not support Dom0 operation\n");
>> +        return -EINVAL;
>> +    }
>> +
>> +    return 0;
>> +}
>>  
>>  /*
>>   * If allocation isn't specified, reserve 1/16th of available memory for
>> diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
>> index d69a83b089..f95a00acfd 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(d, &parms)) != 0 )
>> +        return rc;
> 
> I would do the check ahead of check_and_adjust_load_address(), as then
> we could avoid the load address adjustment if we detect earlier than
> the dom0 feature is not present.  But that's just my taste.

+1, perhaps even yet a few more lines further up.

Jan

Re: [PATCH v2] xen/x86: Check supported features even for PVH dom0
Posted by Frediano Ziglio 1 week, 1 day ago
On Fri, 3 Apr 2026 at 11:11, Roger Pau Monné <roger.pau@citrix.com> wrote:
>
> On Thu, Apr 02, 2026 at 04:55:10PM +0100, Frediano Ziglio wrote:
> > The supported features ELF notes 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@cloud.com>
> > --
> > Changes since v1:
> > - fix typo in title;
> > - fix minor formatting issue;
> > - use is_hardware_domain instead of checking is_pv_shim;
> > - reduce indentation returning earlier;
> > - return error instead of jumping to cleanup code.
> > ---
> >  xen/arch/x86/dom0_build.c             | 14 ++++++++++++++
> >  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, 21 insertions(+), 8 deletions(-)
> >
> > diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
> > index 864dd9e53e..a33ce77321 100644
> > --- a/xen/arch/x86/dom0_build.c
> > +++ b/xen/arch/x86/dom0_build.c
> > @@ -320,6 +320,20 @@ unsigned long __init dom0_paging_pages(const struct domain *d,
> >      return DIV_ROUND_UP(memkb, 1024) << (20 - PAGE_SHIFT);
> >  }
> >
> > +int __init dom0_check_parms(
> > +    struct domain *d, const struct elf_dom_parms *parms)
>
> d should be const also.
>
> > +{
> > +    if ( parms->elf_notes[XEN_ELFNOTE_SUPPORTED_FEATURES].type == XEN_ENT_NONE )
> > +        return 0;
> > +
> > +    if ( is_hardware_domain(d) && !test_bit(XENFEAT_dom0, parms->f_supported) )
> > +    {
> > +        printk("Kernel does not support Dom0 operation\n");
> > +        return -EINVAL;
> > +    }
> > +
> > +    return 0;
> > +}
> >
> >  /*
> >   * If allocation isn't specified, reserve 1/16th of available memory for
> > diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
> > index d69a83b089..f95a00acfd 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(d, &parms)) != 0 )
> > +        return rc;
>
> I would do the check ahead of check_and_adjust_load_address(), as then
> we could avoid the load address adjustment if we detect earlier than
> the dom0 feature is not present.  But that's just my taste.
>
> I can adjust the const-ification of d on commit if there are no
> further objections:
>
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
>
> Thanks, Roger.

Fine with me.

Frediano