Received-SPF: pass (google.com: domain of linux-kernel+bounces-177875-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Received-SPF: Pass (protection.outlook.com: domain of amd.com designates
 165.204.84.17 as permitted sender) receiver=protection.outlook.com;
 client-ip=165.204.84.17; helo=SATLEXMB04.amd.com; pr=C
Date: Mon, 13 May 2024 12:05:35 -0500
From: Michael Roth <michael.roth@amd.com>
To: Paolo Bonzini <pbonzini@redhat.com>
CC: Nathan Chancellor <nathan@kernel.org>, <kvm@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, Sean Christopherson <seanjc@google.com>,
	<llvm@lists.linux.dev>
Subject: Re: [PULL 18/19] KVM: SEV: Provide support for
 SNP_EXTENDED_GUEST_REQUEST NAE event
Message-ID: <20240513170535.je74yhujxpogijga@amd.com>
References: <20240510211024.556136-1-michael.roth@amd.com>
 <20240510211024.556136-19-michael.roth@amd.com>
 <20240513151920.GA3061950@thelio-3990X>
 <0ceafce9-0e08-4d47-813d-6b3f52ac5fd6@redhat.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <0ceafce9-0e08-4d47-813d-6b3f52ac5fd6@redhat.com>
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 May 2024 17:06:28.3062
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: f6dfc88a-fb7d-40c0-28c5-08dc736f077b
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=3dd8961f-e488-4e60-8e11-a82d994e183d;Ip=[165.204.84.17];Helo=[SATLEXMB04.amd.com]
X-MS-Exchange-CrossTenant-AuthSource:
	DS2PEPF00003439.namprd02.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ2PR12MB9210

On Mon, May 13, 2024 at 06:53:24PM +0200, Paolo Bonzini wrote:
> On 5/13/24 17:19, Nathan Chancellor wrote:
> > > +static int snp_begin_ext_guest_req(struct kvm_vcpu *vcpu)
> > > +{
> > > +	int vmm_ret = SNP_GUEST_VMM_ERR_GENERIC;
> > > +	struct vcpu_svm *svm = to_svm(vcpu);
> > > +	unsigned long data_npages;
> > > +	sev_ret_code fw_err;
> > > +	gpa_t data_gpa;
> > > +
> > > +	if (!sev_snp_guest(vcpu->kvm))
> > > +		goto abort_request;
> > > +
> > > +	data_gpa = vcpu->arch.regs[VCPU_REGS_RAX];
> > > +	data_npages = vcpu->arch.regs[VCPU_REGS_RBX];
> > > +
> > > +	if (!IS_ALIGNED(data_gpa, PAGE_SIZE))
> > > +		goto abort_request;
> > 
> > [...]
> > 
> > > +abort_request:
> > > +	ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, SNP_GUEST_ERR(vmm_ret, fw_err));
> > > +	return 1; /* resume guest */
> > > +}
> > 
> > This patch is now in -next as commit 32fde9e18b3f ("KVM: SEV: Provide
> > support for SNP_EXTENDED_GUEST_REQUEST NAE event"), where it causes a
> > clang warning (or hard error when CONFIG_WERROR is enabled) [...]
> > Seems legitimate to me. What was the intention here?
> 
> Mike, I think this should just be 0?

Hi Paolo,

Yes, I was just about to submit a patch that does just that:

  https://github.com/mdroth/linux/commit/df55e9c5b97542fe037f5b5293c11a49f7c658ef

Sorry for the breakage,

Mike

> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index c7a0971149f2..affb4fb47f91 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -3911,7 +3911,6 @@ static int snp_begin_ext_guest_req(struct kvm_vcpu *vcpu)
>  	int vmm_ret = SNP_GUEST_VMM_ERR_GENERIC;
>  	struct vcpu_svm *svm = to_svm(vcpu);
>  	unsigned long data_npages;
> -	sev_ret_code fw_err;
>  	gpa_t data_gpa;
>  	if (!sev_snp_guest(vcpu->kvm))
> @@ -3938,7 +3937,7 @@ static int snp_begin_ext_guest_req(struct kvm_vcpu *vcpu)
>  	return 0; /* forward request to userspace */
>  abort_request:
> -	ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, SNP_GUEST_ERR(vmm_ret, fw_err));
> +	ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, SNP_GUEST_ERR(vmm_ret, 0));
>  	return 1; /* resume guest */
>  }
> Paolo
> 
>