All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] KVM: Start populating /sys/hypervisor with KVM entries
@ 2018-10-09  9:54 Filippo Sironi
  2018-10-09 10:41 ` Christian Borntraeger
                   ` (4 more replies)
  0 siblings, 5 replies; 77+ messages in thread
From: Filippo Sironi @ 2018-10-09  9:54 UTC (permalink / raw)
  To: sironi, linux-kernel, kvm

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


^ permalink raw reply related	[flat|nested] 77+ messages in thread

* Re: [PATCH] KVM: Start populating /sys/hypervisor with KVM entries
  2018-10-09  9:54 [PATCH] KVM: Start populating /sys/hypervisor with KVM entries Filippo Sironi
@ 2018-10-09 10:41 ` Christian Borntraeger
  2018-10-09 16:21   ` Boris Ostrovsky
  2018-10-09 16:21   ` Boris Ostrovsky
  2018-10-09 15:00 ` Konrad Rzeszutek Wilk
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 77+ messages in thread
From: Christian Borntraeger @ 2018-10-09 10:41 UTC (permalink / raw)
  To: Filippo Sironi, linux-kernel, kvm



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);
> 


^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH] KVM: Start populating /sys/hypervisor with KVM entries
  2018-10-09  9:54 [PATCH] KVM: Start populating /sys/hypervisor with KVM entries Filippo Sironi
  2018-10-09 10:41 ` Christian Borntraeger
@ 2018-10-09 15:00 ` Konrad Rzeszutek Wilk
  2018-10-10  5:19 ` kbuild test robot
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 77+ messages in thread
From: Konrad Rzeszutek Wilk @ 2018-10-09 15:00 UTC (permalink / raw)
  To: Filippo Sironi; +Cc: linux-kernel, kvm

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
> 

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH] KVM: Start populating /sys/hypervisor with KVM entries
  2018-10-09 10:41 ` Christian Borntraeger
  2018-10-09 16:21   ` Boris Ostrovsky
@ 2018-10-09 16:21   ` Boris Ostrovsky
  2018-10-09 17:50     ` Cornelia Huck
  2018-10-09 17:50     ` Cornelia Huck
  1 sibling, 2 replies; 77+ messages in thread
From: Boris Ostrovsky @ 2018-10-09 16:21 UTC (permalink / raw)
  To: Christian Borntraeger, Filippo Sironi, linux-kernel, kvm, xen-devel
  Cc: vasu.srinivasan, Konrad Rzeszutek Wilk

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



^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH] KVM: Start populating /sys/hypervisor with KVM entries
  2018-10-09 10:41 ` Christian Borntraeger
@ 2018-10-09 16:21   ` Boris Ostrovsky
  2018-10-09 16:21   ` Boris Ostrovsky
  1 sibling, 0 replies; 77+ messages in thread
From: Boris Ostrovsky @ 2018-10-09 16:21 UTC (permalink / raw)
  To: Christian Borntraeger, Filippo Sironi, linux-kernel, kvm, xen-devel
  Cc: vasu.srinivasan, Konrad Rzeszutek Wilk

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



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH] KVM: Start populating /sys/hypervisor with KVM entries
  2018-10-09 16:21   ` Boris Ostrovsky
@ 2018-10-09 17:50     ` Cornelia Huck
  2018-10-09 17:50     ` Cornelia Huck
  1 sibling, 0 replies; 77+ messages in thread
From: Cornelia Huck @ 2018-10-09 17:50 UTC (permalink / raw)
  To: Boris Ostrovsky
  Cc: Christian Borntraeger, Filippo Sironi, linux-kernel, kvm,
	xen-devel, vasu.srinivasan, Konrad Rzeszutek Wilk

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.

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH] KVM: Start populating /sys/hypervisor with KVM entries
  2018-10-09 16:21   ` Boris Ostrovsky
  2018-10-09 17:50     ` Cornelia Huck
@ 2018-10-09 17:50     ` Cornelia Huck
  1 sibling, 0 replies; 77+ messages in thread
From: Cornelia Huck @ 2018-10-09 17:50 UTC (permalink / raw)
  To: Boris Ostrovsky
  Cc: kvm, Konrad Rzeszutek Wilk, Filippo Sironi, linux-kernel,
	Christian Borntraeger, xen-devel, vasu.srinivasan

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.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH] KVM: Start populating /sys/hypervisor with KVM entries
  2018-10-09  9:54 [PATCH] KVM: Start populating /sys/hypervisor with KVM entries Filippo Sironi
  2018-10-09 10:41 ` Christian Borntraeger
  2018-10-09 15:00 ` Konrad Rzeszutek Wilk
@ 2018-10-10  5:19 ` kbuild test robot
  2019-05-14 15:16   ` [Xen-devel] " Filippo Sironi
  2019-05-14 15:16 ` KVM: Start populating /sys/hypervisor with KVM entries Filippo Sironi
  4 siblings, 0 replies; 77+ messages in thread
From: kbuild test robot @ 2018-10-10  5:19 UTC (permalink / raw)
  To: Filippo Sironi; +Cc: kbuild-all, sironi, linux-kernel, kvm

[-- 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 --]

^ permalink raw reply	[flat|nested] 77+ messages in thread

* KVM: Start populating /sys/hypervisor with KVM entries
  2018-10-09  9:54 [PATCH] KVM: Start populating /sys/hypervisor with KVM entries Filippo Sironi
@ 2019-05-14 15:16   ` Filippo Sironi
  2018-10-09 15:00 ` Konrad Rzeszutek Wilk
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 77+ messages in thread
From: Filippo Sironi @ 2019-05-14 15:16 UTC (permalink / raw)
  To: linux-kernel, kvm, borntraeger, boris.ostrovsky, cohuck,
	konrad.wilk, xen-devel, vasu.srinivasan

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


^ permalink raw reply	[flat|nested] 77+ messages in thread

* KVM: Start populating /sys/hypervisor with KVM entries
  2018-10-09  9:54 [PATCH] KVM: Start populating /sys/hypervisor with KVM entries Filippo Sironi
                   ` (3 preceding siblings ...)
  2019-05-14 15:16   ` [Xen-devel] " Filippo Sironi
@ 2019-05-14 15:16 ` Filippo Sironi
  4 siblings, 0 replies; 77+ messages in thread
From: Filippo Sironi @ 2019-05-14 15:16 UTC (permalink / raw)
  To: linux-kernel, kvm, borntraeger, boris.ostrovsky, cohuck,
	konrad.wilk, xen-devel, vasu.srinivasan

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


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* [Xen-devel] KVM: Start populating /sys/hypervisor with KVM entries
@ 2019-05-14 15:16   ` Filippo Sironi
  0 siblings, 0 replies; 77+ messages in thread
From: Filippo Sironi @ 2019-05-14 15:16 UTC (permalink / raw)
  To: linux-kernel, kvm, borntraeger, boris.ostrovsky, cohuck,
	konrad.wilk, xen-devel, vasu.srinivasan

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


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
  2019-05-14 15:16   ` [Xen-devel] " Filippo Sironi
@ 2019-05-14 15:16     ` Filippo Sironi
  -1 siblings, 0 replies; 77+ messages in thread
From: Filippo Sironi @ 2019-05-14 15:16 UTC (permalink / raw)
  To: linux-kernel, kvm, borntraeger, boris.ostrovsky, cohuck,
	konrad.wilk, xen-devel, vasu.srinivasan
  Cc: 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


^ permalink raw reply related	[flat|nested] 77+ messages in thread

* [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
  2019-05-14 15:16   ` [Xen-devel] " Filippo Sironi
  (?)
  (?)
@ 2019-05-14 15:16   ` Filippo Sironi
  -1 siblings, 0 replies; 77+ messages in thread
From: Filippo Sironi @ 2019-05-14 15:16 UTC (permalink / raw)
  To: linux-kernel, kvm, borntraeger, boris.ostrovsky, cohuck,
	konrad.wilk, xen-devel, vasu.srinivasan
  Cc: 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


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 77+ messages in thread

* [Xen-devel] [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
@ 2019-05-14 15:16     ` Filippo Sironi
  0 siblings, 0 replies; 77+ messages in thread
From: Filippo Sironi @ 2019-05-14 15:16 UTC (permalink / raw)
  To: linux-kernel, kvm, borntraeger, boris.ostrovsky, cohuck,
	konrad.wilk, xen-devel, vasu.srinivasan
  Cc: 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


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 77+ messages in thread

* [PATCH v2 2/2] KVM: x86: Implement the arch-specific hook to report the VM UUID
  2019-05-14 15:16   ` [Xen-devel] " Filippo Sironi
@ 2019-05-14 15:16     ` Filippo Sironi
  -1 siblings, 0 replies; 77+ messages in thread
From: Filippo Sironi @ 2019-05-14 15:16 UTC (permalink / raw)
  To: linux-kernel, kvm, borntraeger, boris.ostrovsky, cohuck,
	konrad.wilk, xen-devel, vasu.srinivasan
  Cc: 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


^ permalink raw reply related	[flat|nested] 77+ messages in thread

* [PATCH v2 2/2] KVM: x86: Implement the arch-specific hook to report the VM UUID
  2019-05-14 15:16   ` [Xen-devel] " Filippo Sironi
                     ` (3 preceding siblings ...)
  (?)
@ 2019-05-14 15:16   ` Filippo Sironi
  -1 siblings, 0 replies; 77+ messages in thread
From: Filippo Sironi @ 2019-05-14 15:16 UTC (permalink / raw)
  To: linux-kernel, kvm, borntraeger, boris.ostrovsky, cohuck,
	konrad.wilk, xen-devel, vasu.srinivasan
  Cc: 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


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 77+ messages in thread

* [Xen-devel] [PATCH v2 2/2] KVM: x86: Implement the arch-specific hook to report the VM UUID
@ 2019-05-14 15:16     ` Filippo Sironi
  0 siblings, 0 replies; 77+ messages in thread
From: Filippo Sironi @ 2019-05-14 15:16 UTC (permalink / raw)
  To: linux-kernel, kvm, borntraeger, boris.ostrovsky, cohuck,
	konrad.wilk, xen-devel, vasu.srinivasan
  Cc: 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


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
  2019-05-14 15:16     ` [Xen-devel] " Filippo Sironi
@ 2019-05-14 15:26       ` Christian Borntraeger
  -1 siblings, 0 replies; 77+ messages in thread
From: Christian Borntraeger @ 2019-05-14 15:26 UTC (permalink / raw)
  To: Filippo Sironi, linux-kernel, kvm, boris.ostrovsky, cohuck,
	konrad.wilk, xen-devel, vasu.srinivasan



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);
> 


^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
  2019-05-14 15:16     ` [Xen-devel] " Filippo Sironi
  (?)
  (?)
@ 2019-05-14 15:26     ` Christian Borntraeger
  -1 siblings, 0 replies; 77+ messages in thread
From: Christian Borntraeger @ 2019-05-14 15:26 UTC (permalink / raw)
  To: Filippo Sironi, linux-kernel, kvm, boris.ostrovsky, cohuck,
	konrad.wilk, xen-devel, vasu.srinivasan



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);
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [Xen-devel] [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
@ 2019-05-14 15:26       ` Christian Borntraeger
  0 siblings, 0 replies; 77+ messages in thread
From: Christian Borntraeger @ 2019-05-14 15:26 UTC (permalink / raw)
  To: Filippo Sironi, linux-kernel, kvm, boris.ostrovsky, cohuck,
	konrad.wilk, xen-devel, vasu.srinivasan



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);
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
  2019-05-14 15:26       ` [Xen-devel] " Christian Borntraeger
@ 2019-05-14 16:09         ` Sironi, Filippo
  -1 siblings, 0 replies; 77+ messages in thread
From: Sironi, Filippo @ 2019-05-14 16:09 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: LKML, KVM list, boris.ostrovsky, cohuck, konrad.wilk, xen-devel,
	vasu.srinivasan



> 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



^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
  2019-05-14 15:26       ` [Xen-devel] " Christian Borntraeger
  (?)
@ 2019-05-14 16:09       ` Sironi, Filippo
  -1 siblings, 0 replies; 77+ messages in thread
From: Sironi, Filippo @ 2019-05-14 16:09 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: KVM list, konrad.wilk, cohuck, LKML, xen-devel, boris.ostrovsky,
	vasu.srinivasan



> 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



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [Xen-devel] [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
@ 2019-05-14 16:09         ` Sironi, Filippo
  0 siblings, 0 replies; 77+ messages in thread
From: Sironi, Filippo @ 2019-05-14 16:09 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: KVM list, konrad.wilk, cohuck, LKML, xen-devel, boris.ostrovsky,
	vasu.srinivasan



> 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



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
  2019-05-14 16:09         ` [Xen-devel] " Sironi, Filippo
@ 2019-05-14 16:31           ` Christian Borntraeger
  -1 siblings, 0 replies; 77+ messages in thread
From: Christian Borntraeger @ 2019-05-14 16:31 UTC (permalink / raw)
  To: Sironi, Filippo
  Cc: LKML, KVM list, boris.ostrovsky, cohuck, konrad.wilk, xen-devel,
	vasu.srinivasan



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.


^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
  2019-05-14 16:09         ` [Xen-devel] " Sironi, Filippo
  (?)
@ 2019-05-14 16:31         ` Christian Borntraeger
  -1 siblings, 0 replies; 77+ messages in thread
From: Christian Borntraeger @ 2019-05-14 16:31 UTC (permalink / raw)
  To: Sironi, Filippo
  Cc: KVM list, konrad.wilk, cohuck, LKML, xen-devel, boris.ostrovsky,
	vasu.srinivasan



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.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [Xen-devel] [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
@ 2019-05-14 16:31           ` Christian Borntraeger
  0 siblings, 0 replies; 77+ messages in thread
From: Christian Borntraeger @ 2019-05-14 16:31 UTC (permalink / raw)
  To: Sironi, Filippo
  Cc: KVM list, konrad.wilk, cohuck, LKML, xen-devel, boris.ostrovsky,
	vasu.srinivasan



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.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
  2019-05-14 16:09         ` [Xen-devel] " Sironi, Filippo
@ 2019-05-14 22:08           ` Sironi, Filippo
  -1 siblings, 0 replies; 77+ messages in thread
From: Sironi, Filippo @ 2019-05-14 22:08 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: LKML, KVM list, boris.ostrovsky, cohuck, konrad.wilk, xen-devel,
	vasu.srinivasan



> 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



^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
  2019-05-14 16:09         ` [Xen-devel] " Sironi, Filippo
                           ` (2 preceding siblings ...)
  (?)
@ 2019-05-14 22:08         ` Sironi, Filippo
  -1 siblings, 0 replies; 77+ messages in thread
From: Sironi, Filippo @ 2019-05-14 22:08 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: KVM list, konrad.wilk, cohuck, LKML, xen-devel, boris.ostrovsky,
	vasu.srinivasan



> 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



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [Xen-devel] [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
@ 2019-05-14 22:08           ` Sironi, Filippo
  0 siblings, 0 replies; 77+ messages in thread
From: Sironi, Filippo @ 2019-05-14 22:08 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: KVM list, konrad.wilk, cohuck, LKML, xen-devel, boris.ostrovsky,
	vasu.srinivasan



> 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



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
  2019-05-14 15:16     ` [Xen-devel] " Filippo Sironi
@ 2019-05-16 13:50       ` Alexander Graf
  -1 siblings, 0 replies; 77+ messages in thread
From: Alexander Graf @ 2019-05-16 13:50 UTC (permalink / raw)
  To: Filippo Sironi, linux-kernel, kvm, borntraeger, boris.ostrovsky,
	cohuck, konrad.wilk, xen-devel, vasu.srinivasan

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);
> 


^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
  2019-05-14 15:16     ` [Xen-devel] " Filippo Sironi
                       ` (3 preceding siblings ...)
  (?)
@ 2019-05-16 13:50     ` Alexander Graf
  -1 siblings, 0 replies; 77+ messages in thread
From: Alexander Graf @ 2019-05-16 13:50 UTC (permalink / raw)
  To: Filippo Sironi, linux-kernel, kvm, borntraeger, boris.ostrovsky,
	cohuck, konrad.wilk, xen-devel, vasu.srinivasan

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);
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [Xen-devel] [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
@ 2019-05-16 13:50       ` Alexander Graf
  0 siblings, 0 replies; 77+ messages in thread
From: Alexander Graf @ 2019-05-16 13:50 UTC (permalink / raw)
  To: Filippo Sironi, linux-kernel, kvm, borntraeger, boris.ostrovsky,
	cohuck, konrad.wilk, xen-devel, vasu.srinivasan

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);
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 2/2] KVM: x86: Implement the arch-specific hook to report the VM UUID
  2019-05-14 15:16     ` [Xen-devel] " Filippo Sironi
@ 2019-05-16 13:56       ` Alexander Graf
  -1 siblings, 0 replies; 77+ messages in thread
From: Alexander Graf @ 2019-05-16 13:56 UTC (permalink / raw)
  To: Filippo Sironi, linux-kernel, kvm, borntraeger, boris.ostrovsky,
	cohuck, konrad.wilk, xen-devel, vasu.srinivasan

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);
> 


^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 2/2] KVM: x86: Implement the arch-specific hook to report the VM UUID
  2019-05-14 15:16     ` [Xen-devel] " Filippo Sironi
  (?)
  (?)
@ 2019-05-16 13:56     ` Alexander Graf
  -1 siblings, 0 replies; 77+ messages in thread
From: Alexander Graf @ 2019-05-16 13:56 UTC (permalink / raw)
  To: Filippo Sironi, linux-kernel, kvm, borntraeger, boris.ostrovsky,
	cohuck, konrad.wilk, xen-devel, vasu.srinivasan

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);
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [Xen-devel] [PATCH v2 2/2] KVM: x86: Implement the arch-specific hook to report the VM UUID
@ 2019-05-16 13:56       ` Alexander Graf
  0 siblings, 0 replies; 77+ messages in thread
From: Alexander Graf @ 2019-05-16 13:56 UTC (permalink / raw)
  To: Filippo Sironi, linux-kernel, kvm, borntraeger, boris.ostrovsky,
	cohuck, konrad.wilk, xen-devel, vasu.srinivasan

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);
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [Xen-devel] [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
  2019-05-16 13:50       ` [Xen-devel] " Alexander Graf
  (?)
@ 2019-05-16 14:02         ` Andrew Cooper
  -1 siblings, 0 replies; 77+ messages in thread
From: Andrew Cooper @ 2019-05-16 14:02 UTC (permalink / raw)
  To: Alexander Graf, Filippo Sironi, linux-kernel, kvm, borntraeger,
	boris.ostrovsky, cohuck, konrad.wilk, xen-devel, vasu.srinivasan

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

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
@ 2019-05-16 14:02         ` Andrew Cooper
  0 siblings, 0 replies; 77+ messages in thread
From: Andrew Cooper @ 2019-05-16 14:02 UTC (permalink / raw)
  To: Alexander Graf, Filippo Sironi, linux-kernel, kvm, borntraeger,
	boris.ostrovsky, cohuck, konrad.wilk, xen-devel, vasu.srinivasan

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [Xen-devel] [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
@ 2019-05-16 14:02         ` Andrew Cooper
  0 siblings, 0 replies; 77+ messages in thread
From: Andrew Cooper @ 2019-05-16 14:02 UTC (permalink / raw)
  To: Alexander Graf, Filippo Sironi, linux-kernel, kvm, borntraeger,
	boris.ostrovsky, cohuck, konrad.wilk, xen-devel, vasu.srinivasan

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [Xen-devel] [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
  2019-05-16 14:02         ` Andrew Cooper
@ 2019-05-16 14:08           ` Alexander Graf
  -1 siblings, 0 replies; 77+ messages in thread
From: Alexander Graf @ 2019-05-16 14:08 UTC (permalink / raw)
  To: Andrew Cooper, Filippo Sironi, linux-kernel, kvm, borntraeger,
	boris.ostrovsky, cohuck, konrad.wilk, xen-devel


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



^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
  2019-05-16 14:02         ` Andrew Cooper
  (?)
  (?)
@ 2019-05-16 14:08         ` Alexander Graf
  -1 siblings, 0 replies; 77+ messages in thread
From: Alexander Graf @ 2019-05-16 14:08 UTC (permalink / raw)
  To: Andrew Cooper, Filippo Sironi, linux-kernel, kvm, borntraeger,
	boris.ostrovsky, cohuck, konrad.wilk, xen-devel


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



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [Xen-devel] [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
@ 2019-05-16 14:08           ` Alexander Graf
  0 siblings, 0 replies; 77+ messages in thread
From: Alexander Graf @ 2019-05-16 14:08 UTC (permalink / raw)
  To: Andrew Cooper, Filippo Sironi, linux-kernel, kvm, borntraeger,
	boris.ostrovsky, cohuck, konrad.wilk, xen-devel


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



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [Xen-devel] [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
  2019-05-16 14:08           ` Alexander Graf
@ 2019-05-16 15:02             ` Boris Ostrovsky
  -1 siblings, 0 replies; 77+ messages in thread
From: Boris Ostrovsky @ 2019-05-16 15:02 UTC (permalink / raw)
  To: Alexander Graf, Andrew Cooper, Filippo Sironi, linux-kernel, kvm,
	borntraeger, cohuck, konrad.wilk, xen-devel

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



^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
  2019-05-16 14:08           ` Alexander Graf
  (?)
  (?)
@ 2019-05-16 15:02           ` Boris Ostrovsky
  -1 siblings, 0 replies; 77+ messages in thread
From: Boris Ostrovsky @ 2019-05-16 15:02 UTC (permalink / raw)
  To: Alexander Graf, Andrew Cooper, Filippo Sironi, linux-kernel, kvm,
	borntraeger, cohuck, konrad.wilk, xen-devel

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



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [Xen-devel] [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
@ 2019-05-16 15:02             ` Boris Ostrovsky
  0 siblings, 0 replies; 77+ messages in thread
From: Boris Ostrovsky @ 2019-05-16 15:02 UTC (permalink / raw)
  To: Alexander Graf, Andrew Cooper, Filippo Sironi, linux-kernel, kvm,
	borntraeger, cohuck, konrad.wilk, xen-devel

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



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [Xen-devel] [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
  2019-05-16 15:02             ` Boris Ostrovsky
@ 2019-05-16 15:14               ` Sironi, Filippo
  -1 siblings, 0 replies; 77+ messages in thread
From: Sironi, Filippo @ 2019-05-16 15:14 UTC (permalink / raw)
  To: Boris Ostrovsky
  Cc: Graf, Alexander, Andrew Cooper, LKML, kvm, borntraeger, cohuck,
	konrad.wilk, xen-devel


> 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



^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
  2019-05-16 15:02             ` Boris Ostrovsky
  (?)
@ 2019-05-16 15:14             ` Sironi, Filippo
  -1 siblings, 0 replies; 77+ messages in thread
From: Sironi, Filippo @ 2019-05-16 15:14 UTC (permalink / raw)
  To: Boris Ostrovsky
  Cc: kvm, konrad.wilk, Andrew Cooper, cohuck, LKML, borntraeger, Graf,
	Alexander, xen-devel


> 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



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [Xen-devel] [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
@ 2019-05-16 15:14               ` Sironi, Filippo
  0 siblings, 0 replies; 77+ messages in thread
From: Sironi, Filippo @ 2019-05-16 15:14 UTC (permalink / raw)
  To: Boris Ostrovsky
  Cc: kvm, konrad.wilk, Andrew Cooper, cohuck, LKML, borntraeger, Graf,
	Alexander, xen-devel


> 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



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 2/2] KVM: x86: Implement the arch-specific hook to report the VM UUID
  2019-05-16 13:56       ` [Xen-devel] " Alexander Graf
@ 2019-05-16 15:25         ` Sironi, Filippo
  -1 siblings, 0 replies; 77+ messages in thread
From: Sironi, Filippo @ 2019-05-16 15:25 UTC (permalink / raw)
  To: Graf, Alexander
  Cc: LKML, KVM list, Christian Borntraeger, Boris Ostrovsky, cohuck,
	konrad.wilk, xen-devel, vasu.srinivasan


> 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



^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 2/2] KVM: x86: Implement the arch-specific hook to report the VM UUID
  2019-05-16 13:56       ` [Xen-devel] " Alexander Graf
  (?)
  (?)
@ 2019-05-16 15:25       ` Sironi, Filippo
  -1 siblings, 0 replies; 77+ messages in thread
From: Sironi, Filippo @ 2019-05-16 15:25 UTC (permalink / raw)
  To: Graf, Alexander
  Cc: KVM list, konrad.wilk, cohuck, LKML, Christian Borntraeger,
	xen-devel, Boris Ostrovsky, vasu.srinivasan


> 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



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [Xen-devel] [PATCH v2 2/2] KVM: x86: Implement the arch-specific hook to report the VM UUID
@ 2019-05-16 15:25         ` Sironi, Filippo
  0 siblings, 0 replies; 77+ messages in thread
From: Sironi, Filippo @ 2019-05-16 15:25 UTC (permalink / raw)
  To: Graf, Alexander
  Cc: KVM list, konrad.wilk, cohuck, LKML, Christian Borntraeger,
	xen-devel, Boris Ostrovsky, vasu.srinivasan


> 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



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 2/2] KVM: x86: Implement the arch-specific hook to report the VM UUID
  2019-05-16 15:25         ` [Xen-devel] " Sironi, Filippo
@ 2019-05-16 15:33           ` Alexander Graf
  -1 siblings, 0 replies; 77+ messages in thread
From: Alexander Graf @ 2019-05-16 15:33 UTC (permalink / raw)
  To: Sironi, Filippo
  Cc: LKML, KVM list, Christian Borntraeger, Boris Ostrovsky, cohuck,
	konrad.wilk, xen-devel, vasu.srinivasan


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



^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 2/2] KVM: x86: Implement the arch-specific hook to report the VM UUID
  2019-05-16 15:25         ` [Xen-devel] " Sironi, Filippo
  (?)
  (?)
@ 2019-05-16 15:33         ` Alexander Graf
  -1 siblings, 0 replies; 77+ messages in thread
From: Alexander Graf @ 2019-05-16 15:33 UTC (permalink / raw)
  To: Sironi, Filippo
  Cc: KVM list, konrad.wilk, cohuck, LKML, Christian Borntraeger,
	xen-devel, Boris Ostrovsky, vasu.srinivasan


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



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [Xen-devel] [PATCH v2 2/2] KVM: x86: Implement the arch-specific hook to report the VM UUID
@ 2019-05-16 15:33           ` Alexander Graf
  0 siblings, 0 replies; 77+ messages in thread
From: Alexander Graf @ 2019-05-16 15:33 UTC (permalink / raw)
  To: Sironi, Filippo
  Cc: KVM list, konrad.wilk, cohuck, LKML, Christian Borntraeger,
	xen-devel, Boris Ostrovsky, vasu.srinivasan


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



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 2/2] KVM: x86: Implement the arch-specific hook to report the VM UUID
  2019-05-16 15:33           ` [Xen-devel] " Alexander Graf
@ 2019-05-16 16:40             ` Boris Ostrovsky
  -1 siblings, 0 replies; 77+ messages in thread
From: Boris Ostrovsky @ 2019-05-16 16:40 UTC (permalink / raw)
  To: Alexander Graf, Sironi, Filippo
  Cc: LKML, KVM list, Christian Borntraeger, cohuck, konrad.wilk,
	xen-devel, vasu.srinivasan

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

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 2/2] KVM: x86: Implement the arch-specific hook to report the VM UUID
  2019-05-16 15:33           ` [Xen-devel] " Alexander Graf
  (?)
  (?)
@ 2019-05-16 16:40           ` Boris Ostrovsky
  -1 siblings, 0 replies; 77+ messages in thread
From: Boris Ostrovsky @ 2019-05-16 16:40 UTC (permalink / raw)
  To: Alexander Graf, Sironi, Filippo
  Cc: KVM list, konrad.wilk, cohuck, LKML, Christian Borntraeger,
	xen-devel, vasu.srinivasan

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [Xen-devel] [PATCH v2 2/2] KVM: x86: Implement the arch-specific hook to report the VM UUID
@ 2019-05-16 16:40             ` Boris Ostrovsky
  0 siblings, 0 replies; 77+ messages in thread
From: Boris Ostrovsky @ 2019-05-16 16:40 UTC (permalink / raw)
  To: Alexander Graf, Sironi, Filippo
  Cc: KVM list, konrad.wilk, cohuck, LKML, Christian Borntraeger,
	xen-devel, vasu.srinivasan

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 2/2] KVM: x86: Implement the arch-specific hook to report the VM UUID
  2019-05-16 16:40             ` [Xen-devel] " Boris Ostrovsky
@ 2019-05-16 17:41               ` Sironi, Filippo
  -1 siblings, 0 replies; 77+ messages in thread
From: Sironi, Filippo @ 2019-05-16 17:41 UTC (permalink / raw)
  To: Boris Ostrovsky
  Cc: Graf, Alexander, LKML, KVM list, Christian Borntraeger, cohuck,
	konrad.wilk, xen-devel, vasu.srinivasan


> 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



^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 2/2] KVM: x86: Implement the arch-specific hook to report the VM UUID
  2019-05-16 16:40             ` [Xen-devel] " Boris Ostrovsky
  (?)
  (?)
@ 2019-05-16 17:41             ` Sironi, Filippo
  -1 siblings, 0 replies; 77+ messages in thread
From: Sironi, Filippo @ 2019-05-16 17:41 UTC (permalink / raw)
  To: Boris Ostrovsky
  Cc: KVM list, konrad.wilk, cohuck, LKML, Christian Borntraeger, Graf,
	Alexander, xen-devel, vasu.srinivasan


> 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



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [Xen-devel] [PATCH v2 2/2] KVM: x86: Implement the arch-specific hook to report the VM UUID
@ 2019-05-16 17:41               ` Sironi, Filippo
  0 siblings, 0 replies; 77+ messages in thread
From: Sironi, Filippo @ 2019-05-16 17:41 UTC (permalink / raw)
  To: Boris Ostrovsky
  Cc: KVM list, konrad.wilk, cohuck, LKML, Christian Borntraeger, Graf,
	Alexander, xen-devel, vasu.srinivasan


> 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



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 2/2] KVM: x86: Implement the arch-specific hook to report the VM UUID
  2019-05-16 17:41               ` [Xen-devel] " Sironi, Filippo
@ 2019-05-16 17:49                 ` Alexander Graf
  -1 siblings, 0 replies; 77+ messages in thread
From: Alexander Graf @ 2019-05-16 17:49 UTC (permalink / raw)
  To: Sironi, Filippo, Boris Ostrovsky
  Cc: LKML, KVM list, Christian Borntraeger, cohuck, konrad.wilk,
	xen-devel, vasu.srinivasan


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


^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 2/2] KVM: x86: Implement the arch-specific hook to report the VM UUID
  2019-05-16 17:41               ` [Xen-devel] " Sironi, Filippo
  (?)
  (?)
@ 2019-05-16 17:49               ` Alexander Graf
  -1 siblings, 0 replies; 77+ messages in thread
From: Alexander Graf @ 2019-05-16 17:49 UTC (permalink / raw)
  To: Sironi, Filippo, Boris Ostrovsky
  Cc: KVM list, konrad.wilk, cohuck, LKML, Christian Borntraeger,
	xen-devel, vasu.srinivasan


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


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [Xen-devel] [PATCH v2 2/2] KVM: x86: Implement the arch-specific hook to report the VM UUID
@ 2019-05-16 17:49                 ` Alexander Graf
  0 siblings, 0 replies; 77+ messages in thread
From: Alexander Graf @ 2019-05-16 17:49 UTC (permalink / raw)
  To: Sironi, Filippo, Boris Ostrovsky
  Cc: KVM list, konrad.wilk, cohuck, LKML, Christian Borntraeger,
	xen-devel, vasu.srinivasan


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


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
  2019-05-16 13:50       ` [Xen-devel] " Alexander Graf
@ 2019-05-17 15:41         ` Sironi, Filippo
  -1 siblings, 0 replies; 77+ messages in thread
From: Sironi, Filippo @ 2019-05-17 15:41 UTC (permalink / raw)
  To: Graf, Alexander
  Cc: LKML, KVM list, Christian Borntraeger, Boris Ostrovsky, cohuck,
	konrad.wilk, xen-devel


> 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



^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
  2019-05-16 13:50       ` [Xen-devel] " Alexander Graf
                         ` (2 preceding siblings ...)
  (?)
@ 2019-05-17 15:41       ` Sironi, Filippo
  -1 siblings, 0 replies; 77+ messages in thread
From: Sironi, Filippo @ 2019-05-17 15:41 UTC (permalink / raw)
  To: Graf, Alexander
  Cc: KVM list, konrad.wilk, cohuck, LKML, Christian Borntraeger,
	xen-devel, Boris Ostrovsky


> 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



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [Xen-devel] [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
@ 2019-05-17 15:41         ` Sironi, Filippo
  0 siblings, 0 replies; 77+ messages in thread
From: Sironi, Filippo @ 2019-05-17 15:41 UTC (permalink / raw)
  To: Graf, Alexander
  Cc: KVM list, konrad.wilk, cohuck, LKML, Christian Borntraeger,
	xen-devel, Boris Ostrovsky


> 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



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
  2019-05-17 15:41         ` [Xen-devel] " Sironi, Filippo
@ 2019-05-31  9:06           ` Alexander Graf
  -1 siblings, 0 replies; 77+ messages in thread
From: Alexander Graf @ 2019-05-31  9:06 UTC (permalink / raw)
  To: Sironi, Filippo
  Cc: LKML, KVM list, Christian Borntraeger, Boris Ostrovsky, cohuck,
	konrad.wilk, xen-devel, Marc Zyngier, Christoffer Dall


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);

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
  2019-05-17 15:41         ` [Xen-devel] " Sironi, Filippo
  (?)
  (?)
@ 2019-05-31  9:06         ` Alexander Graf
  -1 siblings, 0 replies; 77+ messages in thread
From: Alexander Graf @ 2019-05-31  9:06 UTC (permalink / raw)
  To: Sironi, Filippo
  Cc: KVM list, konrad.wilk, Marc Zyngier, cohuck, LKML,
	Christian Borntraeger, xen-devel, Boris Ostrovsky,
	Christoffer Dall


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);

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [Xen-devel] [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
@ 2019-05-31  9:06           ` Alexander Graf
  0 siblings, 0 replies; 77+ messages in thread
From: Alexander Graf @ 2019-05-31  9:06 UTC (permalink / raw)
  To: Sironi, Filippo
  Cc: KVM list, konrad.wilk, Marc Zyngier, cohuck, LKML,
	Christian Borntraeger, xen-devel, Boris Ostrovsky,
	Christoffer Dall


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);

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
  2019-05-31  9:06           ` [Xen-devel] " Alexander Graf
@ 2019-05-31  9:12             ` Raslan, KarimAllah
  -1 siblings, 0 replies; 77+ messages in thread
From: Raslan, KarimAllah @ 2019-05-31  9:12 UTC (permalink / raw)
  To: Sironi, Filippo, Graf, Alexander
  Cc: boris.ostrovsky, linux-kernel, kvm, cohuck, konrad.wilk,
	borntraeger, christoffer.dall, Marc.Zyngier, xen-devel

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



^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
  2019-05-31  9:06           ` [Xen-devel] " Alexander Graf
  (?)
  (?)
@ 2019-05-31  9:12           ` Raslan, KarimAllah
  -1 siblings, 0 replies; 77+ messages in thread
From: Raslan, KarimAllah @ 2019-05-31  9:12 UTC (permalink / raw)
  To: Sironi, Filippo, Graf, Alexander
  Cc: kvm, konrad.wilk, Marc.Zyngier, cohuck, linux-kernel,
	borntraeger, xen-devel, boris.ostrovsky, christoffer.dall

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


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [Xen-devel] [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
@ 2019-05-31  9:12             ` Raslan, KarimAllah
  0 siblings, 0 replies; 77+ messages in thread
From: Raslan, KarimAllah @ 2019-05-31  9:12 UTC (permalink / raw)
  To: Sironi, Filippo, Graf, Alexander
  Cc: kvm, konrad.wilk, Marc.Zyngier, cohuck, linux-kernel,
	borntraeger, xen-devel, boris.ostrovsky, christoffer.dall

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


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
  2019-05-31  9:12             ` [Xen-devel] " Raslan, KarimAllah
@ 2019-05-31  9:26               ` Alexander Graf
  -1 siblings, 0 replies; 77+ messages in thread
From: Alexander Graf @ 2019-05-31  9:26 UTC (permalink / raw)
  To: Raslan, KarimAllah, Sironi, Filippo
  Cc: boris.ostrovsky, linux-kernel, kvm, cohuck, konrad.wilk,
	borntraeger, christoffer.dall, Marc.Zyngier, xen-devel


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



^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
  2019-05-31  9:12             ` [Xen-devel] " Raslan, KarimAllah
  (?)
  (?)
@ 2019-05-31  9:26             ` Alexander Graf
  -1 siblings, 0 replies; 77+ messages in thread
From: Alexander Graf @ 2019-05-31  9:26 UTC (permalink / raw)
  To: Raslan, KarimAllah, Sironi, Filippo
  Cc: kvm, konrad.wilk, Marc.Zyngier, cohuck, linux-kernel,
	borntraeger, xen-devel, boris.ostrovsky, christoffer.dall


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



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [Xen-devel] [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
@ 2019-05-31  9:26               ` Alexander Graf
  0 siblings, 0 replies; 77+ messages in thread
From: Alexander Graf @ 2019-05-31  9:26 UTC (permalink / raw)
  To: Raslan, KarimAllah, Sironi, Filippo
  Cc: kvm, konrad.wilk, Marc.Zyngier, cohuck, linux-kernel,
	borntraeger, xen-devel, boris.ostrovsky, christoffer.dall


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



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
  2019-05-31  9:12             ` [Xen-devel] " Raslan, KarimAllah
@ 2019-05-31  9:38               ` Marc Zyngier
  -1 siblings, 0 replies; 77+ messages in thread
From: Marc Zyngier @ 2019-05-31  9:38 UTC (permalink / raw)
  To: Raslan, KarimAllah
  Cc: Sironi, Filippo, Graf, Alexander, boris.ostrovsky, linux-kernel,
	kvm, cohuck, konrad.wilk, borntraeger, xen-devel,
	Christoffer Dall

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.

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
  2019-05-31  9:12             ` [Xen-devel] " Raslan, KarimAllah
                               ` (3 preceding siblings ...)
  (?)
@ 2019-05-31  9:38             ` Marc Zyngier
  -1 siblings, 0 replies; 77+ messages in thread
From: Marc Zyngier @ 2019-05-31  9:38 UTC (permalink / raw)
  To: Raslan, KarimAllah
  Cc: kvm, konrad.wilk, Sironi, Filippo, cohuck, linux-kernel,
	Christoffer Dall, borntraeger, Graf, Alexander, xen-devel,
	boris.ostrovsky

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.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

* Re: [Xen-devel] [PATCH v2 1/2] KVM: Start populating /sys/hypervisor with KVM entries
@ 2019-05-31  9:38               ` Marc Zyngier
  0 siblings, 0 replies; 77+ messages in thread
From: Marc Zyngier @ 2019-05-31  9:38 UTC (permalink / raw)
  To: Raslan, KarimAllah
  Cc: kvm, konrad.wilk, Sironi, Filippo, cohuck, linux-kernel,
	Christoffer Dall, borntraeger, Graf, Alexander, xen-devel,
	boris.ostrovsky

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.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 77+ messages in thread

end of thread, other threads:[~2019-05-31  9:39 UTC | newest]

Thread overview: 77+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-09  9:54 [PATCH] KVM: Start populating /sys/hypervisor with KVM entries Filippo Sironi
2018-10-09 10:41 ` Christian Borntraeger
2018-10-09 16:21   ` Boris Ostrovsky
2018-10-09 16:21   ` Boris Ostrovsky
2018-10-09 17:50     ` Cornelia Huck
2018-10-09 17:50     ` Cornelia Huck
2018-10-09 15:00 ` Konrad Rzeszutek Wilk
2018-10-10  5:19 ` kbuild test robot
2019-05-14 15:16 ` Filippo Sironi
2019-05-14 15:16   ` [Xen-devel] " Filippo Sironi
2019-05-14 15:16   ` [PATCH v2 1/2] " Filippo Sironi
2019-05-14 15:16     ` [Xen-devel] " Filippo Sironi
2019-05-14 15:26     ` Christian Borntraeger
2019-05-14 15:26       ` [Xen-devel] " Christian Borntraeger
2019-05-14 16:09       ` Sironi, Filippo
2019-05-14 16:09       ` Sironi, Filippo
2019-05-14 16:09         ` [Xen-devel] " Sironi, Filippo
2019-05-14 16:31         ` Christian Borntraeger
2019-05-14 16:31         ` Christian Borntraeger
2019-05-14 16:31           ` [Xen-devel] " Christian Borntraeger
2019-05-14 22:08         ` Sironi, Filippo
2019-05-14 22:08         ` Sironi, Filippo
2019-05-14 22:08           ` [Xen-devel] " Sironi, Filippo
2019-05-14 15:26     ` Christian Borntraeger
2019-05-16 13:50     ` Alexander Graf
2019-05-16 13:50       ` [Xen-devel] " Alexander Graf
2019-05-16 14:02       ` Andrew Cooper
2019-05-16 14:02         ` Andrew Cooper
2019-05-16 14:02         ` Andrew Cooper
2019-05-16 14:08         ` Alexander Graf
2019-05-16 14:08         ` [Xen-devel] " Alexander Graf
2019-05-16 14:08           ` Alexander Graf
2019-05-16 15:02           ` Boris Ostrovsky
2019-05-16 15:02             ` Boris Ostrovsky
2019-05-16 15:14             ` Sironi, Filippo
2019-05-16 15:14             ` [Xen-devel] " Sironi, Filippo
2019-05-16 15:14               ` Sironi, Filippo
2019-05-16 15:02           ` Boris Ostrovsky
2019-05-17 15:41       ` Sironi, Filippo
2019-05-17 15:41         ` [Xen-devel] " Sironi, Filippo
2019-05-31  9:06         ` Alexander Graf
2019-05-31  9:06           ` [Xen-devel] " Alexander Graf
2019-05-31  9:12           ` Raslan, KarimAllah
2019-05-31  9:12             ` [Xen-devel] " Raslan, KarimAllah
2019-05-31  9:26             ` Alexander Graf
2019-05-31  9:26               ` [Xen-devel] " Alexander Graf
2019-05-31  9:26             ` Alexander Graf
2019-05-31  9:38             ` Marc Zyngier
2019-05-31  9:38               ` [Xen-devel] " Marc Zyngier
2019-05-31  9:38             ` Marc Zyngier
2019-05-31  9:12           ` Raslan, KarimAllah
2019-05-31  9:06         ` Alexander Graf
2019-05-17 15:41       ` Sironi, Filippo
2019-05-16 13:50     ` Alexander Graf
2019-05-14 15:16   ` Filippo Sironi
2019-05-14 15:16   ` [PATCH v2 2/2] KVM: x86: Implement the arch-specific hook to report the VM UUID Filippo Sironi
2019-05-14 15:16     ` [Xen-devel] " Filippo Sironi
2019-05-16 13:56     ` Alexander Graf
2019-05-16 13:56       ` [Xen-devel] " Alexander Graf
2019-05-16 15:25       ` Sironi, Filippo
2019-05-16 15:25         ` [Xen-devel] " Sironi, Filippo
2019-05-16 15:33         ` Alexander Graf
2019-05-16 15:33           ` [Xen-devel] " Alexander Graf
2019-05-16 16:40           ` Boris Ostrovsky
2019-05-16 16:40             ` [Xen-devel] " Boris Ostrovsky
2019-05-16 17:41             ` Sironi, Filippo
2019-05-16 17:41               ` [Xen-devel] " Sironi, Filippo
2019-05-16 17:49               ` Alexander Graf
2019-05-16 17:49                 ` [Xen-devel] " Alexander Graf
2019-05-16 17:49               ` Alexander Graf
2019-05-16 17:41             ` Sironi, Filippo
2019-05-16 16:40           ` Boris Ostrovsky
2019-05-16 15:33         ` Alexander Graf
2019-05-16 15:25       ` Sironi, Filippo
2019-05-16 13:56     ` Alexander Graf
2019-05-14 15:16   ` Filippo Sironi
2019-05-14 15:16 ` KVM: Start populating /sys/hypervisor with KVM entries Filippo Sironi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.