All of lore.kernel.org
 help / color / mirror / Atom feed
* [Xen-devel] [PATCH v3 for 4.13] x86/microcode: refuse to load the same revision ucode
@ 2019-11-26 15:41 Sergey Dyasli
  2019-11-26 15:51 ` Jan Beulich
  2019-11-27  3:10 ` Chao Gao
  0 siblings, 2 replies; 4+ messages in thread
From: Sergey Dyasli @ 2019-11-26 15:41 UTC (permalink / raw)
  To: xen-devel
  Cc: Juergen Gross, Sergey Dyasli, Andrew Cooper, Jan Beulich,
	Chao Gao, Roger Pau Monné

Currently if a user tries to live-load the same or older ucode revision
than CPU already has, he will get a single message in Xen log like:

    (XEN) 128 cores are to update their microcode

No actual ucode loading will happen and this situation can be quite
confusing. Fix this by starting ucode update only when the provided
ucode revision is higher than the currently cached one (if any).
This is based on the property that if microcode_cache exists, all CPUs
in the system should have at least that ucode revision.

Additionally, print a user friendly message if no matching or newer
ucode can be found in the provided blob. This also requires ignoring
-ENODATA in AMD-side code, otherwise the message given to the user is:

    (XEN) Parsing microcode blob error -61

Which actually means that a ucode blob was parsed fine, but no matching
ucode was found.

Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
---
v2 --> v3:
- move ucode comparison to generic code
- ignore -ENODATA in a different code section

v1 --> v2:
- compare provided ucode with the currently cached one

CC: Jan Beulich <jbeulich@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Chao Gao <chao.gao@intel.com>
CC: Juergen Gross <jgross@suse.com>
---
 xen/arch/x86/microcode.c     | 19 +++++++++++++++++++
 xen/arch/x86/microcode_amd.c |  7 +++++++
 2 files changed, 26 insertions(+)

diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
index 65d1f41e7c..44efc2d9b3 100644
--- a/xen/arch/x86/microcode.c
+++ b/xen/arch/x86/microcode.c
@@ -640,10 +640,29 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) buf, unsigned long len)
 
     if ( !patch )
     {
+        printk(XENLOG_WARNING "microcode: couldn't find any matching ucode in "
+                              "the provided blob!\n");
         ret = -ENOENT;
         goto put;
     }
 
+    /*
+     * If microcode_cache exists, all CPUs in the system should have at least
+     * that ucode revision.
+     */
+    spin_lock(&microcode_mutex);
+    if ( microcode_cache &&
+         microcode_ops->compare_patch(patch, microcode_cache) != NEW_UCODE )
+    {
+        spin_unlock(&microcode_mutex);
+        printk(XENLOG_WARNING "microcode: couldn't find any newer revision "
+                              "in the provided blob!\n");
+        ret = -ENOENT;
+
+        goto put;
+    }
+    spin_unlock(&microcode_mutex);
+
     if ( microcode_ops->start_update )
     {
         ret = microcode_ops->start_update();
diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c
index 1e52f7f49a..00750f7bbb 100644
--- a/xen/arch/x86/microcode_amd.c
+++ b/xen/arch/x86/microcode_amd.c
@@ -502,6 +502,13 @@ static struct microcode_patch *cpu_request_microcode(const void *buf,
 
     if ( error )
     {
+        /*
+         * -ENODATA here means that the blob was parsed fine but no matching
+         * ucode was found. Don't return it to the caller.
+         */
+        if ( error == -ENODATA )
+            error = 0;
+
         xfree(mc_amd->equiv_cpu_table);
         xfree(mc_amd);
         goto out;
-- 
2.17.1


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

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

* Re: [Xen-devel] [PATCH v3 for 4.13] x86/microcode: refuse to load the same revision ucode
  2019-11-26 15:41 [Xen-devel] [PATCH v3 for 4.13] x86/microcode: refuse to load the same revision ucode Sergey Dyasli
@ 2019-11-26 15:51 ` Jan Beulich
  2019-11-27  3:10 ` Chao Gao
  1 sibling, 0 replies; 4+ messages in thread
From: Jan Beulich @ 2019-11-26 15:51 UTC (permalink / raw)
  To: Sergey Dyasli
  Cc: Juergen Gross, Andrew Cooper, Roger Pau Monné, Chao Gao, xen-devel

On 26.11.2019 16:41, Sergey Dyasli wrote:
> Currently if a user tries to live-load the same or older ucode revision
> than CPU already has, he will get a single message in Xen log like:
> 
>     (XEN) 128 cores are to update their microcode
> 
> No actual ucode loading will happen and this situation can be quite
> confusing. Fix this by starting ucode update only when the provided
> ucode revision is higher than the currently cached one (if any).
> This is based on the property that if microcode_cache exists, all CPUs
> in the system should have at least that ucode revision.
> 
> Additionally, print a user friendly message if no matching or newer
> ucode can be found in the provided blob. This also requires ignoring
> -ENODATA in AMD-side code, otherwise the message given to the user is:
> 
>     (XEN) Parsing microcode blob error -61
> 
> Which actually means that a ucode blob was parsed fine, but no matching
> ucode was found.
> 
> Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

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

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

* Re: [Xen-devel] [PATCH v3 for 4.13] x86/microcode: refuse to load the same revision ucode
  2019-11-26 15:41 [Xen-devel] [PATCH v3 for 4.13] x86/microcode: refuse to load the same revision ucode Sergey Dyasli
  2019-11-26 15:51 ` Jan Beulich
@ 2019-11-27  3:10 ` Chao Gao
  2019-11-27 10:11   ` Sergey Dyasli
  1 sibling, 1 reply; 4+ messages in thread
From: Chao Gao @ 2019-11-27  3:10 UTC (permalink / raw)
  To: Sergey Dyasli
  Cc: Juergen Gross, Andrew Cooper, Roger Pau Monné,
	Jan Beulich, xen-devel

On Tue, Nov 26, 2019 at 03:41:53PM +0000, Sergey Dyasli wrote:
>Currently if a user tries to live-load the same or older ucode revision
>than CPU already has, he will get a single message in Xen log like:
>
>    (XEN) 128 cores are to update their microcode
>
>No actual ucode loading will happen and this situation can be quite
>confusing. Fix this by starting ucode update only when the provided
>ucode revision is higher than the currently cached one (if any).
>This is based on the property that if microcode_cache exists, all CPUs
>in the system should have at least that ucode revision.
>
>Additionally, print a user friendly message if no matching or newer
>ucode can be found in the provided blob. This also requires ignoring
>-ENODATA in AMD-side code, otherwise the message given to the user is:
>
>    (XEN) Parsing microcode blob error -61
>
>Which actually means that a ucode blob was parsed fine, but no matching
>ucode was found.
>
>Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
>---
>v2 --> v3:
>- move ucode comparison to generic code
>- ignore -ENODATA in a different code section
>
>v1 --> v2:
>- compare provided ucode with the currently cached one
>
>CC: Jan Beulich <jbeulich@suse.com>
>CC: Andrew Cooper <andrew.cooper3@citrix.com>
>CC: Roger Pau Monné <roger.pau@citrix.com>
>CC: Chao Gao <chao.gao@intel.com>
>CC: Juergen Gross <jgross@suse.com>
>---
> xen/arch/x86/microcode.c     | 19 +++++++++++++++++++
> xen/arch/x86/microcode_amd.c |  7 +++++++
> 2 files changed, 26 insertions(+)
>
>diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
>index 65d1f41e7c..44efc2d9b3 100644
>--- a/xen/arch/x86/microcode.c
>+++ b/xen/arch/x86/microcode.c
>@@ -640,10 +640,29 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) buf, unsigned long len)
> 
>     if ( !patch )
>     {
>+        printk(XENLOG_WARNING "microcode: couldn't find any matching ucode in "
>+                              "the provided blob!\n");
>         ret = -ENOENT;
>         goto put;
>     }
> 
>+    /*
>+     * If microcode_cache exists, all CPUs in the system should have at least
>+     * that ucode revision.
>+     */
>+    spin_lock(&microcode_mutex);
>+    if ( microcode_cache &&
>+         microcode_ops->compare_patch(patch, microcode_cache) != NEW_UCODE )
>+    {
>+        spin_unlock(&microcode_mutex);
>+        printk(XENLOG_WARNING "microcode: couldn't find any newer revision "
>+                              "in the provided blob!\n");

The patch needs to be freed.

With it fixed,
Reviewed-by: Chao Gao <chao.gao@intel.com>

Thanks
Chao

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

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

* Re: [Xen-devel] [PATCH v3 for 4.13] x86/microcode: refuse to load the same revision ucode
  2019-11-27  3:10 ` Chao Gao
@ 2019-11-27 10:11   ` Sergey Dyasli
  0 siblings, 0 replies; 4+ messages in thread
From: Sergey Dyasli @ 2019-11-27 10:11 UTC (permalink / raw)
  To: Chao Gao
  Cc: Juergen Gross, Andrew Cooper, Roger Pau Monné,
	Jan Beulich, xen-devel

On 27/11/2019 03:10, Chao Gao wrote:
> On Tue, Nov 26, 2019 at 03:41:53PM +0000, Sergey Dyasli wrote:
>> Currently if a user tries to live-load the same or older ucode revision
>> than CPU already has, he will get a single message in Xen log like:
>>
>>    (XEN) 128 cores are to update their microcode
>>
>> No actual ucode loading will happen and this situation can be quite
>> confusing. Fix this by starting ucode update only when the provided
>> ucode revision is higher than the currently cached one (if any).
>> This is based on the property that if microcode_cache exists, all CPUs
>> in the system should have at least that ucode revision.
>>
>> Additionally, print a user friendly message if no matching or newer
>> ucode can be found in the provided blob. This also requires ignoring
>> -ENODATA in AMD-side code, otherwise the message given to the user is:
>>
>>    (XEN) Parsing microcode blob error -61
>>
>> Which actually means that a ucode blob was parsed fine, but no matching
>> ucode was found.
>>
>> Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
>> ---
>> v2 --> v3:
>> - move ucode comparison to generic code
>> - ignore -ENODATA in a different code section
>>
>> v1 --> v2:
>> - compare provided ucode with the currently cached one
>>
>> CC: Jan Beulich <jbeulich@suse.com>
>> CC: Andrew Cooper <andrew.cooper3@citrix.com>
>> CC: Roger Pau Monné <roger.pau@citrix.com>
>> CC: Chao Gao <chao.gao@intel.com>
>> CC: Juergen Gross <jgross@suse.com>
>> ---
>> xen/arch/x86/microcode.c     | 19 +++++++++++++++++++
>> xen/arch/x86/microcode_amd.c |  7 +++++++
>> 2 files changed, 26 insertions(+)
>>
>> diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
>> index 65d1f41e7c..44efc2d9b3 100644
>> --- a/xen/arch/x86/microcode.c
>> +++ b/xen/arch/x86/microcode.c
>> @@ -640,10 +640,29 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) buf, unsigned long len)
>>
>>     if ( !patch )
>>     {
>> +        printk(XENLOG_WARNING "microcode: couldn't find any matching ucode in "
>> +                              "the provided blob!\n");
>>         ret = -ENOENT;
>>         goto put;
>>     }
>>
>> +    /*
>> +     * If microcode_cache exists, all CPUs in the system should have at least
>> +     * that ucode revision.
>> +     */
>> +    spin_lock(&microcode_mutex);
>> +    if ( microcode_cache &&
>> +         microcode_ops->compare_patch(patch, microcode_cache) != NEW_UCODE )
>> +    {
>> +        spin_unlock(&microcode_mutex);
>> +        printk(XENLOG_WARNING "microcode: couldn't find any newer revision "
>> +                              "in the provided blob!\n");
> 
> The patch needs to be freed.

Thanks for noticing this!

--
Sergey

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

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

end of thread, other threads:[~2019-11-27 10:12 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-26 15:41 [Xen-devel] [PATCH v3 for 4.13] x86/microcode: refuse to load the same revision ucode Sergey Dyasli
2019-11-26 15:51 ` Jan Beulich
2019-11-27  3:10 ` Chao Gao
2019-11-27 10:11   ` Sergey Dyasli

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.