Re: [Qemu-devel] [PATCH] cpu_defs: Simplify CPUTLB padding logic

[Richard Henderson <rth@twiddle.net>]

On 07/06/2015 09:43 AM, Paolo Bonzini wrote:
>
>
> On 05/07/2015 23:08, Peter Crosthwaite wrote:
>> There was a complicated subtractive arithmetic for determining the
>> padding on the CPUTLBEntry structure. Simplify this with a union.
>>
>> Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
>> ---
>>   include/exec/cpu-defs.h | 23 ++++++++++++-----------
>>   1 file changed, 12 insertions(+), 11 deletions(-)
>>
>> diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h
>> index 98b9cff..5093be2 100644
>> --- a/include/exec/cpu-defs.h
>> +++ b/include/exec/cpu-defs.h
>> @@ -105,17 +105,18 @@ typedef struct CPUTLBEntry {
>>          bit 3                      : indicates that the entry is invalid
>>          bit 2..0                   : zero
>>       */
>> -    target_ulong addr_read;
>> -    target_ulong addr_write;
>> -    target_ulong addr_code;
>> -    /* Addend to virtual address to get host address.  IO accesses
>> -       use the corresponding iotlb value.  */
>> -    uintptr_t addend;
>> -    /* padding to get a power of two size */
>> -    uint8_t dummy[(1 << CPU_TLB_ENTRY_BITS) -
>> -                  (sizeof(target_ulong) * 3 +
>> -                   ((-sizeof(target_ulong) * 3) & (sizeof(uintptr_t) - 1)) +
>> -                   sizeof(uintptr_t))];
>> +    union {
>
> The struct CPUTLBEntry can be changed to union CPUTLBEntry directly,
> with no need for the anonymous struct.

Um, no it can't.  That would put all of the members at the same address.

> Which compiler version started implementing anonymous structs?

A long long time ago -- gcc 2 era.

> Or can we just add
>
> 	 __attribute__((__aligned__(1 << CPU_TLB_ENTRY_BITS)))

The structure isn't currently aligned, and it needn't be.  We only need the 
size to be a power of two for the addressing.



r~