From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 87AD213AD11 for ; Mon, 5 Aug 2024 11:06:43 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.133.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722856006; cv=none; b=g5t8XnODICWumj19Kdn2A2nPG8jf4RFELyoByrY78iS3IR48jiYLsRWEkAU2j4ptSSAVl9dXNNr8E6Q/Y3jKb1aVUquD7zURHdoaYX0p1d7ZiDonxLApGV16Iqj5pb+pQS1ur9Wtd+9s3nm+6wmQhJXoqc8b2NiHWwKe4WpilPk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722856006; c=relaxed/simple; bh=ZROb3dB2VaVaGsnp5zkOSyH+sb3xDkLcZAhsWX2SuaU=; h=Message-ID:Subject:From:To:Cc:Date:In-Reply-To:References: Content-Type:MIME-Version; b=lVp7njHQk7/iOiJ4ySp3cwc0DZOJR+EJG48YsytCya0EpZ7ax1EjjKNSjNRa1n0AecSB7UQr2yxba9SeiRHMwyCed075cnLiWLbgU5l4PGu38/ge3bsFU/of/gKxkwySc3zPT2cC8nwbaLUg3qktRlSr3wxFCJPihyZgr+vQKaM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=iDOc+MBi; arc=none smtp.client-ip=170.10.133.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="iDOc+MBi" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1722856002; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=0EuEhaqkma5nF/MDa1ex/CbEv6H+NURzLYv8ieItZpQ=; b=iDOc+MBiiQvuTmsT/DPTESxV+ApZy9NgcilY05hm8MtkVeoH4F89sxAp/MmfqTmvWsVMb5 mwClmVb9guKbTKMAyK4ceYgrXiFWhuCOpIN4I6aU0auumA52w9vrLiLGurAvdDNv/tUqeI lNbNjRDRAGMg1pmEvYDOycxdH9/b2m0= Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-650-yPep0-6_PPmVSkU6_0fbtw-1; Mon, 05 Aug 2024 07:06:39 -0400 X-MC-Unique: yPep0-6_PPmVSkU6_0fbtw-1 Received: by mail-wr1-f69.google.com with SMTP id ffacd0b85a97d-36878581685so5348515f8f.2 for ; Mon, 05 Aug 2024 04:06:39 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722855998; x=1723460798; h=mime-version:user-agent:content-transfer-encoding:references :in-reply-to:date:cc:to:from:subject:message-id:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=0EuEhaqkma5nF/MDa1ex/CbEv6H+NURzLYv8ieItZpQ=; b=VHcGMZ2ZY4bd2n8n8HNHTULAiN/IQfmwS3WAWOs252ZROxDMZR1rztYDkAOLjZv73p 7R/FGSPNfaL0RFrRmQHwf9pajKQ3urWKIZmJVzeUzmjxFaHTfT3BgFhumbdwEXDXQQq8 atqVqxv9/wvZWVHowQrHoQLzihWhVG9D5JOmhSsPne3e803B9CPd/5YXJ/mkOHKbKhH5 1ibLYv5IfDMK745FbjPXmNpaWgzCQaYfMPCjdYSglsUGa755KPX6/vbqMe/AAQNrIHsE l8u5eFtGnDEpJF1M5pX9U5YK9EzZMaoF7hds0+ymB0wkc20llHtJruDFUN+Uslx9vNFr c/SA== X-Forwarded-Encrypted: i=1; AJvYcCWwfPFyJ2EEMaCa7/tFEvdUbeY7SYkWd5ew+IMjuqy5g/7iEpWXrEi1NOS/QfTVLAmM4vuEySETssnzz7UFslKSVZWfJTxc6KuuVxuF X-Gm-Message-State: AOJu0Yy1zmhBOQWAFemQVInmE85qPFmCVi9tQa5Qyl8v7EofAOG8BRnw pMKQIr+twKY/2mH1suU7VKYnCbrbTc7wg+5TZyco4rCyKZC8ngcCpQ6zkHvY+8QxDtyjui2/AP/ s1suxtcJfV/rknUl8xbeb3YGtWTVlhA4vRFYrK2NVGKibFZdzbovWYH4i3A4EBQ== X-Received: by 2002:a5d:518c:0:b0:366:e7aa:7fa5 with SMTP id ffacd0b85a97d-36bbc0f7f87mr7350780f8f.1.1722855997900; Mon, 05 Aug 2024 04:06:37 -0700 (PDT) X-Google-Smtp-Source: AGHT+IF6dg7LPJI84OZv+IpNHY8eZR1NrMnc4r04apwnaanq3LD1mnPJZ5UQZpkQV2fYA8faTO8Xtg== X-Received: by 2002:a5d:518c:0:b0:366:e7aa:7fa5 with SMTP id ffacd0b85a97d-36bbc0f7f87mr7350755f8f.1.1722855997347; Mon, 05 Aug 2024 04:06:37 -0700 (PDT) Received: from intellaptop.lan ([2a06:c701:778d:5201:3e8a:4c9c:25dd:6ccc]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-36bbcf0cc58sm9526157f8f.2.2024.08.05.04.06.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 05 Aug 2024 04:06:36 -0700 (PDT) Message-ID: <8f35b524cda53aff29a9389c79742fc14f77ec68.camel@redhat.com> Subject: Re: [PATCH v2 22/49] KVM: x86: Add a macro to precisely handle aliased 0x1.EDX CPUID features From: mlevitsk@redhat.com To: Sean Christopherson Cc: Paolo Bonzini , Vitaly Kuznetsov , kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Hou Wenlong , Kechen Lu , Oliver Upton , Binbin Wu , Yang Weijiang , Robert Hoo Date: Mon, 05 Aug 2024 14:06:35 +0300 In-Reply-To: References: <20240517173926.965351-1-seanjc@google.com> <20240517173926.965351-23-seanjc@google.com> <43ef06aca700528d956c8f51101715df86f32a91.camel@redhat.com> <3da2be9507058a15578b5f736bc179dc3b5e970f.camel@redhat.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.44.4 (3.44.4-3.fc36) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 =D0=A3 =D1=87=D1=82, 2024-07-25 =D1=83 11:39 -0700, Sean Christopherson =D0= =BF=D0=B8=D1=88=D0=B5: > > On Wed, Jul 24, 2024, Maxim Levitsky wrote: > > > > On Mon, 2024-07-08 at 14:08 -0700, Sean Christopherson wrote: > > > > > > On Thu, Jul 04, 2024, Maxim Levitsky wrote: > > > > > > > > On Fri, 2024-05-17 at 10:38 -0700, Sean Christopherson wrot= e: > > > > > > > > > > Add a macro to precisely handle CPUID features that AMD= duplicated from > > > > > > > > > > CPUID.0x1.EDX into CPUID.0x8000_0001.EDX.=C2=A0 This wi= ll allow adding an > > > > > > > > > > assert that all features passed to kvm_cpu_cap_init() m= atch the word being > > > > > > > > > > processed, e.g. to prevent passing a feature from CPUID= 0x7 to CPUID 0x1. > > > > > > > > > >=20 > > > > > > > > > > Because the kernel simply reuses the X86_FEATURE_* defi= nitions from > > > > > > > > > > CPUID.0x1.EDX, KVM's use of the aliased features would = result in false > > > > > > > > > > positives from such an assert. > > > > > > > > > >=20 > > > > > > > > > > No functional change intended. > > > > > > > > > >=20 > > > > > > > > > > Signed-off-by: Sean Christopherson > > > > > > > > > > --- > > > > > > > > > > =C2=A0arch/x86/kvm/cpuid.c | 24 +++++++++++++++++------= - > > > > > > > > > > =C2=A01 file changed, 17 insertions(+), 7 deletions(-) > > > > > > > > > >=20 > > > > > > > > > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.= c > > > > > > > > > > index 5e3b97d06374..f2bd2f5c4ea3 100644 > > > > > > > > > > --- a/arch/x86/kvm/cpuid.c > > > > > > > > > > +++ b/arch/x86/kvm/cpuid.c > > > > > > > > > > @@ -88,6 +88,16 @@ u32 xstate_required_size(u64 xstate_= bv, bool compacted) > > > > > > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0F(name)= ;=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0\ > > > > > > > > > > =C2=A0}) > > > > > > > > > > =C2=A0 > > > > > > > > > > +/* > > > > > > > > > > + * Aliased Features - For features in 0x8000_0001.EDX = that are duplicates of > > > > > > > > > > + * identical 0x1.EDX features, and thus are aliased fr= om 0x1 to 0x8000_0001. > > > > > > > > > > + */ > > > > > > > > > > +#define AF(name)=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0\ > > > > > > > > > > +({=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0\ > > > > > > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0BUILD_BUG_ON= (__feature_leaf(X86_FEATURE_##name) !=3D CPUID_1_EDX);=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0\ > > > > > > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0feature_bit(= name);=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0\ > > > > > > > > > > +}) > > > > > > > > > > + > > > > > > > > > > =C2=A0/* > > > > > > > > > > =C2=A0 * Magic value used by KVM when querying userspac= e-provided CPUID entries and > > > > > > > > > > =C2=A0 * doesn't care about the CPIUD index because the= index of the function in > > > > > > > > > > @@ -758,13 +768,13 @@ void kvm_set_cpu_caps(void) > > > > > > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0); > > > > > > > > > > =C2=A0 > > > > > > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0kvm_cpu= _cap_init(CPUID_8000_0001_EDX, > > > > > > > > > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0F(FPU) | F(VME) | F(DE) | F(PSE) | > > > > > > > > > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0F(TSC) | F(MSR) | F(PAE) | F(MCE) | > > > > > > > > > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0F(CX8) | F(APIC) | 0 /* Reserved */ | F= (SYSCALL) | > > > > > > > > > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0F(MTRR) | F(PGE) | F(MCA) | F(CMOV) | > > > > > > > > > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0F(PAT) | F(PSE36) | 0 /* Reserved */ | > > > > > > > > > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0F(NX) | 0 /* Reserved */ | F(MMXEXT) | = F(MMX) | > > > > > > > > > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0F(FXSR) | F(FXSR_OPT) | X86_64_F(GBPAGE= S) | F(RDTSCP) | > > > > > > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0AF(FPU) | AF(VME) | AF(DE) | AF(PSE) | > > > > > > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0AF(TSC) | AF(MSR) | AF(PAE) | AF(MCE) | > > > > > > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0AF(CX8) | AF(APIC) | 0 /* Reserved */ |= F(SYSCALL) | > > > > > > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0AF(MTRR) | AF(PGE) | AF(MCA) | AF(CMOV)= | > > > > > > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0AF(PAT) | AF(PSE36) | 0 /* Reserved */ = | > > > > > > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0F(NX) | 0 /* Reserved */ | F(MMXEXT) | = AF(MMX) | > > > > > > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0AF(FXSR) | F(FXSR_OPT) | X86_64_F(GBPAG= ES) | F(RDTSCP) | > > > > > > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A00 /* Reserved */ | X86_64_F(LM) |= F(3DNOWEXT) | F(3DNOW) > > > > > > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0); > > > > > > > > > > =C2=A0 > > > > > > > >=20 > > > > > > > > Hi, > > > > > > > >=20 > > > > > > > > What if we defined the aliased features instead. > > > > > > > > Something like this: > > > > > > > >=20 > > > > > > > > #define __X86_FEATURE_8000_0001_ALIAS(feature) \ > > > > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0(feature + = (CPUID_8000_0001_EDX - CPUID_1_EDX) * 32) > > > > > > > >=20 > > > > > > > > #define KVM_X86_FEATURE_FPU_ALIAS=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0__X86_FEATURE_8000_0001_ALIAS(KVM_X86_FEATURE_FPU) > > > > > > > > #define KVM_X86_FEATURE_VME_ALIAS=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0__X86_FEATURE_8000_0001_ALIAS(KVM_X86_FEATURE_VME) > > > > > > > >=20 > > > > > > > > And then just use for example the 'F(FPU_ALIAS)' in the CPU= ID_8000_0001_EDX > > > > > >=20 > > > > > > At first glance, I really liked this idea, but after working th= rough the > > > > > > ramifications, I think I prefer "converting" the flag when pass= ing it to > > > > > > kvm_cpu_cap_init().=C2=A0 In-place conversion makes it all but = impossible for KVM to > > > > > > check the alias, e.g. via guest_cpu_cap_has(), especially since= the AF() macro > > > > > > doesn't set the bits in kvm_known_cpu_caps (if/when a non-hacky= validation of > > > > > > usage becomes reality). > > > >=20 > > > > Could you elaborate on this as well? > > > >=20 > > > > My suggestion was that we can just treat aliases as completely inde= pendent > > > > and dummy features, say KVM_X86_FEATURE_FPU_ALIAS, and pass them as= is to the > > > > guest, which means that if an alias is present in host cpuid, it ap= pears in > > > > kvm caps, and thus qemu can then set it in guest cpuid. > > > >=20 > > > > I don't think that we need any special treatment for them if you lo= ok at it > > > > this way.=C2=A0 If you don't agree, can you give me an example? > >=20 > > KVM doesn't honor the aliases beyond telling userspace they can be set = (see below > > for all the aliased features that KVM _should_ be checking).=C2=A0 The = APM clearly > > states that the features are the same as their CPUID.0x1 counterparts, = but Intel > > CPUs don't support the aliases.=C2=A0 So, as you also note below, I thi= nk we could > > unequivocally say that enumerating the aliases but not the "real" featu= res is a > > bogus CPUID model, but we can't say the opposite, i.e. the real feature= s can > > exists without the aliases. > >=20 > > And that means that KVM must never query the aliases, e.g. should never= do > > guest_cpu_cap_has(KVM_X86_FEATURE_FPU_ALIAS), because the result is ess= entially > > meaningless.=C2=A0 It's a small thing, but if KVM_X86_FEATURE_FPU_ALIAS= simply doesn't > > exist, i.e. we do in-place conversion, then it's impossible to feed the= aliases > > into things like guest_cpu_cap_has(). This only makes my case stronger - treating the aliases as just features wi= ll allow us to avoid adding more logic to code which is already too complex IM= HO. If your concern is that features could be queried by guest_cpu_cap_has() that is easy to fix, we can (and should) put them into a separate file and #include them only in cpuid.c. We can even #undef the __X86_FEATURE_8000_0001_ALIAS macro after the kvm_se= t_cpu_caps, then if I understand the macro pre-processor correctly, any use of feature = alias macros will not fully evaluate and cause a compile error. > >=20 > > Heh, on a related topic, __cr4_reserved_bits() fails to account for any= of the > > aliased features.=C2=A0 Unless I'm missing something, VME, DE, TSC, PSE= , PAE, PGE and > > MCE, all need to be handled in __cr4_reserved_bits().=C2=A0 > > =C2=A0Amusingly,=20 > > nested_vmx_cr_fixed1_bits_update() handles the aliased legacy features.= =C2=A0 I don't > > see any reason for nested_vmx_cr_fixed1_bits_update() to manually query= guest > > CPUID, it should be able to use cr4_guest_rsvd_bits verbatim. Yep, this should be fixed - this patch series is about to grow even more I = guess, or rather let me suggest that you split it into several patch series, which can be merged and discussed separately. > >=20 > > > > > > Side topic, if it's not already documented somewhere else, kvm/= x86/cpuid.rst > > > > > > should call out that KVM only honors the features in CPUID.0x1,= i.e. that setting > > > > > > aliased bits in CPUID.0x8000_0001 is supported if and only if t= he bit(s) is also > > > > > > set in CPUID.0x1. > > > >=20 > > > > To be honest if KVM enforces this, such enforcement can be removed = IMHO: > >=20 > > There's no enforcement, and as above I agree that this would be a bogus= CPUID > > model.=C2=A0 I was thinking that it could be helpful to document that K= VM never checks > > the aliases, but on second though, it's probably unnecessary because th= e APM does > > say > >=20 > > =C2=A0 Same as CPUID Fn0000_0001_EDX[...] > >=20 > > for all the bits, i.e. setting the aliases without the real bits is an > > architectural violation. Regardless if this is an architectural violation or not, KVM should allow t= his because it allows many architectural violations, like AVX3 with no XSAVE, a= nd such. IMHO being consistent is more important than being right in only some cases= , and I don't think we want to start enforcing all the CPUID dependencies (I actually won't object to this). Best regards, Maxim Levitsky > >=20