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(-)
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
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
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.
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
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.
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
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
© 2016 - 2026 Red Hat, Inc.