1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH 0/5] Refresh QEMU caps when CPU microcode changes
  4. View patch

[libvirt] [PATCH 2/5] util: introduce virHostCPUGetMicrocodeVersion

Jiri Denemark posted 5 patches 7 years, 4 months ago
[libvirt] [PATCH 2/5] util: introduce virHostCPUGetMicrocodeVersion
Posted by Jiri Denemark 7 years, 4 months ago 
From: Paolo Bonzini <pbonzini@redhat.com>

This new API reads host's CPU microcode version from /proc/cpuinfo.

Unfortunately, there is no other way of reading microcode version which
would be usable from both system and session daemon.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
 src/libvirt_private.syms |  1 +
 src/util/virhostcpu.c    | 43 +++++++++++++++++++++++++++++++++++++++++++
 src/util/virhostcpu.h    |  2 ++
 3 files changed, 46 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8d583e3e74..a705fa846d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1869,6 +1869,7 @@ virHostCPUGetCount;
 virHostCPUGetInfo;
 virHostCPUGetKVMMaxVCPUs;
 virHostCPUGetMap;
+virHostCPUGetMicrocodeVersion;
 virHostCPUGetOnline;
 virHostCPUGetOnlineBitmap;
 virHostCPUGetPresentBitmap;
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index c485a97211..713fdec553 100644
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -1206,3 +1206,46 @@ virHostCPUGetKVMMaxVCPUs(void)
     return -1;
 }
 #endif /* HAVE_LINUX_KVM_H */
+
+
+#ifdef __linux__
+
+unsigned int
+virHostCPUGetMicrocodeVersion(void)
+{
+    char *outbuf = NULL;
+    char *cur;
+    unsigned int version = 0;
+
+    if (virFileReadHeaderQuiet(CPUINFO_PATH, 4096, &outbuf) < 0) {
+        char ebuf[1024];
+        VIR_DEBUG("Failed to read microcode version from %s: %s",
+                  CPUINFO_PATH, virStrerror(errno, ebuf, sizeof(ebuf)));
+        return 0;
+    }
+
+    /* Account for format 'microcode    : XXXX'*/
+    if (!(cur = strstr(outbuf, "microcode")) ||
+        !(cur = strchr(cur, ':')))
+        goto cleanup;
+    cur++;
+
+    /* Linux places the microcode revision in a 32-bit integer, so
+     * ui is fine for us too.  */
+    if (virStrToLong_ui(cur, &cur, 0, &version) < 0)
+        goto cleanup;
+
+ cleanup:
+    VIR_FREE(outbuf);
+    return version;
+}
+
+#else
+
+unsigned int
+virHostCPUGetMicrocodeVersion(void)
+{
+    return 0;
+}
+
+#endif
diff --git a/src/util/virhostcpu.h b/src/util/virhostcpu.h
index 67033de842..f9f3359288 100644
--- a/src/util/virhostcpu.h
+++ b/src/util/virhostcpu.h
@@ -66,4 +66,6 @@ virBitmapPtr virHostCPUGetSiblingsList(unsigned int cpu);
 
 int virHostCPUGetOnline(unsigned int cpu, bool *online);
 
+unsigned int virHostCPUGetMicrocodeVersion(void);
+
 #endif /* __VIR_HOSTCPU_H__*/
-- 
2.15.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] util: introduce virHostCPUGetMicrocodeVersion
Posted by Peter Krempa 7 years, 4 months ago 
On Thu, Jan 04, 2018 at 15:58:09 +0100, Jiri Denemark wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
> 
> This new API reads host's CPU microcode version from /proc/cpuinfo.
> 
> Unfortunately, there is no other way of reading microcode version which
> would be usable from both system and session daemon.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
> ---
>  src/libvirt_private.syms |  1 +
>  src/util/virhostcpu.c    | 43 +++++++++++++++++++++++++++++++++++++++++++
>  src/util/virhostcpu.h    |  2 ++
>  3 files changed, 46 insertions(+)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 8d583e3e74..a705fa846d 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1869,6 +1869,7 @@ virHostCPUGetCount;
>  virHostCPUGetInfo;
>  virHostCPUGetKVMMaxVCPUs;
>  virHostCPUGetMap;
> +virHostCPUGetMicrocodeVersion;
>  virHostCPUGetOnline;
>  virHostCPUGetOnlineBitmap;
>  virHostCPUGetPresentBitmap;
> diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
> index c485a97211..713fdec553 100644
> --- a/src/util/virhostcpu.c
> +++ b/src/util/virhostcpu.c
> @@ -1206,3 +1206,46 @@ virHostCPUGetKVMMaxVCPUs(void)
>      return -1;
>  }
>  #endif /* HAVE_LINUX_KVM_H */
> +
> +
> +#ifdef __linux__
> +
> +unsigned int
> +virHostCPUGetMicrocodeVersion(void)
> +{
> +    char *outbuf = NULL;
> +    char *cur;
> +    unsigned int version = 0;
> +
> +    if (virFileReadHeaderQuiet(CPUINFO_PATH, 4096, &outbuf) < 0) {

In other places we read 1MiB of /proc/cpuinfo:

src/util/virsysinfo.c:#define CPUINFO_FILE_LEN (1024*1024)    /* 1MB limit for /proc/cpuinfo file */

Will this be enough?


> +        char ebuf[1024];
> +        VIR_DEBUG("Failed to read microcode version from %s: %s",
> +                  CPUINFO_PATH, virStrerror(errno, ebuf, sizeof(ebuf)));

Formatting the error message into a debug message does not make much
sense. I'd just log 'errno' in raw since this function does not really
return any errors.

> +        return 0;
> +    }
> +
> +    /* Account for format 'microcode    : XXXX'*/
> +    if (!(cur = strstr(outbuf, "microcode")) ||
> +        !(cur = strchr(cur, ':')))
> +        goto cleanup;
> +    cur++;
> +
> +    /* Linux places the microcode revision in a 32-bit integer, so
> +     * ui is fine for us too.  */
> +    if (virStrToLong_ui(cur, &cur, 0, &version) < 0)
> +        goto cleanup;
> +
> + cleanup:
> +    VIR_FREE(outbuf);
> +    return version;
> +}
> +
> +#else

#else /* __linux__ */

> +
> +unsigned int
> +virHostCPUGetMicrocodeVersion(void)
> +{
> +    return 0;
> +}
> +
> +#endif

#endif /* __linux__ */

> diff --git a/src/util/virhostcpu.h b/src/util/virhostcpu.h
> index 67033de842..f9f3359288 100644
> --- a/src/util/virhostcpu.h
> +++ b/src/util/virhostcpu.h
> @@ -66,4 +66,6 @@ virBitmapPtr virHostCPUGetSiblingsList(unsigned int cpu);
>  
>  int virHostCPUGetOnline(unsigned int cpu, bool *online);
>  
> +unsigned int virHostCPUGetMicrocodeVersion(void);
> +
>  #endif /* __VIR_HOSTCPU_H__*/

ACK with pointless error formatting removed.

> -- 
> 2.15.1
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] util: introduce virHostCPUGetMicrocodeVersion
Posted by Jiri Denemark 7 years, 4 months ago 
On Thu, Jan 04, 2018 at 16:15:46 +0100, Peter Krempa wrote:
> On Thu, Jan 04, 2018 at 15:58:09 +0100, Jiri Denemark wrote:
> > From: Paolo Bonzini <pbonzini@redhat.com>
> > 
> > This new API reads host's CPU microcode version from /proc/cpuinfo.
> > 
> > Unfortunately, there is no other way of reading microcode version which
> > would be usable from both system and session daemon.
> > 
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
> > ---
> >  src/libvirt_private.syms |  1 +
> >  src/util/virhostcpu.c    | 43 +++++++++++++++++++++++++++++++++++++++++++
> >  src/util/virhostcpu.h    |  2 ++
> >  3 files changed, 46 insertions(+)
> > 
> > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> > index 8d583e3e74..a705fa846d 100644
> > --- a/src/libvirt_private.syms
> > +++ b/src/libvirt_private.syms
> > @@ -1869,6 +1869,7 @@ virHostCPUGetCount;
> >  virHostCPUGetInfo;
> >  virHostCPUGetKVMMaxVCPUs;
> >  virHostCPUGetMap;
> > +virHostCPUGetMicrocodeVersion;
> >  virHostCPUGetOnline;
> >  virHostCPUGetOnlineBitmap;
> >  virHostCPUGetPresentBitmap;
> > diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
> > index c485a97211..713fdec553 100644
> > --- a/src/util/virhostcpu.c
> > +++ b/src/util/virhostcpu.c
> > @@ -1206,3 +1206,46 @@ virHostCPUGetKVMMaxVCPUs(void)
> >      return -1;
> >  }
> >  #endif /* HAVE_LINUX_KVM_H */
> > +
> > +
> > +#ifdef __linux__
> > +
> > +unsigned int
> > +virHostCPUGetMicrocodeVersion(void)
> > +{
> > +    char *outbuf = NULL;
> > +    char *cur;
> > +    unsigned int version = 0;
> > +
> > +    if (virFileReadHeaderQuiet(CPUINFO_PATH, 4096, &outbuf) < 0) {
> 
> In other places we read 1MiB of /proc/cpuinfo:
> 
> src/util/virsysinfo.c:#define CPUINFO_FILE_LEN (1024*1024)    /* 1MB limit for /proc/cpuinfo file */
> 
> Will this be enough?

Should be (TM) :-) We're only interested in the first CPU and the
microcode version is printed very early before the endlessly growing
list of CPU features.

> 
> 
> > +        char ebuf[1024];
> > +        VIR_DEBUG("Failed to read microcode version from %s: %s",
> > +                  CPUINFO_PATH, virStrerror(errno, ebuf, sizeof(ebuf)));
> 
> Formatting the error message into a debug message does not make much
> sense. I'd just log 'errno' in raw since this function does not really
> return any errors.
...
> ACK with pointless error formatting removed.

It's not pointless. It makes the log useful without having to look at
/usr/include/asm-generic/errno-base.h. And it will only be done once in
libvirtd lifetime so it's not really wasting CPU cycles or something.

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] util: introduce virHostCPUGetMicrocodeVersion
Posted by Ján Tomko 7 years, 4 months ago 
On Thu, Jan 04, 2018 at 04:27:45PM +0100, Jiri Denemark wrote:
>On Thu, Jan 04, 2018 at 16:15:46 +0100, Peter Krempa wrote:
>> On Thu, Jan 04, 2018 at 15:58:09 +0100, Jiri Denemark wrote:
>> > +        char ebuf[1024];
>> > +        VIR_DEBUG("Failed to read microcode version from %s: %s",
>> > +                  CPUINFO_PATH, virStrerror(errno, ebuf, sizeof(ebuf)));
>>
>> Formatting the error message into a debug message does not make much
>> sense. I'd just log 'errno' in raw since this function does not really
>> return any errors.
>...
>> ACK with pointless error formatting removed.
>
>It's not pointless. It makes the log useful without having to look at
>/usr/include/asm-generic/errno-base.h. And it will only be done once in
>libvirtd lifetime so it's not really wasting CPU cycles or something.
>

ACK including the error formatting. I see the point in it, CPU cycles
are cheaper than human cycles.

Jan

>Jirka
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] util: introduce virHostCPUGetMicrocodeVersion
Posted by Pino Toscano 7 years, 4 months ago 
On Thursday, 4 January 2018 16:27:45 CET Jiri Denemark wrote:
> It's not pointless. It makes the log useful without having to look at
> /usr/include/asm-generic/errno-base.h. And it will only be done once in
> libvirtd lifetime so it's not really wasting CPU cycles or something.

Theoretically you do not need to: if you install 'moreutils', you get
the useful 'errno' utility to map from errno numbers to descriptions
(even localized ones), and vice-versa.  The only issue is that this
works only for the same OS+architecture combination, since values of
errno changes between OSes and architectures in the same OS.

-- 
Pino Toscano--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.