Re: [Qemu-devel] [PATCH RFC v1 1/3] target-ppc: add TLB_NEED_LOCAL_FLUSH flag

[Nikunj A Dadhania]

Alex Bennée <address@hidden> writes:

> Nikunj A Dadhania <address@hidden> writes:
>
> I think we need a little more detail here. In fact when you post the
> next version of the series could you please include a cover letter to
> cover what the series is trying to achieve?

Sure will do that.

>
>> Signed-off-by: Nikunj A Dadhania <address@hidden>
>> ---
>>  target-ppc/cpu.h         | 1 +
>>  target-ppc/helper_regs.h | 2 +-
>>  target-ppc/mmu-hash64.c  | 4 ++--
>>  target-ppc/mmu_helper.c  | 6 +++---
>>  4 files changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>> index 1e808c8..71111dc 100644
>> --- a/target-ppc/cpu.h
>> +++ b/target-ppc/cpu.h
>> @@ -1009,6 +1009,7 @@ struct CPUPPCState {
>>      bool tlb_dirty;   /* Set to non-zero when modifying TLB                 
>>  */
>>      bool kvm_sw_tlb;  /* non-zero if KVM SW TLB API is active               
>>  */
>>      uint32_t tlb_need_flush; /* Delayed flush needed */
>> +#define TLB_NEED_LOCAL_FLUSH   0x1
>>  #endif
>>
>>      /* Other registers */
>> diff --git a/target-ppc/helper_regs.h b/target-ppc/helper_regs.h
>> index 3d279f1..4457a30 100644
>> --- a/target-ppc/helper_regs.h
>> +++ b/target-ppc/helper_regs.h
>> @@ -157,7 +157,7 @@ static inline int hreg_store_msr(CPUPPCState *env, 
>> target_ulong value,
>>  static inline void check_tlb_flush(CPUPPCState *env)
>>  {
>>      CPUState *cs = CPU(ppc_env_get_cpu(env));
>> -    if (env->tlb_need_flush) {
>> +    if ((env->tlb_need_flush & TLB_NEED_LOCAL_FLUSH) == 
>> TLB_NEED_LOCAL_FLUSH) {
>>          env->tlb_need_flush = 0;
>>          tlb_flush(cs, 1);
>>      }
>> diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
>> index 8118143..4c7ceef 100644
>> --- a/target-ppc/mmu-hash64.c
>> +++ b/target-ppc/mmu-hash64.c
>> @@ -110,7 +110,7 @@ void helper_slbia(CPUPPCState *env)
>>               *      and we still don't have a tlb_flush_mask(env, n, mask)
>>               *      in QEMU, we just invalidate all TLBs
>>               */
>> -            env->tlb_need_flush = 1;
>> +            env->tlb_need_flush = TLB_NEED_LOCAL_FLUSH;
>
> I'm not sure what we gain here versus just using a straight bool for the flag.

In the next patches I am adding TLB_NEED_GLOBAL_FLUSH, that is for
broadcast flush for other cpus.

TLB_NEED_LOCAL_FLUSH = 0x1
TLB_NEED_GLOBAL_FLUSH = 0x2

Regards
Nikunj