Re: [Qemu-devel] [PATCH] s390x: initialize virtio dev region

[Alexander Graf <agraf@suse.de>]

On 11/10/2011 03:19 AM, Peter Maydell wrote:
> On 10 November 2011 01:45, Alexander Graf<agraf@suse.de>  wrote:
>> On 10.11.2011, at 02:36, Peter Maydell wrote:
>>> This looks a bit fishy -- cpu_physical_memory_map() takes a
>>> target_phys_addr_t but you're passing it a ram_addr_t.
>> Meh. Always those types ... :)
> In the simple case ("ram starts at 0, not multiply mapped, guest
> and host pointer widths the same") target_phys_addr_t's and
> ram_addr_t's are the same, but this isn't necessarily so; indeed
> one ram_addr_t can be mapped to multiple target_phys_addr_t's
> in some system models. So the two things aren't trivially
> interconvertible. (As it happens I think in this case you're
> actually just doing physical address calculations using the wrong
> type...)

This is the machine init function. The s390 virtio machine's ram layout 
is defined to be exactly as I posted in the previous post. So there 
won't be any ram starts at != 0 or multiple mappings :).

>> On 10.11.2011, at 02:36, Peter Maydell wrote:
>>> Also isn't this second-guessing the ram_addr_t of the region allocated
>>> inside memory_region_init_ram() ?
>> Not sure I understand? On s390-virtio we have to following ram layout:
>>
>> [ ----- RAM ---- | --- virtio device structs --- ]
>>
>> The size of the latter is added to my_ram_size by
>> s390_virtio_bus_init. All I'm trying to do is make sure that this
>> latter part of RAM is always 0, while the first part can still be
>> dynamically allocated.
> That comment was written on the assumption that you were actually
> trying to manipulate a ram_addr_t in your variable of type
> ram_addr_t; to expand on it a bit:
>
> I think that conceptually the only way to get the ram_addr_t for a
> bit of RAM should be that you got it back from qemu_ram_alloc()
> (or that you did the obvious arithmetic to get a ram_addr_t within
> a block given the start of one). As it happens qemu_ram_alloc()
> starts ram_addr_t's at 0 and works up with no gaps, but that's
> an implementation detail. (Also if anybody ever did something that
> meant there was another qemu_ram_alloc() call before the one done
> inside that memory_region_init_ram() then things would break.)
>
> In summary, either:
> (1) you need to ask the memory region for its ram_addr_t
> [there doesn't seem to be an API for this at the moment],
> do arithmetic on ram_addr_t's and use qemu_get_ram_ptr() to
> get a host pointer corresponding to that ram_addr_t
> or (2) you need to do your arithmetic in target_phys_addr_t's
> (and then you can say your RAM starts at physaddr 0, which
> is guaranteed, rather than at ram_addr_t 0, which isn't)
> and use cpu_physical_memory_map/unmap().

I still don't get it. What I want is:

   for (i = ram_size; i < my_ram_size; i++)
     stb_phys(env, i, 0);

cpu_physical_memory_map gives me a pointer to the memory region between 
ram_size and my_ram_size. I memset(0) it. I don't see how I could 
possibly have different offsets anywhere. If cpu_physical_memory_map 
would take anything different than physical address, a lot of 
assumptions in QEMU code would break.


Alex