linux-kernel - Re: [PATCH v2 2/9] powerpc/kvm: make hypervisor state restore a function

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160519142456.GA913@in.ibm.com>
Date:	Thu, 19 May 2016 19:54:56 +0530
From:	Gautham R Shenoy <ego@...ux.vnet.ibm.com>
To:	Shreyas B Prabhu <shreyas@...ux.vnet.ibm.com>
Cc:	ego@...ux.vnet.ibm.com, mpe@...erman.id.au,
	linuxppc-dev@...ts.ozlabs.org, paulus@...abs.org,
	linux-kernel@...r.kernel.org, mikey@...ling.org
Subject: Re: [PATCH v2 2/9] powerpc/kvm: make hypervisor state restore a
 function

Hi Shreyas,

On Wed, May 18, 2016 at 12:37:56PM +0530, Shreyas B Prabhu wrote:

[..snip..]
> >> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> >> index 7716ceb..7ebfbb0 100644
> >> --- a/arch/powerpc/kernel/exceptions-64s.S
> >> +++ b/arch/powerpc/kernel/exceptions-64s.S
> >> @@ -107,25 +107,8 @@ BEGIN_FTR_SECTION
> >>  	beq	9f
> >>
> >>  	cmpwi	cr3,r13,2
> >> +	bl	power7_restore_hyp_resource
> >>
> >> -	/*
> >> -	 * Check if last bit of HSPGR0 is set. This indicates whether we are
> >> -	 * waking up from winkle.
> >> -	 */
> >> -	GET_PACA(r13)
> >> -	clrldi	r5,r13,63
> >> -	clrrdi	r13,r13,1
> >> -	cmpwi	cr4,r5,1
> >> -	mtspr	SPRN_HSPRG0,r13
> >> -
> >> -	lbz	r0,PACA_THREAD_IDLE_STATE(r13)
> >> -	cmpwi   cr2,r0,PNV_THREAD_NAP
> >> -	bgt     cr2,8f				/* Either sleep or Winkle */
> >> -
> >> -	/* Waking up from nap should not cause hypervisor state loss */
> >> -	bgt	cr3,.
> >> -
> >> -	/* Waking up from nap */
> >>  	li	r0,PNV_THREAD_RUNNING
> >>  	stb	r0,PACA_THREAD_IDLE_STATE(r13)	/* Clear thread state */
> >>
> >> @@ -143,13 +126,9 @@ BEGIN_FTR_SECTION
> >>
> >>  	/* Return SRR1 from power7_nap() */
> >>  	mfspr	r3,SPRN_SRR1
> >> -	beq	cr3,2f
> >> -	b	power7_wakeup_noloss
> >> -2:	b	power7_wakeup_loss
> >> -
> >> -	/* Fast Sleep wakeup on PowerNV */
> >> -8:	GET_PACA(r13)
> > 
> > In the old code, we do a GET_PACA(r13) before invoking the
> > power7_wakeup_tb_loss. In the new code we don't. Can you explain
> > this omission ?
> 
> GET_PACA(13) is the called in the beginning of
> power7_restore_hyp_resource. So r13 contains pointer to PACA when
> power7_wakeup_tb_loss invoked later in the same function.

Ah, I see it now. So the GET_PACA(r13) at 8: was anyway redundant in
the older code.

You can add my Reviewed-by: to this patch.

--
Thanks and Regards
gautham.