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? r~
