qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-arm] [Qemu-devel] [PATCH-for-4.2 v8 3/9] hw/acpi: Add ACPI Gen


From: Peter Maydell
Subject: Re: [Qemu-arm] [Qemu-devel] [PATCH-for-4.2 v8 3/9] hw/acpi: Add ACPI Generic Event Device Support
Date: Mon, 5 Aug 2019 16:54:06 +0100

On Mon, 5 Aug 2019 at 16:47, Igor Mammedov <address@hidden> wrote:
> On Mon, 5 Aug 2019 14:42:38 +0100
> Peter Maydell <address@hidden> wrote:
> > This is definitely a bad idea -- devices should not add their
> > own memory regions to the system memory MR. They should
> > expose their MRs (by being a sysbus-device) and let the board
> > code do the wiring up of the MRs into the right memory space
> > at the right address.
>
> it's not the only place in GED that is trying to add to system address
> space, optionally if called acpi_memory_hotplug_init() will do the same,
> then later we could add cpu hotplug memory region over there.
>
> Perhaps we could use bus-less device plug code path,
> in that case memory_region_init_io()/qdev_init_gpio_out_named()
> should be moved to ged_initfn() and mapping part into specialized helper
> (similar to pc_dimm_plug() ) that's called by board (from 
> virt_machine_device_plug_cb)
> callback during completing device realize stage, it would be something like:
>
> virt.c:
>    virt_machine_device_plug_cb()
>       if dev == GED_TYPE
>         machine_ged_plug_helper(system_memory)
>
> generic_event_device.c:
>    machine_ged_plug_helper(as, irq) // similar to sysbus_mmio_map() but ged 
> specialized
>       connect_irq()
>       memory_region_add_subregion(as, ged->ged_base, &ged->io)
>       if ged->memory-hotplug-support
>           memory_region_add_subregion(as, ged->memhp_base , 
> &ged->memhp_state.memhp_io)

I don't really understand why we want to do this complicated
thing, rather than just doing the normal thing for devices
that live at particular addresses, ie make them sysbus devices
and have the board map their memory regions in the right place.

> in this case addresses could be normally hard-codded in board code if device 
> is not optional
> (as in patch 6/9: create_acpi_ged() )
> or potentially they could come from CLI as -device parameters
> (/me thinking about building blocks that allow to create machine from config)

I don't think we want to do this. The user should not have to
know anything about addresses or have to specify them on the
command line. (This is why you can't create sysbus devices
with -device except for some odd special cases to do with passthrough
of hardware.)

> sysbus device might be fine as shortcut if we are thinking about
> only creating device during machine_init (although I have a reservations 
> towards
> sysbus interface (ex: caller of sysbus_mmio_map() has no clue when mapping 
> N-th
> region at some address)).

Not sure entirely what you have in mind here? (though yes, the
sysbus device API has its awkward corners, some of which are
just down to how old it is.)

thanks
-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]