Re: [PATCH] drivers/xen/hypervisor: Expose VM SIF flags to userspace

[Juergen Gross <jgross@xxxxxxxx>]

On 29.11.22 16:00, 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/properties/flags,
> which will coexist with /proc/xen while dependencies are being migrated.
> Possible values are "privileged", "initdomain", "multiboot",
> "mod_start_pfn", and "virtmap", with "initdomain" being the equivalent
> of "control_d".
>
> Signed-off-by: Per Bilse <per.bilse@xxxxxxxxxx>
> ---
>   drivers/xen/sys-hypervisor.c | 26 ++++++++++++++++++++++++++
>   include/xen/interface/xen.h  | 13 ++++++++-----
>   2 files changed, 34 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/xen/sys-hypervisor.c b/drivers/xen/sys-hypervisor.c
> index fcb0792f090e..7393e04bdb6d 100644
> --- a/drivers/xen/sys-hypervisor.c
> +++ b/drivers/xen/sys-hypervisor.c
> @@ -379,6 +379,31 @@ static ssize_t buildid_show(struct hyp_sysfs_attr *attr, 
> char *buffer)
>   
>   HYPERVISOR_ATTR_RO(buildid);
>   
> +static ssize_t flags_show(struct hyp_sysfs_attr *attr, char *buffer)
> +{
> +       static char const *const sifstr[SIFN_NUM_SIFN] = {
> +               [SIFN_PRIV]  = "privileged",
> +               [SIFN_INIT]  = "initdomain",
> +               [SIFN_MULTI] = "multiboot",
> +               [SIFN_PFN]   = "mod_start_pfn",
> +               [SIFN_VIRT]  = "virtmap"
> +       };
> +       unsigned sifnum, sifmask;
> +       ssize_t ret = 0;
> +
> +       sifmask = ~(~0U << SIFN_NUM_SIFN);  // ...0000111...
> +       if (xen_domain() && (xen_start_flags & sifmask) != 0) {
> +               for (sifnum = 0; sifnum != SIFN_NUM_SIFN; sifnum++) {
> +                       if ((xen_start_flags & (1<<sifnum)) != 0)
> +                               ret += sprintf(buffer+ret, "%s ", 
> sifstr[sifnum]);
> +               }
> +               buffer[ret-1] = '\n';
> +       }
> +       return ret;
> +}
> +
> +HYPERVISOR_ATTR_RO(flags);
> +
>   static struct attribute *xen_properties_attrs[] = {
>         &capabilities_attr.attr,
>         &changeset_attr.attr,
> @@ -386,6 +411,7 @@ static struct attribute *xen_properties_attrs[] = {
>         &pagesize_attr.attr,
>         &features_attr.attr,
>         &buildid_attr.attr,
> +       &flags_attr.attr,
>         NULL
>   };
>   
> diff --git a/include/xen/interface/xen.h b/include/xen/interface/xen.h
> index 0ca23eca2a9c..762a348abe3e 100644
> --- a/include/xen/interface/xen.h
> +++ b/include/xen/interface/xen.h
> @@ -648,11 +648,14 @@ struct start_info {
>   };
>   
>   /* These flags are passed in the 'flags' field of start_info_t. */
> -#define SIF_PRIVILEGED      (1<<0)  /* Is the domain privileged? */
> -#define SIF_INITDOMAIN      (1<<1)  /* Is this the initial control domain? */
> -#define SIF_MULTIBOOT_MOD   (1<<2)  /* Is mod_start a multiboot module? */
> -#define SIF_MOD_START_PFN   (1<<3)  /* Is mod_start a PFN? */
> -#define SIF_VIRT_P2M_4TOOLS (1<<4)  /* Do Xen tools understand a virt. mapped 
> */
> +/* Text strings are printed out in sys-hypervisor.c, we guard   */
> +/* against mix-ups and errors by enumerating the flags.         */
> +enum { SIFN_PRIV, SIFN_INIT, SIFN_MULTI, SIFN_PFN, SIFN_VIRT, SIFN_NUM_SIFN };
> +#define SIF_PRIVILEGED      (1<<SIFN_PRIV)  /* Is the domain privileged? */
> +#define SIF_INITDOMAIN      (1<<SIFN_INIT)  /* Is this the initial control 
> domain? */
> +#define SIF_MULTIBOOT_MOD   (1<<SIFN_MULTI) /* Is mod_start a multiboot 
> module? */
> +#define SIF_MOD_START_PFN   (1<<SIFN_PFN)   /* Is mod_start a PFN? */
> +#define SIF_VIRT_P2M_4TOOLS (1<<SIFN_VIRT)  /* Do Xen tools understand a virt. 
> mapped */
Please don't change this header, as it is based on its master
located in the Xen repository.

An acceptable solution would be to send a Xen patch first doing the
xen.h changes, and when that patch has been taken to modify the
related Linux header accordingly.

In case you want to go that route, please add a "XEN_" prefix to the
enum members.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature