linux-kernel - Re: [PATCH] KVM: x86: Update Xen-specific CPUID leaves during mangling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <cd8c29bc349244399b84008ef4c9ce7a@amazon.co.uk>
Date: Thu, 23 Jan 2025 15:33:30 +0000
From: "Griffoul, Fred" <fgriffo@...zon.co.uk>
To: Vitaly Kuznetsov <vkuznets@...hat.com>, Sean Christopherson
	<seanjc@...gle.com>
CC: "kvm@...r.kernel.org" <kvm@...r.kernel.org>, Paolo Bonzini
	<pbonzini@...hat.com>, Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar
	<mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, Dave Hansen
	<dave.hansen@...ux.intel.com>, "x86@...nel.org" <x86@...nel.org>, "H. Peter
 Anvin" <hpa@...or.com>, David Woodhouse <dwmw2@...radead.org>, Paul Durrant
	<paul@....org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] KVM: x86: Update Xen-specific CPUID leaves during mangling

Thanks for your comments: I will post a new patch following Sean's idea to fix the CPUID registers directly in kvm_cpuid().

Br,

--
Fred

________________________________________
From: Vitaly Kuznetsov <vkuznets@...hat.com>
Sent: Thursday, January 23, 2025 1:24:12 PM
To: Sean Christopherson
Cc: Griffoul, Fred; kvm@...r.kernel.org; Paolo Bonzini; Thomas Gleixner; Ingo Molnar; Borislav Petkov; Dave Hansen; x86@...nel.org; H. Peter Anvin; David Woodhouse; Paul Durrant; linux-kernel@...r.kernel.org
Subject: RE: [EXTERNAL] [PATCH] KVM: x86: Update Xen-specific CPUID leaves during mangling


Sean Christopherson <seanjc@...gle.com> writes:

> On Wed, Jan 22, 2025, Vitaly Kuznetsov wrote:
>> > Signed-off-by: Fred Griffoul <fgriffo@...zon.co.uk>
>> > ---
>> >  arch/x86/kvm/cpuid.c | 1 +
>> >  arch/x86/kvm/xen.c   | 5 +++++
>> >  arch/x86/kvm/xen.h   | 5 +++++
>> >  3 files changed, 11 insertions(+)
>> >
>> > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
>> > index edef30359c19..432d8e9e1bab 100644
>> > --- a/arch/x86/kvm/cpuid.c
>> > +++ b/arch/x86/kvm/cpuid.c
>> > @@ -212,6 +212,7 @@ static int kvm_cpuid_check_equal(struct kvm_vcpu *vcpu, struct kvm_cpuid_entry2
>> >     */
>> >    kvm_update_cpuid_runtime(vcpu);
>> >    kvm_apply_cpuid_pv_features_quirk(vcpu);
>> > +  kvm_xen_update_cpuid_runtime(vcpu);
>>
>> This one is weird as we update it in runtime (kvm_guest_time_update())
>> and values may change when we e.g. migrate the guest. First, I do not
>> understand how the guest is supposed to notice the change as CPUID data
>> is normally considered static.
>
> I don't think it does.  Linux-as-a-guest reads the info once during boot (see
> xen_tsc_safe_clocksource()), and if and only if the TSC is constant and non-stop,
> i.e. iff the values won't change.

Right, the values shouldn't change on the same host. What I was thinking
is what happens when we migrate the guest to another
host. kvm_guest_time_update() is going to be called and we will get
something different (maybe just slightly different, but still) in Xen
TSC CPUIDs. The guest, however, is likely not going to notice at all.

>
>>  Second, I do not see how the VMM is
>> supposed to track it as if it tries to supply some different data for
>> these Xen leaves, kvm_cpuid_check_equal() will still fail.
>>
>> Would it make more sense to just ignore these Xen CPUID leaves with TSC
>> information when we do the comparison?
>
> Another alternative would be to modify the register output in kvm_cpuid().  Given
> that Linux reads the info once during boot, and presumably other guests do the
> same, runtime "patching" wouldn't incur meaningful overhead.  And there are no
> feature bits that KVM cares about, i.e. no reason KVM's view needs to be correct.

True, CPUID reading time should not be performance critical.

--
Vitaly