[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditiona
From: |
Avi Kivity |
Subject: |
Re: [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally |
Date: |
Sun, 07 Oct 2012 12:25:50 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120911 Thunderbird/15.0.1 |
On 10/04/2012 08:15 PM, Stefan Weil wrote:
> Am 04.10.2012 12:36, schrieb Avi Kivity:
>> The hassle and compile time overhead of maintaining both 32-bit and
>> 64-bit
>> capable source isn't worth the tiny performance advantage which is
>> seen on
>> a minority of configurations. Switch to compiling libhw only once, with
>> target_phys_addr_t unconditionally typedefed to uint64_t.
>>
>> Signed-off-by: Avi Kivity <address@hidden>
>
> Hi,
>
> I noticed that you replaced target_phys_addr_t by uint64_t in two lines.
In those two lines, int64_t is more correct than t_p_a_t. Explanation
below.
> Are there plans to replace target_phys_addr_t everywhere?
Yes, by hw_addr (or hwaddr, or phys, or ...).
> Should new code use uint64_t, or should it continue to use
> target_phys_addr_t?
target_phys_addr_t.
>
> Using target_phys_addr_t might make support for 128 bit in some years
> easier
> because it allows identifying critical code,
Agree.
> although I think it will be difficult to avoid wrong use of either
> target_phys_addr_t
> or uint64_t as long as both are the same size.
Some languages allow enforcing this, but C doesn't.
>
>
>> -#if TARGET_PHYS_ADDR_BITS > 32
>> - return low | ((target_phys_addr_t)high << 32);
>> -#else
>> - return low;
>> -#endif
>> + return low | ((uint64_t)high << 32);
>>
Shifting by 32 is not defined for types that are 32 bits or narrower.
On x86, for example, a shift by 32 of a 32-bit quantity is the identity
operation (where mathematically you would expect the result to be 0, not
the first argument). Since the context does not guarantee that
target_phys_addr_t is wider than 32 bits, I cast it to a known-wide
type, then (implicitly) cast it back to target_phys_addr_t.
>
>> - *raddrp |= (target_phys_addr_t)(tlb->RPN & 0xF) << 32;
>> + *raddrp |= (uint64_t)(tlb->RPN & 0xF) << 32;
>
Same applies here, of course.
Since target_phys_addr_t is 64 bits, it would have worked "by accident",
but if we'd have switched back to 32 bits in the future (unlikely but
possible) then I would have introduced a bug here.
--
error compiling committee.c: too many arguments to function
- [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally, Avi Kivity, 2012/10/04
- Re: [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally, Edgar E. Iglesias, 2012/10/04
- Re: [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally, Max Filippov, 2012/10/04
- Re: [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally, Anthony Liguori, 2012/10/04
- Re: [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally, Blue Swirl, 2012/10/04
- Re: [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally, Michael Walle, 2012/10/04
- Re: [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally, Stefan Weil, 2012/10/04
- Re: [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally,
Avi Kivity <=
- Re: [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally, Anthony Liguori, 2012/10/04
- Re: [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally, Peter Maydell, 2012/10/05