qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Laurent Vivier
Subject: Re: [Qemu-devel] [Qemu-ppc] [PULL 1/3] ppc: Rework POWER7 & POWER8 exception model
Date: Thu, 7 Apr 2016 12:45:41 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.1


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 <address@hidden>
>>>
>>> From: Benjamin Herrenschmidt <address@hidden>
>>>
>>> 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.

Laurent

> Thanks,
> 
> C.
> 
>>
>> Laurent
>>>
>>> Signed-off-by: Benjamin Herrenschmidt <address@hidden>
>>> [clg: Removed LPES0/1 handling for HV vs. !HV
>>>       Fixed LPCR_ILE case for POWERPC_EXCP_POWER8 ]
>>> Signed-off-by: Cédric Le Goater <address@hidden>
>>> [dwg: This was written as a cleanup, but it also fixes a real bug
>>>       where setting an alternative interrupt location would not be
>>>       correctly migrated]
>>> Signed-off-by: David Gibson <address@hidden>
>>> ---
>>>  hw/ppc/spapr_hcall.c        | 16 +--------------
>>>  include/hw/ppc/spapr.h      |  5 -----
>>>  target-ppc/cpu.h            | 10 +++++++++
>>>  target-ppc/excp_helper.c    | 49 
>>> +++++++++++++++++++++++++++++++++++++++++++--
>>>  target-ppc/translate_init.c |  2 +-
>>>  5 files changed, 59 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>>> index 2dcb676..8f40602 100644
>>> --- a/hw/ppc/spapr_hcall.c
>>> +++ b/hw/ppc/spapr_hcall.c
>>> @@ -824,7 +824,6 @@ static target_ulong 
>>> h_set_mode_resource_addr_trans_mode(PowerPCCPU *cpu,
>>>  {
>>>      CPUState *cs;
>>>      PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
>>> -    target_ulong prefix;
>>>  
>>>      if (!(pcc->insns_flags2 & PPC2_ISA207S)) {
>>>          return H_P2;
>>> @@ -836,25 +835,12 @@ static target_ulong 
>>> h_set_mode_resource_addr_trans_mode(PowerPCCPU *cpu,
>>>          return H_P4;
>>>      }
>>>  
>>> -    switch (mflags) {
>>> -    case H_SET_MODE_ADDR_TRANS_NONE:
>>> -        prefix = 0;
>>> -        break;
>>> -    case H_SET_MODE_ADDR_TRANS_0001_8000:
>>> -        prefix = 0x18000;
>>> -        break;
>>> -    case H_SET_MODE_ADDR_TRANS_C000_0000_0000_4000:
>>> -        prefix = 0xC000000000004000ULL;
>>> -        break;
>>> -    default:
>>> +    if (mflags == AIL_RESERVED) {
>>>          return H_UNSUPPORTED_FLAG;
>>>      }
>>>  
>>>      CPU_FOREACH(cs) {
>>> -        CPUPPCState *env = &POWERPC_CPU(cpu)->env;
>>> -
>>>          set_spr(cs, SPR_LPCR, mflags << LPCR_AIL_SHIFT, LPCR_AIL);
>>> -        env->excp_prefix = prefix;
>>>      }
>>>  
>>>      return H_SUCCESS;
>>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>>> index 098d85d..815d5ee 100644
>>> --- a/include/hw/ppc/spapr.h
>>> +++ b/include/hw/ppc/spapr.h
>>> @@ -204,11 +204,6 @@ struct sPAPRMachineState {
>>>  #define H_SET_MODE_ENDIAN_BIG    0
>>>  #define H_SET_MODE_ENDIAN_LITTLE 1
>>>  
>>> -/* Flags for H_SET_MODE_RESOURCE_ADDR_TRANS_MODE */
>>> -#define H_SET_MODE_ADDR_TRANS_NONE                  0
>>> -#define H_SET_MODE_ADDR_TRANS_0001_8000             2
>>> -#define H_SET_MODE_ADDR_TRANS_C000_0000_0000_4000   3
>>> -
>>>  /* VASI States */
>>>  #define H_VASI_INVALID    0
>>>  #define H_VASI_ENABLED    1
>>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>>> index 676081e..9d4e43c 100644
>>> --- a/target-ppc/cpu.h
>>> +++ b/target-ppc/cpu.h
>>> @@ -167,6 +167,8 @@ enum powerpc_excp_t {
>>>      POWERPC_EXCP_970,
>>>      /* POWER7 exception model           */
>>>      POWERPC_EXCP_POWER7,
>>> +    /* POWER8 exception model           */
>>> +    POWERPC_EXCP_POWER8,
>>>  #endif /* defined(TARGET_PPC64) */
>>>  };
>>>  
>>> @@ -2277,6 +2279,14 @@ enum {
>>>      HMER_XSCOM_STATUS_LSH       = (63 - 23),
>>>  };
>>>  
>>> +/* Alternate Interrupt Location (AIL) */
>>> +enum {
>>> +    AIL_NONE                = 0,
>>> +    AIL_RESERVED            = 1,
>>> +    AIL_0001_8000           = 2,
>>> +    AIL_C000_0000_0000_4000 = 3,
>>> +};
>>> +
>>>  
>>> /*****************************************************************************/
>>>  
>>>  static inline target_ulong cpu_read_xer(CPUPPCState *env)
>>> diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
>>> index c890853..ca4ffe8 100644
>>> --- a/target-ppc/excp_helper.c
>>> +++ b/target-ppc/excp_helper.c
>>> @@ -77,7 +77,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int 
>>> excp_model, int excp)
>>>      CPUPPCState *env = &cpu->env;
>>>      target_ulong msr, new_msr, vector;
>>>      int srr0, srr1, asrr0, asrr1;
>>> -    int lpes0, lpes1, lev;
>>> +    int lpes0, lpes1, lev, ail;
>>>  
>>>      if (0) {
>>>          /* XXX: find a suitable condition to enable the hypervisor mode */
>>> @@ -108,6 +108,25 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int 
>>> excp_model, int excp)
>>>      asrr0 = -1;
>>>      asrr1 = -1;
>>>  
>>> +    /* Exception targetting modifiers
>>> +     *
>>> +     * AIL is initialized here but can be cleared by
>>> +     * selected exceptions
>>> +     */
>>> +#if defined(TARGET_PPC64)
>>> +    if (excp_model == POWERPC_EXCP_POWER7 ||
>>> +        excp_model == POWERPC_EXCP_POWER8) {
>>> +        if (excp_model == POWERPC_EXCP_POWER8) {
>>> +            ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT;
>>> +        } else {
>>> +            ail = 0;
>>> +        }
>>> +    } else
>>> +#endif /* defined(TARGET_PPC64) */
>>> +    {
>>> +        ail = 0;
>>> +    }
>>> +
>>>      switch (excp) {
>>>      case POWERPC_EXCP_NONE:
>>>          /* Should never happen */
>>> @@ -146,6 +165,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int 
>>> excp_model, int excp)
>>>              /* XXX: find a suitable condition to enable the hypervisor 
>>> mode */
>>>              new_msr |= (target_ulong)MSR_HVB;
>>>          }
>>> +        ail = 0;
>>>  
>>>          /* machine check exceptions don't have ME set */
>>>          new_msr &= ~((target_ulong)1 << MSR_ME);
>>> @@ -344,6 +364,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int 
>>> excp_model, int excp)
>>>              /* XXX: find a suitable condition to enable the hypervisor 
>>> mode */
>>>              new_msr |= (target_ulong)MSR_HVB;
>>>          }
>>> +        ail = 0;
>>>          goto store_next;
>>>      case POWERPC_EXCP_DSEG:      /* Data segment exception                 
>>>   */
>>>          if (lpes1 == 0) {
>>> @@ -630,7 +651,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int 
>>> excp_model, int excp)
>>>      }
>>>  
>>>  #ifdef TARGET_PPC64
>>> -    if (excp_model == POWERPC_EXCP_POWER7) {
>>> +    if (excp_model == POWERPC_EXCP_POWER7 ||
>>> +        excp_model == POWERPC_EXCP_POWER8) {
>>>          if (env->spr[SPR_LPCR] & LPCR_ILE) {
>>>              new_msr |= (target_ulong)1 << MSR_LE;
>>>          }
>>> @@ -650,6 +672,29 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int 
>>> excp_model, int excp)
>>>                    excp);
>>>      }
>>>      vector |= env->excp_prefix;
>>> +
>>> +    /* AIL only works if there is no HV transition and we are running with
>>> +     * translations enabled
>>> +     */
>>> +    if (!((msr >> MSR_IR) & 1) || !((msr >> MSR_DR) & 1)) {
>>> +        ail = 0;
>>> +    }
>>> +    /* Handle AIL */
>>> +    if (ail) {
>>> +        new_msr |= (1 << MSR_IR) | (1 << MSR_DR);
>>> +        switch(ail) {
>>> +        case AIL_0001_8000:
>>> +            vector |= 0x18000;
>>> +            break;
>>> +        case AIL_C000_0000_0000_4000:
>>> +            vector |= 0xc000000000004000ull;
>>> +            break;
>>> +        default:
>>> +            cpu_abort(cs, "Invalid AIL combination %d\n", ail);
>>> +            break;
>>> +        }
>>> +    }
>>> +
>>>  #if defined(TARGET_PPC64)
>>>      if (excp_model == POWERPC_EXCP_BOOKE) {
>>>          if (env->spr[SPR_BOOKE_EPCR] & EPCR_ICM) {
>>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>>> index 0a33597..f515725 100644
>>> --- a/target-ppc/translate_init.c
>>> +++ b/target-ppc/translate_init.c
>>> @@ -8487,7 +8487,7 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
>>>      pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
>>>      pcc->sps = &POWER7_POWER8_sps;
>>>  #endif
>>> -    pcc->excp_model = POWERPC_EXCP_POWER7;
>>> +    pcc->excp_model = POWERPC_EXCP_POWER8;
>>>      pcc->bus_model = PPC_FLAGS_INPUT_POWER7;
>>>      pcc->bfd_mach = bfd_mach_ppc64;
>>>      pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
>>>
>>
> 



reply via email to

[Prev in Thread] Current Thread [Next in Thread]