From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 01/14] KVM: PPC: e500: don't translate gfn to pfn with preemption disabled Date: Tue, 01 Nov 2011 11:00:31 +0200 Message-ID: <4EAFB52F.8060909@redhat.com> References: <1320047596-20577-1-git-send-email-agraf@suse.de> <1320047596-20577-2-git-send-email-agraf@suse.de> <4EAE99A0.7050903@redhat.com> <4EAEEE53.3070800@freescale.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Alexander Graf , kvm-ppc@vger.kernel.org, kvm list , Marcelo Tosatti To: Scott Wood Return-path: Received: from mx1.redhat.com ([209.132.183.28]:62091 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752544Ab1KAJAi (ORCPT ); Tue, 1 Nov 2011 05:00:38 -0400 In-Reply-To: <4EAEEE53.3070800@freescale.com> Sender: kvm-owner@vger.kernel.org List-ID: On 10/31/2011 08:52 PM, Scott Wood wrote: > On 10/31/2011 07:50 AM, Avi Kivity wrote: > > On 10/31/2011 09:53 AM, Alexander Graf wrote: > >> +/* sesel is index into the set, not the whole array */ > >> +static void write_stlbe(struct kvmppc_vcpu_e500 *vcpu_e500, > >> + struct tlbe *gtlbe, > >> + struct tlbe *stlbe, > >> + int stlbsel, int sesel) > >> +{ > >> + int stid; > >> + > >> + preempt_disable(); > >> + stid = kvmppc_e500_get_sid(vcpu_e500, get_tlb_ts(gtlbe), > >> + get_tlb_tid(gtlbe), > >> + get_cur_pr(&vcpu_e500->vcpu), 0); > >> + > >> + stlbe->mas1 |= MAS1_TID(stid); > >> + write_host_tlbe(vcpu_e500, stlbsel, sesel, stlbe); > >> + preempt_enable(); > >> +} > >> + > >> > > > > This naked preempt_disable() is fishy. What happens if we're migrated > > immediately afterwards? we fault again and redo? > > Yes, we'll fault again. > > We just want to make sure that the sid is still valid when we write the > TLB entry. If we migrate, we'll get a new sid and the old TLB entry > will be irrelevant, even if we migrate back to the same CPU. The entire > TLB will be flushed before a sid is reused. > > If we don't do the preempt_disable(), we could get the sid on one CPU > and then get migrated and run it on another CPU where that sid is (or > will be) valid for a different context. Or we could run out of sids > while preempted, making the sid allocated before this possibly valid for > a different context. > > Makes sense, thanks. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.