Am 26.10.2010 um 21:26 schrieb Blue Swirl:
On Tue, Oct 26, 2010 at 7:14 PM, Andreas Färber <andreas.faerber@web.de
wrote: Am 26.10.2010 um 20:53 schrieb Blue Swirl:
On Mon, Oct 25, 2010 at 9:08 PM, Andreas Färber <andreas.faerber@web.de
wrote:
Am 20.10.2010 um 21:04 schrieb Blue Swirl:
On Wed, Oct 20, 2010 at 6:36 PM, Andreas Färber <andreas.faerber@web.de
wrote:
Am 20.10.2010 um 18:19 schrieb Blue Swirl:
> On Sun, Oct 17, 2010 at 11:18 PM, Andreas Färber > andreas.faerber@web.de > wrote: >> >> On ppc64, cell size is 32 bits but pointers are 64-bit. Thus, >> direct >> casts >> result in warnings, treated as errors. >> >> Use [u]intptr_t cast or cell2pointer and pointer2cell macros as >> necessary. > >> -int uart_init(uint64_t port, unsigned long speed) >> +int uart_init(uintptr_t port, unsigned long speed) > > This is not correct. The physical addresses on Sparc32 are > really 36 > bits wide, so uintptr_t is not wide enough.
Okay, then I'll revert this one. The patch may be mechanical to _review_ but it's pretty invasive, so I'd rather have some parts of it committed before running into larger merge conflicts.
> For that, we'd need a > physical address type. We could reuse for example QEMU name, > target_phys_addr_t.
Sounds good. Should we go ahead with my patch first and do the uint64_t -> target_phys_addr_t change as a follow-up patch? Or should I strip all changes to drivers/ from my patch?
The cell2pointer etc. changes look OK.
Thanks, applied in r922, with one additional (uintptr_t) -> cell2pointer() fix in mac-parts.c that I forgot to mention.
Attached is what I have not applied from the original patch. I'm thinking we should better avoid the native-bitwidth-equals-host-bitwidth path on ppc64 than fixing the symptoms of using it. For the rest, target_phys_addr_t would seem a nice solution. Additionally, the double-dereference of "myself" and the trampoline issue still need proper fixes.
In some cases, instead of casts, the structure fields (in adb_kbd.c: dev->state) should be widened to uintptr_t or pointer.
I was rather thinking of phys_addr_t.
That should be used for physical addresses. But if the address is "virtual" (for example a pointer to a structure, like in the adb_kbd.c case), then a pointer type would be better. There's in those cases an assumption that the addressed data can be accessed via pointer dereference, so CPU word length is enough.
In other places the cast just extends the address (from 32 to 64 bits), but the upper bits have been lost already earlier. Then the structures should be changed (to target_phys_addr_t) to contain a full physical address.
You're right, I mixed it up with cuda where phys_addr_t is necessary for ->base.
Only QEMU needs to distinguish between host and target though, so I don't think target_ makes sense here. Attached FYI.
Andreas