> -----Original Message----- > From: Alexander Graf [mailto:ag...@suse.de] > Sent: Thursday, October 13, 2011 2:36 PM > To: Bhushan Bharat-R65777 > Cc: <kvm-ppc@vger.kernel.org>; <bharatb.ya...@gmail.com>; Bhushan Bharat- > R65777 > Subject: Re: [PATCH] PPC: Fix race in mtmsr paravirt implementation > > > Am 13.10.2011 um 07:40 schrieb Bharat Bhushan <r65...@freescale.com>: > > > The current implementation of mtmsr and mtmsrd are racy in that it > does: > > > > * check (int_pending == 0) > > ---> host sets int_pending = 1 <--- > > * write shared page > > * done > > > > while instead we should check for int_pending after the shared page is > written. > > > > Signed-off-by: Bharat Bhushan <bharat.bhus...@freescale.com> > > --- > > arch/powerpc/kernel/kvm_emul.S | 22 ++++++++++------------ > > 1 files changed, 10 insertions(+), 12 deletions(-) > > > > diff --git a/arch/powerpc/kernel/kvm_emul.S > > b/arch/powerpc/kernel/kvm_emul.S index f2b1b25..65f853b 100644 > > --- a/arch/powerpc/kernel/kvm_emul.S > > +++ b/arch/powerpc/kernel/kvm_emul.S > > @@ -85,15 +85,15 @@ kvm_emulate_mtmsrd_reg: > > /* Put MSR back into magic page */ > > STL64(r31, KVM_MAGIC_PAGE + KVM_MAGIC_MSR, 0) > > > > + /* Check if we may trigger an interrupt */ > > + andi. r30, r30, MSR_EE > > + beq no_check > > + > > /* Check if we have to fetch an interrupt */ > > lwz r31, (KVM_MAGIC_PAGE + KVM_MAGIC_INT)(0) > > cmpwi r31, 0 > > beq+ no_check > > > > - /* Check if we may trigger an interrupt */ > > - andi. r30, r30, MSR_EE > > - beq no_check > > - > > This chunk should actually be ok already. We're basically doing: > > if(likely(!int_pending) && !(new_msr & MSR_EE)) > goto no_check; > > Since we wrote shared.msr before the check, we're good, no?
Actually I borrowed this from wrtee implementation and keep consistant across mtmsr/ mtmsrd and wrtee/i. I thought of it like: If we are qualified to take interrupt (EE set) then only check for pending interrupt rather than check for pending interrupt and then check whether we are qualified to take it or not. If you think earlier is better way of understanding then I will change the patch. Thanks -Bharat > > > SCRATCH_RESTORE > > > > /* Nag hypervisor */ > > @@ -167,22 +167,20 @@ maybe_stay_in_guest: > > kvm_emulate_mtmsr_reg2: > > ori r30, r0, 0 > > > > - /* Check if we have to fetch an interrupt */ > > - lwz r31, (KVM_MAGIC_PAGE + KVM_MAGIC_INT)(0) > > - cmpwi r31, 0 > > - beq+ no_mtmsr > > + /* Put MSR into magic page because we don't call mtmsr */ > > + STL64(r30, KVM_MAGIC_PAGE + KVM_MAGIC_MSR, 0) > > > > /* Check if we may trigger an interrupt */ > > andi. r31, r30, MSR_EE > > beq no_mtmsr > > > > - b do_mtmsr > > + /* Check if we have to fetch an interrupt */ > > + lwz r31, (KVM_MAGIC_PAGE + KVM_MAGIC_INT)(0) > > + cmpwi r31, 0 > > + bne- do_mtmsr > > > > no_mtmsr: > > > > - /* Put MSR into magic page because we don't call mtmsr */ > > - STL64(r30, KVM_MAGIC_PAGE + KVM_MAGIC_MSR, 0) > > - > > This one looks good. > > Alex > > > SCRATCH_RESTORE > > > > /* Go back to caller */ > > -- > > 1.7.0.4 > > > > -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
