Re: [PATCH] drivers/xen/hypervisor: Expose Xen SIF flags to userspace
From: Per Bilse
Date: Mon Dec 12 2022 - 08:35:54 EST
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@xxxxxxxxxx/);
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