Re: [Qemu-devel] [PATCH 08/39] provide portable sizeof(long) test

[malc]

On Tue, 12 Oct 2010, Paolo Bonzini wrote:

> On 10/12/2010 04:41 PM, malc wrote:
> > > > >  Gives wrong results on Win64.
> > >  
> > >  Then it's not HOST_LONG_BITS, it's HOST_POINTER_BITS.
> > 
> > Not quite, [s]size_t/ptrdiff_t are 64 bits wide udner Win64, little
> > to do with pointers.
> 
> Before (on Win64): sizeof(long) == 4, HOST_LONG_BITS == 64
> 
> After: sizeof(long) == 4, HOST_LONG_BITS == 32
> 
> If you say HOST_LONG_BITS should be 64, then I say it is poorly named: it
> represents sizeof(void*) * CHAR_BIT and should be named HOST_POINTER_BITS.
> 
> BTW, HOST_LONG_BITS is used mostly for user-mode emulation, which does not
> matter for Win64.  In exec-all.h it is used to second-guess
> sizeof(tcg_target_long), which would be the size of a pointer.  However,
> it is safe to give a conservative value based on sizeof(long).
> 
> Finally, in other places it is definitely used to mean sizeof(long).  Even
> though the softmmu is probably not IL32P64-clean, there you have this:
> 
> #if HOST_LONG_BITS == 32 && TARGET_LONG_BITS == 32
> #define CPU_TLB_ENTRY_BITS 4
> #else
> #define CPU_TLB_ENTRY_BITS 5
> #endif
> 
> typedef struct CPUTLBEntry {
>     /* bit TARGET_LONG_BITS to TARGET_PAGE_BITS : virtual address
>        bit TARGET_PAGE_BITS-1..4  : Nonzero for accesses that should not
>                                     go directly to ram.
>        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.  */
>     unsigned long 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(unsigned long) - 
> 1)) + 
>                    sizeof(unsigned long))];
> } CPUTLBEntry;
> 
> _As things are now_ it is more correct to use sizeof(long), or at
> least more conservative.
> 
> I can certainly change it to sizeof(void*) in v2, even though I don't think
> it's a good idea, but it's your call as a maintainer.
> 

Bottom line: QEMU assumes that long is large enough to hold a pointer,
this assumption is wrong, and your patch is wrong (not because it 
miscalculates size of ABI long) but because of the assumptions current
code has. Whether two wrongs makes right is an open question, currently
Win64 isnot supported in any shape or form, even though Filip Navarra
once did a port[1] which never went into mainline.

[1] http://marc.info/?l=qemu-devel&m=124912692531724&w=2

-- 
mailto:address@hidden