[PATCH] KVM: Start populating /sys/hypervisor with KVM entries

[PATCH] KVM: Start populating /sys/hypervisor with KVM entries

[Filippo Sironi]

Start populating /sys/hypervisor with KVM entries when we're running on
KVM. This is to replicate functionality that's available when we're
running on Xen.

Let's start with /sys/hypervisor/uuid, which users prefer over
/sys/devices/virtual/dmi/id/product_uuid as a way to recognize a virtual
machine, since it's also available when running on Xen HVM and on Xen PV
and, on top of that doesn't require root privileges by default.

Signed-off-by: Filippo Sironi <sironi@amazon.de>
---
 drivers/Kconfig              |  2 ++
 drivers/Makefile             |  2 ++
 drivers/kvm/Kconfig          | 14 ++++++++++++++
 drivers/kvm/Makefile         |  1 +
 drivers/kvm/sys-hypervisor.c | 26 ++++++++++++++++++++++++++
 5 files changed, 45 insertions(+)
 create mode 100644 drivers/kvm/Kconfig
 create mode 100644 drivers/kvm/Makefile
 create mode 100644 drivers/kvm/sys-hypervisor.c

diff --git a/drivers/Kconfig b/drivers/Kconfig
index afc942c54814..597519c5f7c8 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -135,6 +135,8 @@ source "drivers/hv/Kconfig"
 
 source "drivers/xen/Kconfig"
 
+source "drivers/kvm/Kconfig"
+
 source "drivers/staging/Kconfig"
 
 source "drivers/platform/Kconfig"
diff --git a/drivers/Makefile b/drivers/Makefile
index 1056f9699192..727205e287fc 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -47,6 +47,8 @@ obj-y				+= soc/
 obj-$(CONFIG_VIRTIO)		+= virtio/
 obj-$(CONFIG_XEN)		+= xen/
 
+obj-$(CONFIG_KVM_GUEST)		+= kvm/
+
 # regulators early, since some subsystems rely on them to initialize
 obj-$(CONFIG_REGULATOR)		+= regulator/
 
diff --git a/drivers/kvm/Kconfig b/drivers/kvm/Kconfig
new file mode 100644
index 000000000000..3fc041df7c11
--- /dev/null
+++ b/drivers/kvm/Kconfig
@@ -0,0 +1,14 @@
+menu "KVM driver support"
+        depends on KVM_GUEST
+
+config KVM_SYS_HYPERVISOR
+        bool "Create KVM entries under /sys/hypervisor"
+        depends on SYSFS
+        select SYS_HYPERVISOR
+        default y
+        help
+          Create KVM entries under /sys/hypervisor (e.g., uuid). When running
+          native or on another hypervisor, /sys/hypervisor may still be
+          present, but it will have no KVM entries.
+
+endmenu
diff --git a/drivers/kvm/Makefile b/drivers/kvm/Makefile
new file mode 100644
index 000000000000..73a43fc994b9
--- /dev/null
+++ b/drivers/kvm/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_KVM_SYS_HYPERVISOR) += sys-hypervisor.o
diff --git a/drivers/kvm/sys-hypervisor.c b/drivers/kvm/sys-hypervisor.c
new file mode 100644
index 000000000000..ef04ca65cf1a
--- /dev/null
+++ b/drivers/kvm/sys-hypervisor.c
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include <asm/kvm_para.h>
+
+#include <linux/dmi.h>
+#include <linux/kobject.h>
+#include <linux/sysfs.h>
+
+static ssize_t uuid_show(struct kobject *obj,
+			 struct kobj_attribute *attr,
+			 char *buf)
+{
+	const char *uuid = dmi_get_system_info(DMI_PRODUCT_UUID);
+	return sprintf(buf, "%s\n", uuid);
+}
+
+static struct kobj_attribute uuid = __ATTR_RO(uuid);
+
+static int __init uuid_init(void)
+{
+	if (!kvm_para_available())
+		return 0;
+	return sysfs_create_file(hypervisor_kobj, &uuid.attr);
+}
+
+device_initcall(uuid_init);
-- 
2.7.4

Re: [PATCH] KVM: Start populating /sys/hypervisor with KVM entries

[Christian Borntraeger]

On 10/09/2018 11:54 AM, Filippo Sironi wrote:
> Start populating /sys/hypervisor with KVM entries when we're running on
> KVM. This is to replicate functionality that's available when we're
> running on Xen.
> 
> Let's start with /sys/hypervisor/uuid, which users prefer over
> /sys/devices/virtual/dmi/id/product_uuid as a way to recognize a virtual
> machine, since it's also available when running on Xen HVM and on Xen PV
> and, on top of that doesn't require root privileges by default.

Can you make this an arch hook? On s390 it is possible to get the uuid with
the stsi instruction. 
See
https://git.kernel.org/pub/scm/virt/kvm/kvm.git/tree/arch/s390/kernel/sysinfo.c#n248


We do use uuid_t, but we can certainly return a char*.




> 
> Signed-off-by: Filippo Sironi <sironi@amazon.de>
> ---
>  drivers/Kconfig              |  2 ++
>  drivers/Makefile             |  2 ++
>  drivers/kvm/Kconfig          | 14 ++++++++++++++
>  drivers/kvm/Makefile         |  1 +
>  drivers/kvm/sys-hypervisor.c | 26 ++++++++++++++++++++++++++
>  5 files changed, 45 insertions(+)
>  create mode 100644 drivers/kvm/Kconfig
>  create mode 100644 drivers/kvm/Makefile
>  create mode 100644 drivers/kvm/sys-hypervisor.c
> 
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index afc942c54814..597519c5f7c8 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -135,6 +135,8 @@ source "drivers/hv/Kconfig"
> 
>  source "drivers/xen/Kconfig"
> 
> +source "drivers/kvm/Kconfig"
> +
>  source "drivers/staging/Kconfig"
> 
>  source "drivers/platform/Kconfig"
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 1056f9699192..727205e287fc 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -47,6 +47,8 @@ obj-y				+= soc/
>  obj-$(CONFIG_VIRTIO)		+= virtio/
>  obj-$(CONFIG_XEN)		+= xen/
> 
> +obj-$(CONFIG_KVM_GUEST)		+= kvm/
> +
>  # regulators early, since some subsystems rely on them to initialize
>  obj-$(CONFIG_REGULATOR)		+= regulator/
> 
> diff --git a/drivers/kvm/Kconfig b/drivers/kvm/Kconfig
> new file mode 100644
> index 000000000000..3fc041df7c11
> --- /dev/null
> +++ b/drivers/kvm/Kconfig
> @@ -0,0 +1,14 @@
> +menu "KVM driver support"
> +        depends on KVM_GUEST
> +
> +config KVM_SYS_HYPERVISOR
> +        bool "Create KVM entries under /sys/hypervisor"
> +        depends on SYSFS
> +        select SYS_HYPERVISOR
> +        default y
> +        help
> +          Create KVM entries under /sys/hypervisor (e.g., uuid). When running
> +          native or on another hypervisor, /sys/hypervisor may still be
> +          present, but it will have no KVM entries.
> +
> +endmenu
> diff --git a/drivers/kvm/Makefile b/drivers/kvm/Makefile
> new file mode 100644
> index 000000000000..73a43fc994b9
> --- /dev/null
> +++ b/drivers/kvm/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_KVM_SYS_HYPERVISOR) += sys-hypervisor.o
> diff --git a/drivers/kvm/sys-hypervisor.c b/drivers/kvm/sys-hypervisor.c
> new file mode 100644
> index 000000000000..ef04ca65cf1a
> --- /dev/null
> +++ b/drivers/kvm/sys-hypervisor.c
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#include <asm/kvm_para.h>
> +
> +#include <linux/dmi.h>
> +#include <linux/kobject.h>
> +#include <linux/sysfs.h>
> +
> +static ssize_t uuid_show(struct kobject *obj,
> +			 struct kobj_attribute *attr,
> +			 char *buf)
> +{
> +	const char *uuid = dmi_get_system_info(DMI_PRODUCT_UUID);
> +	return sprintf(buf, "%s\n", uuid);
> +}
> +
> +static struct kobj_attribute uuid = __ATTR_RO(uuid);
> +
> +static int __init uuid_init(void)
> +{
> +	if (!kvm_para_available())
> +		return 0;
> +	return sysfs_create_file(hypervisor_kobj, &uuid.attr);
> +}
> +
> +device_initcall(uuid_init);
> 

Re: [PATCH] KVM: Start populating /sys/hypervisor with KVM entries

[Boris Ostrovsky]

On 10/9/18 6:41 AM, Christian Borntraeger wrote:
> 
> 
> On 10/09/2018 11:54 AM, Filippo Sironi wrote:
>> Start populating /sys/hypervisor with KVM entries when we're running on
>> KVM. This is to replicate functionality that's available when we're
>> running on Xen.
>>
>> Let's start with /sys/hypervisor/uuid, which users prefer over
>> /sys/devices/virtual/dmi/id/product_uuid as a way to recognize a virtual
>> machine, since it's also available when running on Xen HVM and on Xen PV
>> and, on top of that doesn't require root privileges by default.
> 
> Can you make this an arch hook? On s390 it is possible to get the uuid with
> the stsi instruction. 
> See
> https://git.kernel.org/pub/scm/virt/kvm/kvm.git/tree/arch/s390/kernel/sysinfo.c#n248
> 
> 
> We do use uuid_t, but we can certainly return a char*.


I would suggest having a top-level sys-hypervisor.c that will create
common files that all hypervisors should have (uuid, type, version, etc)
and then have hypervisor- and/or arch-specific hooks.


-boris

Re: [PATCH] KVM: Start populating /sys/hypervisor with KVM entries

[Cornelia Huck]

On Tue, 9 Oct 2018 12:21:46 -0400
Boris Ostrovsky <boris.ostrovsky@oracle.com> wrote:

> On 10/9/18 6:41 AM, Christian Borntraeger wrote:
> > 
> > 
> > On 10/09/2018 11:54 AM, Filippo Sironi wrote:  
> >> Start populating /sys/hypervisor with KVM entries when we're running on
> >> KVM. This is to replicate functionality that's available when we're
> >> running on Xen.
> >>
> >> Let's start with /sys/hypervisor/uuid, which users prefer over
> >> /sys/devices/virtual/dmi/id/product_uuid as a way to recognize a virtual
> >> machine, since it's also available when running on Xen HVM and on Xen PV
> >> and, on top of that doesn't require root privileges by default.  
> > 
> > Can you make this an arch hook? On s390 it is possible to get the uuid with
> > the stsi instruction. 
> > See
> > https://git.kernel.org/pub/scm/virt/kvm/kvm.git/tree/arch/s390/kernel/sysinfo.c#n248
> > 
> > 
> > We do use uuid_t, but we can certainly return a char*.  
> 
> 
> I would suggest having a top-level sys-hypervisor.c that will create
> common files that all hypervisors should have (uuid, type, version, etc)
> and then have hypervisor- and/or arch-specific hooks.

I think we really need *both* hypervisor- and arch-specific hooks.

Re: [PATCH] KVM: Start populating /sys/hypervisor with KVM entries

[Konrad Rzeszutek Wilk]

On Tue, Oct 09, 2018 at 11:54:39AM +0200, Filippo Sironi wrote:
> Start populating /sys/hypervisor with KVM entries when we're running on
> KVM. This is to replicate functionality that's available when we're
> running on Xen.
> 
> Let's start with /sys/hypervisor/uuid, which users prefer over
> /sys/devices/virtual/dmi/id/product_uuid as a way to recognize a virtual
> machine, since it's also available when running on Xen HVM and on Xen PV
> and, on top of that doesn't require root privileges by default.

Yeey! Thank you for doing this. It has been on my TODO but just never
got to it.

I think you also need to update the Documentation/../sysfs file.
> 
> Signed-off-by: Filippo Sironi <sironi@amazon.de>
> ---
>  drivers/Kconfig              |  2 ++
>  drivers/Makefile             |  2 ++
>  drivers/kvm/Kconfig          | 14 ++++++++++++++
>  drivers/kvm/Makefile         |  1 +
>  drivers/kvm/sys-hypervisor.c | 26 ++++++++++++++++++++++++++
>  5 files changed, 45 insertions(+)
>  create mode 100644 drivers/kvm/Kconfig
>  create mode 100644 drivers/kvm/Makefile
>  create mode 100644 drivers/kvm/sys-hypervisor.c
> 
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index afc942c54814..597519c5f7c8 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -135,6 +135,8 @@ source "drivers/hv/Kconfig"
>  
>  source "drivers/xen/Kconfig"
>  
> +source "drivers/kvm/Kconfig"
> +
>  source "drivers/staging/Kconfig"
>  
>  source "drivers/platform/Kconfig"
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 1056f9699192..727205e287fc 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -47,6 +47,8 @@ obj-y				+= soc/
>  obj-$(CONFIG_VIRTIO)		+= virtio/
>  obj-$(CONFIG_XEN)		+= xen/
>  
> +obj-$(CONFIG_KVM_GUEST)		+= kvm/
> +
>  # regulators early, since some subsystems rely on them to initialize
>  obj-$(CONFIG_REGULATOR)		+= regulator/
>  
> diff --git a/drivers/kvm/Kconfig b/drivers/kvm/Kconfig
> new file mode 100644
> index 000000000000..3fc041df7c11
> --- /dev/null
> +++ b/drivers/kvm/Kconfig
> @@ -0,0 +1,14 @@
> +menu "KVM driver support"
> +        depends on KVM_GUEST
> +
> +config KVM_SYS_HYPERVISOR
> +        bool "Create KVM entries under /sys/hypervisor"
> +        depends on SYSFS
> +        select SYS_HYPERVISOR
> +        default y
> +        help
> +          Create KVM entries under /sys/hypervisor (e.g., uuid). When running
> +          native or on another hypervisor, /sys/hypervisor may still be
> +          present, but it will have no KVM entries.
> +
> +endmenu
> diff --git a/drivers/kvm/Makefile b/drivers/kvm/Makefile
> new file mode 100644
> index 000000000000..73a43fc994b9
> --- /dev/null
> +++ b/drivers/kvm/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_KVM_SYS_HYPERVISOR) += sys-hypervisor.o
> diff --git a/drivers/kvm/sys-hypervisor.c b/drivers/kvm/sys-hypervisor.c
> new file mode 100644
> index 000000000000..ef04ca65cf1a
> --- /dev/null
> +++ b/drivers/kvm/sys-hypervisor.c
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#include <asm/kvm_para.h>
> +
> +#include <linux/dmi.h>
> +#include <linux/kobject.h>
> +#include <linux/sysfs.h>
> +
> +static ssize_t uuid_show(struct kobject *obj,
> +			 struct kobj_attribute *attr,
> +			 char *buf)
> +{
> +	const char *uuid = dmi_get_system_info(DMI_PRODUCT_UUID);
> +	return sprintf(buf, "%s\n", uuid);
> +}
> +
> +static struct kobj_attribute uuid = __ATTR_RO(uuid);
> +
> +static int __init uuid_init(void)
> +{
> +	if (!kvm_para_available())
> +		return 0;
> +	return sysfs_create_file(hypervisor_kobj, &uuid.attr);
> +}
> +
> +device_initcall(uuid_init);
> -- 
> 2.7.4
> 

Re: [PATCH] KVM: Start populating /sys/hypervisor with KVM entries

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 7417 bytes --]

Hi Filippo,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linux-sof-driver/master]
[also build test ERROR on v4.19-rc7 next-20181009]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Filippo-Sironi/KVM-Start-populating-sys-hypervisor-with-KVM-entries/20181010-064236
base:   https://github.com/thesofproject/linux master
config: powerpc64-allmodconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=powerpc64 

All error/warnings (new ones prefixed by >>):

   In file included from arch/powerpc/include/uapi/asm/kvm_para.h:82:0,
                    from arch/powerpc/include/asm/kvm_para.h:22,
                    from drivers//kvm/sys-hypervisor.c:3:
>> arch/powerpc/include/asm/epapr_hcalls.h:109:12: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'epapr_paravirt_early_init'
    int __init epapr_paravirt_early_init(void);
               ^~~~~~~~~~~~~~~~~~~~~~~~~
   In file included from arch/powerpc/include/asm/epapr_hcalls.h:53:0,
                    from arch/powerpc/include/uapi/asm/kvm_para.h:82,
                    from arch/powerpc/include/asm/kvm_para.h:22,
                    from drivers//kvm/sys-hypervisor.c:3:
   arch/powerpc/include/asm/kvm_para.h: In function 'kvm_arch_para_features':
>> arch/powerpc/include/asm/kvm_para.h:58:40: error: 'KVM_HC_FEATURES' undeclared (first use in this function); did you mean 'KVM_HCALL_TOKEN'?
     if(epapr_hypercall0_1(KVM_HCALL_TOKEN(KVM_HC_FEATURES), &r))
                                           ^
   arch/powerpc/include/uapi/asm/epapr_hcalls.h:75:51: note: in definition of macro '_EV_HCALL_TOKEN'
    #define _EV_HCALL_TOKEN(id, num) (((id) << 16) | (num))
                                                      ^~~
>> arch/powerpc/include/asm/kvm_para.h:58:24: note: in expansion of macro 'KVM_HCALL_TOKEN'
     if(epapr_hypercall0_1(KVM_HCALL_TOKEN(KVM_HC_FEATURES), &r))
                           ^~~~~~~~~~~~~~~
   arch/powerpc/include/asm/kvm_para.h:58:40: note: each undeclared identifier is reported only once for each function it appears in
     if(epapr_hypercall0_1(KVM_HCALL_TOKEN(KVM_HC_FEATURES), &r))
                                           ^
   arch/powerpc/include/uapi/asm/epapr_hcalls.h:75:51: note: in definition of macro '_EV_HCALL_TOKEN'
    #define _EV_HCALL_TOKEN(id, num) (((id) << 16) | (num))
                                                      ^~~
>> arch/powerpc/include/asm/kvm_para.h:58:24: note: in expansion of macro 'KVM_HCALL_TOKEN'
     if(epapr_hypercall0_1(KVM_HCALL_TOKEN(KVM_HC_FEATURES), &r))
                           ^~~~~~~~~~~~~~~

vim +58 arch/powerpc/include/asm/kvm_para.h

bbf45ba5 include/asm-powerpc/kvm_para.h      Hollis Blanchard 2008-04-16  21  
c3617f72 arch/powerpc/include/asm/kvm_para.h David Howells    2012-10-09 @22  #include <uapi/asm/kvm_para.h>
bbf45ba5 include/asm-powerpc/kvm_para.h      Hollis Blanchard 2008-04-16  23  
2a342ed5 arch/powerpc/include/asm/kvm_para.h Alexander Graf   2010-07-29  24  #ifdef CONFIG_KVM_GUEST
2a342ed5 arch/powerpc/include/asm/kvm_para.h Alexander Graf   2010-07-29  25  
26e673c3 arch/powerpc/include/asm/kvm_para.h Alexander Graf   2010-09-03  26  #include <linux/of.h>
26e673c3 arch/powerpc/include/asm/kvm_para.h Alexander Graf   2010-09-03  27  
2a342ed5 arch/powerpc/include/asm/kvm_para.h Alexander Graf   2010-07-29  28  static inline int kvm_para_available(void)
2a342ed5 arch/powerpc/include/asm/kvm_para.h Alexander Graf   2010-07-29  29  {
2a342ed5 arch/powerpc/include/asm/kvm_para.h Alexander Graf   2010-07-29  30  	struct device_node *hyper_node;
2a342ed5 arch/powerpc/include/asm/kvm_para.h Alexander Graf   2010-07-29  31  
2a342ed5 arch/powerpc/include/asm/kvm_para.h Alexander Graf   2010-07-29  32  	hyper_node = of_find_node_by_path("/hypervisor");
2a342ed5 arch/powerpc/include/asm/kvm_para.h Alexander Graf   2010-07-29  33  	if (!hyper_node)
2a342ed5 arch/powerpc/include/asm/kvm_para.h Alexander Graf   2010-07-29  34  		return 0;
2a342ed5 arch/powerpc/include/asm/kvm_para.h Alexander Graf   2010-07-29  35  
2a342ed5 arch/powerpc/include/asm/kvm_para.h Alexander Graf   2010-07-29  36  	if (!of_device_is_compatible(hyper_node, "linux,kvm"))
2a342ed5 arch/powerpc/include/asm/kvm_para.h Alexander Graf   2010-07-29  37  		return 0;
2a342ed5 arch/powerpc/include/asm/kvm_para.h Alexander Graf   2010-07-29  38  
2a342ed5 arch/powerpc/include/asm/kvm_para.h Alexander Graf   2010-07-29  39  	return 1;
2a342ed5 arch/powerpc/include/asm/kvm_para.h Alexander Graf   2010-07-29  40  }
2a342ed5 arch/powerpc/include/asm/kvm_para.h Alexander Graf   2010-07-29  41  
2a342ed5 arch/powerpc/include/asm/kvm_para.h Alexander Graf   2010-07-29  42  #else
2a342ed5 arch/powerpc/include/asm/kvm_para.h Alexander Graf   2010-07-29  43  
bbf45ba5 include/asm-powerpc/kvm_para.h      Hollis Blanchard 2008-04-16  44  static inline int kvm_para_available(void)
bbf45ba5 include/asm-powerpc/kvm_para.h      Hollis Blanchard 2008-04-16  45  {
bbf45ba5 include/asm-powerpc/kvm_para.h      Hollis Blanchard 2008-04-16  46  	return 0;
bbf45ba5 include/asm-powerpc/kvm_para.h      Hollis Blanchard 2008-04-16  47  }
bbf45ba5 include/asm-powerpc/kvm_para.h      Hollis Blanchard 2008-04-16  48  
2a342ed5 arch/powerpc/include/asm/kvm_para.h Alexander Graf   2010-07-29  49  #endif
2a342ed5 arch/powerpc/include/asm/kvm_para.h Alexander Graf   2010-07-29  50  
bbf45ba5 include/asm-powerpc/kvm_para.h      Hollis Blanchard 2008-04-16  51  static inline unsigned int kvm_arch_para_features(void)
bbf45ba5 include/asm-powerpc/kvm_para.h      Hollis Blanchard 2008-04-16  52  {
2a342ed5 arch/powerpc/include/asm/kvm_para.h Alexander Graf   2010-07-29  53  	unsigned long r;
2a342ed5 arch/powerpc/include/asm/kvm_para.h Alexander Graf   2010-07-29  54  
2a342ed5 arch/powerpc/include/asm/kvm_para.h Alexander Graf   2010-07-29  55  	if (!kvm_para_available())
bbf45ba5 include/asm-powerpc/kvm_para.h      Hollis Blanchard 2008-04-16  56  		return 0;
2a342ed5 arch/powerpc/include/asm/kvm_para.h Alexander Graf   2010-07-29  57  
b1f0d94c arch/powerpc/include/asm/kvm_para.h Bharat Bhushan   2013-10-08 @58  	if(epapr_hypercall0_1(KVM_HCALL_TOKEN(KVM_HC_FEATURES), &r))
2a342ed5 arch/powerpc/include/asm/kvm_para.h Alexander Graf   2010-07-29  59  		return 0;
2a342ed5 arch/powerpc/include/asm/kvm_para.h Alexander Graf   2010-07-29  60  
2a342ed5 arch/powerpc/include/asm/kvm_para.h Alexander Graf   2010-07-29  61  	return r;
bbf45ba5 include/asm-powerpc/kvm_para.h      Hollis Blanchard 2008-04-16  62  }
bbf45ba5 include/asm-powerpc/kvm_para.h      Hollis Blanchard 2008-04-16  63  

:::::: The code at line 58 was first introduced by commit
:::::: b1f0d94c26b64e814243b736f47e7ef40d96432c kvm/powerpc: move kvm_hypercall0() and friends to epapr_hypercall0()

:::::: TO: Bharat Bhushan <r65777@freescale.com>
:::::: CC: Alexander Graf <agraf@suse.de>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 56064 bytes --]

KVM: Start populating /sys/hypervisor with KVM entries

[Filippo Sironi]

Long-time Xen HVM and Xen PV users are missing /sys/hypervisor entries when
moving to KVM.  One report is about getting the VM UUID.  The VM UUID can
already be retrieved using /sys/devices/virtual/dmi/id/product_uuid.  This has
two downsides: (1) it requires root privileges and (2) it is only available on
KVM and Xen HVM.

By exposing /sys/hypervisor/uuid when running on KVM as well, we provide an
interface that's functional for KVM, Xen HVM, and Xen PV.  Let's do so by
providing arch-specific hooks so that different architectures can implement the
hooks in different ways.

Further work can be done by consolidating the creation of the basic
/sys/hypervisor across hypervisors.

Filippo Sironi (2):
      KVM: Start populating /sys/hypervisor with KVM entries
      KVM: x86: Implement the arch-specific hook to report the VM UUID

[PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries

[Filippo Sironi]

Start populating /sys/hypervisor with KVM entries when we're running on
KVM. This is to replicate functionality that's available when we're
running on Xen.

Start with /sys/hypervisor/uuid, which users prefer over
/sys/devices/virtual/dmi/id/product_uuid as a way to recognize a virtual
machine, since it's also available when running on Xen HVM and on Xen PV
and, on top of that doesn't require root privileges by default.
Let's create arch-specific hooks so that different architectures can
provide different implementations.

Signed-off-by: Filippo Sironi <sironi@amazon.de>
---
v2:
* move the retrieval of the VM UUID out of uuid_show and into
  kvm_para_get_uuid, which is a weak function that can be overwritten

 drivers/Kconfig              |  2 ++
 drivers/Makefile             |  2 ++
 drivers/kvm/Kconfig          | 14 ++++++++++++++
 drivers/kvm/Makefile         |  1 +
 drivers/kvm/sys-hypervisor.c | 30 ++++++++++++++++++++++++++++++
 5 files changed, 49 insertions(+)
 create mode 100644 drivers/kvm/Kconfig
 create mode 100644 drivers/kvm/Makefile
 create mode 100644 drivers/kvm/sys-hypervisor.c

diff --git a/drivers/Kconfig b/drivers/Kconfig
index 45f9decb9848..90eb835fe951 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -146,6 +146,8 @@ source "drivers/hv/Kconfig"
 
 source "drivers/xen/Kconfig"
 
+source "drivers/kvm/Kconfig"
+
 source "drivers/staging/Kconfig"
 
 source "drivers/platform/Kconfig"
diff --git a/drivers/Makefile b/drivers/Makefile
index c61cde554340..79cc92a3f6bf 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -44,6 +44,8 @@ obj-y				+= soc/
 obj-$(CONFIG_VIRTIO)		+= virtio/
 obj-$(CONFIG_XEN)		+= xen/
 
+obj-$(CONFIG_KVM_GUEST)		+= kvm/
+
 # regulators early, since some subsystems rely on them to initialize
 obj-$(CONFIG_REGULATOR)		+= regulator/
 
diff --git a/drivers/kvm/Kconfig b/drivers/kvm/Kconfig
new file mode 100644
index 000000000000..3fc041df7c11
--- /dev/null
+++ b/drivers/kvm/Kconfig
@@ -0,0 +1,14 @@
+menu "KVM driver support"
+        depends on KVM_GUEST
+
+config KVM_SYS_HYPERVISOR
+        bool "Create KVM entries under /sys/hypervisor"
+        depends on SYSFS
+        select SYS_HYPERVISOR
+        default y
+        help
+          Create KVM entries under /sys/hypervisor (e.g., uuid). When running
+          native or on another hypervisor, /sys/hypervisor may still be
+          present, but it will have no KVM entries.
+
+endmenu
diff --git a/drivers/kvm/Makefile b/drivers/kvm/Makefile
new file mode 100644
index 000000000000..73a43fc994b9
--- /dev/null
+++ b/drivers/kvm/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_KVM_SYS_HYPERVISOR) += sys-hypervisor.o
diff --git a/drivers/kvm/sys-hypervisor.c b/drivers/kvm/sys-hypervisor.c
new file mode 100644
index 000000000000..43b1d1a09807
--- /dev/null
+++ b/drivers/kvm/sys-hypervisor.c
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include <asm/kvm_para.h>
+
+#include <linux/kobject.h>
+#include <linux/sysfs.h>
+
+__weak const char *kvm_para_get_uuid(void)
+{
+	return NULL;
+}
+
+static ssize_t uuid_show(struct kobject *obj,
+			 struct kobj_attribute *attr,
+			 char *buf)
+{
+	const char *uuid = kvm_para_get_uuid();
+	return sprintf(buf, "%s\n", uuid);
+}
+
+static struct kobj_attribute uuid = __ATTR_RO(uuid);
+
+static int __init uuid_init(void)
+{
+	if (!kvm_para_available())
+		return 0;
+	return sysfs_create_file(hypervisor_kobj, &uuid.attr);
+}
+
+device_initcall(uuid_init);
-- 
2.7.4

Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries

[Christian Borntraeger]

On 14.05.19 17:16, Filippo Sironi wrote:
> Start populating /sys/hypervisor with KVM entries when we're running on
> KVM. This is to replicate functionality that's available when we're
> running on Xen.
> 
> Start with /sys/hypervisor/uuid, which users prefer over
> /sys/devices/virtual/dmi/id/product_uuid as a way to recognize a virtual
> machine, since it's also available when running on Xen HVM and on Xen PV
> and, on top of that doesn't require root privileges by default.
> Let's create arch-specific hooks so that different architectures can
> provide different implementations.
> 
> Signed-off-by: Filippo Sironi <sironi@amazon.de>
> ---
> v2:
> * move the retrieval of the VM UUID out of uuid_show and into
>   kvm_para_get_uuid, which is a weak function that can be overwritten
> 
>  drivers/Kconfig              |  2 ++
>  drivers/Makefile             |  2 ++
>  drivers/kvm/Kconfig          | 14 ++++++++++++++
>  drivers/kvm/Makefile         |  1 +
>  drivers/kvm/sys-hypervisor.c | 30 ++++++++++++++++++++++++++++++
>  5 files changed, 49 insertions(+)
>  create mode 100644 drivers/kvm/Kconfig
>  create mode 100644 drivers/kvm/Makefile
>  create mode 100644 drivers/kvm/sys-hypervisor.c
> 
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 45f9decb9848..90eb835fe951 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -146,6 +146,8 @@ source "drivers/hv/Kconfig"
>  
>  source "drivers/xen/Kconfig"
>  
> +source "drivers/kvm/Kconfig"
> +
>  source "drivers/staging/Kconfig"
>  
>  source "drivers/platform/Kconfig"
> diff --git a/drivers/Makefile b/drivers/Makefile
> index c61cde554340..79cc92a3f6bf 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -44,6 +44,8 @@ obj-y				+= soc/
>  obj-$(CONFIG_VIRTIO)		+= virtio/
>  obj-$(CONFIG_XEN)		+= xen/
>  
> +obj-$(CONFIG_KVM_GUEST)		+= kvm/
> +
>  # regulators early, since some subsystems rely on them to initialize
>  obj-$(CONFIG_REGULATOR)		+= regulator/
>  
> diff --git a/drivers/kvm/Kconfig b/drivers/kvm/Kconfig
> new file mode 100644
> index 000000000000..3fc041df7c11
> --- /dev/null
> +++ b/drivers/kvm/Kconfig
> @@ -0,0 +1,14 @@
> +menu "KVM driver support"
> +        depends on KVM_GUEST
> +
> +config KVM_SYS_HYPERVISOR
> +        bool "Create KVM entries under /sys/hypervisor"
> +        depends on SYSFS
> +        select SYS_HYPERVISOR
> +        default y
> +        help
> +          Create KVM entries under /sys/hypervisor (e.g., uuid). When running
> +          native or on another hypervisor, /sys/hypervisor may still be
> +          present, but it will have no KVM entries.
> +
> +endmenu
> diff --git a/drivers/kvm/Makefile b/drivers/kvm/Makefile
> new file mode 100644
> index 000000000000..73a43fc994b9
> --- /dev/null
> +++ b/drivers/kvm/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_KVM_SYS_HYPERVISOR) += sys-hypervisor.o
> diff --git a/drivers/kvm/sys-hypervisor.c b/drivers/kvm/sys-hypervisor.c
> new file mode 100644
> index 000000000000..43b1d1a09807
> --- /dev/null
> +++ b/drivers/kvm/sys-hypervisor.c
> @@ -0,0 +1,30 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#include <asm/kvm_para.h>
> +
> +#include <linux/kobject.h>
> +#include <linux/sysfs.h>
> +
> +__weak const char *kvm_para_get_uuid(void)
> +{
> +	return NULL;
> +}
> +
> +static ssize_t uuid_show(struct kobject *obj,
> +			 struct kobj_attribute *attr,
> +			 char *buf)
> +{
> +	const char *uuid = kvm_para_get_uuid();

I would prefer to have kvm_para_get_uuid return a uuid_t
but char * will probably work out as well.


> +	return sprintf(buf, "%s\n", uuid);
> +}
> +
> +static struct kobj_attribute uuid = __ATTR_RO(uuid);
> +
> +static int __init uuid_init(void)
> +{
> +	if (!kvm_para_available())

Isnt kvm_para_available a function that is defined in the context of the HOST
and not of the guest?

> +		return 0;
> +	return sysfs_create_file(hypervisor_kobj, &uuid.attr);
> +}
> +
> +device_initcall(uuid_init);
> 

Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries

[Sironi, Filippo]

> On 14. May 2019, at 17:26, Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> 
> 
> 
> On 14.05.19 17:16, Filippo Sironi wrote:
>> Start populating /sys/hypervisor with KVM entries when we're running on
>> KVM. This is to replicate functionality that's available when we're
>> running on Xen.
>> 
>> Start with /sys/hypervisor/uuid, which users prefer over
>> /sys/devices/virtual/dmi/id/product_uuid as a way to recognize a virtual
>> machine, since it's also available when running on Xen HVM and on Xen PV
>> and, on top of that doesn't require root privileges by default.
>> Let's create arch-specific hooks so that different architectures can
>> provide different implementations.
>> 
>> Signed-off-by: Filippo Sironi <sironi@amazon.de>
>> ---
>> v2:
>> * move the retrieval of the VM UUID out of uuid_show and into
>>  kvm_para_get_uuid, which is a weak function that can be overwritten
>> 
>> drivers/Kconfig              |  2 ++
>> drivers/Makefile             |  2 ++
>> drivers/kvm/Kconfig          | 14 ++++++++++++++
>> drivers/kvm/Makefile         |  1 +
>> drivers/kvm/sys-hypervisor.c | 30 ++++++++++++++++++++++++++++++
>> 5 files changed, 49 insertions(+)
>> create mode 100644 drivers/kvm/Kconfig
>> create mode 100644 drivers/kvm/Makefile
>> create mode 100644 drivers/kvm/sys-hypervisor.c
>> 
>> diff --git a/drivers/Kconfig b/drivers/Kconfig
>> index 45f9decb9848..90eb835fe951 100644
>> --- a/drivers/Kconfig
>> +++ b/drivers/Kconfig
>> @@ -146,6 +146,8 @@ source "drivers/hv/Kconfig"
>> 
>> source "drivers/xen/Kconfig"
>> 
>> +source "drivers/kvm/Kconfig"
>> +
>> source "drivers/staging/Kconfig"
>> 
>> source "drivers/platform/Kconfig"
>> diff --git a/drivers/Makefile b/drivers/Makefile
>> index c61cde554340..79cc92a3f6bf 100644
>> --- a/drivers/Makefile
>> +++ b/drivers/Makefile
>> @@ -44,6 +44,8 @@ obj-y				+= soc/
>> obj-$(CONFIG_VIRTIO)		+= virtio/
>> obj-$(CONFIG_XEN)		+= xen/
>> 
>> +obj-$(CONFIG_KVM_GUEST)		+= kvm/
>> +
>> # regulators early, since some subsystems rely on them to initialize
>> obj-$(CONFIG_REGULATOR)		+= regulator/
>> 
>> diff --git a/drivers/kvm/Kconfig b/drivers/kvm/Kconfig
>> new file mode 100644
>> index 000000000000..3fc041df7c11
>> --- /dev/null
>> +++ b/drivers/kvm/Kconfig
>> @@ -0,0 +1,14 @@
>> +menu "KVM driver support"
>> +        depends on KVM_GUEST
>> +
>> +config KVM_SYS_HYPERVISOR
>> +        bool "Create KVM entries under /sys/hypervisor"
>> +        depends on SYSFS
>> +        select SYS_HYPERVISOR
>> +        default y
>> +        help
>> +          Create KVM entries under /sys/hypervisor (e.g., uuid). When running
>> +          native or on another hypervisor, /sys/hypervisor may still be
>> +          present, but it will have no KVM entries.
>> +
>> +endmenu
>> diff --git a/drivers/kvm/Makefile b/drivers/kvm/Makefile
>> new file mode 100644
>> index 000000000000..73a43fc994b9
>> --- /dev/null
>> +++ b/drivers/kvm/Makefile
>> @@ -0,0 +1 @@
>> +obj-$(CONFIG_KVM_SYS_HYPERVISOR) += sys-hypervisor.o
>> diff --git a/drivers/kvm/sys-hypervisor.c b/drivers/kvm/sys-hypervisor.c
>> new file mode 100644
>> index 000000000000..43b1d1a09807
>> --- /dev/null
>> +++ b/drivers/kvm/sys-hypervisor.c
>> @@ -0,0 +1,30 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +
>> +#include <asm/kvm_para.h>
>> +
>> +#include <linux/kobject.h>
>> +#include <linux/sysfs.h>
>> +
>> +__weak const char *kvm_para_get_uuid(void)
>> +{
>> +	return NULL;
>> +}
>> +
>> +static ssize_t uuid_show(struct kobject *obj,
>> +			 struct kobj_attribute *attr,
>> +			 char *buf)
>> +{
>> +	const char *uuid = kvm_para_get_uuid();
> 
> I would prefer to have kvm_para_get_uuid return a uuid_t
> but char * will probably work out as well.

Let me give this a quick spin.

>> +	return sprintf(buf, "%s\n", uuid);
>> +}
>> +
>> +static struct kobj_attribute uuid = __ATTR_RO(uuid);
>> +
>> +static int __init uuid_init(void)
>> +{
>> +	if (!kvm_para_available())
> 
> Isnt kvm_para_available a function that is defined in the context of the HOST
> and not of the guest?

No, kvm_para_available is defined in the guest context.
On x86, it checks for the presence of the KVM CPUID leafs.

>> +		return 0;
>> +	return sysfs_create_file(hypervisor_kobj, &uuid.attr);
>> +}
>> +
>> +device_initcall(uuid_init);




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B

Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries

[Christian Borntraeger]

On 14.05.19 18:09, Sironi, Filippo wrote:

>> Isnt kvm_para_available a function that is defined in the context of the HOST
>> and not of the guest?
> 
> No, kvm_para_available is defined in the guest context.
> On x86, it checks for the presence of the KVM CPUID leafs.
> 

Right you are, I mixed that up.

Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries

[Sironi, Filippo]

> On 14. May 2019, at 18:09, Sironi, Filippo <sironi@amazon.de> wrote:
> 
>> On 14. May 2019, at 17:26, Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>> 
>>> On 14.05.19 17:16, Filippo Sironi wrote:
>>> Start populating /sys/hypervisor with KVM entries when we're running on
>>> KVM. This is to replicate functionality that's available when we're
>>> running on Xen.
>>> 
>>> Start with /sys/hypervisor/uuid, which users prefer over
>>> /sys/devices/virtual/dmi/id/product_uuid as a way to recognize a virtual
>>> machine, since it's also available when running on Xen HVM and on Xen PV
>>> and, on top of that doesn't require root privileges by default.
>>> Let's create arch-specific hooks so that different architectures can
>>> provide different implementations.
>>> 
>>> Signed-off-by: Filippo Sironi <sironi@amazon.de>
>>> ---
>>> v2:
>>> * move the retrieval of the VM UUID out of uuid_show and into
>>> kvm_para_get_uuid, which is a weak function that can be overwritten
>>> 
>>> drivers/Kconfig              |  2 ++
>>> drivers/Makefile             |  2 ++
>>> drivers/kvm/Kconfig          | 14 ++++++++++++++
>>> drivers/kvm/Makefile         |  1 +
>>> drivers/kvm/sys-hypervisor.c | 30 ++++++++++++++++++++++++++++++
>>> 5 files changed, 49 insertions(+)
>>> create mode 100644 drivers/kvm/Kconfig
>>> create mode 100644 drivers/kvm/Makefile
>>> create mode 100644 drivers/kvm/sys-hypervisor.c
>>> 
>>> diff --git a/drivers/Kconfig b/drivers/Kconfig
>>> index 45f9decb9848..90eb835fe951 100644
>>> --- a/drivers/Kconfig
>>> +++ b/drivers/Kconfig
>>> @@ -146,6 +146,8 @@ source "drivers/hv/Kconfig"
>>> 
>>> source "drivers/xen/Kconfig"
>>> 
>>> +source "drivers/kvm/Kconfig"
>>> +
>>> source "drivers/staging/Kconfig"
>>> 
>>> source "drivers/platform/Kconfig"
>>> diff --git a/drivers/Makefile b/drivers/Makefile
>>> index c61cde554340..79cc92a3f6bf 100644
>>> --- a/drivers/Makefile
>>> +++ b/drivers/Makefile
>>> @@ -44,6 +44,8 @@ obj-y				+= soc/
>>> obj-$(CONFIG_VIRTIO)		+= virtio/
>>> obj-$(CONFIG_XEN)		+= xen/
>>> 
>>> +obj-$(CONFIG_KVM_GUEST)		+= kvm/
>>> +
>>> # regulators early, since some subsystems rely on them to initialize
>>> obj-$(CONFIG_REGULATOR)		+= regulator/
>>> 
>>> diff --git a/drivers/kvm/Kconfig b/drivers/kvm/Kconfig
>>> new file mode 100644
>>> index 000000000000..3fc041df7c11
>>> --- /dev/null
>>> +++ b/drivers/kvm/Kconfig
>>> @@ -0,0 +1,14 @@
>>> +menu "KVM driver support"
>>> +        depends on KVM_GUEST
>>> +
>>> +config KVM_SYS_HYPERVISOR
>>> +        bool "Create KVM entries under /sys/hypervisor"
>>> +        depends on SYSFS
>>> +        select SYS_HYPERVISOR
>>> +        default y
>>> +        help
>>> +          Create KVM entries under /sys/hypervisor (e.g., uuid). When running
>>> +          native or on another hypervisor, /sys/hypervisor may still be
>>> +          present, but it will have no KVM entries.
>>> +
>>> +endmenu
>>> diff --git a/drivers/kvm/Makefile b/drivers/kvm/Makefile
>>> new file mode 100644
>>> index 000000000000..73a43fc994b9
>>> --- /dev/null
>>> +++ b/drivers/kvm/Makefile
>>> @@ -0,0 +1 @@
>>> +obj-$(CONFIG_KVM_SYS_HYPERVISOR) += sys-hypervisor.o
>>> diff --git a/drivers/kvm/sys-hypervisor.c b/drivers/kvm/sys-hypervisor.c
>>> new file mode 100644
>>> index 000000000000..43b1d1a09807
>>> --- /dev/null
>>> +++ b/drivers/kvm/sys-hypervisor.c
>>> @@ -0,0 +1,30 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +
>>> +#include <asm/kvm_para.h>
>>> +
>>> +#include <linux/kobject.h>
>>> +#include <linux/sysfs.h>
>>> +
>>> +__weak const char *kvm_para_get_uuid(void)
>>> +{
>>> +	return NULL;
>>> +}
>>> +
>>> +static ssize_t uuid_show(struct kobject *obj,
>>> +			 struct kobj_attribute *attr,
>>> +			 char *buf)
>>> +{
>>> +	const char *uuid = kvm_para_get_uuid();
>> 
>> I would prefer to have kvm_para_get_uuid return a uuid_t
>> but char * will probably work out as well.
> 
> Let me give this a quick spin.

I looked into getting a uuid_t.

At least for architectures where we retrieve that bit of
information from DMI tables, this is undesirable since
the interpretation of the UUID changed with DMI 2.6
(the first 3 fields are now encoded in little-endian).
This means that we wouldn't know how to print it in this
generic code.

I think that it's best if the architecture specific code
turns the UUID into the string representation.

>>> +	return sprintf(buf, "%s\n", uuid);
>>> +}
>>> +
>>> +static struct kobj_attribute uuid = __ATTR_RO(uuid);
>>> +
>>> +static int __init uuid_init(void)
>>> +{
>>> +	if (!kvm_para_available())
>> 
>> Isnt kvm_para_available a function that is defined in the context of the HOST
>> and not of the guest?
> 
> No, kvm_para_available is defined in the guest context.
> On x86, it checks for the presence of the KVM CPUID leafs.
> 
>>> +		return 0;
>>> +	return sysfs_create_file(hypervisor_kobj, &uuid.attr);
>>> +}
>>> +
>>> +device_initcall(uuid_init);




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B

Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries

[Alexander Graf]

On 14.05.19 08:16, Filippo Sironi wrote:
> Start populating /sys/hypervisor with KVM entries when we're running on
> KVM. This is to replicate functionality that's available when we're
> running on Xen.
> 
> Start with /sys/hypervisor/uuid, which users prefer over
> /sys/devices/virtual/dmi/id/product_uuid as a way to recognize a virtual
> machine, since it's also available when running on Xen HVM and on Xen PV
> and, on top of that doesn't require root privileges by default.
> Let's create arch-specific hooks so that different architectures can
> provide different implementations.
> 
> Signed-off-by: Filippo Sironi <sironi@amazon.de>

I think this needs something akin to

  https://www.kernel.org/doc/Documentation/ABI/stable/sysfs-hypervisor-xen

to document which files are available.

> ---
> v2:
> * move the retrieval of the VM UUID out of uuid_show and into
>   kvm_para_get_uuid, which is a weak function that can be overwritten
> 
>  drivers/Kconfig              |  2 ++
>  drivers/Makefile             |  2 ++
>  drivers/kvm/Kconfig          | 14 ++++++++++++++
>  drivers/kvm/Makefile         |  1 +
>  drivers/kvm/sys-hypervisor.c | 30 ++++++++++++++++++++++++++++++
>  5 files changed, 49 insertions(+)
>  create mode 100644 drivers/kvm/Kconfig
>  create mode 100644 drivers/kvm/Makefile
>  create mode 100644 drivers/kvm/sys-hypervisor.c
> 

[...]

> +
> +__weak const char *kvm_para_get_uuid(void)
> +{
> +	return NULL;
> +}
> +
> +static ssize_t uuid_show(struct kobject *obj,
> +			 struct kobj_attribute *attr,
> +			 char *buf)
> +{
> +	const char *uuid = kvm_para_get_uuid();
> +	return sprintf(buf, "%s\n", uuid);

The usual return value for the Xen /sys/hypervisor interface is
"<denied>". Wouldn't it make sense to follow that pattern for the KVM
one too? Currently, if we can not determine the UUID this will just
return (null).

Otherwise, looks good to me. Are you aware of any other files we should
provide? Also, is there any reason not to implement ARM as well while at it?

Alex

> +}
> +
> +static struct kobj_attribute uuid = __ATTR_RO(uuid);
> +
> +static int __init uuid_init(void)
> +{
> +	if (!kvm_para_available())
> +		return 0;
> +	return sysfs_create_file(hypervisor_kobj, &uuid.attr);
> +}
> +
> +device_initcall(uuid_init);
> 

Re: [Xen-devel] [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries

[Andrew Cooper]

On 16/05/2019 14:50, Alexander Graf wrote:
> On 14.05.19 08:16, Filippo Sironi wrote:
>> Start populating /sys/hypervisor with KVM entries when we're running on
>> KVM. This is to replicate functionality that's available when we're
>> running on Xen.
>>
>> Start with /sys/hypervisor/uuid, which users prefer over
>> /sys/devices/virtual/dmi/id/product_uuid as a way to recognize a virtual
>> machine, since it's also available when running on Xen HVM and on Xen PV
>> and, on top of that doesn't require root privileges by default.
>> Let's create arch-specific hooks so that different architectures can
>> provide different implementations.
>>
>> Signed-off-by: Filippo Sironi <sironi@amazon.de>
> I think this needs something akin to
>
>   https://www.kernel.org/doc/Documentation/ABI/stable/sysfs-hypervisor-xen
>
> to document which files are available.
>
>> ---
>> v2:
>> * move the retrieval of the VM UUID out of uuid_show and into
>>   kvm_para_get_uuid, which is a weak function that can be overwritten
>>
>>  drivers/Kconfig              |  2 ++
>>  drivers/Makefile             |  2 ++
>>  drivers/kvm/Kconfig          | 14 ++++++++++++++
>>  drivers/kvm/Makefile         |  1 +
>>  drivers/kvm/sys-hypervisor.c | 30 ++++++++++++++++++++++++++++++
>>  5 files changed, 49 insertions(+)
>>  create mode 100644 drivers/kvm/Kconfig
>>  create mode 100644 drivers/kvm/Makefile
>>  create mode 100644 drivers/kvm/sys-hypervisor.c
>>
> [...]
>
>> +
>> +__weak const char *kvm_para_get_uuid(void)
>> +{
>> +	return NULL;
>> +}
>> +
>> +static ssize_t uuid_show(struct kobject *obj,
>> +			 struct kobj_attribute *attr,
>> +			 char *buf)
>> +{
>> +	const char *uuid = kvm_para_get_uuid();
>> +	return sprintf(buf, "%s\n", uuid);
> The usual return value for the Xen /sys/hypervisor interface is
> "<denied>".

This string comes straight from Xen.

It was an effort to reduce the quantity of interesting fingerprintable
data accessable by default to unprivileged guests.

See
https://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=a2fc8d514df2b38c310d4f4432fe06520b0769ed

~Andrew

Re: [Xen-devel] [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries

[Alexander Graf]

On 16.05.19 07:02, Andrew Cooper wrote:
> On 16/05/2019 14:50, Alexander Graf wrote:
>> On 14.05.19 08:16, Filippo Sironi wrote:
>>> Start populating /sys/hypervisor with KVM entries when we're running on
>>> KVM. This is to replicate functionality that's available when we're
>>> running on Xen.
>>>
>>> Start with /sys/hypervisor/uuid, which users prefer over
>>> /sys/devices/virtual/dmi/id/product_uuid as a way to recognize a virtual
>>> machine, since it's also available when running on Xen HVM and on Xen PV
>>> and, on top of that doesn't require root privileges by default.
>>> Let's create arch-specific hooks so that different architectures can
>>> provide different implementations.
>>>
>>> Signed-off-by: Filippo Sironi <sironi@amazon.de>
>> I think this needs something akin to
>>
>>   https://www.kernel.org/doc/Documentation/ABI/stable/sysfs-hypervisor-xen
>>
>> to document which files are available.
>>
>>> ---
>>> v2:
>>> * move the retrieval of the VM UUID out of uuid_show and into
>>>   kvm_para_get_uuid, which is a weak function that can be overwritten
>>>
>>>  drivers/Kconfig              |  2 ++
>>>  drivers/Makefile             |  2 ++
>>>  drivers/kvm/Kconfig          | 14 ++++++++++++++
>>>  drivers/kvm/Makefile         |  1 +
>>>  drivers/kvm/sys-hypervisor.c | 30 ++++++++++++++++++++++++++++++
>>>  5 files changed, 49 insertions(+)
>>>  create mode 100644 drivers/kvm/Kconfig
>>>  create mode 100644 drivers/kvm/Makefile
>>>  create mode 100644 drivers/kvm/sys-hypervisor.c
>>>
>> [...]
>>
>>> +
>>> +__weak const char *kvm_para_get_uuid(void)
>>> +{
>>> +	return NULL;
>>> +}
>>> +
>>> +static ssize_t uuid_show(struct kobject *obj,
>>> +			 struct kobj_attribute *attr,
>>> +			 char *buf)
>>> +{
>>> +	const char *uuid = kvm_para_get_uuid();
>>> +	return sprintf(buf, "%s\n", uuid);
>> The usual return value for the Xen /sys/hypervisor interface is
>> "<denied>".
> This string comes straight from Xen.
>
> It was an effort to reduce the quantity of interesting fingerprintable
> data accessable by default to unprivileged guests.
>
> See
> https://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=a2fc8d514df2b38c310d4f4432fe06520b0769ed


What a great design :). My point is mostly that we should be as common
as possible when it comes to /sys/hypervisor, so that tools don't have
to care about the HV they're working against.

By being first to implement <denied> you just created precedence, so we
can either simulate the same behavor for KVM or be different. And since
commonality is good, I'd rather be the same.

That said, I couldn't find in the patdch above whether Xen even emits
<denied> for the uuid. Does it have that capability? If not, we may as
well go with (null).


Alex

Re: [Xen-devel] [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries

[Boris Ostrovsky]

On 5/16/19 10:08 AM, Alexander Graf wrote:
>
> My point is mostly that we should be as common
> as possible when it comes to /sys/hypervisor, so that tools don't have
> to care about the HV they're working against.

It might make sense to have a common sys-hypervisor.c file
(drivers/hypervisor/sys-hypervisor.c or some such), with
hypervisor-specific ops/callbacks/etc.

-boris

Re: [Xen-devel] [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries

[Sironi, Filippo]

> On 16. May 2019, at 17:02, Boris Ostrovsky <boris.ostrovsky@oracle.com> wrote:
> 
> On 5/16/19 10:08 AM, Alexander Graf wrote:
>> 
>> My point is mostly that we should be as common
>> as possible when it comes to /sys/hypervisor, so that tools don't have
>> to care about the HV they're working against.
> 
> It might make sense to have a common sys-hypervisor.c file
> (drivers/hypervisor/sys-hypervisor.c or some such), with
> hypervisor-specific ops/callbacks/etc.
> 
> -boris


Yes, it definitely does. I would follow up with future patches to make it
happen.

Filippo





Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B

Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries

[Sironi, Filippo]

> On 16. May 2019, at 15:50, Graf, Alexander <graf@amazon.com> wrote:
> 
> On 14.05.19 08:16, Filippo Sironi wrote:
>> Start populating /sys/hypervisor with KVM entries when we're running on
>> KVM. This is to replicate functionality that's available when we're
>> running on Xen.
>> 
>> Start with /sys/hypervisor/uuid, which users prefer over
>> /sys/devices/virtual/dmi/id/product_uuid as a way to recognize a virtual
>> machine, since it's also available when running on Xen HVM and on Xen PV
>> and, on top of that doesn't require root privileges by default.
>> Let's create arch-specific hooks so that different architectures can
>> provide different implementations.
>> 
>> Signed-off-by: Filippo Sironi <sironi@amazon.de>
> 
> I think this needs something akin to
> 
>  https://www.kernel.org/doc/Documentation/ABI/stable/sysfs-hypervisor-xen
> 
> to document which files are available.
> 
>> ---
>> v2:
>> * move the retrieval of the VM UUID out of uuid_show and into
>>  kvm_para_get_uuid, which is a weak function that can be overwritten
>> 
>> drivers/Kconfig              |  2 ++
>> drivers/Makefile             |  2 ++
>> drivers/kvm/Kconfig          | 14 ++++++++++++++
>> drivers/kvm/Makefile         |  1 +
>> drivers/kvm/sys-hypervisor.c | 30 ++++++++++++++++++++++++++++++
>> 5 files changed, 49 insertions(+)
>> create mode 100644 drivers/kvm/Kconfig
>> create mode 100644 drivers/kvm/Makefile
>> create mode 100644 drivers/kvm/sys-hypervisor.c
>> 
> 
> [...]
> 
>> +
>> +__weak const char *kvm_para_get_uuid(void)
>> +{
>> +	return NULL;
>> +}
>> +
>> +static ssize_t uuid_show(struct kobject *obj,
>> +			 struct kobj_attribute *attr,
>> +			 char *buf)
>> +{
>> +	const char *uuid = kvm_para_get_uuid();
>> +	return sprintf(buf, "%s\n", uuid);
> 
> The usual return value for the Xen /sys/hypervisor interface is
> "<denied>". Wouldn't it make sense to follow that pattern for the KVM
> one too? Currently, if we can not determine the UUID this will just
> return (null).
> 
> Otherwise, looks good to me. Are you aware of any other files we should
> provide? Also, is there any reason not to implement ARM as well while at it?
> 
> Alex

This originated from a customer request that was using /sys/hypervisor/uuid.
My guess is that we would want to expose "type" and "version" moving
forward and that's when we hypervisor hooks will be useful on top
of arch hooks.

On a different note, any idea how to check whether the OS is running
virtualized on KVM on ARM and ARM64?  kvm_para_available() isn't an
option and the same is true for S390 where kvm_para_available()
always returns true and it would even if a KVM enabled kernel would
be running on bare metal.

I think we will need another arch hook to call a function that says
whether the OS is running virtualized on KVM.

>> +}
>> +
>> +static struct kobj_attribute uuid = __ATTR_RO(uuid);
>> +
>> +static int __init uuid_init(void)
>> +{
>> +	if (!kvm_para_available())
>> +		return 0;
>> +	return sysfs_create_file(hypervisor_kobj, &uuid.attr);
>> +}
>> +
>> +device_initcall(uuid_init);




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B

Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries

[Alexander Graf]

On 17.05.19 17:41, Sironi, Filippo wrote:
>> On 16. May 2019, at 15:50, Graf, Alexander <graf@amazon.com> wrote:
>>
>> On 14.05.19 08:16, Filippo Sironi wrote:
>>> Start populating /sys/hypervisor with KVM entries when we're running on
>>> KVM. This is to replicate functionality that's available when we're
>>> running on Xen.
>>>
>>> Start with /sys/hypervisor/uuid, which users prefer over
>>> /sys/devices/virtual/dmi/id/product_uuid as a way to recognize a virtual
>>> machine, since it's also available when running on Xen HVM and on Xen PV
>>> and, on top of that doesn't require root privileges by default.
>>> Let's create arch-specific hooks so that different architectures can
>>> provide different implementations.
>>>
>>> Signed-off-by: Filippo Sironi <sironi@amazon.de>
>> I think this needs something akin to
>>
>>   https://www.kernel.org/doc/Documentation/ABI/stable/sysfs-hypervisor-xen
>>
>> to document which files are available.
>>
>>> ---
>>> v2:
>>> * move the retrieval of the VM UUID out of uuid_show and into
>>>   kvm_para_get_uuid, which is a weak function that can be overwritten
>>>
>>> drivers/Kconfig              |  2 ++
>>> drivers/Makefile             |  2 ++
>>> drivers/kvm/Kconfig          | 14 ++++++++++++++
>>> drivers/kvm/Makefile         |  1 +
>>> drivers/kvm/sys-hypervisor.c | 30 ++++++++++++++++++++++++++++++
>>> 5 files changed, 49 insertions(+)
>>> create mode 100644 drivers/kvm/Kconfig
>>> create mode 100644 drivers/kvm/Makefile
>>> create mode 100644 drivers/kvm/sys-hypervisor.c
>>>
>> [...]
>>
>>> +
>>> +__weak const char *kvm_para_get_uuid(void)
>>> +{
>>> +	return NULL;
>>> +}
>>> +
>>> +static ssize_t uuid_show(struct kobject *obj,
>>> +			 struct kobj_attribute *attr,
>>> +			 char *buf)
>>> +{
>>> +	const char *uuid = kvm_para_get_uuid();
>>> +	return sprintf(buf, "%s\n", uuid);
>> The usual return value for the Xen /sys/hypervisor interface is
>> "<denied>". Wouldn't it make sense to follow that pattern for the KVM
>> one too? Currently, if we can not determine the UUID this will just
>> return (null).
>>
>> Otherwise, looks good to me. Are you aware of any other files we should
>> provide? Also, is there any reason not to implement ARM as well while at it?
>>
>> Alex
> This originated from a customer request that was using /sys/hypervisor/uuid.
> My guess is that we would want to expose "type" and "version" moving
> forward and that's when we hypervisor hooks will be useful on top
> of arch hooks.
>
> On a different note, any idea how to check whether the OS is running
> virtualized on KVM on ARM and ARM64?  kvm_para_available() isn't an


Yeah, ARM doesn't have any KVM PV FWIW. I also can't find any explicit 
hint passed into guests that we are indeed running in KVM. The closest 
thing I can see is the SMBIOS product identifier in QEMU which gets 
patched to "KVM Virtual Machine". Maybe we'll have to do with that for 
the sake of backwards compatibility ...


> option and the same is true for S390 where kvm_para_available()
> always returns true and it would even if a KVM enabled kernel would
> be running on bare metal.


For s390, you can figure the topology out using the sthyi instruction. 
I'm not sure if there is a nice in-kernel API to leverage that though. 
In fact, kvm_para_available() probably should check sthyi output to 
determine whether we really can use it, no? Christian?


Alex


>
> I think we will need another arch hook to call a function that says
> whether the OS is running virtualized on KVM.
>
>>> +}
>>> +
>>> +static struct kobj_attribute uuid = __ATTR_RO(uuid);
>>> +
>>> +static int __init uuid_init(void)
>>> +{
>>> +	if (!kvm_para_available())
>>> +		return 0;
>>> +	return sysfs_create_file(hypervisor_kobj, &uuid.attr);
>>> +}
>>> +
>>> +device_initcall(uuid_init);

Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries

[Raslan, KarimAllah]

On Fri, 2019-05-31 at 11:06 +0200, Alexander Graf wrote:
> On 17.05.19 17:41, Sironi, Filippo wrote:
> > 
> > > 
> > > On 16. May 2019, at 15:50, Graf, Alexander <graf@amazon.com> wrote:
> > > 
> > > On 14.05.19 08:16, Filippo Sironi wrote:
> > > > 
> > > > Start populating /sys/hypervisor with KVM entries when we're running on
> > > > KVM. This is to replicate functionality that's available when we're
> > > > running on Xen.
> > > > 
> > > > Start with /sys/hypervisor/uuid, which users prefer over
> > > > /sys/devices/virtual/dmi/id/product_uuid as a way to recognize a virtual
> > > > machine, since it's also available when running on Xen HVM and on Xen PV
> > > > and, on top of that doesn't require root privileges by default.
> > > > Let's create arch-specific hooks so that different architectures can
> > > > provide different implementations.
> > > > 
> > > > Signed-off-by: Filippo Sironi <sironi@amazon.de>
> > > I think this needs something akin to
> > > 
> > >   https://www.kernel.org/doc/Documentation/ABI/stable/sysfs-hypervisor-xen
> > > 
> > > to document which files are available.
> > > 
> > > > 
> > > > ---
> > > > v2:
> > > > * move the retrieval of the VM UUID out of uuid_show and into
> > > >   kvm_para_get_uuid, which is a weak function that can be overwritten
> > > > 
> > > > drivers/Kconfig              |  2 ++
> > > > drivers/Makefile             |  2 ++
> > > > drivers/kvm/Kconfig          | 14 ++++++++++++++
> > > > drivers/kvm/Makefile         |  1 +
> > > > drivers/kvm/sys-hypervisor.c | 30 ++++++++++++++++++++++++++++++
> > > > 5 files changed, 49 insertions(+)
> > > > create mode 100644 drivers/kvm/Kconfig
> > > > create mode 100644 drivers/kvm/Makefile
> > > > create mode 100644 drivers/kvm/sys-hypervisor.c
> > > > 
> > > [...]
> > > 
> > > > 
> > > > +
> > > > +__weak const char *kvm_para_get_uuid(void)
> > > > +{
> > > > +	return NULL;
> > > > +}
> > > > +
> > > > +static ssize_t uuid_show(struct kobject *obj,
> > > > +			 struct kobj_attribute *attr,
> > > > +			 char *buf)
> > > > +{
> > > > +	const char *uuid = kvm_para_get_uuid();
> > > > +	return sprintf(buf, "%s\n", uuid);
> > > The usual return value for the Xen /sys/hypervisor interface is
> > > "<denied>". Wouldn't it make sense to follow that pattern for the KVM
> > > one too? Currently, if we can not determine the UUID this will just
> > > return (null).
> > > 
> > > Otherwise, looks good to me. Are you aware of any other files we should
> > > provide? Also, is there any reason not to implement ARM as well while at it?
> > > 
> > > Alex
> > This originated from a customer request that was using /sys/hypervisor/uuid.
> > My guess is that we would want to expose "type" and "version" moving
> > forward and that's when we hypervisor hooks will be useful on top
> > of arch hooks.
> > 
> > On a different note, any idea how to check whether the OS is running
> > virtualized on KVM on ARM and ARM64?  kvm_para_available() isn't an
> 
> 
> Yeah, ARM doesn't have any KVM PV FWIW. I also can't find any explicit 
> hint passed into guests that we are indeed running in KVM. The closest 
> thing I can see is the SMBIOS product identifier in QEMU which gets 
> patched to "KVM Virtual Machine". Maybe we'll have to do with that for 
> the sake of backwards compatibility ...

How about "psci_ops.conduit" (PSCI_CONDUIT_HVC vs PSCI_CONDUIT_SMC)?

> 
> 
> > 
> > option and the same is true for S390 where kvm_para_available()
> > always returns true and it would even if a KVM enabled kernel would
> > be running on bare metal.
> 
> 
> For s390, you can figure the topology out using the sthyi instruction. 
> I'm not sure if there is a nice in-kernel API to leverage that though. 
> In fact, kvm_para_available() probably should check sthyi output to 
> determine whether we really can use it, no? Christian?
> 
> 
> Alex
> 
> 
> > 
> > 
> > I think we will need another arch hook to call a function that says
> > whether the OS is running virtualized on KVM.
> > 
> > > 
> > > > 
> > > > +}
> > > > +
> > > > +static struct kobj_attribute uuid = __ATTR_RO(uuid);
> > > > +
> > > > +static int __init uuid_init(void)
> > > > +{
> > > > +	if (!kvm_para_available())
> > > > +		return 0;
> > > > +	return sysfs_create_file(hypervisor_kobj, &uuid.attr);
> > > > +}
> > > > +
> > > > +device_initcall(uuid_init);



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879

Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries

[Alexander Graf]

On 31.05.19 11:12, Raslan, KarimAllah wrote:
> On Fri, 2019-05-31 at 11:06 +0200, Alexander Graf wrote:
>> On 17.05.19 17:41, Sironi, Filippo wrote:
>>>> On 16. May 2019, at 15:50, Graf, Alexander <graf@amazon.com> wrote:
>>>>
>>>> On 14.05.19 08:16, Filippo Sironi wrote:
>>>>> Start populating /sys/hypervisor with KVM entries when we're running on
>>>>> KVM. This is to replicate functionality that's available when we're
>>>>> running on Xen.
>>>>>
>>>>> Start with /sys/hypervisor/uuid, which users prefer over
>>>>> /sys/devices/virtual/dmi/id/product_uuid as a way to recognize a virtual
>>>>> machine, since it's also available when running on Xen HVM and on Xen PV
>>>>> and, on top of that doesn't require root privileges by default.
>>>>> Let's create arch-specific hooks so that different architectures can
>>>>> provide different implementations.
>>>>>
>>>>> Signed-off-by: Filippo Sironi <sironi@amazon.de>
>>>> I think this needs something akin to
>>>>
>>>>    https://www.kernel.org/doc/Documentation/ABI/stable/sysfs-hypervisor-xen
>>>>
>>>> to document which files are available.
>>>>
>>>>> ---
>>>>> v2:
>>>>> * move the retrieval of the VM UUID out of uuid_show and into
>>>>>    kvm_para_get_uuid, which is a weak function that can be overwritten
>>>>>
>>>>> drivers/Kconfig              |  2 ++
>>>>> drivers/Makefile             |  2 ++
>>>>> drivers/kvm/Kconfig          | 14 ++++++++++++++
>>>>> drivers/kvm/Makefile         |  1 +
>>>>> drivers/kvm/sys-hypervisor.c | 30 ++++++++++++++++++++++++++++++
>>>>> 5 files changed, 49 insertions(+)
>>>>> create mode 100644 drivers/kvm/Kconfig
>>>>> create mode 100644 drivers/kvm/Makefile
>>>>> create mode 100644 drivers/kvm/sys-hypervisor.c
>>>>>
>>>> [...]
>>>>
>>>>> +
>>>>> +__weak const char *kvm_para_get_uuid(void)
>>>>> +{
>>>>> +	return NULL;
>>>>> +}
>>>>> +
>>>>> +static ssize_t uuid_show(struct kobject *obj,
>>>>> +			 struct kobj_attribute *attr,
>>>>> +			 char *buf)
>>>>> +{
>>>>> +	const char *uuid = kvm_para_get_uuid();
>>>>> +	return sprintf(buf, "%s\n", uuid);
>>>> The usual return value for the Xen /sys/hypervisor interface is
>>>> "<denied>". Wouldn't it make sense to follow that pattern for the KVM
>>>> one too? Currently, if we can not determine the UUID this will just
>>>> return (null).
>>>>
>>>> Otherwise, looks good to me. Are you aware of any other files we should
>>>> provide? Also, is there any reason not to implement ARM as well while at it?
>>>>
>>>> Alex
>>> This originated from a customer request that was using /sys/hypervisor/uuid.
>>> My guess is that we would want to expose "type" and "version" moving
>>> forward and that's when we hypervisor hooks will be useful on top
>>> of arch hooks.
>>>
>>> On a different note, any idea how to check whether the OS is running
>>> virtualized on KVM on ARM and ARM64?  kvm_para_available() isn't an
>>
>> Yeah, ARM doesn't have any KVM PV FWIW. I also can't find any explicit
>> hint passed into guests that we are indeed running in KVM. The closest
>> thing I can see is the SMBIOS product identifier in QEMU which gets
>> patched to "KVM Virtual Machine". Maybe we'll have to do with that for
>> the sake of backwards compatibility ...
> How about "psci_ops.conduit" (PSCI_CONDUIT_HVC vs PSCI_CONDUIT_SMC)?


This won't work for 2 reasons:

   a) You don't know it's KVM. You only know you might be running in EL1.
   b) KVM may choose to just use SMC for PSCI going forward and trap on it.


Alex

Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries

[Marc Zyngier]

On Fri, 31 May 2019 10:12:03 +0100,
"Raslan, KarimAllah" <karahmed@amazon.de> wrote:
> 
> On Fri, 2019-05-31 at 11:06 +0200, Alexander Graf wrote:
> > On 17.05.19 17:41, Sironi, Filippo wrote:
> > > 
> > > > 
> > > > On 16. May 2019, at 15:50, Graf, Alexander <graf@amazon.com> wrote:
> > > > 
> > > > On 14.05.19 08:16, Filippo Sironi wrote:
> > > > > 
> > > > > Start populating /sys/hypervisor with KVM entries when we're running on
> > > > > KVM. This is to replicate functionality that's available when we're
> > > > > running on Xen.
> > > > > 
> > > > > Start with /sys/hypervisor/uuid, which users prefer over
> > > > > /sys/devices/virtual/dmi/id/product_uuid as a way to recognize a virtual
> > > > > machine, since it's also available when running on Xen HVM and on Xen PV
> > > > > and, on top of that doesn't require root privileges by default.
> > > > > Let's create arch-specific hooks so that different architectures can
> > > > > provide different implementations.
> > > > > 
> > > > > Signed-off-by: Filippo Sironi <sironi@amazon.de>
> > > > I think this needs something akin to
> > > > 
> > > >   https://www.kernel.org/doc/Documentation/ABI/stable/sysfs-hypervisor-xen
> > > > 
> > > > to document which files are available.
> > > > 
> > > > > 
> > > > > ---
> > > > > v2:
> > > > > * move the retrieval of the VM UUID out of uuid_show and into
> > > > >   kvm_para_get_uuid, which is a weak function that can be overwritten
> > > > > 
> > > > > drivers/Kconfig              |  2 ++
> > > > > drivers/Makefile             |  2 ++
> > > > > drivers/kvm/Kconfig          | 14 ++++++++++++++
> > > > > drivers/kvm/Makefile         |  1 +
> > > > > drivers/kvm/sys-hypervisor.c | 30 ++++++++++++++++++++++++++++++
> > > > > 5 files changed, 49 insertions(+)
> > > > > create mode 100644 drivers/kvm/Kconfig
> > > > > create mode 100644 drivers/kvm/Makefile
> > > > > create mode 100644 drivers/kvm/sys-hypervisor.c
> > > > > 
> > > > [...]
> > > > 
> > > > > 
> > > > > +
> > > > > +__weak const char *kvm_para_get_uuid(void)
> > > > > +{
> > > > > +	return NULL;
> > > > > +}
> > > > > +
> > > > > +static ssize_t uuid_show(struct kobject *obj,
> > > > > +			 struct kobj_attribute *attr,
> > > > > +			 char *buf)
> > > > > +{
> > > > > +	const char *uuid = kvm_para_get_uuid();
> > > > > +	return sprintf(buf, "%s\n", uuid);
> > > > The usual return value for the Xen /sys/hypervisor interface is
> > > > "<denied>". Wouldn't it make sense to follow that pattern for the KVM
> > > > one too? Currently, if we can not determine the UUID this will just
> > > > return (null).
> > > > 
> > > > Otherwise, looks good to me. Are you aware of any other files we should
> > > > provide? Also, is there any reason not to implement ARM as well while at it?
> > > > 
> > > > Alex
> > > This originated from a customer request that was using /sys/hypervisor/uuid.
> > > My guess is that we would want to expose "type" and "version" moving
> > > forward and that's when we hypervisor hooks will be useful on top
> > > of arch hooks.
> > > 
> > > On a different note, any idea how to check whether the OS is running
> > > virtualized on KVM on ARM and ARM64?  kvm_para_available() isn't an
> > 
> > 
> > Yeah, ARM doesn't have any KVM PV FWIW. I also can't find any explicit 
> > hint passed into guests that we are indeed running in KVM. The closest 
> > thing I can see is the SMBIOS product identifier in QEMU which gets 
> > patched to "KVM Virtual Machine". Maybe we'll have to do with that for 
> > the sake of backwards compatibility ...
> 
> How about "psci_ops.conduit" (PSCI_CONDUIT_HVC vs PSCI_CONDUIT_SMC)?

[changing Christoffer address for one that actually]

That's not enough. HVC only tells you about the fact that you are
running under a hypervisor without telling you which one, and doesn't
cater for nested virt. It doesn't tell you anything about a hypervisor
that doesn't use HVC at all (it could only advertise SMC, for example).

If you want to identify the hypervisor, don't guess. Use the SMCCC
discovery mechanism, and make KVM identify itself as the hypervisor. I
have some code for that stashed at [1] as part of an unrelated series,
which I may post at some point.

Thanks,

	M.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=kvm-arm64/pvcy

-- 
Jazz is not dead, it just smell funny.

[PATCH v2 2/2] KVM: x86: Implement the arch-specific hook to report the VM UUID

[Filippo Sironi]

On x86, we report the UUID in DMI System Information (i.e., DMI Type 1)
as VM UUID.

Signed-off-by: Filippo Sironi <sironi@amazon.de>
---
 arch/x86/kernel/kvm.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 5c93a65ee1e5..441cab08a09d 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -25,6 +25,7 @@
 #include <linux/kernel.h>
 #include <linux/kvm_para.h>
 #include <linux/cpu.h>
+#include <linux/dmi.h>
 #include <linux/mm.h>
 #include <linux/highmem.h>
 #include <linux/hardirq.h>
@@ -694,6 +695,12 @@ bool kvm_para_available(void)
 }
 EXPORT_SYMBOL_GPL(kvm_para_available);
 
+const char *kvm_para_get_uuid(void)
+{
+	return dmi_get_system_info(DMI_PRODUCT_UUID);
+}
+EXPORT_SYMBOL_GPL(kvm_para_get_uuid);
+
 unsigned int kvm_arch_para_features(void)
 {
 	return cpuid_eax(kvm_cpuid_base() | KVM_CPUID_FEATURES);
-- 
2.7.4

Re: [PATCH v2 2/2] KVM: x86: Implement the arch-specific hook to report the VM UUID

[Alexander Graf]

On 14.05.19 08:16, Filippo Sironi wrote:
> On x86, we report the UUID in DMI System Information (i.e., DMI Type 1)
> as VM UUID.
> 
> Signed-off-by: Filippo Sironi <sironi@amazon.de>
> ---
>  arch/x86/kernel/kvm.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 5c93a65ee1e5..441cab08a09d 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -25,6 +25,7 @@
>  #include <linux/kernel.h>
>  #include <linux/kvm_para.h>
>  #include <linux/cpu.h>
> +#include <linux/dmi.h>
>  #include <linux/mm.h>
>  #include <linux/highmem.h>
>  #include <linux/hardirq.h>
> @@ -694,6 +695,12 @@ bool kvm_para_available(void)
>  }
>  EXPORT_SYMBOL_GPL(kvm_para_available);
>  
> +const char *kvm_para_get_uuid(void)
> +{
> +	return dmi_get_system_info(DMI_PRODUCT_UUID);

This adds a new dependency on CONFIG_DMI. Probably best to guard it with
an #if IS_ENABLED(CONFIG_DMI).

The concept seems sound though.


Alex

> +}
> +EXPORT_SYMBOL_GPL(kvm_para_get_uuid);
> +
>  unsigned int kvm_arch_para_features(void)
>  {
>  	return cpuid_eax(kvm_cpuid_base() | KVM_CPUID_FEATURES);
> 

Re: [PATCH v2 2/2] KVM: x86: Implement the arch-specific hook to report the VM UUID

[Sironi, Filippo]

> On 16. May 2019, at 15:56, Graf, Alexander <graf@amazon.com> wrote:
> 
> On 14.05.19 08:16, Filippo Sironi wrote:
>> On x86, we report the UUID in DMI System Information (i.e., DMI Type 1)
>> as VM UUID.
>> 
>> Signed-off-by: Filippo Sironi <sironi@amazon.de>
>> ---
>> arch/x86/kernel/kvm.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>> 
>> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
>> index 5c93a65ee1e5..441cab08a09d 100644
>> --- a/arch/x86/kernel/kvm.c
>> +++ b/arch/x86/kernel/kvm.c
>> @@ -25,6 +25,7 @@
>> #include <linux/kernel.h>
>> #include <linux/kvm_para.h>
>> #include <linux/cpu.h>
>> +#include <linux/dmi.h>
>> #include <linux/mm.h>
>> #include <linux/highmem.h>
>> #include <linux/hardirq.h>
>> @@ -694,6 +695,12 @@ bool kvm_para_available(void)
>> }
>> EXPORT_SYMBOL_GPL(kvm_para_available);
>> 
>> +const char *kvm_para_get_uuid(void)
>> +{
>> +	return dmi_get_system_info(DMI_PRODUCT_UUID);
> 
> This adds a new dependency on CONFIG_DMI. Probably best to guard it with
> an #if IS_ENABLED(CONFIG_DMI).
> 
> The concept seems sound though.
> 
> Alex

include/linux/dmi.h contains a dummy implementation of
dmi_get_system_info that returns NULL if CONFIG_DMI isn't defined.
This is enough unless we decide to return "<denied>" like in Xen.
If then, we can have the check in the generic code to turn NULL
into "<denied>".

Filippo


>> +}
>> +EXPORT_SYMBOL_GPL(kvm_para_get_uuid);
>> +
>> unsigned int kvm_arch_para_features(void)
>> {
>> 	return cpuid_eax(kvm_cpuid_base() | KVM_CPUID_FEATURES);




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B

Re: [PATCH v2 2/2] KVM: x86: Implement the arch-specific hook to report the VM UUID

[Alexander Graf]

On 16.05.19 08:25, Sironi, Filippo wrote:
>> On 16. May 2019, at 15:56, Graf, Alexander <graf@amazon.com> wrote:
>>
>> On 14.05.19 08:16, Filippo Sironi wrote:
>>> On x86, we report the UUID in DMI System Information (i.e., DMI Type 1)
>>> as VM UUID.
>>>
>>> Signed-off-by: Filippo Sironi <sironi@amazon.de>
>>> ---
>>> arch/x86/kernel/kvm.c | 7 +++++++
>>> 1 file changed, 7 insertions(+)
>>>
>>> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
>>> index 5c93a65ee1e5..441cab08a09d 100644
>>> --- a/arch/x86/kernel/kvm.c
>>> +++ b/arch/x86/kernel/kvm.c
>>> @@ -25,6 +25,7 @@
>>> #include <linux/kernel.h>
>>> #include <linux/kvm_para.h>
>>> #include <linux/cpu.h>
>>> +#include <linux/dmi.h>
>>> #include <linux/mm.h>
>>> #include <linux/highmem.h>
>>> #include <linux/hardirq.h>
>>> @@ -694,6 +695,12 @@ bool kvm_para_available(void)
>>> }
>>> EXPORT_SYMBOL_GPL(kvm_para_available);
>>>
>>> +const char *kvm_para_get_uuid(void)
>>> +{
>>> +	return dmi_get_system_info(DMI_PRODUCT_UUID);
>> This adds a new dependency on CONFIG_DMI. Probably best to guard it with
>> an #if IS_ENABLED(CONFIG_DMI).
>>
>> The concept seems sound though.
>>
>> Alex
> include/linux/dmi.h contains a dummy implementation of
> dmi_get_system_info that returns NULL if CONFIG_DMI isn't defined.


Oh, I missed that bit. Awesome! Less work :).


> This is enough unless we decide to return "<denied>" like in Xen.
> If then, we can have the check in the generic code to turn NULL
> into "<denied>".


Yes. Waiting for someone from Xen to answer this :)


Alex

Re: [PATCH v2 2/2] KVM: x86: Implement the arch-specific hook to report the VM UUID

[Boris Ostrovsky]

On 5/16/19 11:33 AM, Alexander Graf wrote:
> On 16.05.19 08:25, Sironi, Filippo wrote:
>>> On 16. May 2019, at 15:56, Graf, Alexander <graf@amazon.com> wrote:
>>>
>>> On 14.05.19 08:16, Filippo Sironi wrote:
>>>> On x86, we report the UUID in DMI System Information (i.e., DMI Type 1)
>>>> as VM UUID.
>>>>
>>>> Signed-off-by: Filippo Sironi <sironi@amazon.de>
>>>> ---
>>>> arch/x86/kernel/kvm.c | 7 +++++++
>>>> 1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
>>>> index 5c93a65ee1e5..441cab08a09d 100644
>>>> --- a/arch/x86/kernel/kvm.c
>>>> +++ b/arch/x86/kernel/kvm.c
>>>> @@ -25,6 +25,7 @@
>>>> #include <linux/kernel.h>
>>>> #include <linux/kvm_para.h>
>>>> #include <linux/cpu.h>
>>>> +#include <linux/dmi.h>
>>>> #include <linux/mm.h>
>>>> #include <linux/highmem.h>
>>>> #include <linux/hardirq.h>
>>>> @@ -694,6 +695,12 @@ bool kvm_para_available(void)
>>>> }
>>>> EXPORT_SYMBOL_GPL(kvm_para_available);
>>>>
>>>> +const char *kvm_para_get_uuid(void)
>>>> +{
>>>> +	return dmi_get_system_info(DMI_PRODUCT_UUID);
>>> This adds a new dependency on CONFIG_DMI. Probably best to guard it with
>>> an #if IS_ENABLED(CONFIG_DMI).
>>>
>>> The concept seems sound though.
>>>
>>> Alex
>> include/linux/dmi.h contains a dummy implementation of
>> dmi_get_system_info that returns NULL if CONFIG_DMI isn't defined.
>
> Oh, I missed that bit. Awesome! Less work :).
>
>
>> This is enough unless we decide to return "<denied>" like in Xen.
>> If then, we can have the check in the generic code to turn NULL
>> into "<denied>".
>
> Yes. Waiting for someone from Xen to answer this :)

Not sure I am answering your question but on Xen we return UUID value
zero if access permissions are not sufficient. Not <denied>.

http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/common/kernel.c;h=612575430f1ce7faf5bd66e7a99f1758c63fb3cb;hb=HEAD#l506

-boris

Re: [PATCH v2 2/2] KVM: x86: Implement the arch-specific hook to report the VM UUID

[Sironi, Filippo]

> On 16. May 2019, at 18:40, Boris Ostrovsky <boris.ostrovsky@oracle.com> wrote:
> 
> On 5/16/19 11:33 AM, Alexander Graf wrote:
>> On 16.05.19 08:25, Sironi, Filippo wrote:
>>>> On 16. May 2019, at 15:56, Graf, Alexander <graf@amazon.com> wrote:
>>>> 
>>>> On 14.05.19 08:16, Filippo Sironi wrote:
>>>>> On x86, we report the UUID in DMI System Information (i.e., DMI Type 1)
>>>>> as VM UUID.
>>>>> 
>>>>> Signed-off-by: Filippo Sironi <sironi@amazon.de>
>>>>> ---
>>>>> arch/x86/kernel/kvm.c | 7 +++++++
>>>>> 1 file changed, 7 insertions(+)
>>>>> 
>>>>> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
>>>>> index 5c93a65ee1e5..441cab08a09d 100644
>>>>> --- a/arch/x86/kernel/kvm.c
>>>>> +++ b/arch/x86/kernel/kvm.c
>>>>> @@ -25,6 +25,7 @@
>>>>> #include <linux/kernel.h>
>>>>> #include <linux/kvm_para.h>
>>>>> #include <linux/cpu.h>
>>>>> +#include <linux/dmi.h>
>>>>> #include <linux/mm.h>
>>>>> #include <linux/highmem.h>
>>>>> #include <linux/hardirq.h>
>>>>> @@ -694,6 +695,12 @@ bool kvm_para_available(void)
>>>>> }
>>>>> EXPORT_SYMBOL_GPL(kvm_para_available);
>>>>> 
>>>>> +const char *kvm_para_get_uuid(void)
>>>>> +{
>>>>> +	return dmi_get_system_info(DMI_PRODUCT_UUID);
>>>> This adds a new dependency on CONFIG_DMI. Probably best to guard it with
>>>> an #if IS_ENABLED(CONFIG_DMI).
>>>> 
>>>> The concept seems sound though.
>>>> 
>>>> Alex
>>> include/linux/dmi.h contains a dummy implementation of
>>> dmi_get_system_info that returns NULL if CONFIG_DMI isn't defined.
>> 
>> Oh, I missed that bit. Awesome! Less work :).
>> 
>> 
>>> This is enough unless we decide to return "<denied>" like in Xen.
>>> If then, we can have the check in the generic code to turn NULL
>>> into "<denied>".
>> 
>> Yes. Waiting for someone from Xen to answer this :)
> 
> Not sure I am answering your question but on Xen we return UUID value
> zero if access permissions are not sufficient. Not <denied>.
> 
> http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/common/kernel.c;h=612575430f1ce7faf5bd66e7a99f1758c63fb3cb;hb=HEAD#l506
> 
> -boris

Then, I believe that returning 00000000-0000-0000-0000-000000000000
instead of NULL in the weak implementation of 1/2 and translating
NULL into 00000000-0000-0000-0000-000000000000 is the better approach.

I'll repost.

Filippo





Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B

Re: [PATCH v2 2/2] KVM: x86: Implement the arch-specific hook to report the VM UUID

[Alexander Graf]

On 16.05.19 10:41, Sironi, Filippo wrote:
>> On 16. May 2019, at 18:40, Boris Ostrovsky <boris.ostrovsky@oracle.com> wrote:
>>
>> On 5/16/19 11:33 AM, Alexander Graf wrote:
>>> On 16.05.19 08:25, Sironi, Filippo wrote:
>>>>> On 16. May 2019, at 15:56, Graf, Alexander <graf@amazon.com> wrote:
>>>>>
>>>>> On 14.05.19 08:16, Filippo Sironi wrote:
>>>>>> On x86, we report the UUID in DMI System Information (i.e., DMI Type 1)
>>>>>> as VM UUID.
>>>>>>
>>>>>> Signed-off-by: Filippo Sironi <sironi@amazon.de>
>>>>>> ---
>>>>>> arch/x86/kernel/kvm.c | 7 +++++++
>>>>>> 1 file changed, 7 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
>>>>>> index 5c93a65ee1e5..441cab08a09d 100644
>>>>>> --- a/arch/x86/kernel/kvm.c
>>>>>> +++ b/arch/x86/kernel/kvm.c
>>>>>> @@ -25,6 +25,7 @@
>>>>>> #include <linux/kernel.h>
>>>>>> #include <linux/kvm_para.h>
>>>>>> #include <linux/cpu.h>
>>>>>> +#include <linux/dmi.h>
>>>>>> #include <linux/mm.h>
>>>>>> #include <linux/highmem.h>
>>>>>> #include <linux/hardirq.h>
>>>>>> @@ -694,6 +695,12 @@ bool kvm_para_available(void)
>>>>>> }
>>>>>> EXPORT_SYMBOL_GPL(kvm_para_available);
>>>>>>
>>>>>> +const char *kvm_para_get_uuid(void)
>>>>>> +{
>>>>>> +	return dmi_get_system_info(DMI_PRODUCT_UUID);
>>>>> This adds a new dependency on CONFIG_DMI. Probably best to guard it with
>>>>> an #if IS_ENABLED(CONFIG_DMI).
>>>>>
>>>>> The concept seems sound though.
>>>>>
>>>>> Alex
>>>> include/linux/dmi.h contains a dummy implementation of
>>>> dmi_get_system_info that returns NULL if CONFIG_DMI isn't defined.
>>> Oh, I missed that bit. Awesome! Less work :).
>>>
>>>
>>>> This is enough unless we decide to return "<denied>" like in Xen.
>>>> If then, we can have the check in the generic code to turn NULL
>>>> into "<denied>".
>>> Yes. Waiting for someone from Xen to answer this :)
>> Not sure I am answering your question but on Xen we return UUID value
>> zero if access permissions are not sufficient. Not <denied>.
>>
>> http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/common/kernel.c;h=612575430f1ce7faf5bd66e7a99f1758c63fb3cb;hb=HEAD#l506
>>
>> -boris
> Then, I believe that returning 00000000-0000-0000-0000-000000000000
> instead of NULL in the weak implementation of 1/2 and translating
> NULL into 00000000-0000-0000-0000-000000000000 is the better approach.


Just keep it at NULL in kvm_para_get_uuid() and convert to the canonical
00000000-0000-0000-0000-000000000000 in uuid_show().

Alex