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

[Avi Kivity <avi@redhat.com>]

On 08/24/2011 02:18 PM, Avi Kivity wrote:
> On 08/24/2011 01:53 PM, Peter Maydell wrote:
>> >  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.
>>
>
> I think you're right.  This also allows eventual removal of system_io 
> on anything non-x86.
>
> So a replacement would look like:
>
> (before)
>
> -static void pc_init_isa(ram_addr_t ram_size,
> +static void pc_init_isa(MemoryRegion *address_space_mem,
> +                        MemoryRegion *address_space_io,
> +                        ram_addr_t ram_size,
>                          const char *boot_device,
>                          const char *kernel_filename,
>                          const char *kernel_cmdline,
> @@ -259,15 +265,17 @@ static void pc_init_isa(ram_addr_t ram_size,
>  {
>      if (cpu_model == NULL)
>          cpu_model = "486";
> -    pc_init1(get_system_memory(),
> -             get_system_io(),
> +    pc_init1(address_space_mem,
> +             address_space_io,
>               ram_size, boot_device,
>               kernel_filename, kernel_cmdline,
>               initrd_filename, cpu_model, 0, 1);
>  }
>
> (after)
> @@ -259,15 +265,17 @@ static void pc_init_isa(ram_addr_t ram_size,
>  {
> +    MemoryRegion *address_space_mem, *address_space_io;
> +
> +    setup_system_memory(&address_space_mem,&address_space_io);
>      if (cpu_model == NULL)
>          cpu_model = "486";
> -    pc_init1(get_system_memory(),
> -             get_system_io(),
> +    pc_init1(address_space_mem,
> +             address_space_io,
>               ram_size, boot_device,
>               kernel_filename, kernel_cmdline,
>               initrd_filename, cpu_model, 0, 1);
>  }
>
>
> Later on, we'd refine the setup_system_memory() calls, for example not 
> to create the io space on non-x86.
>
> A possible complication is whether anything currently uses 
> system_memory before ->init is called.  Anyone know?
>

Okay, it looks like I can just drop the patch and call 
get_system_memory() in the various _init functions.  This avoids the 
complication.  We can tidy up get_system_memory() later.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.