| Message ID | 20240220192625.17944-1-philmd@linaro.org |
|---|---|
| Headers | show |
| Series | hw: Replace cpu_interrupt() calls by qemu_irq(QDev GPIO) | expand |
On 20/02/2024 19:26, Philippe Mathieu-Daudé wrote: > Hi, > > cpu_interrupt() doesn't scale well with heterogenous machines > because its mask is target specific (defined in target/ARCH/cpu.h). > > While it is (often...) used by target-specific hw to notify cpu, > there is no restriction to use such target-specific hw in a > heterogeneous setup, where it'd still target the same kind of > target cpus. > > The Alpha Typhoon HW is unlikely to be used heterogeneously, > but it is the simplest case I found to illustrate how I plan > to remove cpu_interrupt() calls from hw/: use the QDev GPIO API. > > Does that sound good enough? I think the basic mechanism of setting/resetting the interrupt using qdev GPIOs should work, but I wonder if there isn't a better way of defining them to avoid more #ifdeffery. Is it possible to come up with some declarative syntax in either CPUClass or CPUClass::sysemu_ops that would avoid the developer manually having to call qdev_init_gpio_in_named() manually during CPU init() and/or create the various _irq() callback functions by hand? > Thanks, > > Phil. > > Philippe Mathieu-Daudé (2): > target/alpha: Expose TMR and SMP IRQ lines via QDev > hw/alpha/typhoon: Set CPU IRQs using QDev API > > hw/alpha/typhoon.c | 15 +++++++++------ > target/alpha/cpu.c | 30 ++++++++++++++++++++++++++++++ > 2 files changed, 39 insertions(+), 6 deletions(-) ATB, Mark.
On Tue, 20 Feb 2024 at 19:28, Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > cpu_interrupt() doesn't scale well with heterogenous machines > because its mask is target specific (defined in target/ARCH/cpu.h). > > While it is (often...) used by target-specific hw to notify cpu, > there is no restriction to use such target-specific hw in a > heterogeneous setup, where it'd still target the same kind of > target cpus. When would this target-specific hw call cpu_interrupt() on a CPU which is not of that target architecture? For instance, in the typhoon code you change in patch 2, the CPUState argument to cpu_interrupt() and cpu_reset_interrupt() is always that of one of the 4 CPUs pointed to by the TyphoonState struct, which are guaranteed to be Alpha CPUs. In some hypothetical world where this machine type had an Arm board-management CPU on it as well, that CPU would not be one of the ones pointed to by the TyphoonState struct, which would still all be Alpha. I can see that you would run into slight awkwardness on a device that wanted to do this same kind of thing on two CPU types at once, just because the defines are in cpu.h and you can't #include them both at once. But are we going to in practice have those on a heterogenous machine? > The Alpha Typhoon HW is unlikely to be used heterogeneously, > but it is the simplest case I found to illustrate how I plan > to remove cpu_interrupt() calls from hw/: use the QDev GPIO API. I do generally think that it's probably nicer design to have cpu_interrupt() be internal to target/foo (it's how arm does it, except for a handful of calls in obsolete SoC models). But I think it might be helpful if you could give a description of what the immediate issue is that means that we need to do this cleanup to progress with the heterogenous machine work (and to what extent we can leave existing code in old non-heterogenous board and device models alone). thanks -- PMM