Re: [Qemu-devel] [PATCH -V3 4/4] target-ppc: Use #define for max slb entries

[Andreas Färber]

Am 25.08.2013 20:33, schrieb Alexander Graf:
> 
> On 23.08.2013, at 06:20, Aneesh Kumar K.V wrote:
> 
>> From: "Aneesh Kumar K.V" <address@hidden>
>>
>> Instead of opencoding 64 use MAX_SLB_ENTRIES. We don't update the kernel
>> header here.
> 
> Ah, here you're fixing up the hardcoded 64 :). Could you please check whether 
> ARRAY_SIZE() works in all these as well? If not, this patch is good.
> 
> 
> Alex
> 
>>
>> Signed-off-by: Aneesh Kumar K.V <address@hidden>
>> ---
>> target-ppc/cpu.h     | 3 ++-
>> target-ppc/kvm.c     | 6 +++---
>> target-ppc/machine.c | 2 +-
>> 3 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>> index 711db08..b06818e 100644
>> --- a/target-ppc/cpu.h
>> +++ b/target-ppc/cpu.h
>> @@ -405,6 +405,7 @@ struct ppc_slb_t {
>>  uint64_t vsid;
>> };
>>
>> +#define MAX_SLB_ENTRIES         64
>> #define SEGMENT_SHIFT_256M      28
>> #define SEGMENT_MASK_256M       (~((1ULL << SEGMENT_SHIFT_256M) - 1))
>>
>> @@ -947,7 +948,7 @@ struct CPUPPCState {
>> #if !defined(CONFIG_USER_ONLY)
>> #if defined(TARGET_PPC64)
>>  /* PowerPC 64 SLB area */
>> -    ppc_slb_t slb[64];
>> +    ppc_slb_t slb[MAX_SLB_ENTRIES];
>>  int32_t slb_nr;
>> #endif
>>  /* segment registers */
>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>> index bcc6544..fce8835 100644
>> --- a/target-ppc/kvm.c
>> +++ b/target-ppc/kvm.c
>> @@ -818,7 +818,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>>
>>      /* Sync SLB */
>> #ifdef TARGET_PPC64
>> -        for (i = 0; i < 64; i++) {
>> +        for (i = 0; i < MAX_SLB_ENTRIES; i++) {
>>          sregs.u.s.ppc64.slb[i].slbe = env->slb[i].esid;
>>          sregs.u.s.ppc64.slb[i].slbv = env->slb[i].vsid;
>>      }
>> @@ -1042,8 +1042,8 @@ int kvm_arch_get_registers(CPUState *cs)
>>       * the env->slb array first so that we mark all entries invalid and
>>       * update with only valid SLB entries.
>>       */
>> -        memset(env->slb, 0, 64 * sizeof(ppc_slb_t));
>> -        for (i = 0; i < 64; i++) {
>> +        memset(env->slb, 0, MAX_SLB_ENTRIES * sizeof(ppc_slb_t));
>> +        for (i = 0; i < MAX_SLB_ENTRIES; i++) {
>>          target_ulong rb = sregs.u.s.ppc64.slb[i].slbe;
>>          target_ulong rs = sregs.u.s.ppc64.slb[i].slbv;
>>          /*
>> diff --git a/target-ppc/machine.c b/target-ppc/machine.c
>> index 12e1512..12c174f 100644
>> --- a/target-ppc/machine.c
>> +++ b/target-ppc/machine.c
>> @@ -312,7 +312,7 @@ static const VMStateDescription vmstate_slb = {
>>  .minimum_version_id_old = 1,
>>  .fields      = (VMStateField []) {
>>      VMSTATE_INT32_EQUAL(env.slb_nr, PowerPCCPU),
>> -        VMSTATE_SLB_ARRAY(env.slb, PowerPCCPU, 64),
>> +        VMSTATE_SLB_ARRAY(env.slb, PowerPCCPU, MAX_SLB_ENTRIES),

IMO use of ARRAY_SIZE() makes it too easy to break VMState here.
Don't know how likely it is to be touched in the future, of course.

Andreas

>>      VMSTATE_END_OF_LIST()
>>  }
>> };
>> -- 
>> 1.8.1.2
>>
> 
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg