From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f74.google.com (mail-pj1-f74.google.com [209.85.216.74]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D567722A4F8 for ; Thu, 18 Sep 2025 20:34:36 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.74 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1758227678; cv=none; b=MI7WfCjGVxpE+igVQvwNohzEnJyGqreudhXsvvgTsl4O4IHOOtj3XG1aEO31mH65JsdfrtRwrwjBvvLtQJHh9BV/zPGU8WQrkfeGQ3CjPKtZD7XOhigWCYCzrBvL39s6CvHRAkrkeNRZzRkx3qB3dGJpBbvuvMDE5H+c/yHSRJI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1758227678; c=relaxed/simple; bh=TNlRLl4c9UDfjC84aA8G4KJ4ljf2HUpu3dv4ZBWDSDo=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=XuXX47P9F39RlGV7q+laB4LN67IXrlQqUSIlvT/tCEpnNtp7iIzCh9+LMzwA1JAUmxpuIFP2HC9O0/cGqsWuuG2ouYT9OVvxv4Xid5t3vxx1twoftBT/JhsaIi5hoorv3ytKHtNgZbGYVAalsttDWL8qm2mhMkkSojZmbGcC6Zo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=ugo19fez; arc=none smtp.client-ip=209.85.216.74 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="ugo19fez" Received: by mail-pj1-f74.google.com with SMTP id 98e67ed59e1d1-32ec69d22b2so1428566a91.1 for ; Thu, 18 Sep 2025 13:34:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1758227676; x=1758832476; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=XBODMdbQGpyv3o1i/C7a9yrUe2tedQBqvCU7igvUACQ=; b=ugo19fezcS05F1sa4Gdb4bUJmMsyB+r7GowBGWFuHWnHlHC339NBIMmjVN/yUt8zwn i9ErNkSKj4ubjF0DdJICGKPRbly27rhqkTliz32Ahhs3sbPY6d+oRIcaPOF+WSp8aEBW KTV1GxyIPzKHVHHQi7PgVPliBg1o2RM/r4V0miaA9Fa8Dhip6aOAXVkI5KRObW6IBSL1 feEK5XBGHH4r5nSm5hhXLdkcA9G4wqDEYKT5KmLll5PyRL20FyIFI3fViNPFtNBZ8XcI WuyfM+CVld31MPFXXltv8sZTDtEevNnMwn03lvN/geUMcsFIO52PkmC9puLbh2KNNP6m ohWA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1758227676; x=1758832476; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=XBODMdbQGpyv3o1i/C7a9yrUe2tedQBqvCU7igvUACQ=; b=Q6eqWD80YG/Lf63EofGd162MyN8qLbrnvZg3YJtF7IctDksnp9cLemzWPcdisanjA8 raWRWExN4Nh/Lq7Ss0iXXcJFjtMTeiHT/p3qnFS4hLPzA1S8FVZRNhxUJbaZ3zUqFmGY SESuDQKTo2Ewo95y9ZOTEBu5DViIUZDMk4j9fAFL1F2n3JVWAjbRcM9RhKBoMvLk7JZy QiXTfS+1oa5SbNCT1ednxc2zZjwVmsB+2bwVlDcbQO2GUkcQLj9VAc2unrL2ixQpPVjl 7hNCGACTCLNzcwdljjZvRzT4hM+NNtxaa6drRsgEPrcWbxdy/JkeIiFpD8he3wtkHru5 UzoQ== X-Forwarded-Encrypted: i=1; AJvYcCUrz82qkI7hO7JPetQ0HmMYLNeAbM3SFCEC512RJZN5mgoIgq1MGERs62QzQBKkAjWH/lmN1as4YbcdFGs=@vger.kernel.org X-Gm-Message-State: AOJu0Yx3YWJAvw3VHjxgJHDiWlYHcHJsxwhjaGuzh9kb/I53vZP6RShk aEdEFLoZguNnoYJX2S/7mjAfZIsneCn8i8IKuDmqxu8Q7Xose7cxP097WZQVdVZjFsKbSV8EMde K1aYXHw== X-Google-Smtp-Source: AGHT+IERMigXNRqnLSigGWkoYGfo80aGu0GfE+30vipr87ezRtoiMl4FHd/dkvILTWnAeCpXl1EhKp58i6w= X-Received: from pjyf8.prod.google.com ([2002:a17:90a:ec88:b0:330:429d:b28c]) (user=seanjc job=prod-delivery.src-stubby-dispatcher) by 2002:a17:90b:4cd1:b0:32e:e18a:3691 with SMTP id 98e67ed59e1d1-33098386c95mr936918a91.35.1758227676194; Thu, 18 Sep 2025 13:34:36 -0700 (PDT) Date: Thu, 18 Sep 2025 13:34:34 -0700 In-Reply-To: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20250912232319.429659-1-seanjc@google.com> <20250912232319.429659-30-seanjc@google.com> Message-ID: Subject: Re: [PATCH v15 29/41] KVM: SEV: Synchronize MSR_IA32_XSS from the GHCB when it's valid From: Sean Christopherson To: John Allen Cc: Paolo Bonzini , kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Tom Lendacky , Mathias Krause , Rick Edgecombe , Chao Gao , Maxim Levitsky , Xiaoyao Li Content-Type: text/plain; charset="us-ascii" On Thu, Sep 18, 2025, John Allen wrote: > On Tue, Sep 16, 2025 at 05:55:33PM -0500, John Allen wrote: > > On Tue, Sep 16, 2025 at 02:38:52PM -0700, Sean Christopherson wrote: > > > On Tue, Sep 16, 2025, John Allen wrote: > > > > On Tue, Sep 16, 2025 at 12:53:58PM -0700, Sean Christopherson wrote: > > > > > On Tue, Sep 16, 2025, John Allen wrote: > > > > > > On Fri, Sep 12, 2025 at 04:23:07PM -0700, Sean Christopherson wrote: > > > > > > > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c > > > > > > > index 0cd77a87dd84..0cd32df7b9b6 100644 > > > > > > > --- a/arch/x86/kvm/svm/sev.c > > > > > > > +++ b/arch/x86/kvm/svm/sev.c > > > > > > > @@ -3306,6 +3306,9 @@ static void sev_es_sync_from_ghcb(struct vcpu_svm *svm) > > > > > > > if (kvm_ghcb_xcr0_is_valid(svm)) > > > > > > > __kvm_set_xcr(vcpu, 0, kvm_ghcb_get_xcr0(ghcb)); > > > > > > > > > > > > > > + if (kvm_ghcb_xss_is_valid(svm)) > > > > > > > + __kvm_emulate_msr_write(vcpu, MSR_IA32_XSS, kvm_ghcb_get_xss(ghcb)); > > > > > > > + > > > > > > > > > > > > It looks like this is the change that caused the selftest regression > > > > > > with sev-es. It's not yet clear to me what the problem is though. > > > > > > > > > > Do you see any WARNs in the guest kernel log? > > > > > > > > > > The most obvious potential bug is that KVM is missing a CPUID update, e.g. due > > > > > to dropping an XSS write, consuming stale data, not setting cpuid_dynamic_bits_dirty, > > > > > etc. But AFAICT, CPUID.0xD.1.EBX (only thing that consumes the current XSS) is > > > > > only used by init_xstate_size(), and I would expect the guest kernel's sanity > > > > > checks in paranoid_xstate_size_valid() to yell if KVM botches CPUID emulation. > > > > > > > > Yes, actually that looks to be the case: > > > > > > > > [ 0.463504] ------------[ cut here ]------------ > > > > [ 0.464443] XSAVE consistency problem: size 880 != kernel_size 840 > > > > [ 0.465445] WARNING: CPU: 0 PID: 0 at arch/x86/kernel/fpu/xstate.c:638 paranoid_xstate_size_valid+0x101/0x140 > > > > > > Can you run with the below printk tracing in the host (and optionally tracing in > > > the guest for its updates)? Compile tested only. > > > > Interesting, I see "Guest CPUID doesn't have XSAVES" times the number of > > cpus followed by "XSS already set to val = 0, eliding updates" times the > > number of cpus. This is with host tracing only. I can try with guest > > tracing too in the morning. > > Ok, I think I see the problem. The cases above where we were seeing the > added print statements from kvm_set_msr_common were not situations where > we were going through the __kvm_emulate_msr_write via > sev_es_sync_from_ghcb. When we call __kvm_emulate_msr_write from this > context, we never end up getting to kvm_set_msr_common because we hit > the following statement at the top of svm_set_msr: > > if (sev_es_prevent_msr_access(vcpu, msr)) > return vcpu->kvm->arch.has_protected_state ? -EINVAL : 0; Gah, I was looking for something like that but couldn't find it, obviously. > So I'm not sure if this would force using the original method of > directly setting arch.ia32_xss or if there's some additional handling > here that we need in this scenario to allow the msr access. Does this fix things? If so, I'll slot in a patch to extract setting XSS to the helper, and then this patch can use that API. I like the symmetry between __kvm_set_xcr() and __kvm_set_xss(), and I especially like not doing a generic end-around on svm_set_msr() by calling kvm_set_msr_common() directly. diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 945f7da60107..ace9f321d2c9 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -2213,6 +2213,7 @@ unsigned long kvm_get_cr8(struct kvm_vcpu *vcpu); void kvm_lmsw(struct kvm_vcpu *vcpu, unsigned long msw); int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr); int kvm_emulate_xsetbv(struct kvm_vcpu *vcpu); +int __kvm_set_xss(struct kvm_vcpu *vcpu, u64 xss); int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr); int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr); diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index 94d9acc94c9a..462aebc54135 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -3355,7 +3355,7 @@ static void sev_es_sync_from_ghcb(struct vcpu_svm *svm) __kvm_set_xcr(vcpu, 0, kvm_ghcb_get_xcr0(svm)); if (kvm_ghcb_xss_is_valid(svm)) - __kvm_emulate_msr_write(vcpu, MSR_IA32_XSS, kvm_ghcb_get_xss(svm)); + __kvm_set_xss(vcpu, kvm_ghcb_get_xss(svm)); /* Copy the GHCB exit information into the VMCB fields */ exit_code = kvm_ghcb_get_sw_exit_code(svm); diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 5bbc187ab428..9b81e92a8de5 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -1313,6 +1313,22 @@ int kvm_emulate_xsetbv(struct kvm_vcpu *vcpu) } EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_emulate_xsetbv); +int __kvm_set_xss(struct kvm_vcpu *vcpu, u64 xss) +{ + if (!guest_cpuid_has(vcpu, X86_FEATURE_XSAVES)) + return KVM_MSR_RET_UNSUPPORTED; + + if (xss & ~vcpu->arch.guest_supported_xss) + return 1; + if (vcpu->arch.ia32_xss == xss) + return 0; + + vcpu->arch.ia32_xss = xss; + vcpu->arch.cpuid_dynamic_bits_dirty = true; + return 0; +} +EXPORT_SYMBOL_GPL(__kvm_set_xss); + static bool kvm_is_valid_cr4(struct kvm_vcpu *vcpu, unsigned long cr4) { return __kvm_is_valid_cr4(vcpu, cr4) && @@ -4119,16 +4135,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) } break; case MSR_IA32_XSS: - if (!guest_cpuid_has(vcpu, X86_FEATURE_XSAVES)) - return KVM_MSR_RET_UNSUPPORTED; - - if (data & ~vcpu->arch.guest_supported_xss) - return 1; - if (vcpu->arch.ia32_xss == data) - break; - vcpu->arch.ia32_xss = data; - vcpu->arch.cpuid_dynamic_bits_dirty = true; - break; + return __kvm_set_xss(vcpu, data); case MSR_SMI_COUNT: if (!msr_info->host_initiated) return 1;