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

From: Andrew Cooper <andrew.cooper3@citrix.com>
To: George Dunlap <george.dunlap@citrix.com>,
	Jan Beulich <jbeulich@suse.com>
Cc: Anthony Perard <anthony.perard@citrix.com>,
	IanJackson <ian.jackson@citrix.com>,
	Steven Haigh <netwiz@crc.id.au>, Andreas Kinzler <hfp@posteo.de>,
	xen-devel@lists.xenproject.org
Subject: Re: [Xen-devel] [PATCH RFC] x86: Add hack to disable "Fake HT" mode
Date: Fri, 15 Nov 2019 14:14:01 +0000	[thread overview]
Message-ID: <be1f75d1-7cdb-4beb-2486-61dc514739ce@citrix.com> (raw)
In-Reply-To: <380e865b-3092-6a85-0d77-dc99e33678e4@citrix.com>

On 15/11/2019 14: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.

I'd also be happy with a single "goto hvm_common;".  These tweaks are
already at the end of the block, so suitably positioned.

None of this code is going to survive for long.

~Andrew

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