kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Isaku Yamahata <isaku.yamahata@linux.intel.com>
Cc: Jim Mattson <jmattson@google.com>,
	isaku.yamahata@intel.com, kvm@vger.kernel.org,
	 linux-kernel@vger.kernel.org, isaku.yamahata@gmail.com,
	 Paolo Bonzini <pbonzini@redhat.com>,
	erdemaktas@google.com,  Vishal Annapurve <vannapurve@google.com>
Subject: Re: KVM: X86: Make bus clock frequency for vapic timer (bus lock -> bus clock) (was Re: [PATCH 0/2] KVM: X86: Make bus lock frequency for vapic timer) configurable
Date: Thu, 9 Nov 2023 07:55:45 -0800	[thread overview]
Message-ID: <ZU0BASXWcck85r90@google.com> (raw)
In-Reply-To: <20231108235456.GB1132821@ls.amr.corp.intel.com>

On Wed, Nov 08, 2023, Isaku Yamahata wrote:
> On Tue, Nov 07, 2023 at 12:03:35PM -0800, Jim Mattson <jmattson@google.com> wrote:
> > I think I know the answer, but do you have any tests for this new feature?
> 
> If you mean kvm kselftest, no.
> I have
> - TDX patched qemu
> - kvm-unit-tests: test_apic_timer_one_shot() @ kvm-unit-tests/x86/apic.c
>   TDX version is found at https://github.com/intel/kvm-unit-tests-tdx
>   We're planning to upstream the changes for TDX
> 
> How far do we want to go?
> - Run kvm-unit-tests with TDX. What I have right now.
> - kvm-unit-tests: extend qemu for default VM case and update
>   test_apic_timer_one_host()

Hrm, I'm not sure that we can do a whole lot for test_apic_timer_one_shot().  Or
rather, I'm not sure it's worth the effort to try and add coverage beyond what's
already there.

As for TDX, *if* we extend KUT, please don't make it depend on TDX.  Very few people
have access to TDX platforms and anything CoCo is pretty much guaranteed to be harder
to debug.

> - kselftest
>   Right now kvm kselftest doesn't have test cases even for in-kernel IRQCHIP
>   creation.

Selftests always create an in-kernel APIC.  And I think selftests are perfectly
suited to complement the coverage provided by KUT.  Specifically, the failure
scenario for this is that KVM emulates at 1Ghz whereas TDX advertises 25Mhz, i.e.
the test case we want is to verify that the APIC timer doesn't expire early.

There's no need for any APIC infrastructure, e.g. a selftest doesn't even need to
handle an interrupt.  Get the TSC frequency from KVM, program up an arbitrary APIC
bus clock frequency, set TMICT such that it expires waaaay in the future, and then
verify that the APIC timer counts reasonably close to the programmed frequency.
E.g. if the test sets the bus clock to 25Mhz, the "drift" due to KVM counting at
1Ghz should be super obvious.

LOL, side topic, KUT has this:

	/*
	 * For LVT Timer clock, SDM vol 3 10.5.4 says it should be
	 * derived from processor's bus clock (IIUC which is the same  <======
	 * as TSC), however QEMU seems to be using nanosecond. In all
	 * cases, the following should satisfy on all modern
	 * processors.
	 */
	report((lvtt_counter == 1) && (tsc2 - tsc1 >= interval),
	       "APIC LVT timer one shot");

  reply	other threads:[~2023-11-09 15:55 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-07 19:22 [PATCH 0/2] KVM: X86: Make bus lock frequency for vapic timer configurable isaku.yamahata
2023-11-07 19:22 ` [PATCH 1/2] KVM: x86: Make the hardcoded APIC bus frequency vm variable isaku.yamahata
2023-11-07 19:22 ` [PATCH 2/2] KVM: X86: Add a capability to configure bus frequency for APIC timer isaku.yamahata
2023-11-07 19:59   ` Jim Mattson
2023-11-08 23:41     ` Isaku Yamahata
2023-11-10  5:37   ` kernel test robot
2023-11-10 14:42     ` Sean Christopherson
2023-11-07 19:29 ` KVM: X86: Make bus clock frequency for vapic timer (bus lock -> bus clock) (was Re: [PATCH 0/2] KVM: X86: Make bus lock frequency for vapic timer) configurable Isaku Yamahata
2023-11-07 20:03   ` Jim Mattson
2023-11-08 23:54     ` Isaku Yamahata
2023-11-09 15:55       ` Sean Christopherson [this message]
2023-11-10  0:29         ` Isaku Yamahata

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZU0BASXWcck85r90@google.com \
    --to=seanjc@google.com \
    --cc=erdemaktas@google.com \
    --cc=isaku.yamahata@gmail.com \
    --cc=isaku.yamahata@intel.com \
    --cc=isaku.yamahata@linux.intel.com \
    --cc=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=vannapurve@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).