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

Re: [Xen-devel] [PATCH RFC] x86: Add hack to disable "Fake HT" mode


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: George Dunlap <george.dunlap@xxxxxxxxxx>
  • Date: Fri, 15 Nov 2019 14:55:41 +0000
  • Authentication-results: esa2.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none; spf=None smtp.pra=george.dunlap@xxxxxxxxxx; spf=Pass smtp.mailfrom=George.Dunlap@xxxxxxxxxx; spf=None smtp.helo=postmaster@xxxxxxxxxxxxxxx
  • Autocrypt: addr=george.dunlap@xxxxxxxxxx; prefer-encrypt=mutual; keydata= mQINBFPqG+MBEACwPYTQpHepyshcufo0dVmqxDo917iWPslB8lauFxVf4WZtGvQSsKStHJSj 92Qkxp4CH2DwudI8qpVbnWCXsZxodDWac9c3PordLwz5/XL41LevEoM3NWRm5TNgJ3ckPA+J K5OfSK04QtmwSHFP3G/SXDJpGs+oDJgASta2AOl9vPV+t3xG6xyfa2NMGn9wmEvvVMD44Z7R W3RhZPn/NEZ5gaJhIUMgTChGwwWDOX0YPY19vcy5fT4bTIxvoZsLOkLSGoZb/jHIzkAAznug Q7PPeZJ1kXpbW9EHHaUHiCD9C87dMyty0N3TmWfp0VvBCaw32yFtM9jUgB7UVneoZUMUKeHA fgIXhJ7I7JFmw3J0PjGLxCLHf2Q5JOD8jeEXpdxugqF7B/fWYYmyIgwKutiGZeoPhl9c/7RE Bf6f9Qv4AtQoJwtLw6+5pDXsTD5q/GwhPjt7ohF7aQZTMMHhZuS52/izKhDzIufl6uiqUBge 0lqG+/ViLKwCkxHDREuSUTtfjRc9/AoAt2V2HOfgKORSCjFC1eI0+8UMxlfdq2z1AAchinU0 eSkRpX2An3CPEjgGFmu2Je4a/R/Kd6nGU8AFaE8ta0oq5BSFDRYdcKchw4TSxetkG6iUtqOO ZFS7VAdF00eqFJNQpi6IUQryhnrOByw+zSobqlOPUO7XC5fjnwARAQABtCRHZW9yZ2UgVy4g RHVubGFwIDxkdW5sYXBnQHVtaWNoLmVkdT6JAlcEEwEKAEECGwMFCwkIBwMFFQoJCAsFFgID AQACHgECF4ACGQEWIQTXqBy2bTNXPzpOYFimNjwxBZC0bQUCXEowWQUJDCJ7dgAKCRCmNjwx BZC0beKvEACJ75YlJXd7TnNHgFyiCJkm/qPeoQ3sFGSDZuZh7SKcdt9+3V2bFEb0Mii1hQaz 3hRqZb8sYPHJrGP0ljK09k3wf8k3OuNxziLQBJyzvn7WNlE4wBEcy/Ejo9TVBdA4ph5D0YaZ nqdsPmxe/xlTFuSkgu4ep1v9dfVP1TQR0e+JIBa/Ss+cKC5intKm+8JxpOploAHuzaPu0L/X FapzsIXqgT9eIQeBEgO2hge6h9Jov3WeED/vh8kA7f8c6zQ/gs5E7VGALwsiLrhr0LZFcKcw kI3oCCrB/C/wyPZv789Ra8EXbeRSJmTjcnBwHRPjnjwQmetRDD1t+VyrkC6uujT5jmgOBzaj KCqZ8PcMAssOzdzQtKmjUQ2b3ICPs2X13xZ5M5/OVs1W3TG5gkvMh4YoHi4ilFnOk+v3/j7q 65FG6N0JLb94Ndi80HkIOQQ1XVGTyu6bUPaBg3rWK91Csp1682kD/dNVF3FKHrRLmSVtmEQR 5rK0+VGc/FmR6vd4haKGWIRuPxzg+pBR77avIZpU7C7+UXGuZ5CbHwIdY8LojJg2TuUdqaVj yxmEZLOA8rVHipCGrslRNthVbJrGN/pqtKjCClFZHIAYJQ9EGLHXLG9Pj76opfjHij3MpR3o pCGAh6KsCrfrsvjnpDwqSbngGyEVH030irSk4SwIqZ7FwLkBDQRUWmc6AQgAzpc8Ng5Opbrh iZrn69Xr3js28p+b4a+0BOvC48NfrNovZw4eFeKIzmI/t6EkJkSqBIxobWRpBkwGweENsqnd 0qigmsDw4N7J9Xx0h9ARDqiWxX4jr7u9xauI+CRJ1rBNO3VV30QdACwQ4LqhR/WA+IjdhyMH wj3EJGE61NdP/h0zfaLYAbvEg47/TPThFsm4m8Rd6bX7RkrrOgBbL/AOnYOMEivyfZZKX1vv iEemAvLfdk2lZt7Vm6X/fbKbV8tPUuZELzNedJvTTBS3/l1FVz9OUcLDeWhGEdlxqXH0sYWh E9+PXTAfz5JxKH+LMetwEM8DbuOoDIpmIGZKrZ+2fQARAQABiQNbBBgBCgAmAhsCFiEE16gc tm0zVz86TmBYpjY8MQWQtG0FAlxKMJ4FCQnQ/OQBKcBdIAQZAQoABgUCVFpnOgAKCRCyFcen x4Qb7cXrCAC0qQeEWmLa9oEAPa+5U6wvG1t/mi22gZN6uzQXH1faIOoDehr7PPESE6tuR/vI CTTnaSrd4UDPNeqOqVF07YexWD1LDcQG6PnRqC5DIX1RGE3BaSaMl2pFJP8y+chews11yP8G DBbxaIsTcHZI1iVIC9XLhoeegWi84vYc8F4ziADVfowbmbvcVw11gE8tmALCwTeBeZVteXjh 0OELHwrc1/4j4yvENjIXRO+QLIgk43kB57Upr4tP2MEcs0odgPM+Q+oETOJ00xzLgkTnLPim C1FIW2bOZdTj+Uq6ezRS2LKsNmW+PRRvNyA5ojEbA/faxmAjMZtLdSSSeFK8y4SoCRCmNjwx BZC0bevWEACRu+GyQgrdGmorUptniIeO1jQlpTiP5WpVnk9Oe8SiLoXUhXXNj6EtzyLGpYmf kEAbki+S6WAKnzZd3shL58AuMyDxtFNNjNeKJOcl6FL7JPBIIgIp3wR401Ep+/s5pl3Nw8Ii 157f0T7o8CPb54w6S1WsMkU78WzTxIs/1lLblSMcvyz1Jq64g4OqiWI85JfkzPLlloVf1rzy ebIBLrrmjhCE2tL1RONpE/KRVb+Q+PIs5+YcZ+Q1e0vXWA7NhTWFbWx3+N6WW6gaGpbFbopo FkYRpj+2TA5cX5zW148/xU5/ATEb5vdUkFLUFVy5YNUSyeBHuaf6fGmBrDc47rQjAOt1rmyD 56MUBHpLUbvA6NkPezb7T6bQpupyzGRkMUmSwHiLyQNJQhVe+9NiJJvtEE3jol0JVJoQ9WVn FAzPNCgHQyvbsIF3gYkCYKI0w8EhEoH5FHYLoKS6Jg880IY5rXzoAEfPvLXegy6mhYl+mNVN QUBD4h9XtOvcdzR559lZuC0Ksy7Xqw3BMolmKsRO3gWKhXSna3zKl4UuheyZtubVWoNWP/bn vbyiYnLwuiKDfNAinEWERC8nPKlv3PkZw5d3t46F1Dx0TMf16NmP+azsRpnMZyzpY8BL2eur feSGAOB9qjZNyzbo5nEKHldKWCKE7Ye0EPEjECS1gjKDwbkBDQRUWrq9AQgA7aJ0i1pQSmUR 6ZXZD2YEDxia2ByR0uZoTS7N0NYv1OjU8v6p017u0Fco5+Qoju/fZ97ScHhp5xGVAk5kxZBF DT4ovJd0nIeSr3bbWwfNzGx1waztfdzXt6n3MBKr7AhioB1m+vuk31redUdnhbtvN7O40MC+ fgSk5/+jRGxY3IOVPooQKzUO7M51GoOg4wl9ia3H2EzOoGhN2vpTbT8qCcL92ZZZwkBRldoA Wn7c1hEKSTuT3f1VpSmhjnX0J4uvKZ1V2R7rooKJYFBcySC0wa8aTmAtAvLgfcpe+legOtgq DKzLuN45xzEjyjCiI521t8zxNMPJY9FiCPNv0sCkDwARAQABiQI8BBgBCgAmAhsMFiEE16gc tm0zVz86TmBYpjY8MQWQtG0FAlxKNJYFCQnQrVkACgkQpjY8MQWQtG2Xxg//RrRP+PFYuNXt 9C5hec/JoY24TkGPPd2tMC9usWZVImIk7VlHlAeqHeE0lWU0LRGIvOBITbS9izw6fOVQBvCA Fni56S12fKLusWgWhgu03toT9ZGxZ9W22yfw5uThSHQ4y09wRWAIYvhJsKnPGGC2KDxFvtz5 4pYYNe8Icy4bwsxcgbaSFaRh+mYtts6wE9VzyJvyfTqbe8VrvE+3InG5rrlNn51AO6M4Wv20 iFEgYanJXfhicl0WCQrHyTLfdB5p1w+072CL8uryHQVfD0FcDe+J/wl3bmYze+aD1SlPzFoI MaSIXKejC6oh6DAT4rvU8kMAbX90T834Mvbc3jplaWorNJEwjAH/r+v877AI9Vsmptis+rni JwUissjRbcdlkKBisoUZRPmxQeUifxUpqgulZcYwbEC/a49+WvbaYUriaDLHzg9xisijHwD2 yWV8igBeg+cmwnk0mPz8tIVvwi4lICAgXob7HZiaqKnwaDXs4LiS4vdG5s/ElnE3rIc87yru 24n3ypeDZ6f5LkdqL1UNp5/0Aqbr3EiN7/ina4YVyscy9754l944kyHnnMRLVykg0v+kakj0 h0RJ5LbfLAMM8M52KIA3y14g0Fb7kHLcOUMVcgfQ3PrN6chtC+5l6ouDIlSLR3toxH8Aam7E rIFfe2Dk+lD9A9BVd2rfoHA=
  • Cc: Steven Haigh <netwiz@xxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Andreas Kinzler <hfp@xxxxxxxxx>, IanJackson <ian.jackson@xxxxxxxxxx>, Anthony Perard <anthony.perard@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Fri, 15 Nov 2019 14:55:47 +0000
  • Ironport-sdr: 8r0vQ2E06GhdUMuXElwn0gQbbg9HYeI4w5rc9fUZUrMNVv/ms5LqoPt8SLV8P6Uounme39dpm2 5GtM3PfAA8oLDfYGTGlaR5fvZj7y/BaVdN0Rt+eDUIyjs6XkW5dEIqTNfj9cyW921O66hmoABw RDmkp9biuDjZPTOd3dKGTvTp9Zr3VqWJCsUfG/IXnP1BE8V0QIfPfYZfTEYWY+/WELVlpERod/ UVH34G+gQNrqP1mdEbbK2GpjTPoKjYD0FjSlGRqHkfBgGMctcYFR+tatpiE6Y/RvsVUs6iIt0S tt8=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Openpgp: preference=signencrypt

On 11/15/19 2:42 PM, Jan Beulich wrote:
> On 15.11.2019 15:29, George Dunlap wrote:
>> On 11/15/19 2:18 PM, Jan Beulich wrote:
>>> On 15.11.2019 15:10, George Dunlap wrote:
>>>> On 11/15/19 2:06 PM, Andrew Cooper wrote:
>>>>> On 15/11/2019 14:04, George Dunlap wrote:
>>>>>>>> It's not entirely uncommon to (also) consider how the resulting
>>>>>>>> diff would look like when putting together a change. And besides
>>>>>>>> the review aspect, there's also the archeology one - "git blame"
>>>>>>>> yields much more helpful results when code doesn't get moved
>>>>>>>> around more than necessary. But yes, there's no very clear "this
>>>>>>>> is the better option" here. I've taken another look at the code
>>>>>>>> before your change though - everything is already nicely in one
>>>>>>>> place with Andrew's most recent code reorg. So I'm now having an
>>>>>>>> even harder time seeing why you want to move things around again.
>>>>>>> We don't.  I've recommend twice now to have a single "else if" hunk
>>>>>>> which is nearly empty, and much more obviously a gross "make it work for
>>>>>>> 4.13" bodge.
>>>>>> The results are a tiny bit better, but not much really (see attached).
>>>>>
>>>>> What I meant was:
>>>>>
>>>>>> diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
>>>>>> index 312c481f1e..bc088e45f0 100644
>>>>>> --- a/tools/libxc/xc_cpuid_x86.c
>>>>>> +++ b/tools/libxc/xc_cpuid_x86.c
>>>>>> @@ -579,52 +579,71 @@ int xc_cpuid_apply_policy(xc_interface *xch, 
>>>>>> uint32_t domid,
>>>>>>      }
>>>>>
>>>>> else if ( getenv() )
>>>>> {
>>>>>     ...
>>>>> }
>>>>>
>>>>>>      else
>>>>>>      {
>>>>>
>>>>> With no delta to this block at all.
>>>>
>>>> Then we have to duplicate this code in both blocks:
>>>>
>>>>         /*
>>>>          * These settings are necessary to cause earlier
>>>> HVM_PARAM_NESTEDHVM /
>>>>          * XEN_DOMCTL_disable_migrate settings to be reflected correctly in
>>>>          * CPUID.  Xen will discard these bits if configuration hasn't been
>>>>          * set for the domain.
>>>>          */
>>>>         p->extd.itsc = true;
>>>>         p->basic.vmx = true;
>>>>         p->extd.svm = true;
>>>>
>>>> I won't object if that's what you guys really want.
>>>
>>> Personally I think the duplication is less bad than the far
>>> heavier original code churn, but to be honest, especially with
>>> this intended to go away again soon anyway, I'd not be opposed
>>> at all to
>>>
>>>     ...
>>>     else if ( getenv() )
>>>         goto no_fake_ht;
>>
>> This isn't correct, because you do need to clear htt and cmp_legacy, as
>> well as zeroing out cores_per_package and threads_per_cache on Intel.
>> (At least, that's what XenServer's patch does, and it's the best tested.)
>>
>>>     else
>>>     {
>>>     ...
>>>  no_fake_ht:
>>>         /*
>>>          * These settings are necessary to cause earlier 
>>> HVM_PARAM_NESTEDHVM /
>>>          * XEN_DOMCTL_disable_migrate settings to be reflected correctly in
>>>          * CPUID.  Xen will discard these bits if configuration hasn't been
>>>          * set for the domain.
>>>          */
>>>         p->extd.itsc = true;
>>>         p->basic.vmx = true;
>>>         p->extd.svm = true;
>>>     }
>>>
>>> (despite my general dislike of goto).
>>
>> Well I think gotos into other blocks is even worse. :-)
>>
>> I think the result is a lot nicer to review for sure.
> 
> Trying to comment despite this having been an attachment:
> 
>> --- a/tools/libxc/xc_cpuid_x86.c
>> +++ b/tools/libxc/xc_cpuid_x86.c
>> @@ -579,6 +579,26 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t 
>> domid,
>>     }
>>     else
>>     {
>> +        if ( getenv("XEN_LIBXC_DISABLE_FAKEHT") ) {
> 
> The brace wants to move onto its own line.

Ack

> 
>> +            p->basic.htt = false;
> 
> I think everything below here indeed simply undoes what said old
> commit did, but I can't match up this one. And together with the
> question of whether instead leaving it alone, cmp_legacy then
> would have the same question raised.

This is based on a XenServer patch which reverts the entire commit, and
has been maintained in the patchqueue since the commit made it upstream,
AFAICT.  So I'll let someone from that team comment on the wherefores;
but as I said, it's by far the best tested option we're going to get.

 -George

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

 


Rackspace

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