| Message ID | 1557847002-23519-3-git-send-email-sironi@amazon.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/2] KVM: Start populating /sys/hypervisor with KVM entries | expand |
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); >
> 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
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
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
> 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
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
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);
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(+)