2021-04-22 15:11:54

by Jürgen Groß

[permalink] [raw]
Subject: [PATCH 1/3] xen: check required Xen features

Linux kernel is not supported to run on Xen versions older than 4.0.

Add tests for required Xen features always being present in Xen 4.0
and newer.

Signed-off-by: Juergen Gross <[email protected]>
---
drivers/xen/features.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/drivers/xen/features.c b/drivers/xen/features.c
index 25c053b09605..60503299c9bc 100644
--- a/drivers/xen/features.c
+++ b/drivers/xen/features.c
@@ -9,13 +9,26 @@
#include <linux/types.h>
#include <linux/cache.h>
#include <linux/export.h>
+#include <linux/printk.h>

#include <asm/xen/hypercall.h>

+#include <xen/xen.h>
#include <xen/interface/xen.h>
#include <xen/interface/version.h>
#include <xen/features.h>

+/*
+ * Linux kernel expects at least Xen 4.0.
+ *
+ * Assume some features to be available for that reason (depending on guest
+ * mode, of course).
+ */
+#define chk_feature(f) { \
+ if (!xen_feature(f)) \
+ pr_err("Xen: feature %s not available!\n", #f); \
+ }
+
u8 xen_features[XENFEAT_NR_SUBMAPS * 32] __read_mostly;
EXPORT_SYMBOL_GPL(xen_features);

@@ -31,4 +44,9 @@ void xen_setup_features(void)
for (j = 0; j < 32; j++)
xen_features[i * 32 + j] = !!(fi.submap & 1<<j);
}
+
+ if (xen_pv_domain()) {
+ chk_feature(XENFEAT_mmu_pt_update_preserve_ad);
+ chk_feature(XENFEAT_gnttab_map_avail_bits);
+ }
}
--
2.26.2


2021-04-22 15:27:42

by Stefano Stabellini

[permalink] [raw]
Subject: Re: [PATCH 1/3] xen: check required Xen features

On Thu, 22 Apr 2021, Juergen Gross wrote:
> Linux kernel is not supported to run on Xen versions older than 4.0.
>
> Add tests for required Xen features always being present in Xen 4.0
> and newer.
>
> Signed-off-by: Juergen Gross <[email protected]>
> ---
> drivers/xen/features.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/xen/features.c b/drivers/xen/features.c
> index 25c053b09605..60503299c9bc 100644
> --- a/drivers/xen/features.c
> +++ b/drivers/xen/features.c
> @@ -9,13 +9,26 @@
> #include <linux/types.h>
> #include <linux/cache.h>
> #include <linux/export.h>
> +#include <linux/printk.h>
>
> #include <asm/xen/hypercall.h>
>
> +#include <xen/xen.h>
> #include <xen/interface/xen.h>
> #include <xen/interface/version.h>
> #include <xen/features.h>
>
> +/*
> + * Linux kernel expects at least Xen 4.0.
> + *
> + * Assume some features to be available for that reason (depending on guest
> + * mode, of course).
> + */
> +#define chk_feature(f) { \
> + if (!xen_feature(f)) \
> + pr_err("Xen: feature %s not available!\n", #f); \
> + }

I think this could be done as a static inline function in
include/xen/features.h. That way it would be available everywhere. Also,
static inlines are better than macro when it is possible to use them in
terms of code safety.


> u8 xen_features[XENFEAT_NR_SUBMAPS * 32] __read_mostly;
> EXPORT_SYMBOL_GPL(xen_features);
>
> @@ -31,4 +44,9 @@ void xen_setup_features(void)
> for (j = 0; j < 32; j++)
> xen_features[i * 32 + j] = !!(fi.submap & 1<<j);
> }
> +
> + if (xen_pv_domain()) {
> + chk_feature(XENFEAT_mmu_pt_update_preserve_ad);
> + chk_feature(XENFEAT_gnttab_map_avail_bits);
> + }
> }
> --
> 2.26.2
>

2021-04-22 15:33:54

by Jürgen Groß

[permalink] [raw]
Subject: Re: [PATCH 1/3] xen: check required Xen features

On 22.04.21 17:26, Stefano Stabellini wrote:
> On Thu, 22 Apr 2021, Juergen Gross wrote:
>> Linux kernel is not supported to run on Xen versions older than 4.0.
>>
>> Add tests for required Xen features always being present in Xen 4.0
>> and newer.
>>
>> Signed-off-by: Juergen Gross <[email protected]>
>> ---
>> drivers/xen/features.c | 18 ++++++++++++++++++
>> 1 file changed, 18 insertions(+)
>>
>> diff --git a/drivers/xen/features.c b/drivers/xen/features.c
>> index 25c053b09605..60503299c9bc 100644
>> --- a/drivers/xen/features.c
>> +++ b/drivers/xen/features.c
>> @@ -9,13 +9,26 @@
>> #include <linux/types.h>
>> #include <linux/cache.h>
>> #include <linux/export.h>
>> +#include <linux/printk.h>
>>
>> #include <asm/xen/hypercall.h>
>>
>> +#include <xen/xen.h>
>> #include <xen/interface/xen.h>
>> #include <xen/interface/version.h>
>> #include <xen/features.h>
>>
>> +/*
>> + * Linux kernel expects at least Xen 4.0.
>> + *
>> + * Assume some features to be available for that reason (depending on guest
>> + * mode, of course).
>> + */
>> +#define chk_feature(f) { \
>> + if (!xen_feature(f)) \
>> + pr_err("Xen: feature %s not available!\n", #f); \
>> + }
>
> I think this could be done as a static inline function in
> include/xen/features.h. That way it would be available everywhere. Also,
> static inlines are better than macro when it is possible to use them in
> terms of code safety.

It is a macro in order to have only one parameter.

And being a local macro is rendering the code safety reasoning moot.

Additionally I don't want this testing to be scattered all over the
code base. It should be done in one place only.


Juergen


Attachments:
OpenPGP_0xB0DE9DD628BF132F.asc (3.06 kB)
OpenPGP_signature (505.00 B)
OpenPGP digital signature
Download all attachments

2021-05-10 13:07:17

by Boris Ostrovsky

[permalink] [raw]
Subject: Re: [PATCH 1/3] xen: check required Xen features


On 4/22/21 11:10 AM, Juergen Gross wrote:
>
> +/*
> + * Linux kernel expects at least Xen 4.0.
> + *
> + * Assume some features to be available for that reason (depending on guest
> + * mode, of course).
> + */
> +#define chk_feature(f) { \
> + if (!xen_feature(f)) \
> + pr_err("Xen: feature %s not available!\n", #f); \
> + }


With your changes in the subsequent patches, are we still going to function properly without those features? (i.e. maybe we should just panic)


(Also, chk_required_features() perhaps?)


-boris


> +
> u8 xen_features[XENFEAT_NR_SUBMAPS * 32] __read_mostly;
> EXPORT_SYMBOL_GPL(xen_features);
>
> @@ -31,4 +44,9 @@ void xen_setup_features(void)
> for (j = 0; j < 32; j++)
> xen_features[i * 32 + j] = !!(fi.submap & 1<<j);
> }
> +
> + if (xen_pv_domain()) {
> + chk_feature(XENFEAT_mmu_pt_update_preserve_ad);
> + chk_feature(XENFEAT_gnttab_map_avail_bits);
> + }
> }

2021-05-10 13:30:49

by Jürgen Groß

[permalink] [raw]
Subject: Re: [PATCH 1/3] xen: check required Xen features

On 10.05.21 14:11, Boris Ostrovsky wrote:
>
> On 4/22/21 11:10 AM, Juergen Gross wrote:
>>
>> +/*
>> + * Linux kernel expects at least Xen 4.0.
>> + *
>> + * Assume some features to be available for that reason (depending on guest
>> + * mode, of course).
>> + */
>> +#define chk_feature(f) { \
>> + if (!xen_feature(f)) \
>> + pr_err("Xen: feature %s not available!\n", #f); \
>> + }
>
>
> With your changes in the subsequent patches, are we still going to function properly without those features? (i.e. maybe we should just panic)

Depends on the use case.

XENFEAT_gnttab_map_avail_bits is relevant for driver domains using
user space backends only. In case it is not available "interesting"
things might happen.

XENFEAT_mmu_pt_update_preserve_ad not being present would result in
a subsequent mmu-update function using that feature returning -ENOSYS,
so this wouldn't be unrecognized.

So panic() might be a good idea in case the features are not available.

> (Also, chk_required_features() perhaps?)

Fine with me.


Juergen


Attachments:
OpenPGP_0xB0DE9DD628BF132F.asc (3.06 kB)
OpenPGP_signature (505.00 B)
OpenPGP digital signature
Download all attachments