Re: [Qemu-devel] [Qemu-ppc] [PULL 1/3] ppc: Rework POWER7 & POWER8 exception model

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2170 bytes --]

On Thu, Apr 07, 2016 at 12:45:41PM +0200, Laurent Vivier wrote:
> 
> 
> On 07/04/2016 12:27, Cédric Le Goater wrote:
> > Hello Laurent,
> > 
> > On 04/07/2016 11:13 AM, Laurent Vivier wrote:
> >>
> >>
> >> On 05/04/2016 04:17, David Gibson wrote:
> >>> From: Cédric Le Goater <clg@fr.ibm.com>
> >>>
> >>> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> >>>
> >>> This patch fixes the current AIL implementation for POWER8. The
> >>> interrupt vector address can be calculated directly from LPCR when the
> >>> exception is handled. The excp_prefix update becomes useless and we
> >>> can cleanup the H_SET_MODE hcall.
> >>
> >> I know it's a little bit late to comment this patch but:
> >>
> >> what about the initialization of the NIP in ppc_cpu_reset()?
> >>
> >>     env->nip = env->hreset_vector | env->excp_prefix;
> >>
> >> on POWER8 "env->excp_prefix" is always 0, but LPCR can have an AIL defined?
> > 
> > yes. env->spr[SPR_LPCR] still has the previous value at that time and 
> > it is reseted right below in the same routine. 
> > 
> > The cpu should restart in a valid state after that and later on, use the 
> > H_SET_MODE hcall from the guest kernel to set the AIL bits back in LPCR. 
> > It looks fine to me but I might be missing something. 
> 
> What I mean is if we want to keep the previous behavior we should have
> something like:
> 
>     env->nip = env->hreset_vector | env->excp_prefix;
> #if defined(TARGET_PPC64)
>     switch((env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT) {
>     case AIL_0001_8000:
>         env->nip |= 0x18000;
>         break;
>     case AIL_C000_0000_0000_4000:
>         env->nip |= 0xc000000000004000ull;
>         break;
>     }
> #endif
> 
> But I don't know how behaves really a POWER8.

I'm pretty certain the previous behaviour was wrong.  The LPCR AIL
bits shouldn't survive a reset, so the new NIP after the reset should
be based on a cleared AIL.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]