Re: [Qemu-devel] [PATCH] arm highbank: force ramsize to INT_MAX when loading

["Andreas Färber" <afaerber@suse.de>]

Am 09.03.2012 19:22, schrieb Alexander Graf:
> 
> On 09.03.2012, at 17:40, Mark Langsdorf wrote:
> 
>> On 03/09/2012 10:13 AM, Peter Maydell wrote:
>>> On 9 March 2012 15:57, Mark Langsdorf <mark.langsdorf@calxeda.com> wrote:
>>>> Since the ram_size field of arm_boot_info is only an int, don't set
>>>> that field to more than INT_MAX. Signed vs unsigned comparison
>>>> overruns are possible otherwise.
>>>
>>> Can't we just make arm_boot_info.ram_size a uint32_t (propagating through
>>> signedness fixes as required) ?
>>>
>>> Actually it should probably be a target_phys_addr_t, thinking ahead
>>> to adding LPAE support.
>>
>> It really should be a size_t, per the upthread discussion with Andreas
>> Faerber.
> 
> No, Andreas is wrong. Host data types have nothing to do in target ram fiddling code. The type you're searching for is "the size of physical address space the guest can handle" and that's target_phys_addr_t. Period.

That is exactly where we disagree: It's not "target ram fiddling code".

It's the host loading binaries from disk to host RAM.

The Memory API constructs for wiring it up in guest RAM are outside the
scope of this discussion.

If we can't load a 4 GB file into our host RAM, there's no point
bloating our APIs with unreadable types as arguments such as
"target_phys_addr_t max_size" just because a 64-bit guest on a 32-bit
host has a larger virtual address space.

It's not an address, so if we end up needing a special type it should
certainly not be any ...addr_t type. Typedef target_phys_size_t if you
see a need but keep our APIs clean please. Alex, you say you don't care
about infrastructure, well this IS infrastructure and I care about its
API and usability. :)

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg