From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Date: Tue, 01 Nov 2011 09:00:31 +0000 Subject: Re: [PATCH 01/14] KVM: PPC: e500: don't translate gfn to pfn with Message-Id: <4EAFB52F.8060909@redhat.com> List-Id: 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> In-Reply-To: <4EAEEE53.3070800@freescale.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Scott Wood Cc: Alexander Graf , kvm-ppc@vger.kernel.org, kvm list , Marcelo Tosatti 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.