On 2017-09-15 23:36, Ralf Ramsauer wrote: > On 15/09/17 18:55, Jan Kiszka wrote: >> On 2017-09-13 16:29, Ralf Ramsauer wrote: >>> When receiving an SGI on a GICv2 system, the receiving CPU expects the >>> caller's CPU ID in the IAR. Jailhouse did not forward the caller's CPU >>> ID, so cells might erroneously assume that the calling CPU is (always) >>> CPU0. >>> >>> GICv3+ doesn't support this feature. >>> >>> Let's fix this in two steps: >>> - expand irqchip_set_pending() by a u16 caller parameter that is only >>> respected in case of SGIs >>> - change the data type of pending_irqs from u16 to u32 to be able to >>> encode the calling interface in the upper bits >>> >>> Signed-off-by: Ralf Ramsauer <[email protected]> >>> --- >>> hypervisor/arch/arm-common/control.c | 2 +- >>> hypervisor/arch/arm-common/gic-v2.c | 4 +++- >>> hypervisor/arch/arm-common/include/asm/irqchip.h | 4 ++-- >>> hypervisor/arch/arm-common/irqchip.c | 22 >>> ++++++++++++++-------- >>> hypervisor/arch/arm-common/ivshmem.c | 2 +- >>> hypervisor/arch/arm/gic-v3.c | 5 ++++- >>> hypervisor/arch/arm/include/asm/percpu.h | 4 +++- >>> hypervisor/arch/arm64/include/asm/percpu.h | 4 +++- >>> 8 files changed, 31 insertions(+), 16 deletions(-) >>> >>> diff --git a/hypervisor/arch/arm-common/control.c >>> b/hypervisor/arch/arm-common/control.c >>> index 4d3abd25..8aa39028 100644 >>> --- a/hypervisor/arch/arm-common/control.c >>> +++ b/hypervisor/arch/arm-common/control.c >>> @@ -188,7 +188,7 @@ bool arch_handle_phys_irq(struct per_cpu *cpu_data, u32 >>> irqn, >>> } >>> >>> cpu_data->stats[JAILHOUSE_CPU_STAT_VMEXITS_VIRQ] += count_event; >>> - irqchip_set_pending(cpu_data, irqn); >>> + irqchip_set_pending(cpu_data, irqn, -1); >>> >>> return false; >>> } >>> diff --git a/hypervisor/arch/arm-common/gic-v2.c >>> b/hypervisor/arch/arm-common/gic-v2.c >>> index 1aec2a2f..7255cd50 100644 >>> --- a/hypervisor/arch/arm-common/gic-v2.c >>> +++ b/hypervisor/arch/arm-common/gic-v2.c >>> @@ -265,7 +265,7 @@ static int gic_send_sgi(struct sgi *sgi) >>> return 0; >>> } >>> >>> -static int gic_inject_irq(struct per_cpu *cpu_data, u16 irq_id) >>> +static int gic_inject_irq(struct per_cpu *cpu_data, u16 irq_id, u16 caller) >>> { >>> int i; >>> int first_free = -1; >>> @@ -298,6 +298,8 @@ static int gic_inject_irq(struct per_cpu *cpu_data, u16 >>> irq_id) >>> if (!is_sgi(irq_id)) { >>> lr |= GICH_LR_HW_BIT; >>> lr |= (u32)irq_id << GICH_LR_PHYS_ID_SHIFT; >>> + } else { >>> + lr |= (caller & 0x7) << GICH_LR_CPUID_SHIFT; >>> } >>> >>> gic_write_lr(first_free, lr); >>> diff --git a/hypervisor/arch/arm-common/include/asm/irqchip.h >>> b/hypervisor/arch/arm-common/include/asm/irqchip.h >>> index 7cf5d704..5a4a3705 100644 >>> --- a/hypervisor/arch/arm-common/include/asm/irqchip.h >>> +++ b/hypervisor/arch/arm-common/include/asm/irqchip.h >>> @@ -48,7 +48,7 @@ struct irqchip { >>> int (*send_sgi)(struct sgi *sgi); >>> u32 (*read_iar_irqn)(void); >>> void (*eoi_irq)(u32 irqn, bool deactivate); >>> - int (*inject_irq)(struct per_cpu *cpu_data, u16 irq_id); >>> + int (*inject_irq)(struct per_cpu *cpu_data, u16 irq_id, u16 caller); >>> void (*enable_maint_irq)(bool enable); >>> bool (*has_pending_irqs)(void); >>> >>> @@ -85,7 +85,7 @@ void irqchip_handle_irq(struct per_cpu *cpu_data); >>> bool irqchip_has_pending_irqs(void); >>> >>> void irqchip_inject_pending(struct per_cpu *cpu_data); >>> -void irqchip_set_pending(struct per_cpu *cpu_data, u16 irq_id); >>> +void irqchip_set_pending(struct per_cpu *cpu_data, u16 irq_id, u16 caller); >>> >>> bool irqchip_irq_in_cell(struct cell *cell, unsigned int irq_id); >>> >>> diff --git a/hypervisor/arch/arm-common/irqchip.c >>> b/hypervisor/arch/arm-common/irqchip.c >>> index f0f84c92..5e8e28ba 100644 >>> --- a/hypervisor/arch/arm-common/irqchip.c >>> +++ b/hypervisor/arch/arm-common/irqchip.c >>> @@ -106,10 +106,11 @@ void gic_handle_sgir_write(struct sgi *sgi) >>> struct per_cpu *cpu_data = this_cpu_data(); >>> unsigned int cpu, target; >>> u64 cluster; >>> + const u16 caller = cpu_data->cpu_id; >>> >>> if (sgi->routing_mode == 2) >>> /* Route to the caller itself */ >>> - irqchip_set_pending(cpu_data, sgi->id); >>> + irqchip_set_pending(cpu_data, sgi->id, caller); >>> else >>> for_each_cpu(cpu, cpu_data->cell->cpu_set) { >>> if (sgi->routing_mode == 1) { >>> @@ -126,7 +127,7 @@ void gic_handle_sgir_write(struct sgi *sgi) >>> continue; >>> } >>> >>> - irqchip_set_pending(per_cpu(cpu), sgi->id); >>> + irqchip_set_pending(per_cpu(cpu), sgi->id, caller); > [1] >>> } >>> } >>> >>> @@ -220,7 +221,7 @@ bool irqchip_has_pending_irqs(void) >>> return irqchip.has_pending_irqs(); >>> } >>> >>> -void irqchip_set_pending(struct per_cpu *cpu_data, u16 irq_id) >>> +void irqchip_set_pending(struct per_cpu *cpu_data, u16 irq_id, u16 caller) >> >> Sender (not "caller") ID should better derived in-place, i.e. in this >> function, than passed down. That prevents that all users of this > How can I derive it in-place? cpu_data doesn't necessarily have to be > the SGI sending CPU. Cf. [1].
All SIG set_pending requests are issued over the sender CPU, also the case you cited above, just look closer. ;) Jan -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux -- You received this message because you are subscribed to the Google Groups "Jailhouse" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
