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

[andrzej zaborowski]

On 22 February 2012 13:00, Peter Maydell <address@hidden> wrote:
> On 22 February 2012 11:36, andrzej zaborowski <address@hidden> wrote:
>> On 22 February 2012 11:15, Igor Mitsyanko <address@hidden> wrote:
>>> Convert three variables in DMAChannel state from type target_phys_addr_t to 
>>> uint32_t,
>>> use VMSTATE_UINT32 instead of VMSTATE_UINTTL for these variables.
>>> We can do it safely because:
>>> 1) pxa2xx has 32-bit physical address;
>>> 2) rest of the code in this file treats these variables as uint32_t;
>>
>> 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 a safe hack, but I don't see the rationale.
>
> Because we might change target_phys_addr_t to 64 bits globally
> some day (it's certainly been mooted) and that shouldn't suddenly
> change the register width and certainly shouldn't change the
> migration state.
>
> Basically VMSTATE_UINTTL in hw/ is always a bug, because its
> behaviour depends on the size of target_ulong, which is a
> property of the CPU, which is a completely separate device.

I'm not really discussing that, my question is unrelated to
migration/savevm because the patch touches parts that shouldn't be
concerned with migration.  If a particular function (like migration)
needs the type converted to something then that's why C has type
conversions.  A type conversion that compiles to no code is still a
type conversion.

>
>> 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.
>
> Yuck.

Or a general 32-bit memory address type, but as I said uint32_t
conveys less information.  Do you disagree?

Cheers