Received-SPF: pass (google.com: domain of linux-kernel+bounces-157733-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <ad48dd75-3d14-461c-91e4-bad41c325ae7@intel.com>
Date: Wed, 24 Apr 2024 14:20:54 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH V4 0/4] KVM: x86: Make bus clock frequency for vAPIC timer
 configurable
To: Sean Christopherson <seanjc@google.com>, Rick P Edgecombe
	<rick.p.edgecombe@intel.com>
CC: "jmattson@google.com" <jmattson@google.com>, Chao Gao
	<chao.gao@intel.com>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>, "vkuznets@redhat.com" <vkuznets@redhat.com>,
	Vishal Annapurve <vannapurve@google.com>, Xiaoyao Li <xiaoyao.li@intel.com>,
	Erdem Aktas <erdemaktas@google.com>, "pbonzini@redhat.com"
	<pbonzini@redhat.com>, "mlevitsk@redhat.com" <mlevitsk@redhat.com>, "Isaku
 Yamahata" <isaku.yamahata@intel.com>, "kvm@vger.kernel.org"
	<kvm@vger.kernel.org>
References: <cover.1711035400.git.reinette.chatre@intel.com>
 <6fae9b07de98d7f56b903031be4490490042ff90.camel@intel.com>
 <Ziku9m_1hQhJgm_m@google.com>
 <26073e608fc450c6c0dcfe1f5cb1590f14c71e96.camel@intel.com>
 <ZilAEhUS-mmgjBK8@google.com>
Content-Language: en-US
From: Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <ZilAEhUS-mmgjBK8@google.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?NzdMek50UXhhZUdheFNkQzNySGsyNVNsSTZ6dDArSlROOWhuWEpIRUZUSjZu?=
 =?utf-8?B?WnlLZHhkb3p1dGR4UmNzdEE1TVhnclEvbTZPRkNnNU1FNzRZcW0zbUJjSWJn?=
 =?utf-8?B?alpvQzRJQ0V4YnA3V283QVZKSmlMdHRhSjllWjRMcGVnQzVCQlVlQjNPY2ti?=
 =?utf-8?B?Zy94bDNadVV3SlNoSjI3V0FMdWFrTjRTSDhSRjhCUHdqdXZuRUFtT1pRWHc0?=
 =?utf-8?B?VUhRcmszc0JHeUNpMjh0STlZZEtlRkwyd1p4K1lCNWFNSTZqejRuM0llcmhB?=
 =?utf-8?B?dDlIT2lndzQwTWNTSTJXMFhEVUtQVWtQUjhRaHJmNHdlaE1Zem9sSDFjUmIr?=
 =?utf-8?B?QlRzU3dUTFRKMHBxQ0hicUhnbXJnQlp0aUIrMkJwRzUxMFpScmlmS2QrVDA0?=
 =?utf-8?B?OHhVVng0T3lvclBDVXlUaXFacExMeDZwR0xFQWlIazZxbE5lbzlEUEFjdDhP?=
 =?utf-8?B?RlFtQzI2WFlUOHhzOHZUMHFJODd0dUlLdVh1Y1Q2TjRRdFhuckZiMllvWElL?=
 =?utf-8?B?N0JKc0JWd3dDK0xudU5mOVFhV2lsMzJJakVscm5aU2oweWpUbm5BY0YrT1Ji?=
 =?utf-8?B?aDY0U3drc1UvdlJqUlZ6ZWkyZlk1a1JMUElRdWNTbUdPanU2dmVvQXQvUGVp?=
 =?utf-8?B?OGljaHd1eGRERE9DdzlWYk90ZXlrQTUzZVU5YzBiYVVjc1JEZW5IN3JVeUFP?=
 =?utf-8?B?Mmg0dFNzTzZMaWd6YVFYZHYrRnlocmhUNitrYVBFNE4ydS9uMFNxMkpZNWlG?=
 =?utf-8?B?clUrRU85bkFseVVQSjRjSXFGQ0l4MTlMMmtrUHh0amZTR3p2SytBZWh6NXJ1?=
 =?utf-8?B?ZnlxOXlvZDVtR1FXWXJTMG0yTmZrNk5kTDQ3TEkyTkt1aFNHL2twMjQyTDdQ?=
 =?utf-8?B?dkR4VWNadnExWms1RGVycVJiUTE1aFJqcHEvUnQ0cVlIZDVGTkk5QzRqMlla?=
 =?utf-8?B?Mjg4VUZteVR3Uyt2anYvNjBKNXAwVVFBMk5qUlRQLzVEeGxEaEtHZ08zZEMr?=
 =?utf-8?B?cmp5M01CYVJ1a3o0dHRVQzAwQXRKcFp0b0M1S0JxQ1pGZkp4Ui9wVDM2TzYy?=
 =?utf-8?B?aG5HZi9UcUZBU2VqK09oZ0VnOWpoZlRJdE9vYWpGQklVRDlocUV6NXBMaUVL?=
 =?utf-8?B?NmRuZzVhbVJvMENMbFdVd0YveHV1QjkrblN3bTFlclU3akF0ZTdJTHFUaTVC?=
 =?utf-8?B?RHNQekk2QUkzemxnN2MvOW5zanhxQlhORUZzdXZWMTVYL2NCRWVFSFFwS2wv?=
 =?utf-8?B?SmIwdlRDWUFnMnZBdEs4QVdid1JIRXp5bDA3aFFhWWYrcjBiVFFKYUJUZVB6?=
 =?utf-8?B?cTFBOXMzV2xiZXozSlFDTEltQ25UOFpUVHZpSUdhK1Y1Z0F2MElOaXMrSnVZ?=
 =?utf-8?B?d05UaGdMcmVGS1NQYnRWNzhYMHZLNGtXMVg5Nmd3R25QZDVnSWlUUFVrbmEz?=
 =?utf-8?B?Rnl3YVBtMWcxRkwvMEtuVnBHN0VmZDBRM0tJVmhFbHZIVExvZDc3b2pIakdj?=
 =?utf-8?B?aXlJK3lJbnR3bkF3Tkc5V0xpRG1aUXc5ZFRPYkRnOU81UEllTmNRMW5sZ0Fr?=
 =?utf-8?B?TjVkNC9FRk1MSXNtU0tyRy9jQ2wwajRqUXF4K3lEMFdrVTJxdjJ0Y0J4NXlz?=
 =?utf-8?B?aVNTN2lIYW9YOXJoMTNBMCtmSFM1RFRkNWVFdjJjS05hQjdJOGorWnk4ODF1?=
 =?utf-8?B?dERxUnIvSnVUdHpjMHZyc1o1NFBaejEzeTljVVVUc01lbzgzTXF1cUY0S2J2?=
 =?utf-8?B?VkZ2eW1SeXI1U3pneEx3NXY4N3JvbWttSUpiN3BrOFo4MUlBNGRWUVZPWmRG?=
 =?utf-8?B?SVUrUysxYy8wVzBoSHJLZGFYZnFCbFo5NktaOGJaRWg2cUJjUkhNaTJidkgr?=
 =?utf-8?B?YUI4SnZKdGRoY1BsVy93U0VOakhvcG92SDNoQk13MHRhYlc0RGRNNDhRbjc2?=
 =?utf-8?B?Y2Z4SFhKWTQ0R29PU0FnUEFkMEFTa2ZyQllnd2EvT3dMYUpKcVNMMUV1cWVl?=
 =?utf-8?B?WHVmSmlGcGUyc3YzeDAvRVpYb1QyZ0UvYmU3ZjBWaW85ZjNwNlJSMm9uV0Ri?=
 =?utf-8?B?UVhZMHF5STA5alp3RXY4WUtGYy9PbEdVVENGa2JCUFNaU0xibTNVQVMzdTRo?=
 =?utf-8?B?THVzUEh1MTdPQVhYdEp2MlhwQllvWk5Pd2svVlZSMk0xcDFBcWxBU3hVV01D?=
 =?utf-8?B?UXc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 550d7dca-2448-4c0b-c2c8-08dc64a46e7d
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Apr 2024 21:20:57.2294
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: liZX7wryfW4fz2coNrVK8k1u4wXGJvKg7Y+BcTS8PVW+f1DHz57TXuYq8CLmQBpCx6kdr4hmX8HmdL0RWtDqd+z68m2czA2S0ZPlG9Z1qJM=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR11MB5099
X-OriginatorOrg: intel.com

Hi Sean,

On 4/24/2024 10:23 AM, Sean Christopherson wrote:
> On Wed, Apr 24, 2024, Rick P Edgecombe wrote:
>> On Wed, 2024-04-24 at 09:13 -0700, Sean Christopherson wrote:
>>> On Tue, Apr 16, 2024, Rick P Edgecombe wrote:
>>>> On Thu, 2024-03-21 at 09:37 -0700, Reinette Chatre wrote:
>>>>>
>>>>> Summary
>>>>> -------
>>>>> Add KVM_CAP_X86_APIC_BUS_FREQUENCY capability to configure the APIC
>>>>> bus clock frequency for APIC timer emulation.
>>>>> Allow KVM_ENABLE_CAPABILITY(KVM_CAP_X86_APIC_BUS_FREQUENCY) to set the
>>>>> frequency in nanoseconds. When using this capability, the user space
>>>>> VMM should configure CPUID leaf 0x15 to advertise the frequency.
>>>>
>>>> Looks good to me and...
>>>> Tested-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
>>>>
>>>> The only thing missing is actually integrating it into TDX qemu patches and
>>>> testing the resulting TD. I think we are making a fair assumption that the
>>>> problem should be resolved based on the analysis, but we have not actually
>>>> tested that part. Is that right?
>>>
>>> Please tell me that Rick is wrong, and that this actually has been tested with
>>> a TDX guest.  I don't care _who_ tested it, or with what VMM it has been
>>> tested, but _someone_ needs to verify that this actually fixes the TDX issue.
>>
>> It is in the process of getting a TDX test developed (or rather updated).
>> Agreed, it requires verification that it fixes the original TDX issue. That is
>> why I raised it.
>>
>> Reinette was working on this internally and some iterations were happening, but
>> we are trying to work on the public list as much as possible per your earlier
>> comments. So that is why she posted it.
> 
> I have no problem posting "early", but Documentation/process/maintainer-kvm-x86.rst
> clearly states under Testing that:
> 
>   If you can't fully test a change, e.g. due to lack of hardware, clearly state
>   what level of testing you were able to do, e.g. in the cover letter.
> 
> I was assuming that this was actually *fully* tested, because nothing suggests
> otherwise.  And _that_ is a problem, e.g. I was planning on applying this series
> for 6.10, which would have made for quite the mess if it turns out that this
> doesn't actually solve the TDX problem.

There was one vote for the capability name to rather be KVM_CAP_X86_APIC_BUS_CYCLES_NS [1] 

I'd be happy to resubmit with the name changed but after reading your statement above it
is not clear to me what name is preferred: KVM_CAP_X86_APIC_BUS_FREQUENCY
as used in this series that seem to meet your approval or KVM_CAP_X86_APIC_BUS_CYCLES_NS.

Please let me know what you prefer.

Thank you.

Reinette

[1] https://lore.kernel.org/lkml/1e26b405-f382-45f4-9dd5-3ea5db68302a@intel.com/