Received: by 2002:ac0:bc90:0:0:0:0:0 with SMTP id a16csp4258851img; Tue, 26 Mar 2019 06:13:21 -0700 (PDT) X-Google-Smtp-Source: APXvYqz3ycTDNM2H0QImW/YSCcCccSrC29a+gORhZp4LjmAV1gGfKK7UMPjRaRMWfQsXYyOLWll/ X-Received: by 2002:a63:3185:: with SMTP id x127mr28548737pgx.299.1553606001800; Tue, 26 Mar 2019 06:13:21 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1553606001; cv=none; d=google.com; s=arc-20160816; b=FzqnJRZs1RkWO4fwiPNZpRId2+9D4jlvkkDvCKDk0J7xeHcL+E/43Y1YU1WYo0n/fT vtQM9ZpaSS6ZHi3tN29d8pbEVAjENltDu3fF4PZOEvsm4aC+3keQMTbUMSoF542mPh9I FgblS5uJmottSJxHQIn4IfaFoH8IRTkZKb55cqHaMsy2U65gxJIUCYlFEXxFRc92sfxK qnPkoXvv02oBBNK47+aSPGnT7amDf6fa0tAgkCIgj/IQDxO94Ti4v1JmpzWtqVJ1PEX1 Uej4TphC4lP3+ssglFemiTpG7Q9L54Z8AZt0sZvv/UUeR0ZRA4Xyb4gXpTe2IxIxjDKR 182Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:to:references:message-id :content-transfer-encoding:cc:date:in-reply-to:from:subject :mime-version:dkim-signature; bh=VG36/NktXjB1U/a+gHTVu5iFnhtk8979yzArM7G/WC8=; b=aDAThDFPZO5XUKZKPt7H2TL0lOcQioXLK0NNAJmjbLrKEbBIYNOkvWVg+NCtF79N9/ 8ZYaaI43oOgjgl+RsPa0qpGWCBKM5XnCl0i38Yc4AvhkCu9SFrDEp09vCLAATqA1u6Hr ipUueY+MWHkmOrARnU0qJSJfKphGPMBRbP2MC1B2i1z0tDelfLUDvCBKjKDrajwbPKg3 GL/1q2rF5OI7cTD9tIBo79DbKeiXF0Ov6DNS4v+oKvfMNa0xQa+KIrsQMAYihenPXGzp fZSFo0COhHpujF1+z47ru8NoL4pCf4EZlscK4hb315UxiO+9Tb48Kci7E/v+848aK+OA I/Yw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@oracle.com header.s=corp-2018-07-02 header.b=xrud4aes; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=oracle.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id v22si15438527pfm.263.2019.03.26.06.13.06; Tue, 26 Mar 2019 06:13:21 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@oracle.com header.s=corp-2018-07-02 header.b=xrud4aes; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=oracle.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731511AbfCZNMX (ORCPT + 99 others); Tue, 26 Mar 2019 09:12:23 -0400 Received: from aserp2130.oracle.com ([141.146.126.79]:55386 "EHLO aserp2130.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726177AbfCZNMW (ORCPT ); Tue, 26 Mar 2019 09:12:22 -0400 Received: from pps.filterd (aserp2130.oracle.com [127.0.0.1]) by aserp2130.oracle.com (8.16.0.27/8.16.0.27) with SMTP id x2QD3sHo185426; Tue, 26 Mar 2019 13:12:02 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=content-type : mime-version : subject : from : in-reply-to : date : cc : content-transfer-encoding : message-id : references : to; s=corp-2018-07-02; bh=VG36/NktXjB1U/a+gHTVu5iFnhtk8979yzArM7G/WC8=; b=xrud4aespGXDTxjHpKOrT55LnVXtdGEP3Jvj5hmEUaLPITLLuxsaRQJa+cwABf9xRcDx dCeCbrWxVd4OrVrGVXw0toGO2bL353C08plQzJqjbYf5yr5DdYmsYS7wEKJk/NQo6aPl zTtOeHYzijQLsWZoRFUWJsx5UO5VoMRdxsDDwaNaukp64OoFU+cwCeFP/mjvJPHbp30S z9Hncu/jCE8U1QSGji4KqAuta/6jLLz6ERzanoscQr7fpQK0uLq1gJZeAgfOyULpOrii VnFe5wAWMyOoQtm7Jmr3wClOmJKxcjUDzNxx17UU744cw5SByb5T/EH4ZEUkYfPZMKz+ sQ== Received: from userv0021.oracle.com (userv0021.oracle.com [156.151.31.71]) by aserp2130.oracle.com with ESMTP id 2re6g0t8v7-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 26 Mar 2019 13:12:02 +0000 Received: from userv0122.oracle.com (userv0122.oracle.com [156.151.31.75]) by userv0021.oracle.com (8.14.4/8.14.4) with ESMTP id x2QDC1vu010614 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 26 Mar 2019 13:12:01 GMT Received: from abhmp0003.oracle.com (abhmp0003.oracle.com [141.146.116.9]) by userv0122.oracle.com (8.14.4/8.14.4) with ESMTP id x2QDC0tG030477; Tue, 26 Mar 2019 13:12:01 GMT Received: from [10.0.5.57] (/213.57.127.10) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Tue, 26 Mar 2019 06:12:00 -0700 Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 11.1 \(3445.4.7\)) Subject: Re: [PATCH] KVM: x86: nVMX: allow RSM to restore VMXE CR4 flag From: Liran Alon In-Reply-To: <20190326130746.28748-1-vkuznets@redhat.com> Date: Tue, 26 Mar 2019 15:11:56 +0200 Cc: kvm@vger.kernel.org, Paolo Bonzini , =?utf-8?B?UmFkaW0gS3LEjW3DocWZ?= , Jon Doron , Sean Christopherson , linux-kernel@vger.kernel.org Content-Transfer-Encoding: quoted-printable Message-Id: References: <20190326130746.28748-1-vkuznets@redhat.com> To: Vitaly Kuznetsov X-Mailer: Apple Mail (2.3445.4.7) X-Proofpoint-Virus-Version: vendor=nai engine=5900 definitions=9206 signatures=668685 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 priorityscore=1501 malwarescore=0 suspectscore=2 phishscore=0 bulkscore=0 spamscore=0 clxscore=1011 lowpriorityscore=0 mlxscore=0 impostorscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1810050000 definitions=main-1903260092 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > On 26 Mar 2019, at 15:07, Vitaly Kuznetsov = wrote: >=20 > Commit 5bea5123cbf0 ("KVM: VMX: check nested state and CR4.VMXE = against > SMM") introduced a check to vmx_set_cr4() forbidding to set VMXE from = SMM. > The check is correct, however, there is a special case when RSM is = called > to leave SMM: rsm_enter_protected_mode() is called with HF_SMM_MASK = still > set and in case VMXE was set before entering SMM we're failing to = return. >=20 > Resolve the issue by temporary dropping HF_SMM_MASK around set_cr4() = calls > when ops->set_cr() is called from RSM. >=20 > Reported-by: Jon Doron > Suggested-by: Liran Alon > Fixes: 5bea5123cbf0 ("KVM: VMX: check nested state and CR4.VMXE = against SMM") > Signed-off-by: Vitaly Kuznetsov Patch looks good to me. Reviewed-by: Liran Alon > --- > - Instread of putting the temporary HF_SMM_MASK drop to > rsm_enter_protected_mode() (as was suggested by Liran), move it to > emulator_set_cr() modifying its interface. emulate.c seems to be > vcpu-specifics-free at this moment, we may want to keep it this way. > - It seems that Hyper-V+UEFI on KVM is still broken, I'm observing = sporadic > hangs even with this patch. These hangs, however, seem to be = unrelated to > rsm. Feel free to share details on these hangs ;) Great work, -Liran > --- > arch/x86/include/asm/kvm_emulate.h | 3 ++- > arch/x86/kvm/emulate.c | 27 ++++++++++++++------------- > arch/x86/kvm/x86.c | 12 +++++++++++- > 3 files changed, 27 insertions(+), 15 deletions(-) >=20 > diff --git a/arch/x86/include/asm/kvm_emulate.h = b/arch/x86/include/asm/kvm_emulate.h > index 93c4bf598fb0..6c33caa82fa5 100644 > --- a/arch/x86/include/asm/kvm_emulate.h > +++ b/arch/x86/include/asm/kvm_emulate.h > @@ -203,7 +203,8 @@ struct x86_emulate_ops { > void (*set_gdt)(struct x86_emulate_ctxt *ctxt, struct desc_ptr = *dt); > void (*set_idt)(struct x86_emulate_ctxt *ctxt, struct desc_ptr = *dt); > ulong (*get_cr)(struct x86_emulate_ctxt *ctxt, int cr); > - int (*set_cr)(struct x86_emulate_ctxt *ctxt, int cr, ulong val); > + int (*set_cr)(struct x86_emulate_ctxt *ctxt, int cr, ulong val, > + bool from_rsm); > int (*cpl)(struct x86_emulate_ctxt *ctxt); > int (*get_dr)(struct x86_emulate_ctxt *ctxt, int dr, ulong = *dest); > int (*set_dr)(struct x86_emulate_ctxt *ctxt, int dr, ulong = value); > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c > index c338984c850d..a6204105d4d7 100644 > --- a/arch/x86/kvm/emulate.c > +++ b/arch/x86/kvm/emulate.c > @@ -2413,7 +2413,7 @@ static int rsm_enter_protected_mode(struct = x86_emulate_ctxt *ctxt, > cr3 &=3D ~0xfff; > } >=20 > - bad =3D ctxt->ops->set_cr(ctxt, 3, cr3); > + bad =3D ctxt->ops->set_cr(ctxt, 3, cr3, true); > if (bad) > return X86EMUL_UNHANDLEABLE; >=20 > @@ -2422,20 +2422,20 @@ static int rsm_enter_protected_mode(struct = x86_emulate_ctxt *ctxt, > * Then enable protected mode. However, PCID cannot be enabled > * if EFER.LMA=3D0, so set it separately. > */ > - bad =3D ctxt->ops->set_cr(ctxt, 4, cr4 & ~X86_CR4_PCIDE); > + bad =3D ctxt->ops->set_cr(ctxt, 4, cr4 & ~X86_CR4_PCIDE, true); > if (bad) > return X86EMUL_UNHANDLEABLE; >=20 > - bad =3D ctxt->ops->set_cr(ctxt, 0, cr0); > + bad =3D ctxt->ops->set_cr(ctxt, 0, cr0, true); > if (bad) > return X86EMUL_UNHANDLEABLE; >=20 > if (cr4 & X86_CR4_PCIDE) { > - bad =3D ctxt->ops->set_cr(ctxt, 4, cr4); > + bad =3D ctxt->ops->set_cr(ctxt, 4, cr4, true); > if (bad) > return X86EMUL_UNHANDLEABLE; > if (pcid) { > - bad =3D ctxt->ops->set_cr(ctxt, 3, cr3 | pcid); > + bad =3D ctxt->ops->set_cr(ctxt, 3, cr3 | pcid, = true); > if (bad) > return X86EMUL_UNHANDLEABLE; > } > @@ -2581,7 +2581,7 @@ static int em_rsm(struct x86_emulate_ctxt *ctxt) >=20 > /* Zero CR4.PCIDE before CR0.PG. */ > if (cr4 & X86_CR4_PCIDE) { > - ctxt->ops->set_cr(ctxt, 4, cr4 & = ~X86_CR4_PCIDE); > + ctxt->ops->set_cr(ctxt, 4, cr4 & ~X86_CR4_PCIDE, = true); > cr4 &=3D ~X86_CR4_PCIDE; > } >=20 > @@ -2595,11 +2595,12 @@ static int em_rsm(struct x86_emulate_ctxt = *ctxt) > /* For the 64-bit case, this will clear EFER.LMA. */ > cr0 =3D ctxt->ops->get_cr(ctxt, 0); > if (cr0 & X86_CR0_PE) > - ctxt->ops->set_cr(ctxt, 0, cr0 & ~(X86_CR0_PG | = X86_CR0_PE)); > + ctxt->ops->set_cr(ctxt, 0, cr0 & ~(X86_CR0_PG | = X86_CR0_PE), > + true); >=20 > /* Now clear CR4.PAE (which must be done before clearing = EFER.LME). */ > if (cr4 & X86_CR4_PAE) > - ctxt->ops->set_cr(ctxt, 4, cr4 & ~X86_CR4_PAE); > + ctxt->ops->set_cr(ctxt, 4, cr4 & ~X86_CR4_PAE, true); >=20 > /* And finally go back to 32-bit mode. */ > efer =3D 0; > @@ -3131,7 +3132,7 @@ static int load_state_from_tss32(struct = x86_emulate_ctxt *ctxt, > int ret; > u8 cpl; >=20 > - if (ctxt->ops->set_cr(ctxt, 3, tss->cr3)) > + if (ctxt->ops->set_cr(ctxt, 3, tss->cr3, false)) > return emulate_gp(ctxt, 0); > ctxt->_eip =3D tss->eip; > ctxt->eflags =3D tss->eflags | 2; > @@ -3331,7 +3332,7 @@ static int emulator_do_task_switch(struct = x86_emulate_ctxt *ctxt, > write_segment_descriptor(ctxt, tss_selector, = &next_tss_desc); > } >=20 > - ops->set_cr(ctxt, 0, ops->get_cr(ctxt, 0) | X86_CR0_TS); > + ops->set_cr(ctxt, 0, ops->get_cr(ctxt, 0) | X86_CR0_TS, false); > ops->set_segment(ctxt, tss_selector, &next_tss_desc, 0, = VCPU_SREG_TR); >=20 > if (has_error_code) { > @@ -3633,7 +3634,7 @@ static int em_movbe(struct x86_emulate_ctxt = *ctxt) >=20 > static int em_cr_write(struct x86_emulate_ctxt *ctxt) > { > - if (ctxt->ops->set_cr(ctxt, ctxt->modrm_reg, ctxt->src.val)) > + if (ctxt->ops->set_cr(ctxt, ctxt->modrm_reg, ctxt->src.val, = false)) > return emulate_gp(ctxt, 0); >=20 > /* Disable writeback. */ > @@ -3766,7 +3767,7 @@ static int em_clts(struct x86_emulate_ctxt = *ctxt) >=20 > cr0 =3D ctxt->ops->get_cr(ctxt, 0); > cr0 &=3D ~X86_CR0_TS; > - ctxt->ops->set_cr(ctxt, 0, cr0); > + ctxt->ops->set_cr(ctxt, 0, cr0, false); > return X86EMUL_CONTINUE; > } >=20 > @@ -3866,7 +3867,7 @@ static int em_smsw(struct x86_emulate_ctxt = *ctxt) > static int em_lmsw(struct x86_emulate_ctxt *ctxt) > { > ctxt->ops->set_cr(ctxt, 0, (ctxt->ops->get_cr(ctxt, 0) & = ~0x0eul) > - | (ctxt->src.val & 0x0f)); > + | (ctxt->src.val & 0x0f), false); > ctxt->dst.type =3D OP_NONE; > return X86EMUL_CONTINUE; > } > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index a419656521b6..f2745e3170b6 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -5739,7 +5739,8 @@ static unsigned long emulator_get_cr(struct = x86_emulate_ctxt *ctxt, int cr) > return value; > } >=20 > -static int emulator_set_cr(struct x86_emulate_ctxt *ctxt, int cr, = ulong val) > +static int emulator_set_cr(struct x86_emulate_ctxt *ctxt, int cr, = ulong val, > + bool from_rsm) > { > struct kvm_vcpu *vcpu =3D emul_to_vcpu(ctxt); > int res =3D 0; > @@ -5755,7 +5756,16 @@ static int emulator_set_cr(struct = x86_emulate_ctxt *ctxt, int cr, ulong val) > res =3D kvm_set_cr3(vcpu, val); > break; > case 4: > + /* > + * set_cr4() may forbid to set certain flags (e.g. VMXE) = from > + * SMM but we're actually leaving it; temporary drop = HF_SMM_MASK > + * when setting CR4. > + */ > + if (from_rsm) > + vcpu->arch.hflags &=3D ~HF_SMM_MASK; > res =3D kvm_set_cr4(vcpu, mk_cr_64(kvm_read_cr4(vcpu), = val)); > + if (from_rsm) > + vcpu->arch.hflags |=3D HF_SMM_MASK; > break; > case 8: > res =3D kvm_set_cr8(vcpu, val); > --=20 > 2.20.1 >=20