| Message ID | 20250207015341.1208429-6-stefano.stabellini@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Guest XenStore page allocation for 11 Dom0less domUs | expand |
On 07/02/2025 02:53, Stefano Stabellini wrote: > The new xenstore page allocation scheme might break older unpatches s/unpatches/unpatched > 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. > > The older behavior triggered by this option is to allocate the xenstore > page in init-dom0less. That does not work with static-mem guests. > However, it will make it possible to run as regular guests older Linux > kernel versions that are left unpatched. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com> > --- > Changes in v6: > - improve wording in commit message and doc > - code style > - add separate alloc_xenstore_params function > > docs/misc/arm/device-tree/booting.txt | 5 +++ > xen/arch/arm/dom0less-build.c | 45 +++++++++++++++++++-------- > xen/arch/arm/include/asm/kernel.h | 30 +++++++++++------- > 3 files changed, 56 insertions(+), 24 deletions(-) > > diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt > index 9c881baccc..4d6d859c66 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 static-mem 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 6c51f91999..56eb5c6da6 100644 > --- a/xen/arch/arm/dom0less-build.c > +++ b/xen/arch/arm/dom0less-build.c > @@ -755,6 +755,30 @@ static int __init alloc_xenstore_page(struct domain *d) > return 0; > } > > +static int __init alloc_xenstore_params(struct domain *d, > + struct kernel_info *kinfo) NIT: We should not pass struct domain if we pass struct kernel_info from which we can simply derive domain pointer. I see this as a bad code practice. I know we have many functions like that but for new functions I try not to repeat this. > +{ > + int rc = 0; > + > + 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; > + } > + > + return rc; > +} > + > static int __init construct_domU(struct domain *d, > const struct dt_device_node *node) > { > @@ -800,6 +824,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; > > @@ -849,19 +880,7 @@ static int __init construct_domU(struct domain *d, > if ( rc < 0 ) > return rc; > > - if ( kinfo.dom0less_feature & DOM0LESS_XENSTORE ) > - { > - ASSERT(hardware_domain); > - rc = alloc_xenstore_evtchn(d); > - if ( rc < 0 ) > - return rc; > - > - rc = alloc_xenstore_page(d); > - if ( rc < 0 ) > - return rc; > - } > - > - return rc; > + return alloc_xenstore_params(d, &kinfo); > } > > void __init create_domUs(void) > diff --git a/xen/arch/arm/include/asm/kernel.h b/xen/arch/arm/include/asm/kernel.h > index de3f945ae5..bdc96f4c18 100644 > --- a/xen/arch/arm/include/asm/kernel.h > +++ b/xen/arch/arm/include/asm/kernel.h > @@ -13,20 +13,28 @@ > /* > * List of possible features for dom0less domUs > * > - * DOM0LESS_ENHANCED_NO_XS: Notify the OS it is running on top of Xen. All the > - * 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_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_NO_XS: Notify the OS it is running on top of Xen. All the > + * 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. 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 { Other than that: Reviewed-by: Michal Orzel <michal.orzel@amd.com> ~Michal
diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt index 9c881baccc..4d6d859c66 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 static-mem 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 6c51f91999..56eb5c6da6 100644 --- a/xen/arch/arm/dom0less-build.c +++ b/xen/arch/arm/dom0less-build.c @@ -755,6 +755,30 @@ static int __init alloc_xenstore_page(struct domain *d) return 0; } +static int __init alloc_xenstore_params(struct domain *d, + struct kernel_info *kinfo) +{ + int rc = 0; + + 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; + } + + return rc; +} + static int __init construct_domU(struct domain *d, const struct dt_device_node *node) { @@ -800,6 +824,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; @@ -849,19 +880,7 @@ static int __init construct_domU(struct domain *d, if ( rc < 0 ) return rc; - if ( kinfo.dom0less_feature & DOM0LESS_XENSTORE ) - { - ASSERT(hardware_domain); - rc = alloc_xenstore_evtchn(d); - if ( rc < 0 ) - return rc; - - rc = alloc_xenstore_page(d); - if ( rc < 0 ) - return rc; - } - - return rc; + return alloc_xenstore_params(d, &kinfo); } void __init create_domUs(void) diff --git a/xen/arch/arm/include/asm/kernel.h b/xen/arch/arm/include/asm/kernel.h index de3f945ae5..bdc96f4c18 100644 --- a/xen/arch/arm/include/asm/kernel.h +++ b/xen/arch/arm/include/asm/kernel.h @@ -13,20 +13,28 @@ /* * List of possible features for dom0less domUs * - * DOM0LESS_ENHANCED_NO_XS: Notify the OS it is running on top of Xen. All the - * 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_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_NO_XS: Notify the OS it is running on top of Xen. All the + * 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. 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 {
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. The older behavior triggered by this option is to allocate the xenstore page in init-dom0less. That does not work with static-mem guests. However, it will make it possible to run as regular guests older Linux kernel versions that are left unpatched. Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com> --- Changes in v6: - improve wording in commit message and doc - code style - add separate alloc_xenstore_params function docs/misc/arm/device-tree/booting.txt | 5 +++ xen/arch/arm/dom0less-build.c | 45 +++++++++++++++++++-------- xen/arch/arm/include/asm/kernel.h | 30 +++++++++++------- 3 files changed, 56 insertions(+), 24 deletions(-)