On 28.09.2017 22:36, David Hildenbrand wrote:
> External interrupts are currently all handled like floating external
> interrupts, they are queued. Let's prepare for a split of floating
> and local interrupts by turning INTERRUPT_EXT into a mask.
>
> While we can have various floating external interrupts of one kind, there
> is usually only one (or a fixed number) of the local external interrupts.
>
> So turn INTERRUPT_EXT into a mask and properly indicate the kind of
> external interrupt. Floating interrupts will have to moved out of
> one CPU instance later once we have SMP support.
>
> The only floating external interrupts used right now are SERVICE
> interrupts, so let's use that name. Following patches will clean up
> SERVICE interrupt injection.
>
> This get's rid of the ugly special handling for cpu timer and clock
> comparator interrupts. And we really only store the parameters as
> defined by the PoP.
>
> Reviewed-by: Richard Henderson <richard.hender...@linaro.org>
> Signed-off-by: David Hildenbrand <da...@redhat.com>
> ---
> target/s390x/cpu.h | 13 ++++++----
> target/s390x/excp_helper.c | 63
> +++++++++++++++++++++++-----------------------
> target/s390x/helper.c | 12 ++-------
> target/s390x/internal.h | 2 ++
> target/s390x/interrupt.c | 18 ++++++++++++-
> 5 files changed, 61 insertions(+), 47 deletions(-)
[...]
> diff --git a/target/s390x/interrupt.c b/target/s390x/interrupt.c
> index 058e219fe5..b9c30f86d7 100644
> --- a/target/s390x/interrupt.c
> +++ b/target/s390x/interrupt.c
> @@ -71,7 +71,23 @@ void cpu_inject_ext(S390CPU *cpu, uint32_t code, uint32_t
> param,
> env->ext_queue[env->ext_index].param = param;
> env->ext_queue[env->ext_index].param64 = param64;
>
> - env->pending_int |= INTERRUPT_EXT;
> + env->pending_int |= INTERRUPT_EXT_SERVICE;
> + cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HARD);
> +}
> +
> +void cpu_inject_clock_comparator(S390CPU *cpu)
> +{
> + CPUS390XState *env = &cpu->env;
> +
> + env->pending_int |= INTERRUPT_EXT_CLOCK_COMPARATOR;
> + cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HARD);
> +}
> +
> +void cpu_inject_cpu_timer(S390CPU *cpu)
> +{
> + CPUS390XState *env = &cpu->env;
> +
> + env->pending_int |= INTERRUPT_EXT_CPU_TIMER;
> cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HARD);
> }
The last two function look similar enough that you could merge the
functions, e.g.:
void cpu_inject_ext_pending_bit(S390CPU *cpu, int bit)
{
CPUS390XState *env = &cpu->env;
env->pending_int |= bit;
cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HARD);
}
?
Apart from that, the patch looks fine to me.
Thomas
