Re: [Qemu-devel] [PATCH 06/24] QEMUMachine: pass address space to machine init function

[Peter Maydell]

On 24 August 2011 11:46, Avi Kivity <address@hidden> wrote:
> On 08/24/2011 01:32 PM, Peter Maydell wrote:
>>
>> On 24 August 2011 11:11, Avi Kivity<address@hidden>  wrote:
>> >  Avoids get_system_memory() everywhere.
>>
>> >  -    machine->init(ram_size, boot_devices,
>> >  +    machine->init(get_system_memory(), get_system_io(), ram_size,
>> > boot_devices,
>> >                     kernel_filename, kernel_cmdline, initrd_filename,
>> > cpu_model);
>>
>> I think this is conceptually wrong. The system memory/IO address spaces
>> are not configurable properties of the machine model (in the way that
>> the other arguments to machine->init() are), they're purely an internal
>> implementation detail of it. (In fact for many of the machines we support
>> there isn't even any such thing as the system IO space...)
>
> I agree with all you say, but does it make the patch incorrect?

My assertion is that patches which are conceptually wrong are
also incorrect :-)

> The purpose here is to allow removal of get_system_memory() from
> the general code base.

The right way to remove get_system_memory() from the general code base
is to actually model things correctly, for instance by having machine
models create a container memory region into which they insert the
memory regions for all the devices which currently use sysbus_mmio_map
to map themselves, and then pass the container memory region to the
"master" end of the bus, ie the CPU.

-- PMM