Received-SPF: pass (google.com: domain of linux-kernel+bounces-73367-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Tue, 20 Feb 2024 10:33:46 -0600
From: John Allen <john.allen@amd.com>
To: Sean Christopherson <seanjc@google.com>
Cc: mlevitsk@redhat.com, kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	pbonzini@redhat.com, weijiang.yang@intel.com,
	rick.p.edgecombe@intel.com, x86@kernel.org, thomas.lendacky@amd.com,
	bp@alien8.de
Subject: Re: [PATCH 6/9] KVM: SVM: Add MSR_IA32_XSS to the GHCB for
 hypervisor kernel
Message-ID: <ZdTUap7xy8tu15QY@AUS-L1-JOHALLEN.amd.com>
References: <20231010200220.897953-1-john.allen@amd.com>
 <20231010200220.897953-7-john.allen@amd.com>
 <5e413e05de559971cdc2d1a9281a8a271590f62b.camel@redhat.com>
 <ZUQvNIE9iU5TqJfw@google.com>
 <c077e005c64aa82c7eaf4252f322c4ca29a2d0af.camel@redhat.com>
 <Zc5MRqmspThUoB+n@AUS-L1-JOHALLEN.amd.com>
 <ZdTRVNt5GWXEKL8h@google.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZdTRVNt5GWXEKL8h@google.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?PxSdARwT3TKExkt4O/MVTLXMmaSbwSHFsqbbLNmO8pNb1RP6vp38+3BRvL5o?=
 =?us-ascii?Q?G0jscx4TkhHgkhZ8n8apHgE9YNkJ6w0RkIQmZhHZJas1yX9jsf+gMsIcuLpl?=
 =?us-ascii?Q?6Dk4VPAZUC4z+uHgLlVofBbU+wuau8+lZGsCSpjINDJRdS8OmQO8Rr3Zr1Xi?=
 =?us-ascii?Q?KSXIonkmEMyIAtvX73xQWd45K65obUrZIo4mmfQKM7X3o986Qi91VtCTHEBy?=
 =?us-ascii?Q?RoR1AI38yQwV1q99DBJo9f0rr3YEX5opOmphHVmHGt5kbDc239wtM6uVnteW?=
 =?us-ascii?Q?F1qzRl/lIiCUHOtsz8bl+o2LCAZAHf53TlFeKDJlZr+FpjSQ82W8zvklqtMp?=
 =?us-ascii?Q?IBCm8mbJA4uNx270+9s950jSOK4t4WHgdt7VD8mo/o7Sqv6Mq7QIf3yTHEw+?=
 =?us-ascii?Q?wXouCEf3HCLoIUSxH2XAO4oBeMDsflzqpeUks4TvQ+WcaG+dFFpc2zSUh9AK?=
 =?us-ascii?Q?rtqkZLWpGjmVPkGiPxCTsaW3n1bE+NtkbWKBx1f5QsA6g4RR0iKaB39GpA33?=
 =?us-ascii?Q?HblvElgbzolCiRuaS9+no0tMpMnTSNOXna/A7yv2laxME4AMF9UC24lC+dSD?=
 =?us-ascii?Q?cc255F/wqHRuJpvV1yui5RXWk/KqKINp+GakPago+xnzLIEZsqEAa+Qf4hGi?=
 =?us-ascii?Q?pl8uRZloG6Z1dxb7+llL1/88dhkmjzgu/4muX7a7i4ofLSFCs0ClmNXZ0+os?=
 =?us-ascii?Q?EUrncX4WaxqWw2VciQ+xtR+V/CcLpTnpke7DHPE/TZ8xwELAZabqCL9O/WwE?=
 =?us-ascii?Q?fuutumDhapPyPyLSPhiagVkccQutGS1TbofWI4y9ttTWLFzKT5i49poBx9fA?=
 =?us-ascii?Q?Krzq+qH74Snp/qd4VFGLXiMPFStMKoJPMp5VZlf/RjxvrvrD87Gw6uWEtkEp?=
 =?us-ascii?Q?K71qPSN1+ws8Gyb/668a5jysZSvxnlojKjZ+xBnPogCa0D8pN0d2GZ8+yDsv?=
 =?us-ascii?Q?fhPVfwYDr0QOsW8jMFwBqnXUB/ZPisvHLuYjZnGiJuVKTMNZPqX1Gm6fpsGE?=
 =?us-ascii?Q?wzTBIKY8yOm13OLVqlkMqxC0dryjaTILzr/T9Qr5PAs5yj75mbIGZEaifF43?=
 =?us-ascii?Q?+fbIpAkofuCJL5pQynqlDkxNJQeVGdGiNlpLwUyolCVCsZxLBHj1lEA8k+3x?=
 =?us-ascii?Q?qO+aeDWKLzObtR95FTFpG2S/gN0KkBF/zyF2WS3W2HTrt7PrE4GuR/6UF0Es?=
 =?us-ascii?Q?RPxcXZ62e11MbyTD6eIblGCTbTY6C6YNpyUY+i32st8Zkz/K6yAwxLdAU8zk?=
 =?us-ascii?Q?7g483LujWh6SD3wuLtYmxyWseq2ZaeqCg/rIN+84fzjp9STTsUd+Vtri1WvH?=
 =?us-ascii?Q?b3+68G5DUC5soKXyFmkjTtdbjdwpbCWCVxTc0Ewem5bCN5KVEfcyt3RwjNWD?=
 =?us-ascii?Q?wfLvmZofII2xT4jCwF+gUghd72bdTwtMUjo8FCgHrTE6/54l6jgsh9QX2Bhs?=
 =?us-ascii?Q?6YRgxswVBsrAUlp/gotZui6WPvqULaFAveRAofIdPpb6jEr+u5dzClkyqVXm?=
 =?us-ascii?Q?5PpfGES0F/YPvW5ItFYFfvKEOL7HO+T0YxwpU5mENd6OnBBEmeLqxMZzN9Zi?=
 =?us-ascii?Q?gSm1+PrHEANKxSnV4/cPGrbUkzom0s4uvoJTS6ma?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e5c83641-5f2d-4fde-caae-08dc3231bb54
X-MS-Exchange-CrossTenant-AuthSource: BL1PR12MB5995.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Feb 2024 16:33:56.0324
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: jNxDmPc+WbibZeBG1ADBeKMEkazF57WlsSP+Bl+pAEnLKhKaCk36QrcTxtzl9sdtlsx93HTBPbhuckb25WHEgg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4185

On Tue, Feb 20, 2024 at 08:20:36AM -0800, Sean Christopherson wrote:
> On Thu, Feb 15, 2024, John Allen wrote:
> > On Tue, Nov 07, 2023 at 08:20:52PM +0200, Maxim Levitsky wrote:
> > > On Thu, 2023-11-02 at 16:22 -0700, Sean Christopherson wrote:
> > > > On Thu, Nov 02, 2023, Maxim Levitsky wrote:
> > > > > On Tue, 2023-10-10 at 20:02 +0000, John Allen wrote:
> > > > > > @@ -3032,6 +3037,9 @@ static void sev_es_init_vmcb(struct vcpu_svm *svm)
> > > > > >  		if (guest_cpuid_has(&svm->vcpu, X86_FEATURE_RDTSCP))
> > > > > >  			svm_clr_intercept(svm, INTERCEPT_RDTSCP);
> > > > > >  	}
> > > > > > +
> > > > > > +	if (kvm_caps.supported_xss)
> > > > > > +		set_msr_interception(vcpu, svm->msrpm, MSR_IA32_XSS, 1, 1);
> > > > > 
> > > > > This is not just a virtualization hole. This allows the guest to set MSR_IA32_XSS
> > > > > to whatever value it wants, and thus it might allow XSAVES to access some host msrs
> > > > > that guest must not be able to access.
> > > > > 
> > > > > AMD might not yet have such msrs, but on Intel side I do see various components
> > > > > like 'HDC State', 'HWP state' and such.
> > > > 
> > > > The approach AMD has taken with SEV-ES+ is to have ucode context switch everything
> > > > that the guest can access.  So, in theory, if/when AMD adds more XCR0/XSS-based
> > > > features, that state will also be context switched.
> > > > 
> > > > Don't get me wrong, I hate this with a passion, but it's not *quite* fatally unsafe,
> > > > just horrific.
> > > > 
> > > > > I understand that this is needed so that #VC handler could read this msr, and
> > > > > trying to read it will cause another #VC which is probably not allowed (I
> > > > > don't know this detail of SEV-ES)
> > > > > 
> > > > > I guess #VC handler should instead use a kernel cached value of this msr
> > > > > instead, or at least KVM should only allow reads and not writes to it.
> > > > 
> > > > Nope, doesn't work.  In addition to automatically context switching state, SEV-ES
> > > > also encrypts the guest state, i.e. KVM *can't* correctly virtualize XSS (or XCR0)
> > > > for the guest, because KVM *can't* load the guest's desired value into hardware.
> > > > 
> > > > The guest can do #VMGEXIT (a.k.a. VMMCALL) all it wants to request a certain XSS
> > > > or XCR0, and there's not a damn thing KVM can do to service the request.
> > > > 
> > > 
> > > Ah, I understand now. Everything makes sense, and yes, this is really ugly.
> > 
> > Hi Maxim and Sean,
> > 
> > It looks as though there are some broad changes that will need to happen
> > over the long term WRT to SEV-ES/SEV-SNP. In the short term, how would
> > you suggest I proceed with the SVM shstk series? Can we omit the SEV-ES
> > changes for now with an additional patch that disallows guest shstk when
> > SEV-ES is enabled? Subsequently, when we have a proper solution for the
> > concerns discussed here, we could submit another series for SEV-ES
> > support.
> 
> The SEV-ES mess was already addressed by commit a26b7cd22546 ("KVM: SEV: Do not
> intercept accesses to MSR_IA32_XSS for SEV-ES guests").  Or is there more that's
> needed for shadow stacks?

Ah, yes, you are right. That patch should address the controversial
change discussed above at least. Patch 5/9 and 7/9 of this series also
address different SEV-ES issues and will still need to included.

Thanks,
John