[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 2/2] x86/cpuid: support LFENCE always serializing CPUID bit


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Thu, 15 Apr 2021 15:31:18 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=mixK3H5yKy8vkzxJMSMbjHfRcDAWkTugFFXd2m9beUo=; b=ITGwLDkjr3kGVngnHe76xpNL+PMaBZN7tK6EBAJBDooyMhQ3dN+Z3EjAaoh14Nji+xX4mdpUSSBfR0gbCraD0Jceq7G5SxR7kAur5cZSEW6lRKD5PpWb9KOL1E2urHze/OGIJw8bdXr1AN273tRRD4wOMIwqIW4tbjBSBWgeN4tLvJMtX5+7yrANHUzLv9WS6RQutkeilwu4FKr3dHJsVTL20XB7ckyV4AHV0vYJco54zUTZwt0fv2QB7ayYxHRNI1zBJkrBJ71ta/ler5ZeV5jy0s/e5PVeB+KlKw55h0lCY7lLdbuJRICKTaQx/setrQQQ6CphQ5ZHwMECtJG7iQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=nqeZngYPiL9uybW7DXHkbRSJ5PSNO/HeQD//SowSTMQpLYC2cxUVgxGVtTWFN+aSuUN3Co6jK7HMUMTZXMqAhaOgw9aUK1EWgDb2C5Q8kwH94NvhDY3c6rqSQpg+SCAbij7orXh/4CZMwIphvxpuDkfz/HbyUYykbXhYenmoxKZUVkphkbQwLwbuG37Ggl2eURJNyEPI6dgbTSTPiLS2IsQMgFvweAmXM0QmmE56nlkoYWQomFcaDR3sp/eZ8CKR0+CmXmQ7w7ttOuzl2VrUKKiC2poJAcX7mJU+DrOMKYXSb23xV4Z462qOUQvV+pezhprgLYzNwA96jfjSfzBowQ==
  • Authentication-results: esa4.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, "Ian Jackson" <iwj@xxxxxxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Thu, 15 Apr 2021 13:31:31 +0000
  • Ironport-hdrordr: A9a23:FmM6iK6I+n2F1bmpRgPXwXqEI+orLtY04lQ7vn1ZYSd+NuSFis Gjm+ka3xfoiDAXHEotg8yEJbPoex3h3LZPy800Ma25VAfr/FGpIoZr8Jf4z1TbdBHW3tV2kZ 1te60WMrHNJHBnkMf35xS5Gd48wN+BtJuln/va0m0Fd2FXQotLhj0JbDqzOEtwWQVAGN4VFI CE4NBGujqnfh0sH7mGL1MCWPXOoMCOqYnvZgQICwVixA6Fiz6p77CSKWnl4j41VTRTzbA+tV XUigCR3NTYj9iX6D/5k1XS4ZNfhcf7xrJ4ZfCkp8AJJlzX+2OVTat7XbnqhkFQnMiO7xIQnM DIs1McOa1ImgzsV0WUhTeo5AX6yjYp7BbZuCylqF/uu9bwSj5/K+cpv/MgTjLj50AtvM5x3c twtgrz3fcnbmKj7VHAzuPFWB1wmk2/rWBKq59ps1VlXZYDc7gUlIQD/SpuYec9NRjn44MqGv QGNrCk2N9qdzqhHhfkl1gq6tmtUnMvJwyBU0gPt+eEugIm7UxR/g82wtcSkWwH8494Y55Y5/ 7cOqAtr71WSNQKBJgNS9spcI+SMCjgUBjMOGWdLRDOE7wGAWvEr9rS7K8u7O+nVZQUxPIJ6d r8eWIdkVR3V1PlCMWI0pEO2AvKWn+BUTPkzdwbz4Rlu5XnLYCbchGreRQLqY+Nsv8fCsrUV7 KYI5RNGcLuKmPoBMJgwxD+YZ9PMnMTOfdl+uoTaharmIbmO4fqvuvUfLL4P7z2CwspXWv5Hz 8tRz72CMJc7l26e3PxjRTLMkmdP3DXzNZVKuz37uITwI8COslnqQ4Ok2m04cmNNHljv8UNDQ 9DCYKitpn+iXi9/G7O4WksEAFaFFxp7LLpVG4PgQcLNkjzYIsSotn3QxEU4FK3YjtEC+/GGg 9WoFp6vYitKYaL+CwkA9W7dkWXkmUUv3DPa5sHgKWM6YPEd/oDf9cbcZ00MT+OOw1+mA5spm sGQhQDXFXjGjTnjrjgqocVCuHZf9xVmxyqPsZQlHLauSyn1IMSb0peewTrfd+cgA4oSTYRrE Z26bUjjL2JnivqFXEym90iMFpHaH2eBZVPCAjtXvQTppnbPCVLCUuajz2TjB8+Pk7n7V8biG DaISqIQv3TGVZGtndE0qHlzUNsegymDjBNQ0E/lbc4OXXNu3513+POXKa13meLQnYpw+0WMl j+EHEvCzIr4+ry+A+emT6EG3lj+44nOfbFCq8/N5vJ3Gm2FYGOnaYaPvNd8Zp/LuryuusTXe /3QX7NEBrIT8cSnyCFrHcsPyd57EQ+mfTzwRv/8SyW2mU8Dfe6GiUue5grZ/Wnq07qSPaD3M 8n0ZYbve6sPn7wbdDD46fNdDJHIg7Sp2nzb+xAk+EigYsC8J9IW7/cWn/08VsC+jMUBsL9jl kfT6R2+6qpAP4lQ+UiPwZiumM0n9GOJnYxugP4AuUCbUgg5kWrS++h0v7tk/4TGUWPqwv7BE mH/wBc9/nDWTGf1bRyMdNHHU1mLGw94m9l5uWMasn5DxirbfhK+DOBQzKAWY4YbKiOArMLqB lmp/mOgu+MbiL9nCTdpyFyLK4L02GpR6qJcU6xMN8N19yxIlKXhKS2pOa1kTfsUDO+L30iur ctTz1ZUu1zzh84jIM21SCuSqv45mId+mEunw1PpxrKwYip4GDSAEdcFxbW668mBQVuDg==
  • Ironport-sdr: lW3m7Vf3qU58HxPiHgqlmW60L8Pegctbcdon6jEu+k6chU3ExAJbPb4qgmAJlJNGS4HgGA5gVB YS3Q08Sl7wd9WbxPByoMd/hbSjlRR/ItaZgm9nLpO6efKWNL2DWsw9tOAWIX6Qr5ndHJyb2SD1 auyXiL7xm3+4FZNxlLHCvPjdvAov+BBmPyZ0dTnei0D2M6s4HuTDYWpteB67RpKbtpr53rkcwK FoP8BuZnP2WserqjY95gTKGPYQy0rD3ciY+WOLJxIsOgyj49uQxj6/3HdstHnNvF+H7TZJGIJf vnE=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Wed, Apr 14, 2021 at 03:57:22PM +0200, Jan Beulich wrote:
> On 14.04.2021 15:25, Andrew Cooper wrote:
> > On 14/04/2021 14:05, Andrew Cooper wrote:
> >> On 14/04/2021 13:57, Jan Beulich wrote:
> >>> On 14.04.2021 13:04, Roger Pau Monne wrote:
> >>>> @@ -264,6 +265,38 @@ struct cpuid_policy
> >>>>              };
> >>>>              uint32_t nc:8, :4, apic_id_size:4, :16;
> >>>>              uint32_t /* d */:32;
> >>>> +
> >>>> +            uint64_t :64, :64; /* Leaf 0x80000009. */
> >>>> +            uint64_t :64, :64; /* Leaf 0x8000000a - SVM rev and 
> >>>> features. */
> >>>> +            uint64_t :64, :64; /* Leaf 0x8000000b. */
> >>>> +            uint64_t :64, :64; /* Leaf 0x8000000c. */
> >>>> +            uint64_t :64, :64; /* Leaf 0x8000000d. */
> >>>> +            uint64_t :64, :64; /* Leaf 0x8000000e. */
> >>>> +            uint64_t :64, :64; /* Leaf 0x8000000f. */
> >>>> +            uint64_t :64, :64; /* Leaf 0x80000010. */
> >>>> +            uint64_t :64, :64; /* Leaf 0x80000011. */
> >>>> +            uint64_t :64, :64; /* Leaf 0x80000012. */
> >>>> +            uint64_t :64, :64; /* Leaf 0x80000013. */
> >>>> +            uint64_t :64, :64; /* Leaf 0x80000014. */
> >>>> +            uint64_t :64, :64; /* Leaf 0x80000015. */
> >>>> +            uint64_t :64, :64; /* Leaf 0x80000016. */
> >>>> +            uint64_t :64, :64; /* Leaf 0x80000017. */
> >>>> +            uint64_t :64, :64; /* Leaf 0x80000018. */
> >>>> +            uint64_t :64, :64; /* Leaf 0x80000019 - TLB 1GB 
> >>>> Identifiers. */
> >>>> +            uint64_t :64, :64; /* Leaf 0x8000001a - Performance related 
> >>>> info. */
> >>>> +            uint64_t :64, :64; /* Leaf 0x8000001b - IBS feature 
> >>>> information. */
> >>>> +            uint64_t :64, :64; /* Leaf 0x8000001c. */
> >>>> +            uint64_t :64, :64; /* Leaf 0x8000001d - Cache properties. */
> >>>> +            uint64_t :64, :64; /* Leaf 0x8000001e - Extd APIC/Core/Node 
> >>>> IDs. */
> >>>> +            uint64_t :64, :64; /* Leaf 0x8000001f - AMD Secure 
> >>>> Encryption. */
> >>>> +            uint64_t :64, :64; /* Leaf 0x80000020 - Platform QoS. */
> >>>> +
> >>>> +            /* Leaf 0x80000021 - Extended Feature 2 */
> >>>> +            union {
> >>>> +                uint32_t e21a;
> >>>> +                struct { DECL_BITFIELD(e21a); };
> >>>> +            };
> >>>> +            uint32_t /* b */:32, /* c */:32, /* d */:32;
> >>>>          };
> >>>>      } extd;
> >>> Due to the effect of this on what guests get to see, I think this
> >>> wants to take my "x86/CPUID: shrink max_{,sub}leaf fields according
> >>> to actual leaf contents" as a prereq, which in turn may better
> >>> remain on top of "x86/CPUID: adjust extended leaves out of range
> >>> clearing" (both are neighbors in that over 4 months old series,
> >>> fair parts of which could imo go in irrespective of the unsettled
> >>> dispute on xvmalloc() - unfortunately I had made that patch 2 of
> >>> the series, not expecting it to be blocked for so long, and then
> >>> presumably signaling to others that the rest of the series is also
> >>> blocked).
> >> There is no shrinking to be done in this case.  The bit is set across
> >> the board on AMD/Hygon hardware, even on older parts.
> >>
> >> What does need changing however is the logic to trim max_extd_leaf down
> >> to what hardware supports, so the bit is visible on Rome/older
> >> hardware.  I.e. after this change, all VMs should get 0x80000021 by
> >> default on AMD hardware.
> >>
> >> (A curious observation of Milan hardware is that it actually advertises
> >> 0x80000023 as max_extd_leaf, and has two leaves of zeros at the end. 
> >> I've got an open query about this.)
> > 
> > Something like this:
> > 
> > diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
> > index 050cd5713e..d9eb2878c5 100644
> > --- a/xen/arch/x86/cpuid.c
> > +++ b/xen/arch/x86/cpuid.c
> > @@ -311,6 +311,7 @@ static void __init calculate_raw_policy(void)
> >  static void __init calculate_host_policy(void)
> >  {
> >      struct cpuid_policy *p = &host_cpuid_policy;
> > +    unsigned int max_extd_leaf;
> >  
> >      *p = raw_cpuid_policy;
> >  
> > @@ -318,7 +319,18 @@ static void __init calculate_host_policy(void)
> >          min_t(uint32_t, p->basic.max_leaf,   ARRAY_SIZE(p->basic.raw) - 1);
> >      p->feat.max_subleaf =
> >          min_t(uint32_t, p->feat.max_subleaf, ARRAY_SIZE(p->feat.raw) - 1);
> > -    p->extd.max_leaf = 0x80000000 | min_t(uint32_t, p->extd.max_leaf &
> > 0xffff,
> > +
> > +    max_extd_leaf = p->extd.max_leaf;
> > +
> > +    /*
> > +     * For AMD/Hygon hardware before Zen3, we modify LFENCE to be dispatch
> > +     * serialsing.  Extend max_extd_leaf beyond what hardware supports, to
> > +     * include the feature leaf containing this information.
> > +     */
> > +    if ( cpu_has_lfence_dispatch )
> > +        max_extd_leaf = max(max_extd_leaf, 0x80000021);
> > +
> > +    p->extd.max_leaf = 0x80000000 | min_t(uint32_t, max_extd_leaf & 0xffff,
> >                                            ARRAY_SIZE(p->extd.raw) - 1);
> >  
> >      cpuid_featureset_to_policy(boot_cpu_data.x86_capability, p);
> 
> Well, why not set it to ARRAY_SIZE() and then have
> x86_cpuid_policy_shrink_max_leaves() (from "x86/CPUID: shrink
> max_{,sub}leaf fields according to actual leaf contents") have
> a go? It'll recognize the non-zero leaf ... Otherwise, if we
> gain a few more such special cases, things are going to get
> ugly here.

I will wait for Jan to post the updated version of his shrink patch
and then rebase mine on top in order to set extd.max_leaf to
ARRAY_SIZE and let the shrink logic deal with it.

Thanks, Roger.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.