Re: [Qemu-devel] [PATCH 3/5] hw/pxa2xx_lcd.c: drop VMSTATE_UINTTL usage

[Mitsyanko Igor]

On 02/22/2012 04:48 PM, andrzej zaborowski wrote:
> On 22 February 2012 13:26, Mitsyanko Igor<address@hidden>  wrote:
> > On 02/22/2012 03:36 PM, andrzej zaborowski wrote:
> > > Why's uint32_t more correct though?  The purpose of using a named type
> > > across qemu is to mark fields as memory addresses (similar to size_t
> > > being used for sizes, etc.), uint32_t conveys less information -- only
> > > the size.
> >
> > It's obviously more informative, but I thought it's main purpose is to be
> > used with code that could be executed for a different targets (with
> > different address bus width).
>
> In some case for sure, but I believe not in most cases.
>
> > > It's a safe hack, but I don't see the rationale.
> >
> >
> > I don't consider this a hack, we are trying to emulate real hardware, and
> > pxa lcd and dma controllers are intended to work with 32-bit bus. We should
> > not have a possibility to use them with 64-bit targets.
> >
> >
> > > If it's because VMSTATE_UINT32 requires that specific type than a less
> > > ugly hack would be to make a pxa specific memory address type.
> >
> > Introducing new type doesn't look pretty to me,
>
> Why?

Peter already answered, this fields should be exactly 32-bit wide 
(hardware is implemented this way) and we already have a type that is 
exactly 32-bit wide. Implementing each device without any assumptions 
about other parts of emulated system seems like a right approach to me.
Doing something like typedef uint32_t pxalcd_phys_addr_t is fun, but 
then we could end up introducing typedefs like this for every device in hw/.
Also, currently we can't save custom types in VMSTATE.

> > maybe just rename variables
> > to source_addr, dest_addr e.t.c?
>
> Wouldn't it be analogous to changing pointer typed variables to void *
> and adding the actual type in their names?  The result is that at
> language level they'll all be the same type even though they are not.
>
> (or changing le32 and be32 to uint32 in Linux)

Yes, but you got to admit they would be more informative for human :)


--
Mitsyanko Igor
ASWG, Moscow R&D center, Samsung Electronics
email: address@hidden