The new xenstore page allocation scheme might break older unpatches
Linux kernels that do not check for the Xenstore connection status
before proceeding with Xenstore initialization.
Introduce a dom0less configuration option to retain the older behavior,
which is not compatible with 1:1 mapped guests, but it will work with
older legacy kernel versions.
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---
docs/misc/arm/device-tree/booting.txt | 5 +++++
xen/arch/arm/dom0less-build.c | 13 ++++++++++++-
xen/arch/arm/include/asm/kernel.h | 14 +++++++++++---
3 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt
index ff70d44462..8fa3da95be 100644
--- a/docs/misc/arm/device-tree/booting.txt
+++ b/docs/misc/arm/device-tree/booting.txt
@@ -222,6 +222,11 @@ with the following properties:
Xen PV interfaces, including grant-table and xenstore, will be
enabled for the VM.
+ - "legacy"
+ Same as above, but the way the xenstore page is allocated is not
+ compatible with 1:1 mapped guests. On the other hand, it works with
+ older Linux kernels.
+
- "disabled"
Xen PV interfaces are disabled.
diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
index 046439eb87..9afdbca8b8 100644
--- a/xen/arch/arm/dom0less-build.c
+++ b/xen/arch/arm/dom0less-build.c
@@ -799,6 +799,13 @@ static int __init construct_domU(struct domain *d,
else
panic("At the moment, Xenstore support requires dom0 to be present\n");
}
+ else if ( rc == 0 && !strcmp(dom0less_enhanced, "legacy") )
+ {
+ if ( hardware_domain )
+ kinfo.dom0less_feature = DOM0LESS_ENHANCED_LEGACY;
+ else
+ panic("At the moment, Xenstore support requires dom0 to be present\n");
+ }
else if ( rc == 0 && !strcmp(dom0less_enhanced, "no-xenstore") )
kinfo.dom0less_feature = DOM0LESS_ENHANCED_NO_XS;
@@ -848,13 +855,17 @@ static int __init construct_domU(struct domain *d,
if ( rc < 0 )
return rc;
- if ( kinfo.dom0less_feature & DOM0LESS_XENSTORE )
+ if ( kinfo.dom0less_feature & (DOM0LESS_XENSTORE|DOM0LESS_XS_LEGACY) )
{
ASSERT(hardware_domain);
rc = alloc_xenstore_evtchn(d);
if ( rc < 0 )
return rc;
+ d->arch.hvm.params[HVM_PARAM_STORE_PFN] = ~0ULL;
+ }
+ if ( kinfo.dom0less_feature & DOM0LESS_XENSTORE )
+ {
rc = alloc_xenstore_page(d);
if ( rc < 0 )
return rc;
diff --git a/xen/arch/arm/include/asm/kernel.h b/xen/arch/arm/include/asm/kernel.h
index de3f945ae5..4c2ae0b32b 100644
--- a/xen/arch/arm/include/asm/kernel.h
+++ b/xen/arch/arm/include/asm/kernel.h
@@ -17,16 +17,24 @@
* default features (excluding Xenstore) will be
* available. Note that an OS *must* not rely on the
* availability of Xen features if this is not set.
- * DOM0LESS_XENSTORE: Xenstore will be enabled for the VM. This feature
- * can't be enabled without the
- * DOM0LESS_ENHANCED_NO_XS.
+ * DOM0LESS_XENSTORE: Xenstore will be enabled for the VM. The
+ * xenstore page allocation is done by Xen at
+ * domain creation. This feature can't be
+ * enabled without the DOM0LESS_ENHANCED_NO_XS.
+ * DOM0LESS_XS_LEGACY Xenstore will be enabled for the VM, the
+ * xenstore page allocation will happen in
+ * init-dom0less. This feature can't be enabled
+ * without the DOM0LESS_ENHANCED_NO_XS.
* DOM0LESS_ENHANCED: Notify the OS it is running on top of Xen. All the
* default features (including Xenstore) will be
* available. Note that an OS *must* not rely on the
* availability of Xen features if this is not set.
+ * DOM0LESS_ENHANCED_LEGACY:Same as before, but using DOM0LESS_XS_LEGACY.
*/
#define DOM0LESS_ENHANCED_NO_XS BIT(0, U)
#define DOM0LESS_XENSTORE BIT(1, U)
+#define DOM0LESS_XS_LEGACY BIT(2, U)
+#define DOM0LESS_ENHANCED_LEGACY (DOM0LESS_ENHANCED_NO_XS | DOM0LESS_XS_LEGACY)
#define DOM0LESS_ENHANCED (DOM0LESS_ENHANCED_NO_XS | DOM0LESS_XENSTORE)
struct kernel_info {
--
2.25.1
On 06/02/2025 02:08, Stefano Stabellini wrote:
> The new xenstore page allocation scheme might break older unpatches
> Linux kernels that do not check for the Xenstore connection status
> before proceeding with Xenstore initialization.
>
> Introduce a dom0less configuration option to retain the older behavior,
> which is not compatible with 1:1 mapped guests, but it will work with
The issue is for static domains in general - not only for 1:1 guests.
Static domains without direct map will simply fail on acquire_reserved_page().
> older legacy kernel versions.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> ---
> docs/misc/arm/device-tree/booting.txt | 5 +++++
> xen/arch/arm/dom0less-build.c | 13 ++++++++++++-
> xen/arch/arm/include/asm/kernel.h | 14 +++++++++++---
> 3 files changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt
> index ff70d44462..8fa3da95be 100644
> --- a/docs/misc/arm/device-tree/booting.txt
> +++ b/docs/misc/arm/device-tree/booting.txt
> @@ -222,6 +222,11 @@ with the following properties:
> Xen PV interfaces, including grant-table and xenstore, will be
> enabled for the VM.
>
> + - "legacy"
> + Same as above, but the way the xenstore page is allocated is not
> + compatible with 1:1 mapped guests. On the other hand, it works with
Same remark about 1:1
> + older Linux kernels.
> +
> - "disabled"
> Xen PV interfaces are disabled.
>
> diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
> index 046439eb87..9afdbca8b8 100644
> --- a/xen/arch/arm/dom0less-build.c
> +++ b/xen/arch/arm/dom0less-build.c
> @@ -799,6 +799,13 @@ static int __init construct_domU(struct domain *d,
> else
> panic("At the moment, Xenstore support requires dom0 to be present\n");
> }
> + else if ( rc == 0 && !strcmp(dom0less_enhanced, "legacy") )
> + {
> + if ( hardware_domain )
> + kinfo.dom0less_feature = DOM0LESS_ENHANCED_LEGACY;
> + else
> + panic("At the moment, Xenstore support requires dom0 to be present\n");
> + }
> else if ( rc == 0 && !strcmp(dom0less_enhanced, "no-xenstore") )
> kinfo.dom0less_feature = DOM0LESS_ENHANCED_NO_XS;
>
> @@ -848,13 +855,17 @@ static int __init construct_domU(struct domain *d,
> if ( rc < 0 )
> return rc;
>
> - if ( kinfo.dom0less_feature & DOM0LESS_XENSTORE )
> + if ( kinfo.dom0less_feature & (DOM0LESS_XENSTORE|DOM0LESS_XS_LEGACY) )
Spaces around | operator.
> {
> ASSERT(hardware_domain);
> rc = alloc_xenstore_evtchn(d);
> if ( rc < 0 )
> return rc;
> + d->arch.hvm.params[HVM_PARAM_STORE_PFN] = ~0ULL;
> + }
>
> + if ( kinfo.dom0less_feature & DOM0LESS_XENSTORE )
> + {
Can I talk you into moving all of these into separate function e.g. alloc_xenstore_params(struct kernel_info *kinfo)?
It would simplify construct_domU() in which we tend to just call functions responsible for a given functionality.
> rc = alloc_xenstore_page(d);
> if ( rc < 0 )
> return rc;
> diff --git a/xen/arch/arm/include/asm/kernel.h b/xen/arch/arm/include/asm/kernel.h
> index de3f945ae5..4c2ae0b32b 100644
> --- a/xen/arch/arm/include/asm/kernel.h
> +++ b/xen/arch/arm/include/asm/kernel.h
> @@ -17,16 +17,24 @@
> * default features (excluding Xenstore) will be
> * available. Note that an OS *must* not rely on the
> * availability of Xen features if this is not set.
> - * DOM0LESS_XENSTORE: Xenstore will be enabled for the VM. This feature
> - * can't be enabled without the
> - * DOM0LESS_ENHANCED_NO_XS.
> + * DOM0LESS_XENSTORE: Xenstore will be enabled for the VM. The
> + * xenstore page allocation is done by Xen at
> + * domain creation. This feature can't be
> + * enabled without the DOM0LESS_ENHANCED_NO_XS.
> + * DOM0LESS_XS_LEGACY Xenstore will be enabled for the VM, the
> + * xenstore page allocation will happen in
> + * init-dom0less. This feature can't be enabled
> + * without the DOM0LESS_ENHANCED_NO_XS.
> * DOM0LESS_ENHANCED: Notify the OS it is running on top of Xen. All the
> * default features (including Xenstore) will be
> * available. Note that an OS *must* not rely on the
> * availability of Xen features if this is not set.
> + * DOM0LESS_ENHANCED_LEGACY:Same as before, but using DOM0LESS_XS_LEGACY.
NIT: I would just >> all text by one to have a space after :
> */
> #define DOM0LESS_ENHANCED_NO_XS BIT(0, U)
> #define DOM0LESS_XENSTORE BIT(1, U)
> +#define DOM0LESS_XS_LEGACY BIT(2, U)
> +#define DOM0LESS_ENHANCED_LEGACY (DOM0LESS_ENHANCED_NO_XS | DOM0LESS_XS_LEGACY)
> #define DOM0LESS_ENHANCED (DOM0LESS_ENHANCED_NO_XS | DOM0LESS_XENSTORE)
>
> struct kernel_info {
Otherwise, patch is ok.
~Michal
On Thu, 6 Feb 2025, Orzel, Michal wrote:
> On 06/02/2025 02:08, Stefano Stabellini wrote:
> > The new xenstore page allocation scheme might break older unpatches
> > Linux kernels that do not check for the Xenstore connection status
> > before proceeding with Xenstore initialization.
> >
> > Introduce a dom0less configuration option to retain the older behavior,
> > which is not compatible with 1:1 mapped guests, but it will work with
> The issue is for static domains in general - not only for 1:1 guests.
> Static domains without direct map will simply fail on acquire_reserved_page().
I'll clarify
> > older legacy kernel versions.
> >
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> > ---
> > docs/misc/arm/device-tree/booting.txt | 5 +++++
> > xen/arch/arm/dom0less-build.c | 13 ++++++++++++-
> > xen/arch/arm/include/asm/kernel.h | 14 +++++++++++---
> > 3 files changed, 28 insertions(+), 4 deletions(-)
> >
> > diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt
> > index ff70d44462..8fa3da95be 100644
> > --- a/docs/misc/arm/device-tree/booting.txt
> > +++ b/docs/misc/arm/device-tree/booting.txt
> > @@ -222,6 +222,11 @@ with the following properties:
> > Xen PV interfaces, including grant-table and xenstore, will be
> > enabled for the VM.
> >
> > + - "legacy"
> > + Same as above, but the way the xenstore page is allocated is not
> > + compatible with 1:1 mapped guests. On the other hand, it works with
> Same remark about 1:1
>
> > + older Linux kernels.
> > +
> > - "disabled"
> > Xen PV interfaces are disabled.
> >
> > diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
> > index 046439eb87..9afdbca8b8 100644
> > --- a/xen/arch/arm/dom0less-build.c
> > +++ b/xen/arch/arm/dom0less-build.c
> > @@ -799,6 +799,13 @@ static int __init construct_domU(struct domain *d,
> > else
> > panic("At the moment, Xenstore support requires dom0 to be present\n");
> > }
> > + else if ( rc == 0 && !strcmp(dom0less_enhanced, "legacy") )
> > + {
> > + if ( hardware_domain )
> > + kinfo.dom0less_feature = DOM0LESS_ENHANCED_LEGACY;
> > + else
> > + panic("At the moment, Xenstore support requires dom0 to be present\n");
> > + }
> > else if ( rc == 0 && !strcmp(dom0less_enhanced, "no-xenstore") )
> > kinfo.dom0less_feature = DOM0LESS_ENHANCED_NO_XS;
> >
> > @@ -848,13 +855,17 @@ static int __init construct_domU(struct domain *d,
> > if ( rc < 0 )
> > return rc;
> >
> > - if ( kinfo.dom0less_feature & DOM0LESS_XENSTORE )
> > + if ( kinfo.dom0less_feature & (DOM0LESS_XENSTORE|DOM0LESS_XS_LEGACY) )
> Spaces around | operator.
OK
>
> > {
> > ASSERT(hardware_domain);
> > rc = alloc_xenstore_evtchn(d);
> > if ( rc < 0 )
> > return rc;
> > + d->arch.hvm.params[HVM_PARAM_STORE_PFN] = ~0ULL;
> > + }
> >
> > + if ( kinfo.dom0less_feature & DOM0LESS_XENSTORE )
> > + {
> Can I talk you into moving all of these into separate function e.g. alloc_xenstore_params(struct kernel_info *kinfo)?
> It would simplify construct_domU() in which we tend to just call functions responsible for a given functionality.
OK
> > rc = alloc_xenstore_page(d);
> > if ( rc < 0 )
> > return rc;
> > diff --git a/xen/arch/arm/include/asm/kernel.h b/xen/arch/arm/include/asm/kernel.h
> > index de3f945ae5..4c2ae0b32b 100644
> > --- a/xen/arch/arm/include/asm/kernel.h
> > +++ b/xen/arch/arm/include/asm/kernel.h
> > @@ -17,16 +17,24 @@
> > * default features (excluding Xenstore) will be
> > * available. Note that an OS *must* not rely on the
> > * availability of Xen features if this is not set.
> > - * DOM0LESS_XENSTORE: Xenstore will be enabled for the VM. This feature
> > - * can't be enabled without the
> > - * DOM0LESS_ENHANCED_NO_XS.
> > + * DOM0LESS_XENSTORE: Xenstore will be enabled for the VM. The
> > + * xenstore page allocation is done by Xen at
> > + * domain creation. This feature can't be
> > + * enabled without the DOM0LESS_ENHANCED_NO_XS.
> > + * DOM0LESS_XS_LEGACY Xenstore will be enabled for the VM, the
> > + * xenstore page allocation will happen in
> > + * init-dom0less. This feature can't be enabled
> > + * without the DOM0LESS_ENHANCED_NO_XS.
> > * DOM0LESS_ENHANCED: Notify the OS it is running on top of Xen. All the
> > * default features (including Xenstore) will be
> > * available. Note that an OS *must* not rely on the
> > * availability of Xen features if this is not set.
> > + * DOM0LESS_ENHANCED_LEGACY:Same as before, but using DOM0LESS_XS_LEGACY.
> NIT: I would just >> all text by one to have a space after :
OK
> > #define DOM0LESS_ENHANCED_NO_XS BIT(0, U)
> > #define DOM0LESS_XENSTORE BIT(1, U)
> > +#define DOM0LESS_XS_LEGACY BIT(2, U)
> > +#define DOM0LESS_ENHANCED_LEGACY (DOM0LESS_ENHANCED_NO_XS | DOM0LESS_XS_LEGACY)
> > #define DOM0LESS_ENHANCED (DOM0LESS_ENHANCED_NO_XS | DOM0LESS_XENSTORE)
> >
> > struct kernel_info {
>
> Otherwise, patch is ok.
Thanks for the review
© 2016 - 2025 Red Hat, Inc.