From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Subject: Re: [PATCH 01/14] KVM: PPC: e500: don't translate gfn to pfn with preemption disabled Date: Mon, 31 Oct 2011 13:52:03 -0500 Message-ID: <4EAEEE53.3070800@freescale.com> References: <1320047596-20577-1-git-send-email-agraf@suse.de> <1320047596-20577-2-git-send-email-agraf@suse.de> <4EAE99A0.7050903@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Cc: Alexander Graf , , kvm list , Marcelo Tosatti To: Avi Kivity Return-path: In-Reply-To: <4EAE99A0.7050903@redhat.com> Sender: kvm-ppc-owner@vger.kernel.org List-Id: kvm.vger.kernel.org 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. -Scott From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Date: Mon, 31 Oct 2011 18:52:03 +0000 Subject: Re: [PATCH 01/14] KVM: PPC: e500: don't translate gfn to pfn with Message-Id: <4EAEEE53.3070800@freescale.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> In-Reply-To: <4EAE99A0.7050903@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Avi Kivity Cc: Alexander Graf , kvm-ppc@vger.kernel.org, kvm list , Marcelo Tosatti 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. -Scott