Re: [RFC v2 05/12] target/ppc: powerpc_excp: Standardize arguments to interrupt code

[Fabiano Rosas <farosas@linux.ibm.com>]

David Gibson <david@gibson.dropbear.id.au> writes:

> On Mon, Dec 20, 2021 at 03:18:56PM -0300, Fabiano Rosas wrote:
>> In preparation to moving the interrupt code into separate functions,
>> create a PPCIntrArgs structure to serve as a consistent API.
>
> The patch doesn't seem to match this description - I see no new
> structure here.

I didn't want to create the new ppc_intr.h header in this patch so I
might have been careless and put the structure right before the
powerpc_excp definition. See below.

>> 
>> No functional change intended.
>> 
>> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
>> ---
>>  target/ppc/excp_helper.c | 213 +++++++++++++++++++++------------------
>>  1 file changed, 113 insertions(+), 100 deletions(-)
>> 
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index 45641f6d1d..f478ff8a87 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -164,7 +164,7 @@ static int powerpc_reset_wakeup(CPUState *cs, CPUPPCState *env, int excp,
>>  static inline void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp_model, int excp,
>>                                        target_ulong msr,
>>                                        target_ulong *new_msr,
>> -                                      target_ulong *vector)
>> +                                      target_ulong *new_nip)
>>  {
>>  #if defined(TARGET_PPC64)
>>      CPUPPCState *env = &cpu->env;
>> @@ -241,9 +241,9 @@ static inline void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp_model, int excp,
>>  
>>      if (excp != POWERPC_EXCP_SYSCALL_VECTORED) {
>>          if (ail == 2) {
>> -            *vector |= 0x0000000000018000ull;
>> +            *new_nip |= 0x0000000000018000ull;
>>          } else if (ail == 3) {
>> -            *vector |= 0xc000000000004000ull;
>> +            *new_nip |= 0xc000000000004000ull;
>>          }
>>      } else {
>>          /*
>> @@ -251,15 +251,15 @@ static inline void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp_model, int excp,
>>           * only the MSR. AIL=3 replaces the 0x17000 base with 0xc...3000.
>>           */
>>          if (ail == 3) {
>> -            *vector &= ~0x0000000000017000ull; /* Un-apply the base offset */
>> -            *vector |= 0xc000000000003000ull; /* Apply scv's AIL=3 offset */
>> +            *new_nip &= ~0x0000000000017000ull; /* Un-apply the base offset */
>> +            *new_nip |= 0xc000000000003000ull; /* Apply scv's AIL=3 offset */
>>          }
>>      }
>>  #endif
>>  }
>>  
>> -static inline void powerpc_set_excp_state(PowerPCCPU *cpu,
>> -                                          target_ulong vector, target_ulong msr)
>> +static inline void powerpc_set_excp_state(PowerPCCPU *cpu, target_ulong new_nip,
>> +                                          target_ulong new_msr)
>>  {
>>      CPUState *cs = CPU(cpu);
>>      CPUPPCState *env = &cpu->env;
>> @@ -272,9 +272,9 @@ static inline void powerpc_set_excp_state(PowerPCCPU *cpu,
>>       * will prevent setting of the HV bit which some exceptions might need
>>       * to do.
>>       */
>> -    env->msr = msr & env->msr_mask;
>> +    env->msr = new_msr & env->msr_mask;
>>      hreg_compute_hflags(env);
>> -    env->nip = vector;
>> +    env->nip = new_nip;
>>      /* Reset exception state */
>>      cs->exception_index = POWERPC_EXCP_NONE;
>>      env->error_code = 0;
>> @@ -289,6 +289,15 @@ static inline void powerpc_set_excp_state(PowerPCCPU *cpu,
>>      check_tlb_flush(env, false);
>>  }
>>  
>> +typedef struct PPCIntrArgs {
>> +    target_ulong nip;
>> +    target_ulong msr;
>> +    target_ulong new_nip;
>> +    target_ulong new_msr;
>> +    int sprn_srr0;
>> +    int sprn_srr1;
>> +} PPCIntrArgs;

here ^

I can have new header in this patch for the next version.

>> +
>>  /*
>>   * Note that this function should be greatly optimized when called
>>   * with a constant excp, from ppc_hw_interrupt
>> @@ -298,35 +307,35 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp)
>>      CPUState *cs = CPU(cpu);
>>      CPUPPCState *env = &cpu->env;
>>      int excp_model = env->excp_model;
>> -    target_ulong msr, new_msr, vector;
>> -    int srr0, srr1, lev = -1;
>> +    PPCIntrArgs regs;
>> +    int lev = -1;