From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoffer Dall Subject: Re: [PATCH v2 2/2] kvm: arm/arm64: vgic-v3: Tighten synchronization for guests using v2 on v3 Date: Mon, 12 Mar 2018 17:45:37 -0700 Message-ID: <20180313004537.GB16740@lvm> References: <20180311124956.7683-1-marc.zyngier@arm.com> <20180311124956.7683-3-marc.zyngier@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: linux-arm-kernel@lists.infradead.org, Eric Auger , kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, Andre Przywara To: Marc Zyngier Return-path: Content-Disposition: inline In-Reply-To: <20180311124956.7683-3-marc.zyngier@arm.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org List-Id: kvm.vger.kernel.org On Sun, Mar 11, 2018 at 12:49:56PM +0000, Marc Zyngier wrote: > On guest exit, and when using GICv2 on GICv3, we use a dsb(st) to > force synchronization between the memory-mapped guest view and > the system-register view that the hypervisor uses. > > This is incorrect, as the spec calls out the need for "a DSB whose > required access type is both loads and stores with any Shareability > attribute", while we're only synchronizing stores. > > We also lack an isb after the dsb to ensure that the latter has > actually been executed before we start reading stuff from the sysregs. > > The fix is pretty easy: turn dsb(st) into dsb(sy), and slap an isb() > just after. > > Cc: stable@vger.kernel.org > Fixes: f68d2b1b73cc ("arm64: KVM: Implement vgic-v3 save/restore") > Reviewed-by: Andre Przywara > Signed-off-by: Marc Zyngier Acked-by: Christoffer Dall > --- > virt/kvm/arm/hyp/vgic-v3-sr.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/virt/kvm/arm/hyp/vgic-v3-sr.c b/virt/kvm/arm/hyp/vgic-v3-sr.c > index f5c3d6d7019e..b89ce5432214 100644 > --- a/virt/kvm/arm/hyp/vgic-v3-sr.c > +++ b/virt/kvm/arm/hyp/vgic-v3-sr.c > @@ -215,7 +215,8 @@ void __hyp_text __vgic_v3_save_state(struct kvm_vcpu *vcpu) > * are now visible to the system register interface. > */ > if (!cpu_if->vgic_sre) { > - dsb(st); > + dsb(sy); > + isb(); > cpu_if->vgic_vmcr = read_gicreg(ICH_VMCR_EL2); > } > > -- > 2.14.2 >