Re: [PATCH 08/21] hw/alpha/dp264: Create the RAM in the board

[Philippe Mathieu-Daudé]

On 10/21/19 7:27 PM, Richard Henderson wrote:
> On 10/20/19 3:56 PM, Philippe Mathieu-Daudé wrote:
> > @@ -73,8 +74,16 @@ static void clipper_init(MachineState *machine)
> >       cpus[0]->env.trap_arg1 = 0;
> >       cpus[0]->env.trap_arg2 = smp_cpus;
> >   
> > +    /*
> > +     * Main memory region, 0x00.0000.0000.  Real hardware supports 32GB,
> > +     * but the address space hole reserved at this point is 8TB.
> > +     */
> > +    memory_region_allocate_system_memory(&ram_region, NULL, "ram",
> > +                                         ram_size);
> > +    memory_region_add_subregion(get_system_memory(), 0, &ram_region);
>
> The thing that I don't like about this is that ...
>
> > @@ -849,12 +848,6 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
> >   
> >       *p_rtc_irq = qemu_allocate_irq(typhoon_set_timer_irq, s, 0);
> >   
> > -    /* Main memory region, 0x00.0000.0000.  Real hardware supports 32GB,
> > -       but the address space hole reserved at this point is 8TB.  */
> > -    memory_region_allocate_system_memory(&s->ram_region, NULL, "ram",
> > -                                         ram_size);
> > -    memory_region_add_subregion(addr_space, 0, &s->ram_region);
> > -
> >       /* TIGbus, 0x801.0000.0000, 1GB.  */
> >       /* ??? The TIGbus is used for delivering interrupts, and access to
> >          the flash ROM.  I'm not sure that we need to implement it at all.  */
>
> ... previously, it was clear that Typhoon owns the entire address space layout.
>   With this moved to dp264.c, the "address space hole" comment is out of context
> and makes no sense.
>
> Would it be too weird to pass in a MemoryRegion* to typhoon_init and leave the
> memory_region_add_subregion() call where it is?

Yes, this will also better match the hardware.
I have a WiP series that first QOM'ify the CY82C693UB southbridge then 
the Typhoon northbridge and pass the DRAM MemoryRegion* as a link, so in 
the non-QOM form your suggestion is the same. I'll do your change.