[Xen-devel] [PATCH RFC] x86: Don't increase ApicIdCoreSize past 7

George Dunlap posted 1 patch 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/xen tags/patchew/20191125123923.2000028-1-george.dunlap@citrix.com
tools/libxc/xc_cpuid_x86.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

[Xen-devel] [PATCH RFC] x86: Don't increase ApicIdCoreSize past 7

Posted by George Dunlap 1 week ago
Changeset ca2eee92df44 ("x86, hvm: Expose host core/HT topology to HVM
guests") attempted to "fake up" a topology which would induce guest
operating systems to not treat vcpus as sibling hyperthreads.  This
involved actually reporting hyperthreading as available, but giving
vcpus every other ApicId; which in turn led to doubling the ApicIds
per core by bumping the ApicIdCoreSize by one.  In particular, Ryzen
3xxx series processors, and reportedly EPYC "Rome" cpus -- have an
ApicIdCoreSize of 7; the "fake" topology increases this to 8.

Unfortunately, Windows running on modern AMD hardware -- including
Ryzen 3xxx series processors, and reportedly EPYC "Rome" cpus --
doesn't seem to cope with this value being higher than 7.  (Linux
guests have so far continued to cope.)

A "proper" fix is complicated and it's too late to fix it either for
4.13, or to backport to supported branches.  As a short-term fix,
limit this value to 7.

This does mean that a Linux guest, booted on such a system without
this change, and then migrating to a system with this change, with
more than 64 vcpus, would see an apparent topology change.  This is a
low enough risk in practice that enabling this limit unilaterally, to
allow other guests to boot without manual intervention, is worth it.

Reported-by: Steven Haigh <netwiz@crc.id.au>
Reported-by: Andreas Kinzler <hfp@posteo.de>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Juergen Gross <jgross@suse.com>
---
 tools/libxc/xc_cpuid_x86.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index 312c481f1e..519d6d8bd0 100644
--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -616,10 +616,15 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid,
              * - going out of sync with leaf 1 EBX[23:16],
              * - incrementing ApicIdCoreSize when it's zero (which changes the
              *   meaning of bits 7:0).
+             *
+             * UPDATE: I addition to avoiding overflow, some
+             * proprietary operating systems have trouble with
+             * apic_id_size values greater than 7.  Limit the value to
+             * 7 for now.
              */
             if ( p->extd.nc < 0x7f )
             {
-                if ( p->extd.apic_id_size != 0 && p->extd.apic_id_size != 0xf )
+                if ( p->extd.apic_id_size != 0 && p->extd.apic_id_size < 0x7 )
                     p->extd.apic_id_size++;
 
                 p->extd.nc = (p->extd.nc << 1) | 1;
-- 
2.24.0


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

Re: [Xen-devel] [PATCH RFC] x86: Don't increase ApicIdCoreSize past 7

Posted by Wei Liu 1 week ago
On Mon, Nov 25, 2019 at 12:39:23PM +0000, George Dunlap wrote:
> Changeset ca2eee92df44 ("x86, hvm: Expose host core/HT topology to HVM
> guests") attempted to "fake up" a topology which would induce guest
> operating systems to not treat vcpus as sibling hyperthreads.  This
> involved actually reporting hyperthreading as available, but giving
> vcpus every other ApicId; which in turn led to doubling the ApicIds
> per core by bumping the ApicIdCoreSize by one.  In particular, Ryzen
> 3xxx series processors, and reportedly EPYC "Rome" cpus -- have an
> ApicIdCoreSize of 7; the "fake" topology increases this to 8.
> 
> Unfortunately, Windows running on modern AMD hardware -- including
> Ryzen 3xxx series processors, and reportedly EPYC "Rome" cpus --
> doesn't seem to cope with this value being higher than 7.  (Linux
> guests have so far continued to cope.)
> 
> A "proper" fix is complicated and it's too late to fix it either for
> 4.13, or to backport to supported branches.  As a short-term fix,
> limit this value to 7.
> 
> This does mean that a Linux guest, booted on such a system without
> this change, and then migrating to a system with this change, with
> more than 64 vcpus, would see an apparent topology change.  This is a
> low enough risk in practice that enabling this limit unilaterally, to
> allow other guests to boot without manual intervention, is worth it.
> 
> Reported-by: Steven Haigh <netwiz@crc.id.au>
> Reported-by: Andreas Kinzler <hfp@posteo.de>
> Signed-off-by: George Dunlap <george.dunlap@citrix.com>
> ---
> CC: Ian Jackson <ian.jackson@citrix.com>
> CC: Wei Liu <wl@xen.org>
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Jan Beulich <jbeulich@suse.com>
> CC: Juergen Gross <jgross@suse.com>
> ---
>  tools/libxc/xc_cpuid_x86.c | 7 ++++++-

I will defer this to x86 maintainers.

Seeing that you already have an Ack from Jan, feel free to add mine if
necessary.

Wei.

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

Re: [Xen-devel] [PATCH RFC] x86: Don't increase ApicIdCoreSize past 7

Posted by Jan Beulich 1 week ago
On 25.11.2019 13:39, George Dunlap wrote:
> Changeset ca2eee92df44 ("x86, hvm: Expose host core/HT topology to HVM
> guests") attempted to "fake up" a topology which would induce guest
> operating systems to not treat vcpus as sibling hyperthreads.  This
> involved actually reporting hyperthreading as available, but giving
> vcpus every other ApicId; which in turn led to doubling the ApicIds
> per core by bumping the ApicIdCoreSize by one.  In particular, Ryzen
> 3xxx series processors, and reportedly EPYC "Rome" cpus -- have an
> ApicIdCoreSize of 7; the "fake" topology increases this to 8.
> 
> Unfortunately, Windows running on modern AMD hardware -- including
> Ryzen 3xxx series processors, and reportedly EPYC "Rome" cpus --
> doesn't seem to cope with this value being higher than 7.  (Linux
> guests have so far continued to cope.)
> 
> A "proper" fix is complicated and it's too late to fix it either for
> 4.13, or to backport to supported branches.  As a short-term fix,
> limit this value to 7.
> 
> This does mean that a Linux guest, booted on such a system without
> this change, and then migrating to a system with this change, with
> more than 64 vcpus, would see an apparent topology change.  This is a
> low enough risk in practice that enabling this limit unilaterally, to
> allow other guests to boot without manual intervention, is worth it.
> 
> Reported-by: Steven Haigh <netwiz@crc.id.au>
> Reported-by: Andreas Kinzler <hfp@posteo.de>
> Signed-off-by: George Dunlap <george.dunlap@citrix.com>

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

> --- a/tools/libxc/xc_cpuid_x86.c
> +++ b/tools/libxc/xc_cpuid_x86.c
> @@ -616,10 +616,15 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid,
>               * - going out of sync with leaf 1 EBX[23:16],
>               * - incrementing ApicIdCoreSize when it's zero (which changes the
>               *   meaning of bits 7:0).
> +             *
> +             * UPDATE: I addition to avoiding overflow, some

... this becoming "UPDATE: In ...".

Jan

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

Re: [Xen-devel] [PATCH RFC] x86: Don't increase ApicIdCoreSize past 7

Posted by George Dunlap 1 week ago
On 11/25/19 12:49 PM, Jan Beulich wrote:
> On 25.11.2019 13:39, George Dunlap wrote:
>> Changeset ca2eee92df44 ("x86, hvm: Expose host core/HT topology to HVM
>> guests") attempted to "fake up" a topology which would induce guest
>> operating systems to not treat vcpus as sibling hyperthreads.  This
>> involved actually reporting hyperthreading as available, but giving
>> vcpus every other ApicId; which in turn led to doubling the ApicIds
>> per core by bumping the ApicIdCoreSize by one.  In particular, Ryzen
>> 3xxx series processors, and reportedly EPYC "Rome" cpus -- have an
>> ApicIdCoreSize of 7; the "fake" topology increases this to 8.
>>
>> Unfortunately, Windows running on modern AMD hardware -- including
>> Ryzen 3xxx series processors, and reportedly EPYC "Rome" cpus --
>> doesn't seem to cope with this value being higher than 7.  (Linux
>> guests have so far continued to cope.)
>>
>> A "proper" fix is complicated and it's too late to fix it either for
>> 4.13, or to backport to supported branches.  As a short-term fix,
>> limit this value to 7.
>>
>> This does mean that a Linux guest, booted on such a system without
>> this change, and then migrating to a system with this change, with
>> more than 64 vcpus, would see an apparent topology change.  This is a
>> low enough risk in practice that enabling this limit unilaterally, to
>> allow other guests to boot without manual intervention, is worth it.
>>
>> Reported-by: Steven Haigh <netwiz@crc.id.au>
>> Reported-by: Andreas Kinzler <hfp@posteo.de>
>> Signed-off-by: George Dunlap <george.dunlap@citrix.com>
> 
> Acked-by: Jan Beulich <jbeulich@suse.com>
> with ...
> 
>> --- a/tools/libxc/xc_cpuid_x86.c
>> +++ b/tools/libxc/xc_cpuid_x86.c
>> @@ -616,10 +616,15 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid,
>>               * - going out of sync with leaf 1 EBX[23:16],
>>               * - incrementing ApicIdCoreSize when it's zero (which changes the
>>               *   meaning of bits 7:0).
>> +             *
>> +             * UPDATE: I addition to avoiding overflow, some
> 
> ... this becoming "UPDATE: In ...".

Gah... Sorry, meant to apply this change on check-in, but screwed it up
(accidentally edited the wrong buffer).  Let me know if you want a
follow-up patch to fix it.

 -George

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

Re: [Xen-devel] [PATCH RFC] x86: Don't increase ApicIdCoreSize past 7

Posted by Jürgen Groß 1 week ago
On 25.11.19 13:39, George Dunlap wrote:
> Changeset ca2eee92df44 ("x86, hvm: Expose host core/HT topology to HVM
> guests") attempted to "fake up" a topology which would induce guest
> operating systems to not treat vcpus as sibling hyperthreads.  This
> involved actually reporting hyperthreading as available, but giving
> vcpus every other ApicId; which in turn led to doubling the ApicIds
> per core by bumping the ApicIdCoreSize by one.  In particular, Ryzen
> 3xxx series processors, and reportedly EPYC "Rome" cpus -- have an
> ApicIdCoreSize of 7; the "fake" topology increases this to 8.
> 
> Unfortunately, Windows running on modern AMD hardware -- including
> Ryzen 3xxx series processors, and reportedly EPYC "Rome" cpus --
> doesn't seem to cope with this value being higher than 7.  (Linux
> guests have so far continued to cope.)
> 
> A "proper" fix is complicated and it's too late to fix it either for
> 4.13, or to backport to supported branches.  As a short-term fix,
> limit this value to 7.
> 
> This does mean that a Linux guest, booted on such a system without
> this change, and then migrating to a system with this change, with
> more than 64 vcpus, would see an apparent topology change.  This is a
> low enough risk in practice that enabling this limit unilaterally, to
> allow other guests to boot without manual intervention, is worth it.
> 
> Reported-by: Steven Haigh <netwiz@crc.id.au>
> Reported-by: Andreas Kinzler <hfp@posteo.de>
> Signed-off-by: George Dunlap <george.dunlap@citrix.com>

Release-acked-by: Juergen Gross <jgross@suse.com>


Juergen

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