Documentation/ABI/stable/sysfs-hypervisor-xen | 8 +++ drivers/xen/sys-hypervisor.c | 70 +++++++++++++++++-- 2 files changed, 74 insertions(+), 4 deletions(-)
/proc/xen is a legacy pseudo filesystem which predates Xen support
getting merged into Linux. It has largely been replaced with more
normal locations for data (/sys/hypervisor/ for info, /dev/xen/ for
user devices). We want to compile xenfs support out of the dom0 kernel.
There is one item which only exists in /proc/xen, namely
/proc/xen/capabilities with "control_d" being the signal of "you're in
the control domain". This ultimately comes from the SIF flags provided
at VM start.
This patch exposes all SIF flags in /sys/hypervisor/start_flags/ as
boolean files, one for each bit, returning '1' if set, '0' otherwise.
Two known flags, 'privileged' and 'initdomain', are explicitly named,
and all remaining flags can be accessed via generically named files,
as suggested by Andrew Cooper.
This patch replaces previous suggestion for SIF flags exposure in its
entirety.
Signed-off-by: Per Bilse <per.bilse@citrix.com>
---
Documentation/ABI/stable/sysfs-hypervisor-xen | 8 +++
drivers/xen/sys-hypervisor.c | 70 +++++++++++++++++--
2 files changed, 74 insertions(+), 4 deletions(-)
diff --git a/Documentation/ABI/stable/sysfs-hypervisor-xen b/Documentation/ABI/stable/sysfs-hypervisor-xen
index 748593c64568..f52f98548184 100644
--- a/Documentation/ABI/stable/sysfs-hypervisor-xen
+++ b/Documentation/ABI/stable/sysfs-hypervisor-xen
@@ -120,3 +120,11 @@ Contact: xen-devel@lists.xenproject.org
Description: If running under Xen:
The Xen version is in the format <major>.<minor><extra>
This is the <minor> part of it.
+
+What: /sys/hypervisor/start_flags/*
+Date: December 2022
+KernelVersion: 6.1.0
+Contact: xen-devel@lists.xenproject.org
+Description: If running under Xen:
+ All bits in Xen's start-flags are represented as
+ boolean files, returning '1' if set, '0' otherwise.
diff --git a/drivers/xen/sys-hypervisor.c b/drivers/xen/sys-hypervisor.c
index fcb0792f090e..e15d3ff2c56f 100644
--- a/drivers/xen/sys-hypervisor.c
+++ b/drivers/xen/sys-hypervisor.c
@@ -31,7 +31,10 @@ struct hyp_sysfs_attr {
struct attribute attr;
ssize_t (*show)(struct hyp_sysfs_attr *, char *);
ssize_t (*store)(struct hyp_sysfs_attr *, const char *, size_t);
- void *hyp_attr_data;
+ union {
+ void *hyp_attr_data;
+ unsigned long hyp_attr_value;
+ };
};
static ssize_t type_show(struct hyp_sysfs_attr *attr, char *buffer)
@@ -399,6 +402,61 @@ static int __init xen_sysfs_properties_init(void)
return sysfs_create_group(hypervisor_kobj, &xen_properties_group);
}
+#define FLAG_UNAME "unknown"
+#define FLAG_UNAME_FMT FLAG_UNAME "%02u"
+#define FLAG_UNAME_MAX sizeof(FLAG_UNAME "XX")
+#define FLAG_COUNT (sizeof(xen_start_flags) * BITS_PER_BYTE)
+static_assert(sizeof(xen_start_flags)
+ <= sizeof_field(struct hyp_sysfs_attr, hyp_attr_value));
+
+static ssize_t flag_show(struct hyp_sysfs_attr *attr, char *buffer)
+{
+ char *p = buffer;
+
+ *p++ = '0' + ((xen_start_flags & attr->hyp_attr_value) != 0);
+ *p++ = '\n';
+ return p - buffer;
+}
+
+/*
+* Add new, known flags here. No other changes are required, but
+* note that each known flag wastes one entry in flag_unames[].
+* The code/complexity machinations to avoid this isn't worth it
+* for a few entries, but keep it in mind.
+*/
+static struct hyp_sysfs_attr flag_attrs[FLAG_COUNT] = {
+ [ilog2(SIF_PRIVILEGED)] = {
+ .attr = { .name = "privileged", .mode = 0444 },
+ .show = flag_show,
+ .hyp_attr_value = SIF_PRIVILEGED
+ },
+ [ilog2(SIF_INITDOMAIN)] = {
+ .attr = { .name = "initdomain", .mode = 0444 },
+ .show = flag_show,
+ .hyp_attr_value = SIF_INITDOMAIN
+ }
+};
+static struct attribute_group xen_flags_group = {
+ .name = "start_flags",
+ .attrs = (struct attribute *[FLAG_COUNT + 1]){}
+};
+static char flag_unames[FLAG_COUNT][FLAG_UNAME_MAX];
+
+static int __init xen_sysfs_flags_init(void)
+{
+ for (unsigned fnum = 0; fnum != FLAG_COUNT; fnum++) {
+ if (likely(flag_attrs[fnum].attr.name == NULL)) {
+ sprintf(flag_unames[fnum], FLAG_UNAME_FMT, fnum);
+ flag_attrs[fnum].attr.name = flag_unames[fnum];
+ flag_attrs[fnum].attr.mode = 0444;
+ flag_attrs[fnum].show = flag_show;
+ flag_attrs[fnum].hyp_attr_value = 1 << fnum;
+ }
+ xen_flags_group.attrs[fnum] = &flag_attrs[fnum].attr;
+ }
+ return sysfs_create_group(hypervisor_kobj, &xen_flags_group);
+}
+
#ifdef CONFIG_XEN_HAVE_VPMU
struct pmu_mode {
const char *name;
@@ -539,18 +597,22 @@ static int __init hyper_sysfs_init(void)
ret = xen_sysfs_properties_init();
if (ret)
goto prop_out;
+ ret = xen_sysfs_flags_init();
+ if (ret)
+ goto flags_out;
#ifdef CONFIG_XEN_HAVE_VPMU
if (xen_initial_domain()) {
ret = xen_sysfs_pmu_init();
if (ret) {
- sysfs_remove_group(hypervisor_kobj,
- &xen_properties_group);
- goto prop_out;
+ sysfs_remove_group(hypervisor_kobj, &xen_flags_group);
+ goto flags_out;
}
}
#endif
goto out;
+flags_out:
+ sysfs_remove_group(hypervisor_kobj, &xen_properties_group);
prop_out:
sysfs_remove_file(hypervisor_kobj, &uuid_attr.attr);
uuid_out:
--
2.30.2On 02.12.22 19:22, Per Bilse wrote:
> /proc/xen is a legacy pseudo filesystem which predates Xen support
> getting merged into Linux. It has largely been replaced with more
> normal locations for data (/sys/hypervisor/ for info, /dev/xen/ for
> user devices). We want to compile xenfs support out of the dom0 kernel.
>
> There is one item which only exists in /proc/xen, namely
> /proc/xen/capabilities with "control_d" being the signal of "you're in
> the control domain". This ultimately comes from the SIF flags provided
> at VM start.
>
> This patch exposes all SIF flags in /sys/hypervisor/start_flags/ as
> boolean files, one for each bit, returning '1' if set, '0' otherwise.
> Two known flags, 'privileged' and 'initdomain', are explicitly named,
> and all remaining flags can be accessed via generically named files,
> as suggested by Andrew Cooper.
>
> This patch replaces previous suggestion for SIF flags exposure in its
> entirety.
>
> Signed-off-by: Per Bilse <per.bilse@citrix.com>
> ---
> Documentation/ABI/stable/sysfs-hypervisor-xen | 8 +++
> drivers/xen/sys-hypervisor.c | 70 +++++++++++++++++--
> 2 files changed, 74 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/ABI/stable/sysfs-hypervisor-xen b/Documentation/ABI/stable/sysfs-hypervisor-xen
> index 748593c64568..f52f98548184 100644
> --- a/Documentation/ABI/stable/sysfs-hypervisor-xen
> +++ b/Documentation/ABI/stable/sysfs-hypervisor-xen
> @@ -120,3 +120,11 @@ Contact: xen-devel@lists.xenproject.org
> Description: If running under Xen:
> The Xen version is in the format <major>.<minor><extra>
> This is the <minor> part of it.
> +
> +What: /sys/hypervisor/start_flags/*
> +Date: December 2022
> +KernelVersion: 6.1.0
> +Contact: xen-devel@lists.xenproject.org
> +Description: If running under Xen:
> + All bits in Xen's start-flags are represented as
> + boolean files, returning '1' if set, '0' otherwise.
I think at least the files which want to be used by e.g. systemd
("initdomain" as replacement for the "control_d" string in capabilities,
but I think "privileged" as well) should be explicitly added to this
description, as those are meant to be used as a stable ABI.
> diff --git a/drivers/xen/sys-hypervisor.c b/drivers/xen/sys-hypervisor.c
> index fcb0792f090e..e15d3ff2c56f 100644
> --- a/drivers/xen/sys-hypervisor.c
> +++ b/drivers/xen/sys-hypervisor.c
> @@ -31,7 +31,10 @@ struct hyp_sysfs_attr {
> struct attribute attr;
> ssize_t (*show)(struct hyp_sysfs_attr *, char *);
> ssize_t (*store)(struct hyp_sysfs_attr *, const char *, size_t);
> - void *hyp_attr_data;
> + union {
> + void *hyp_attr_data;
> + unsigned long hyp_attr_value;
> + };
> };
>
> static ssize_t type_show(struct hyp_sysfs_attr *attr, char *buffer)
> @@ -399,6 +402,61 @@ static int __init xen_sysfs_properties_init(void)
> return sysfs_create_group(hypervisor_kobj, &xen_properties_group);
> }
>
> +#define FLAG_UNAME "unknown"
> +#define FLAG_UNAME_FMT FLAG_UNAME "%02u"
> +#define FLAG_UNAME_MAX sizeof(FLAG_UNAME "XX")
> +#define FLAG_COUNT (sizeof(xen_start_flags) * BITS_PER_BYTE)
> +static_assert(sizeof(xen_start_flags)
> + <= sizeof_field(struct hyp_sysfs_attr, hyp_attr_value));
> +
> +static ssize_t flag_show(struct hyp_sysfs_attr *attr, char *buffer)
> +{
> + char *p = buffer;
> +
> + *p++ = '0' + ((xen_start_flags & attr->hyp_attr_value) != 0);
> + *p++ = '\n';
> + return p - buffer;
> +}
> +
> +/*
> +* Add new, known flags here. No other changes are required, but
> +* note that each known flag wastes one entry in flag_unames[].
> +* The code/complexity machinations to avoid this isn't worth it
> +* for a few entries, but keep it in mind.
> +*/
> +static struct hyp_sysfs_attr flag_attrs[FLAG_COUNT] = {
> + [ilog2(SIF_PRIVILEGED)] = {
> + .attr = { .name = "privileged", .mode = 0444 },
> + .show = flag_show,
> + .hyp_attr_value = SIF_PRIVILEGED
> + },
What about:
#define FLAG_NODE(bit, node) \
[ilog2(bit)] = { \
.attr = { .name = #node, .mode = 0444 }, \
.show = flag_show, \
.hyp_attr_value = bit \
}
FLAG_NODE(SIF_PRIVILEGED, privileged),
FLAG_NODE(SIF_INITDOMAIN, initdomain)
> + [ilog2(SIF_INITDOMAIN)] = {
> + .attr = { .name = "initdomain", .mode = 0444 },
> + .show = flag_show,
> + .hyp_attr_value = SIF_INITDOMAIN
> + }
In order to avoid a consumer having to look into the sources for any other
set flag, maybe add names for currently defined flags, too? Or just skip
the other flags and add a single additional node ("flags"?) with the whole
value of xen_start_flags either in hex or binary form?
Please note that this is a suggestion only, I'm not insisting on it.
Juergen
On 09/12/2022 09:04, Juergen Gross wrote:
> On 02.12.22 19:22, Per Bilse wrote:
>> +Description: If running under Xen:
>> + All bits in Xen's start-flags are represented as
>> + boolean files, returning '1' if set, '0' otherwise.
>
> I think at least the files which want to be used by e.g. systemd
> ("initdomain" as replacement for the "control_d" string in capabilities,
> but I think "privileged" as well) should be explicitly added to this
> description, as those are meant to be used as a stable ABI.
OK, yes, it's a bit vague as is.
>> +static struct hyp_sysfs_attr flag_attrs[FLAG_COUNT] = {
>> + [ilog2(SIF_PRIVILEGED)] = {
>> + .attr = { .name = "privileged", .mode = 0444 },
>> + .show = flag_show,
>> + .hyp_attr_value = SIF_PRIVILEGED
>> + },
>
> What about:
>
> #define FLAG_NODE(bit, node) \
> [ilog2(bit)] = { \
> .attr = { .name = #node, .mode = 0444 }, \
> .show = flag_show, \
> .hyp_attr_value = bit \
> }
Yes ... I was thinking that maybe flags wouldn't normally share most
characteristics, but they probably do.
> In order to avoid a consumer having to look into the sources for any other
> set flag, maybe add names for currently defined flags, too? Or just skip > the other flags and add a single additional node ("flags"?) with the
whole
> value of xen_start_flags either in hex or binary form?
The current format is as suggested by Andrew Cooper in email about a
previous incarnation
(https://lore.kernel.org/lkml/92300a81-b97b-f5d6-e3e8-363d8a7d3742@citrix.com/);
specifically, that of the known flags only initdomain and privileged are
relevant outside a kernel context, and that all other flags be exposed
individually.
> Please note that this is a suggestion only, I'm not insisting on it.
Please, many thanks for the feedback, it's always good to hear other
people's thoughts.
Best,
-- Per
© 2016 - 2024 Red Hat, Inc.