[PATCH 09/22] arm: Remove cpuid from pending_irq

[Jan Kiszka]

Was always set to 0.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---
hypervisor/arch/arm/gic-v2.c | 2 --
hypervisor/arch/arm/include/asm/irqchip.h | 4 ----
hypervisor/arch/arm/irqchip.c | 4 +---
3 files changed, 1 insertion(+), 9 deletions(-)

diff --git a/hypervisor/arch/arm/gic-v2.c b/hypervisor/arch/arm/gic-v2.c
index 9268191..88024b4 100644
--- a/hypervisor/arch/arm/gic-v2.c
+++ b/hypervisor/arch/arm/gic-v2.c
@@ -268,8 +268,6 @@ static int gic_inject_irq(struct per_cpu *cpu_data, struct pending_irq *irq)
if (!is_sgi(irq->virt_id)) {
lr |= GICH_LR_HW_BIT;
lr |= irq->type.irq << GICH_LR_PHYS_ID_SHIFT;
- } else {
- lr |= irq->type.sgi.cpuid << GICH_LR_CPUID_SHIFT;
}

gic_write_lr(first_free, lr);
diff --git a/hypervisor/arch/arm/include/asm/irqchip.h b/hypervisor/arch/arm/include/asm/irqchip.h
index a6d2a0c..6d94c19 100644
--- a/hypervisor/arch/arm/include/asm/irqchip.h
+++ b/hypervisor/arch/arm/include/asm/irqchip.h
@@ -67,10 +67,6 @@ struct pending_irq {
union {
/* Physical id, when hw is 1 */
u16 irq;
- struct {
- /* GICv2 needs cpuid for SGIs */
- u16 cpuid : 15;
- } sgi __attribute__((packed));
} type;

struct pending_irq *next;
diff --git a/hypervisor/arch/arm/irqchip.c b/hypervisor/arch/arm/irqchip.c
index 72c1364..d2dc445 100644
--- a/hypervisor/arch/arm/irqchip.c
+++ b/hypervisor/arch/arm/irqchip.c
@@ -135,9 +135,7 @@ int irqchip_set_pending(struct per_cpu *cpu_data, u32 irq_id, bool try_inject)

pending.virt_id = irq_id;

- if (is_sgi(irq_id)) {
- pending.type.sgi.cpuid = 0;
- } else {
+ if (!is_sgi(irq_id)) {
pending.type.irq = irq_id;
}

--
2.1.4

[Jan Kiszka]

Always identical to virt_id.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/arm/gic-v2.c | 2 +-
hypervisor/arch/arm/gic-v3.c | 2 +-
hypervisor/arch/arm/include/asm/irqchip.h | 5 -----
hypervisor/arch/arm/irqchip.c | 4 ----
4 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/hypervisor/arch/arm/gic-v2.c b/hypervisor/arch/arm/gic-v2.c
index 88024b4..ce5a61c 100644
--- a/hypervisor/arch/arm/gic-v2.c
+++ b/hypervisor/arch/arm/gic-v2.c
@@ -267,7 +267,7 @@ static int gic_inject_irq(struct per_cpu *cpu_data, struct pending_irq *irq)

if (!is_sgi(irq->virt_id)) {
lr |= GICH_LR_HW_BIT;

- lr |= irq->type.irq << GICH_LR_PHYS_ID_SHIFT;
+ lr |= irq->virt_id << GICH_LR_PHYS_ID_SHIFT;
}

gic_write_lr(first_free, lr);
diff --git a/hypervisor/arch/arm/gic-v3.c b/hypervisor/arch/arm/gic-v3.c
index 710813c..6beab34 100644
--- a/hypervisor/arch/arm/gic-v3.c
+++ b/hypervisor/arch/arm/gic-v3.c
@@ -389,7 +389,7 @@ static int gic_inject_irq(struct per_cpu *cpu_data, struct pending_irq *irq)
lr |= ICH_LR_PENDING;
if (!is_sgi(irq->virt_id)) {
lr |= ICH_LR_HW_BIT;
- lr |= (u64)irq->type.irq << ICH_LR_PHYS_ID_SHIFT;
+ lr |= (u64)irq->virt_id << ICH_LR_PHYS_ID_SHIFT;
}

gic_write_lr(free_lr, lr);
diff --git a/hypervisor/arch/arm/include/asm/irqchip.h b/hypervisor/arch/arm/include/asm/irqchip.h
index 6d94c19..65d4d4f 100644
--- a/hypervisor/arch/arm/include/asm/irqchip.h
+++ b/hypervisor/arch/arm/include/asm/irqchip.h
@@ -64,11 +64,6 @@ struct irqchip_ops {
struct pending_irq {
u32 virt_id;

- union {
- /* Physical id, when hw is 1 */
- u16 irq;
- } type;
-
struct pending_irq *next;
struct pending_irq *prev;
} __attribute__((packed));
diff --git a/hypervisor/arch/arm/irqchip.c b/hypervisor/arch/arm/irqchip.c
index d2dc445..a4e2d99 100644
--- a/hypervisor/arch/arm/irqchip.c
+++ b/hypervisor/arch/arm/irqchip.c
@@ -135,10 +135,6 @@ int irqchip_set_pending(struct per_cpu *cpu_data, u32 irq_id, bool try_inject)

pending.virt_id = irq_id;

- if (!is_sgi(irq_id)) {
- pending.type.irq = irq_id;
- }
-
if (try_inject && irqchip.inject_irq(cpu_data, &pending) == 0)
return 0;

--
2.1.4

[Jan Kiszka]

Likely, we will never support alternative irqchips to the GIC (only
cascaded ones). So this copying-over is not required.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---
hypervisor/arch/arm/gic-v2.c | 2 +-
hypervisor/arch/arm/gic-v3.c | 2 +-

hypervisor/arch/arm/irqchip.c | 7 ++-----
3 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/hypervisor/arch/arm/gic-v2.c b/hypervisor/arch/arm/gic-v2.c
index 8f5fc0b..ff0036b 100644
--- a/hypervisor/arch/arm/gic-v2.c
+++ b/hypervisor/arch/arm/gic-v2.c
@@ -293,7 +293,7 @@ unsigned int irqchip_mmio_count_regions(struct cell *cell)
return 1;
}

-struct irqchip_ops gic_irqchip = {
+struct irqchip_ops irqchip = {
.init = gic_init,
.cpu_init = gic_cpu_init,
.cpu_reset = gic_cpu_reset,
diff --git a/hypervisor/arch/arm/gic-v3.c b/hypervisor/arch/arm/gic-v3.c
index 4ab03ad..b422200 100644
--- a/hypervisor/arch/arm/gic-v3.c
+++ b/hypervisor/arch/arm/gic-v3.c
@@ -410,7 +410,7 @@ unsigned int irqchip_mmio_count_regions(struct cell *cell)
return 2;
}

-struct irqchip_ops gic_irqchip = {
+struct irqchip_ops irqchip = {
.init = gic_init,
.cpu_init = gic_cpu_init,
.cpu_reset = gic_cpu_reset,
diff --git a/hypervisor/arch/arm/irqchip.c b/hypervisor/arch/arm/irqchip.c
index 26b3026..0b1193c 100644
--- a/hypervisor/arch/arm/irqchip.c
+++ b/hypervisor/arch/arm/irqchip.c
@@ -24,6 +24,8 @@
/* AMBA's biosfood */
#define AMBA_DEVICE 0xb105f00d

+extern struct irqchip_ops irqchip;
+
void *gicd_base;
unsigned long gicd_size;

@@ -32,7 +34,6 @@ unsigned long gicd_size;
* per-cpu setup, which means that a bool must be set by the master CPU
*/
static bool irqchip_is_init;
-static struct irqchip_ops irqchip;

bool spi_in_cell(struct cell *cell, unsigned int spi)
{
@@ -189,9 +190,6 @@ void irqchip_root_cell_shrink(struct cell *cell)
root_cell.arch.spis &= ~(cell->arch.spis);
}

-/* Only the GIC is implemented */
-extern struct irqchip_ops gic_irqchip;
-
int irqchip_init(void)
{
int i, err;
@@ -222,7 +220,6 @@ int irqchip_init(void)
case 0x2:
case 0x3:
case 0x4:
- memcpy(&irqchip, &gic_irqchip, sizeof(struct irqchip_ops));
break;
default:
goto err_no_distributor;
--
2.1.4

[Jan Kiszka]

We do not support interrupt priorities so far, and we may have to model
them differently into queues once we do. Remove the de facto unused
field.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/arm/include/asm/irqchip.h | 1 -
hypervisor/arch/arm/irqchip.c | 2 --
2 files changed, 3 deletions(-)

diff --git a/hypervisor/arch/arm/include/asm/irqchip.h b/hypervisor/arch/arm/include/asm/irqchip.h
index 1457fd7..02da6bb 100644
--- a/hypervisor/arch/arm/include/asm/irqchip.h
+++ b/hypervisor/arch/arm/include/asm/irqchip.h
@@ -64,7 +64,6 @@ struct irqchip_ops {
struct pending_irq {
u32 virt_id;

- u8 priority;
u8 hw;
union {

/* Physical id, when hw is 1 */

diff --git a/hypervisor/arch/arm/irqchip.c b/hypervisor/arch/arm/irqchip.c
index 7a6d4ca..a80577f 100644
--- a/hypervisor/arch/arm/irqchip.c
+++ b/hypervisor/arch/arm/irqchip.c
@@ -134,8 +134,6 @@ int irqchip_set_pending(struct per_cpu *cpu_data, u32 irq_id, bool try_inject)
struct pending_irq pending;

pending.virt_id = irq_id;
- /* Priority must be less than ICC_PMR */
- pending.priority = 0;

if (is_sgi(irq_id)) {
pending.hw = 0;
--
2.1.4

[Jan Kiszka]

This is my stack of patches that piled up while walking through the
interrupt injection logic of arm (and soon also arm64). Besides a lot of
more or less minor cleanups, it primarily reworks the software queue for
pending interrupts into a ring and fixes setup, handling and protection
of the maintenance interrupt.

One open issue, though still a minor one: We should probably bring all
distributor registers related to hypervisor SGIs and the maintenance
interrupt into defined state during handover from Linux.

The whole thing is lightly tested so far, only on the Banana Pi (I lost
my Jetson board, need to reclaim next). That also means, GICv3 is
compile-tested only.

Tony, if you can reintegrate you patches on top soon, I will refrain
from doing this with the current wip/arm64 state and will wait for your
test results.

Thorough review welcome, specifically as I started reading the related
sections in the manuals only last weekend.

Thanks,
Jan

Jan Kiszka (22):
arm: Use asm-defines.h for struct per_cpu members
arm: Add missing printk.h include
arm: Fix build warning in gic-v3
arm: Remove unneeded include from irqchip.h
arm: Un-inline spi_in_cell
arm: Remove write-only priority field from pending_irq
arm: Remove maintenance flag from pending_irq.type.sgi
arm: Remove hw flag from pending_irq
arm: Remove cpuid from pending_irq
arm: Remove irq field from pending_irq
arm: Remove return code from irqchip_inject_pending
arm: Remove unused return code of irqchip_set_pending
arm: Disable maintenance interrupt on successful injection
arm: Make sure to not queue interrupt that were rejected as duplicates
arm: Convert software queue of pending interrupts into a ring
arm: Remove try_inject parameter from irqchip_set_pending
arm: Enable maintenance interrupt also from irqchip_set_pending
arm: Enable / disable maintenance interrupt in distributor
arm: Protect hypervisor used SGIs and PPIs from cell changes
arm: Make cpu_init and cpu_reset callbacks mandatory
arm: Reject unknown GIC versions
arm: Consolidate gic_irqchip to irqchip

hypervisor/arch/arm/asm-defines.c | 12 ++
hypervisor/arch/arm/control.c | 2 +-
hypervisor/arch/arm/entry.S | 3 +-
hypervisor/arch/arm/gic-common.c | 28 ++--
hypervisor/arch/arm/gic-v2.c | 62 +++++----
hypervisor/arch/arm/gic-v3.c | 65 +++++-----
hypervisor/arch/arm/include/asm/irqchip.h | 52 ++------
hypervisor/arch/arm/include/asm/percpu.h | 36 ++---
hypervisor/arch/arm/irqchip.c | 209 +++++++++---------------------
hypervisor/arch/arm/setup.c | 2 +-
hypervisor/arch/arm/smp-vexpress.c | 1 +
11 files changed, 184 insertions(+), 288 deletions(-)

--
2.1.4

[Jan Kiszka]

Some implicit inclusion disappeared.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/arm/smp-vexpress.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/hypervisor/arch/arm/smp-vexpress.c b/hypervisor/arch/arm/smp-vexpress.c
index 0553448..07875fa 100644
--- a/hypervisor/arch/arm/smp-vexpress.c
+++ b/hypervisor/arch/arm/smp-vexpress.c
@@ -11,6 +11,7 @@
*/

#include <jailhouse/control.h>
+#include <jailhouse/printk.h>
#include <jailhouse/processor.h>
#include <asm/irqchip.h>
#include <asm/paging.h>
--
2.1.4

[Jan Kiszka]

It's always 0.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/arm/include/asm/irqchip.h | 2 +-
hypervisor/arch/arm/irqchip.c | 4 +---
2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/hypervisor/arch/arm/include/asm/irqchip.h b/hypervisor/arch/arm/include/asm/irqchip.h
index 65d4d4f..8b4b035 100644
--- a/hypervisor/arch/arm/include/asm/irqchip.h
+++ b/hypervisor/arch/arm/include/asm/irqchip.h
@@ -83,7 +83,7 @@ int irqchip_send_sgi(struct sgi *sgi);
void irqchip_handle_irq(struct per_cpu *cpu_data);
void irqchip_eoi_irq(u32 irqn, bool deactivate);

-int irqchip_inject_pending(struct per_cpu *cpu_data);
+void irqchip_inject_pending(struct per_cpu *cpu_data);
int irqchip_insert_pending(struct per_cpu *cpu_data, struct pending_irq *irq);
int irqchip_remove_pending(struct per_cpu *cpu_data, struct pending_irq *irq);
int irqchip_set_pending(struct per_cpu *cpu_data, u32 irq_id, bool try_inject);
diff --git a/hypervisor/arch/arm/irqchip.c b/hypervisor/arch/arm/irqchip.c
index a4e2d99..1e2a4e4 100644
--- a/hypervisor/arch/arm/irqchip.c
+++ b/hypervisor/arch/arm/irqchip.c
@@ -160,7 +160,7 @@ int irqchip_remove_pending(struct per_cpu *cpu_data, struct pending_irq *irq)
return 0;
}

-int irqchip_inject_pending(struct per_cpu *cpu_data)
+void irqchip_inject_pending(struct per_cpu *cpu_data)
{
int err;
struct pending_irq *pending = cpu_data->first_pending;
@@ -183,8 +183,6 @@ int irqchip_inject_pending(struct per_cpu *cpu_data)

pending = pending->next;
}
-
- return 0;
}

void irqchip_handle_irq(struct per_cpu *cpu_data)
--
2.1.4

[Jan Kiszka]

In case we set an interrupt pending for the local CPU and cannot queue
it with the hardware, make sure the maintenance interrupt is on.
Otherwise, we risk to delay guest interrupts or cause the guest to get
stuck.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/arm/irqchip.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/hypervisor/arch/arm/irqchip.c b/hypervisor/arch/arm/irqchip.c
index bec3d95..7f81eb8 100644
--- a/hypervisor/arch/arm/irqchip.c
+++ b/hypervisor/arch/arm/irqchip.c
@@ -73,6 +73,14 @@ void irqchip_set_pending(struct per_cpu *cpu_data, u16 irq_id)
}

spin_unlock(&cpu_data->pending_irqs_lock);
+
+ /*
+ * The list registers are full, trigger maintenance interrupt if we are
+ * on the target CPU. In the other case, the caller will send a
+ * SGI_INJECT, and irqchip_inject_pending will take care.
+ */
+ if (local_injection)
+ irqchip.enable_maint_irq(true);
}

void irqchip_inject_pending(struct per_cpu *cpu_data)
--
2.1.4

[Jan Kiszka]

Port the logic over from x86 and also drop CHECK_ASSUMPTION here.

The only slightly ugly detail: the PERCPU_SIZE_SHIFT define is now
duplicated in both asm/percpu.h instances because there is no good
generic header yet to hold it. Can be cleaned up later on.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/arm/asm-defines.c | 12 ++++++++++++
hypervisor/arch/arm/entry.S | 3 ++-
hypervisor/arch/arm/include/asm/percpu.h | 25 +++++--------------------
hypervisor/arch/arm/setup.c | 2 +-
4 files changed, 20 insertions(+), 22 deletions(-)

diff --git a/hypervisor/arch/arm/asm-defines.c b/hypervisor/arch/arm/asm-defines.c
index 15ff68d..c54ebf4 100644
--- a/hypervisor/arch/arm/asm-defines.c
+++ b/hypervisor/arch/arm/asm-defines.c
@@ -11,9 +11,21 @@
*/

#include <jailhouse/gen-defines.h>
+#include <jailhouse/utils.h>
+#include <asm/percpu.h>

void common(void);

void common(void)
{
+ OFFSET(PERCPU_LINUX_SP, per_cpu, linux_sp);
+ BLANK();
+
+ /* GCC evaluates constant expressions involving built-ins
+ * at compilation time, so this yields computed value.
+ */
+ DEFINE(PERCPU_STACK_END,
+ __builtin_offsetof(struct per_cpu, stack) + \
+ FIELD_SIZEOF(struct per_cpu, stack));
+ DEFINE(PERCPU_SIZE_SHIFT_ASM, PERCPU_SIZE_SHIFT);
}
diff --git a/hypervisor/arch/arm/entry.S b/hypervisor/arch/arm/entry.S
index 6f9178c..430bb3e 100644
--- a/hypervisor/arch/arm/entry.S
+++ b/hypervisor/arch/arm/entry.S
@@ -10,6 +10,7 @@
* the COPYING file in the top-level directory.
*/

+#include <asm/asm-defines.h>
#include <asm/head.h>
#include <asm/percpu.h>

@@ -22,7 +23,7 @@ arch_entry:

ldr r1, =__page_pool
mov r4, #1
- lsl r4, #PERCPU_SIZE_SHIFT
+ lsl r4, #PERCPU_SIZE_SHIFT_ASM
/*
* percpu data = pool + cpuid * shift
* TODO: handle aff1 and aff2
diff --git a/hypervisor/arch/arm/include/asm/percpu.h b/hypervisor/arch/arm/include/asm/percpu.h
index 3ab3a68..e6562da 100644
--- a/hypervisor/arch/arm/include/asm/percpu.h
+++ b/hypervisor/arch/arm/include/asm/percpu.h
@@ -18,11 +18,6 @@

#define NUM_ENTRY_REGS 13

-/* Keep in sync with struct per_cpu! */
-#define PERCPU_SIZE_SHIFT 13
-#define PERCPU_STACK_END PAGE_SIZE
-#define PERCPU_LINUX_SP PERCPU_STACK_END
-
#ifndef __ASSEMBLY__

#include <jailhouse/cell.h>
@@ -30,10 +25,13 @@
#include <asm/spinlock.h>
#include <asm/sysregs.h>

+/* Round up sizeof(struct per_cpu) to the next power of two. */
+#define PERCPU_SIZE_SHIFT \
+ (BITS_PER_LONG - __builtin_clzl(sizeof(struct per_cpu) - 1))
+
struct pending_irq;

struct per_cpu {
- /* Keep these two in sync with defines above! */
u8 stack[PAGE_SIZE];
unsigned long linux_sp;
unsigned long linux_ret;
@@ -93,7 +91,7 @@ static inline struct per_cpu *per_cpu(unsigned int cpu)
static inline struct registers *guest_regs(struct per_cpu *cpu_data)
{
/* Assumes that the trap handler is entered with an empty stack */
- return (struct registers *)(cpu_data->stack + PERCPU_STACK_END
+ return (struct registers *)(cpu_data->stack + sizeof(cpu_data->stack)
- sizeof(struct registers));
}

@@ -103,19 +101,6 @@ static inline unsigned int arm_cpu_phys2virt(unsigned int cpu_id)
}

unsigned int arm_cpu_virt2phys(struct cell *cell, unsigned int virt_id);
-
-/* Validate defines */
-#define CHECK_ASSUMPTION(assume) ((void)sizeof(char[1 - 2*!(assume)]))
-
-static inline void __check_assumptions(void)
-{
- struct per_cpu cpu_data;
-
- CHECK_ASSUMPTION(sizeof(struct per_cpu) == (1 << PERCPU_SIZE_SHIFT));
- CHECK_ASSUMPTION(sizeof(cpu_data.stack) == PERCPU_STACK_END);
- CHECK_ASSUMPTION(__builtin_offsetof(struct per_cpu, linux_sp) ==
- PERCPU_LINUX_SP);
-}
#endif /* !__ASSEMBLY__ */

#endif /* !_JAILHOUSE_ASM_PERCPU_H */
diff --git a/hypervisor/arch/arm/setup.c b/hypervisor/arch/arm/setup.c
index ef6f9e0..f9f59d8 100644
--- a/hypervisor/arch/arm/setup.c
+++ b/hypervisor/arch/arm/setup.c
@@ -129,7 +129,7 @@ void __attribute__((noreturn)) arch_cpu_activate_vmm(struct per_cpu *cpu_data)
*/
"eret\n\t"
:
- : "r" (cpu_data->stack + PERCPU_STACK_END),
+ : "r" (cpu_data->stack + sizeof(cpu_data->stack)),
"r" (cpu_data->linux_reg));

__builtin_unreachable();
--
2.1.4

[Jan Kiszka]

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/arm/include/asm/irqchip.h | 1 -
1 file changed, 1 deletion(-)

diff --git a/hypervisor/arch/arm/include/asm/irqchip.h b/hypervisor/arch/arm/include/asm/irqchip.h
index 17ba90a..34b16c4 100644
--- a/hypervisor/arch/arm/include/asm/irqchip.h
+++ b/hypervisor/arch/arm/include/asm/irqchip.h
@@ -24,7 +24,6 @@
#include <jailhouse/cell.h>
#include <jailhouse/mmio.h>
#include <asm/percpu.h>
-#include <asm/traps.h>

#ifndef __ASSEMBLY__

--
2.1.4

[Jan Kiszka]

We must not allow the cells to manipulate distributor registers or
register bits related to the hypervisor SGIs or the maintenance
interrupt.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/arm/gic-common.c | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/hypervisor/arch/arm/gic-common.c b/hypervisor/arch/arm/gic-common.c
index 958102c..ff32114 100644
--- a/hypervisor/arch/arm/gic-common.c
+++ b/hypervisor/arch/arm/gic-common.c
@@ -57,18 +57,22 @@ restrict_bitmask_access(struct mmio_access *mmio, unsigned int reg_index,
/* First, extract the first interrupt affected by this access */
unsigned int first_irq = reg_index * irqs_per_reg;

- /* For SGIs or PPIs, let the caller do the mmio access */
if (!is_spi(first_irq)) {
- mmio_perform_access(gicd_base, mmio);
- return MMIO_HANDLED;
- }
-
- /* For SPIs, compare against the cell config mask */
- first_irq -= 32;
- for (spi = first_irq; spi < first_irq + irqs_per_reg; spi++) {
- unsigned int bit_nr = (spi - first_irq) * bits_per_irq;
- if (spi_in_cell(cell, spi))
- access_mask |= spi_bits << bit_nr;
+ /*
+ * For SGIs or PPIs, let the caller do the mmio access, except
+ * for the hypervisor used SGIs and the maintenance PPI.
+ */
+ access_mask = 0xffffffff & ~((1 << SGI_INJECT) |
+ (1 << SGI_CPU_OFF) |
+ (1 << MAINTENANCE_IRQ));
+ } else {
+ /* For SPIs, compare against the cell config mask */
+ first_irq -= 32;
+ for (spi = first_irq; spi < first_irq + irqs_per_reg; spi++) {
+ unsigned int bit_nr = (spi - first_irq) * bits_per_irq;
+ if (spi_in_cell(cell, spi))
+ access_mask |= spi_bits << bit_nr;
+ }
}

if (!mmio->is_write) {
--
2.1.4

[Jan Kiszka]

We would likely crash anyway due to irqchip containing only NULL
pointers.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/arm/irqchip.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hypervisor/arch/arm/irqchip.c b/hypervisor/arch/arm/irqchip.c
index a177220..26b3026 100644
--- a/hypervisor/arch/arm/irqchip.c
+++ b/hypervisor/arch/arm/irqchip.c
@@ -224,6 +224,8 @@ int irqchip_init(void)
case 0x4:

memcpy(&irqchip, &gic_irqchip, sizeof(struct irqchip_ops));
break;

+ default:
+ goto err_no_distributor;
}

if (irqchip.init) {
@@ -234,7 +236,7 @@ int irqchip_init(void)
}

err_no_distributor:
- printk("GIC: no distributor found\n");
+ printk("GIC: no supported distributor found\n");
arch_unmap_device(gicd_base, gicd_size);

return -ENODEV;
--
2.1.4

[Jan Kiszka]

No caller evaluated it so far, and none of them has a use case for it.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/arm/include/asm/irqchip.h | 2 +-
hypervisor/arch/arm/irqchip.c | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hypervisor/arch/arm/include/asm/irqchip.h b/hypervisor/arch/arm/include/asm/irqchip.h
index 8b4b035..c4b8289 100644
--- a/hypervisor/arch/arm/include/asm/irqchip.h
+++ b/hypervisor/arch/arm/include/asm/irqchip.h
@@ -86,7 +86,7 @@ void irqchip_eoi_irq(u32 irqn, bool deactivate);

void irqchip_inject_pending(struct per_cpu *cpu_data);
int irqchip_insert_pending(struct per_cpu *cpu_data, struct pending_irq *irq);
int irqchip_remove_pending(struct per_cpu *cpu_data, struct pending_irq *irq);

-int irqchip_set_pending(struct per_cpu *cpu_data, u32 irq_id, bool try_inject);
+void irqchip_set_pending(struct per_cpu *cpu_data, u32 irq_id, bool try_inject);

bool spi_in_cell(struct cell *cell, unsigned int spi);

diff --git a/hypervisor/arch/arm/irqchip.c b/hypervisor/arch/arm/irqchip.c
index 1e2a4e4..7ace354 100644
--- a/hypervisor/arch/arm/irqchip.c
+++ b/hypervisor/arch/arm/irqchip.c
@@ -129,16 +129,16 @@ int irqchip_insert_pending(struct per_cpu *cpu_data, struct pending_irq *irq)
return 0;
}

-int irqchip_set_pending(struct per_cpu *cpu_data, u32 irq_id, bool try_inject)
+void irqchip_set_pending(struct per_cpu *cpu_data, u32 irq_id, bool try_inject)
{

struct pending_irq pending;

pending.virt_id = irq_id;

if (try_inject && irqchip.inject_irq(cpu_data, &pending) == 0)

- return 0;
+ return;

- return irqchip_insert_pending(cpu_data, &pending);
+ irqchip_insert_pending(cpu_data, &pending);
}

/*
--
2.1.4

[Jan Kiszka]

To big to be inlined, and we also want to avoid dereferencing struct
cell in the header due to upcoming include reordering.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/arm/include/asm/irqchip.h | 15 +--------------
hypervisor/arch/arm/irqchip.c | 15 +++++++++++++++
2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/hypervisor/arch/arm/include/asm/irqchip.h b/hypervisor/arch/arm/include/asm/irqchip.h
index 34b16c4..1457fd7 100644
--- a/hypervisor/arch/arm/include/asm/irqchip.h
+++ b/hypervisor/arch/arm/include/asm/irqchip.h
@@ -101,20 +101,7 @@ int irqchip_insert_pending(struct per_cpu *cpu_data, struct pending_irq *irq);

int irqchip_remove_pending(struct per_cpu *cpu_data, struct pending_irq *irq);

int irqchip_set_pending(struct per_cpu *cpu_data, u32 irq_id, bool try_inject);

-static inline bool spi_in_cell(struct cell *cell, unsigned int spi)
-{
- /* FIXME: Change the configuration to a bitmask range */
- u32 spi_mask;
-
- if (spi >= 64)
- return false;
- else if (spi >= 32)
- spi_mask = cell->arch.spis >> 32;
- else
- spi_mask = cell->arch.spis;
-
- return spi_mask & (1 << (spi & 31));
-}
+bool spi_in_cell(struct cell *cell, unsigned int spi);

#endif /* __ASSEMBLY__ */
#endif /* _JAILHOUSE_ASM_IRQCHIP_H */
diff --git a/hypervisor/arch/arm/irqchip.c b/hypervisor/arch/arm/irqchip.c
index 2d7840e..7a6d4ca 100644
--- a/hypervisor/arch/arm/irqchip.c
+++ b/hypervisor/arch/arm/irqchip.c
@@ -34,6 +34,21 @@ unsigned long gicd_size;
static bool irqchip_is_init;
static struct irqchip_ops irqchip;

+bool spi_in_cell(struct cell *cell, unsigned int spi)
+{
+ /* FIXME: Change the configuration to a bitmask range */
+ u32 spi_mask;
+
+ if (spi >= 64)
+ return false;
+ else if (spi >= 32)
+ spi_mask = cell->arch.spis >> 32;
+ else
+ spi_mask = cell->arch.spis;
+
+ return spi_mask & (1 << (spi & 31));
+}
+
static int irqchip_init_pending(struct per_cpu *cpu_data)
{
struct pending_irq *pend_array;
--
2.1.4

[Jan Kiszka]

We enable the maintenance interrupt when all list registers are in use.
However, there was no disabling of it again. Apparently, it rarely
triggered in the field, otherwise we would have seen a lot of
maintenance interrupt storms, thus locked-up systems.

This introduces another callback to enable or disable the maintenance
interrupt. It is now controlled by irqchip_inject_pending, the function
that is also called when handling a maintenance interrupt.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/arm/gic-v2.c | 22 ++++++++++++++--------
hypervisor/arch/arm/gic-v3.c | 26 +++++++++++++++-----------
hypervisor/arch/arm/include/asm/irqchip.h | 1 +
hypervisor/arch/arm/irqchip.c | 19 +++++++++++++++----
4 files changed, 45 insertions(+), 23 deletions(-)

diff --git a/hypervisor/arch/arm/gic-v2.c b/hypervisor/arch/arm/gic-v2.c
index ce5a61c..e47c142 100644
--- a/hypervisor/arch/arm/gic-v2.c
+++ b/hypervisor/arch/arm/gic-v2.c
@@ -251,15 +251,8 @@ static int gic_inject_irq(struct per_cpu *cpu_data, struct pending_irq *irq)
return -EINVAL;
}

- if (first_free == -1) {
- /* Enable maintenance IRQ */
- u32 hcr;
- hcr = mmio_read32(gich_base + GICH_HCR);
- hcr |= GICH_HCR_UIE;
- mmio_write32(gich_base + GICH_HCR, hcr);
-
+ if (first_free == -1)
return -EBUSY;
- }

/* Inject group 0 interrupt (seen as IRQ by the guest) */
lr = irq->virt_id;
@@ -275,6 +268,18 @@ static int gic_inject_irq(struct per_cpu *cpu_data, struct pending_irq *irq)
return 0;
}

+static void gic_enable_maint_irq(bool enable)
+{
+ u32 hcr;
+
+ hcr = mmio_read32(gich_base + GICH_HCR);
+ if (enable)
+ hcr |= GICH_HCR_UIE;
+ else
+ hcr &= ~GICH_HCR_UIE;
+ mmio_write32(gich_base + GICH_HCR, hcr);
+}
+

unsigned int irqchip_mmio_count_regions(struct cell *cell)
{
return 1;

@@ -290,5 +295,6 @@ struct irqchip_ops gic_irqchip = {
.send_sgi = gic_send_sgi,
.handle_irq = gic_handle_irq,
.inject_irq = gic_inject_irq,
+ .enable_maint_irq = gic_enable_maint_irq,
.eoi_irq = gic_eoi_irq,
};
diff --git a/hypervisor/arch/arm/gic-v3.c b/hypervisor/arch/arm/gic-v3.c
index 6beab34..fe9c261 100644
--- a/hypervisor/arch/arm/gic-v3.c
+++ b/hypervisor/arch/arm/gic-v3.c
@@ -370,18 +370,9 @@ static int gic_inject_irq(struct per_cpu *cpu_data, struct pending_irq *irq)
return -EINVAL;
}

- if (free_lr == -1) {
- u32 hcr;
- /*
- * All list registers are in use, trigger a maintenance
- * interrupt once they are available again.
- */
- arm_read_sysreg(ICH_HCR_EL2, hcr);
- hcr |= ICH_HCR_UIE;
- arm_write_sysreg(ICH_HCR_EL2, hcr);
-
+ if (free_lr == -1)
+ /* All list registers are in use */
return -EBUSY;
- }

lr = irq->virt_id;
/* Only group 1 interrupts */
@@ -397,6 +388,18 @@ static int gic_inject_irq(struct per_cpu *cpu_data, struct pending_irq *irq)
return 0;
}

+static void gicv3_enable_maint_irq(bool enable)
+{
+ u32 hcr;
+
+ arm_read_sysreg(ICH_HCR_EL2, hcr);
+ if (enable)
+ hcr |= ICH_HCR_UIE;
+ else
+ hcr &= ~ICH_HCR_UIE;
+ arm_write_sysreg(ICH_HCR_EL2, hcr);
+}
+

unsigned int irqchip_mmio_count_regions(struct cell *cell)
{
return 2;

@@ -411,5 +414,6 @@ struct irqchip_ops gic_irqchip = {
.send_sgi = gic_send_sgi,
.handle_irq = gic_handle_irq,
.inject_irq = gic_inject_irq,
+ .enable_maint_irq = gicv3_enable_maint_irq,
.eoi_irq = gic_eoi_irq,
};
diff --git a/hypervisor/arch/arm/include/asm/irqchip.h b/hypervisor/arch/arm/include/asm/irqchip.h
index c4b8289..538b432 100644
--- a/hypervisor/arch/arm/include/asm/irqchip.h
+++ b/hypervisor/arch/arm/include/asm/irqchip.h
@@ -56,6 +56,7 @@ struct irqchip_ops {
void (*eoi_irq)(u32 irqn, bool deactivate);
int (*inject_irq)(struct per_cpu *cpu_data,
struct pending_irq *irq);
+ void (*enable_maint_irq)(bool enable);

int (*mmio_access)(struct mmio_access *access);
};
diff --git a/hypervisor/arch/arm/irqchip.c b/hypervisor/arch/arm/irqchip.c
index 7ace354..5187390 100644
--- a/hypervisor/arch/arm/irqchip.c
+++ b/hypervisor/arch/arm/irqchip.c
@@ -167,10 +167,14 @@ void irqchip_inject_pending(struct per_cpu *cpu_data)

while (pending != NULL) {
err = irqchip.inject_irq(cpu_data, pending);
- if (err == -EBUSY)
- /* The list registers are full. */
- break;
- else
+ if (err == -EBUSY) {

+ /*
+ * The list registers are full, trigger maintenance

+ * interrupt and leave.
+ */
+ irqchip.enable_maint_irq(true);
+ return;
+ } else {
/*
* Removal only changes the pointers, but does not
* deallocate anything.
@@ -180,9 +184,16 @@ void irqchip_inject_pending(struct per_cpu *cpu_data)
* after this removal, which isn't an issue.
*/
irqchip_remove_pending(cpu_data, pending);
+ }

pending = pending->next;
}
+
+ /*
+ * The software interrupt queue is empty - turn off the maintenance
+ * interrupt.
+ */
+ irqchip.enable_maint_irq(false);

[Jan Kiszka]

We did not get any maintenance interrupts so far because we didn't
enable the source in the distributor so far. Fix this, but also disable
it again when shutting down.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/arm/gic-v2.c | 22 +++++++++++++++-------
hypervisor/arch/arm/gic-v3.c | 21 +++++++++++++--------
2 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/hypervisor/arch/arm/gic-v2.c b/hypervisor/arch/arm/gic-v2.c
index aae5c0e..8f5fc0b 100644
--- a/hypervisor/arch/arm/gic-v2.c
+++ b/hypervisor/arch/arm/gic-v2.c
@@ -76,11 +76,18 @@ static int gic_cpu_reset(struct per_cpu *cpu_data, bool is_shutdown)
mmio_write32(gicc_base + GICC_DIR, i);
}

- /* Disable PPIs if necessary */
- if (!root_shutdown)
- mmio_write32(gicd_base + GICD_ICENABLER, 0xffff0000);
- /* Ensure IPIs are enabled */
- mmio_write32(gicd_base + GICD_ISENABLER, 0x0000ffff);
+ /* Ensure all IPIs and the maintenance PPI are enabled */
+ mmio_write32(gicd_base + GICD_ISENABLER,
+ 0x0000ffff | (1 << MAINTENANCE_IRQ));
+
+ /*
+ * Disable PPIs, except for the maintenance interrupt.
+ * On shutdown, the root cell expects to find all its PPIs still
+ * enabled - except for the maintenance interrupt we used.
+ */
+ mmio_write32(gicd_base + GICD_ICENABLER,
+ root_shutdown ? 1 << MAINTENANCE_IRQ :
+ 0xffff0000 & ~(1 << MAINTENANCE_IRQ));

if (is_shutdown)
mmio_write32(gich_base + GICH_HCR, 0);
@@ -110,8 +117,9 @@ static int gic_cpu_init(struct per_cpu *cpu_data)
u32 vtr, vmcr;
u32 cell_gicc_ctlr, cell_gicc_pmr;

- /* Ensure all IPIs are enabled */
- mmio_write32(gicd_base + GICD_ISENABLER, 0x0000ffff);
+ /* Ensure all IPIs and the maintenance PPI are enabled. */
+ mmio_write32(gicd_base + GICD_ISENABLER,
+ 0x0000ffff | (1 << MAINTENANCE_IRQ));

cell_gicc_ctlr = mmio_read32(gicc_base + GICC_CTLR);
cell_gicc_pmr = mmio_read32(gicc_base + GICC_PMR);
diff --git a/hypervisor/arch/arm/gic-v3.c b/hypervisor/arch/arm/gic-v3.c
index 65f326c..4ab03ad 100644
--- a/hypervisor/arch/arm/gic-v3.c
+++ b/hypervisor/arch/arm/gic-v3.c
@@ -92,14 +92,18 @@ static int gic_cpu_reset(struct per_cpu *cpu_data, bool is_shutdown)
arm_write_sysreg(ICC_DIR_EL1, i);
}

+ /* Ensure all IPIs and the maintenance PPI are enabled. */
+ mmio_write32(gicr + GICR_ISENABLER,
+ 0x0000ffff | (1 << MAINTENANCE_IRQ));
+
/*
- * Disable all PPIs, ensure IPIs are enabled.
- * On shutdown, the root cell expects to find all its PPIs still enabled
- * when returning to the driver.
+ * Disable PPIs, except for the maintenance interrupt.
+ * On shutdown, the root cell expects to find all its PPIs still
+ * enabled - except for the maintenance interrupt we used.
*/
- if (!root_shutdown)
- mmio_write32(gicr + GICR_ICENABLER, 0xffff0000);
- mmio_write32(gicr + GICR_ISENABLER, 0x0000ffff);
+ mmio_write32(gicr + GICR_ICENABLER,
+ root_shutdown ? 1 << MAINTENANCE_IRQ :
+ 0xffff0000 & ~(1 << MAINTENANCE_IRQ));

if (root_shutdown) {
/* Restore the root config */
@@ -152,8 +156,9 @@ static int gic_cpu_init(struct per_cpu *cpu_data)
return -ENODEV;
}

- /* Ensure all IPIs are enabled */
- mmio_write32(redist_base + GICR_SGI_BASE + GICR_ISENABLER, 0x0000ffff);
+ /* Ensure all IPIs and the maintenance PPI are enabled. */
+ mmio_write32(redist_base + GICR_SGI_BASE + GICR_ISENABLER,
+ 0x0000ffff | (1 << MAINTENANCE_IRQ));

/*
* Set EOIMode to 1
--
2.1.4

[Jan Kiszka]

Leftover from the mmio_perform_access refactoring.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/arm/gic-v3.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hypervisor/arch/arm/gic-v3.c b/hypervisor/arch/arm/gic-v3.c
index 11cd5eb..320caf4 100644
--- a/hypervisor/arch/arm/gic-v3.c
+++ b/hypervisor/arch/arm/gic-v3.c
@@ -263,7 +263,7 @@ static enum mmio_result gic_handle_redist_access(void *arg,
return MMIO_HANDLED;
}
}
- mmio_perform_access((unsigned long)phys_redist, mmio);
+ mmio_perform_access(phys_redist, mmio);
return MMIO_HANDLED;
}

--
2.1.4

[Jan Kiszka]

We can only perform injection (and we also always want to) if target
CPU equals caller CPU, and this is better checked inside the function.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/arm/control.c | 2 +-
hypervisor/arch/arm/gic-common.c | 2 +-
hypervisor/arch/arm/include/asm/irqchip.h | 2 +-
hypervisor/arch/arm/irqchip.c | 5 +++--
4 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/hypervisor/arch/arm/control.c b/hypervisor/arch/arm/control.c
index 1c17c31..918704c 100644
--- a/hypervisor/arch/arm/control.c
+++ b/hypervisor/arch/arm/control.c
@@ -329,7 +329,7 @@ bool arch_handle_phys_irq(struct per_cpu *cpu_data, u32 irqn)

cpu_data->stats[JAILHOUSE_CPU_STAT_VMEXITS_VIRQ]++;

- irqchip_set_pending(cpu_data, irqn, true);
+ irqchip_set_pending(cpu_data, irqn);

return false;
}
diff --git a/hypervisor/arch/arm/gic-common.c b/hypervisor/arch/arm/gic-common.c
index 7bb6cd3..958102c 100644
--- a/hypervisor/arch/arm/gic-common.c
+++ b/hypervisor/arch/arm/gic-common.c
@@ -293,7 +293,7 @@ void gic_handle_sgir_write(struct sgi *sgi, bool virt_input)
if (sgi->routing_mode == 0 && !is_target)
continue;

- irqchip_set_pending(per_cpu(cpu), sgi->id, false);
+ irqchip_set_pending(per_cpu(cpu), sgi->id);
sgi->targets |= (1 << cpu);
}

diff --git a/hypervisor/arch/arm/include/asm/irqchip.h b/hypervisor/arch/arm/include/asm/irqchip.h
index f0f0e7a..bfee624 100644
--- a/hypervisor/arch/arm/include/asm/irqchip.h
+++ b/hypervisor/arch/arm/include/asm/irqchip.h
@@ -79,7 +79,7 @@ void irqchip_handle_irq(struct per_cpu *cpu_data);

void irqchip_eoi_irq(u32 irqn, bool deactivate);

void irqchip_inject_pending(struct per_cpu *cpu_data);

-void irqchip_set_pending(struct per_cpu *cpu_data, u16 irq_id, bool try_inject);
+void irqchip_set_pending(struct per_cpu *cpu_data, u16 irq_id);

bool spi_in_cell(struct cell *cell, unsigned int spi);

diff --git a/hypervisor/arch/arm/irqchip.c b/hypervisor/arch/arm/irqchip.c
index 538bbc0..bec3d95 100644
--- a/hypervisor/arch/arm/irqchip.c
+++ b/hypervisor/arch/arm/irqchip.c
@@ -49,11 +49,12 @@ bool spi_in_cell(struct cell *cell, unsigned int spi)

return spi_mask & (1 << (spi & 31));
}

-void irqchip_set_pending(struct per_cpu *cpu_data, u16 irq_id, bool try_inject)
+void irqchip_set_pending(struct per_cpu *cpu_data, u16 irq_id)
{
+ bool local_injection = (this_cpu_data() == cpu_data);
unsigned int new_tail;

- if (try_inject && irqchip.inject_irq(cpu_data, irq_id) != -EBUSY)
+ if (local_injection && irqchip.inject_irq(cpu_data, irq_id) != -EBUSY)
return;

spin_lock(&cpu_data->pending_irqs_lock);
--
2.1.4

[Jan Kiszka]

This massively simplifies the code and reduces the memory usage in
struct per_cpu. However, adding interrupt priorities later on may
require another rework.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/arm/gic-v2.c | 10 +-
hypervisor/arch/arm/gic-v3.c | 10 +-
hypervisor/arch/arm/include/asm/irqchip.h | 18 +---
hypervisor/arch/arm/include/asm/percpu.h | 13 +--
hypervisor/arch/arm/irqchip.c | 151 +++++-------------------------
5 files changed, 46 insertions(+), 156 deletions(-)

diff --git a/hypervisor/arch/arm/gic-v2.c b/hypervisor/arch/arm/gic-v2.c
index 6e939fa..aae5c0e 100644
--- a/hypervisor/arch/arm/gic-v2.c
+++ b/hypervisor/arch/arm/gic-v2.c
@@ -228,7 +228,7 @@ static int gic_send_sgi(struct sgi *sgi)
return 0;
}

-static int gic_inject_irq(struct per_cpu *cpu_data, struct pending_irq *irq)
+static int gic_inject_irq(struct per_cpu *cpu_data, u16 irq_id)
{
int i;
int first_free = -1;
@@ -247,7 +247,7 @@ static int gic_inject_irq(struct per_cpu *cpu_data, struct pending_irq *irq)

/* Check that there is no overlapping */
lr = gic_read_lr(i);
- if ((lr & GICH_LR_VIRT_ID_MASK) == irq->virt_id)
+ if ((lr & GICH_LR_VIRT_ID_MASK) == irq_id)
return -EEXIST;
}

@@ -255,12 +255,12 @@ static int gic_inject_irq(struct per_cpu *cpu_data, struct pending_irq *irq)
return -EBUSY;

/* Inject group 0 interrupt (seen as IRQ by the guest) */

- lr = irq->virt_id;

+ lr = irq_id;
lr |= GICH_LR_PENDING_BIT;

- if (!is_sgi(irq->virt_id)) {
+ if (!is_sgi(irq_id)) {
lr |= GICH_LR_HW_BIT;
- lr |= irq->virt_id << GICH_LR_PHYS_ID_SHIFT;
+ lr |= (u32)irq_id << GICH_LR_PHYS_ID_SHIFT;
}

gic_write_lr(first_free, lr);
diff --git a/hypervisor/arch/arm/gic-v3.c b/hypervisor/arch/arm/gic-v3.c
index 40526ea..65f326c 100644
--- a/hypervisor/arch/arm/gic-v3.c
+++ b/hypervisor/arch/arm/gic-v3.c
@@ -340,7 +340,7 @@ static void gic_eoi_irq(u32 irq_id, bool deactivate)
arm_write_sysreg(ICC_DIR_EL1, irq_id);
}

-static int gic_inject_irq(struct per_cpu *cpu_data, struct pending_irq *irq)
+static int gic_inject_irq(struct per_cpu *cpu_data, u16 irq_id)
{
int i;
int free_lr = -1;
@@ -366,7 +366,7 @@ static int gic_inject_irq(struct per_cpu *cpu_data, struct pending_irq *irq)
* A strict phys->virt id mapping is used for SPIs, so this test
* should be sufficient.
*/
- if ((u32)lr == irq->virt_id)
+ if ((u32)lr == irq_id)
return -EEXIST;
}

@@ -374,13 +374,13 @@ static int gic_inject_irq(struct per_cpu *cpu_data, struct pending_irq *irq)

/* All list registers are in use */
return -EBUSY;

- lr = irq->virt_id;

+ lr = irq_id;

/* Only group 1 interrupts */

lr |= ICH_LR_GROUP_BIT;
lr |= ICH_LR_PENDING;
- if (!is_sgi(irq->virt_id)) {
+ if (!is_sgi(irq_id)) {
lr |= ICH_LR_HW_BIT;
- lr |= (u64)irq->virt_id << ICH_LR_PHYS_ID_SHIFT;
+ lr |= (u64)irq_id << ICH_LR_PHYS_ID_SHIFT;
}

gic_write_lr(free_lr, lr);
diff --git a/hypervisor/arch/arm/include/asm/irqchip.h b/hypervisor/arch/arm/include/asm/irqchip.h
index 538b432..f0f0e7a 100644
--- a/hypervisor/arch/arm/include/asm/irqchip.h
+++ b/hypervisor/arch/arm/include/asm/irqchip.h
@@ -13,20 +13,15 @@
#ifndef _JAILHOUSE_ASM_IRQCHIP_H
#define _JAILHOUSE_ASM_IRQCHIP_H

-/*
- * Since there is no finer-grained allocation than page-alloc for the moment,
- * and it is very complicated to predict the total size needed at
- * initialisation, each cpu is allocated one page of pending irqs.
- * This allows for 256 pending IRQs, which should be sufficient.
- */
-#define MAX_PENDING_IRQS (PAGE_SIZE / sizeof(struct pending_irq))
+#define MAX_PENDING_IRQS 256

#include <jailhouse/cell.h>
#include <jailhouse/mmio.h>
-#include <asm/percpu.h>

#ifndef __ASSEMBLY__

+struct per_cpu;
+
struct sgi {
/*
* Routing mode values:
@@ -54,8 +49,7 @@ struct irqchip_ops {
int (*send_sgi)(struct sgi *sgi);
void (*handle_irq)(struct per_cpu *cpu_data);

void (*eoi_irq)(u32 irqn, bool deactivate);

- int (*inject_irq)(struct per_cpu *cpu_data,
- struct pending_irq *irq);
+ int (*inject_irq)(struct per_cpu *cpu_data, u16 irq_id);

void (*enable_maint_irq)(bool enable);

int (*mmio_access)(struct mmio_access *access);

@@ -85,9 +79,7 @@ void irqchip_handle_irq(struct per_cpu *cpu_data);

void irqchip_eoi_irq(u32 irqn, bool deactivate);

void irqchip_inject_pending(struct per_cpu *cpu_data);

-int irqchip_insert_pending(struct per_cpu *cpu_data, struct pending_irq *irq);
-int irqchip_remove_pending(struct per_cpu *cpu_data, struct pending_irq *irq);
-void irqchip_set_pending(struct per_cpu *cpu_data, u32 irq_id, bool try_inject);
+void irqchip_set_pending(struct per_cpu *cpu_data, u16 irq_id, bool try_inject);

bool spi_in_cell(struct cell *cell, unsigned int spi);

diff --git a/hypervisor/arch/arm/include/asm/percpu.h b/hypervisor/arch/arm/include/asm/percpu.h
index e6562da..ae026c4 100644
--- a/hypervisor/arch/arm/include/asm/percpu.h
+++ b/hypervisor/arch/arm/include/asm/percpu.h
@@ -21,6 +21,7 @@
#ifndef __ASSEMBLY__

#include <jailhouse/cell.h>
+#include <asm/irqchip.h>
#include <asm/psci.h>
#include <asm/spinlock.h>
#include <asm/sysregs.h>
@@ -29,8 +30,6 @@
#define PERCPU_SIZE_SHIFT \

(BITS_PER_LONG - __builtin_clzl(sizeof(struct per_cpu) - 1))

-struct pending_irq;
-
struct per_cpu {

u8 stack[PAGE_SIZE];
unsigned long linux_sp;

@@ -41,10 +40,12 @@ struct per_cpu {
unsigned int cpu_id;
unsigned int virt_id;

- /* Other CPUs can insert sgis into the pending array */
- spinlock_t gic_lock;
- struct pending_irq *pending_irqs;
- struct pending_irq *first_pending;
+ /* synchronizes parallel insertions of SGIs into the pending ring */
+ spinlock_t pending_irqs_lock;
+ u16 pending_irqs[MAX_PENDING_IRQS];
+ unsigned int pending_irqs_head;
+ /* removal from the ring happens lockless, thus tail is volatile */
+ volatile unsigned int pending_irqs_tail;
/* Only GICv3: redistributor base */
void *gicr_base;

diff --git a/hypervisor/arch/arm/irqchip.c b/hypervisor/arch/arm/irqchip.c
index fb25744..538bbc0 100644
--- a/hypervisor/arch/arm/irqchip.c
+++ b/hypervisor/arch/arm/irqchip.c
@@ -49,142 +49,49 @@ bool spi_in_cell(struct cell *cell, unsigned int spi)

return spi_mask & (1 << (spi & 31));
}

-static int irqchip_init_pending(struct per_cpu *cpu_data)
+void irqchip_set_pending(struct per_cpu *cpu_data, u16 irq_id, bool try_inject)
{
- struct pending_irq *pend_array;
-
- if (cpu_data->pending_irqs == NULL) {
- cpu_data->pending_irqs = pend_array = page_alloc(&mem_pool, 1);
- if (pend_array == NULL)
- return -ENOMEM;
- } else {
- pend_array = cpu_data->pending_irqs;
- }
-
- memset(pend_array, 0, PAGE_SIZE);
-
- cpu_data->pending_irqs = pend_array;
- cpu_data->first_pending = NULL;
-
- return 0;
-}
-
-/*
- * Find the first available pending struct for insertion. The `prev' pointer is
- * set to the previous pending interrupt, if any, to help inserting the new one
- * into the list.
- * Returns NULL when no slot is available
- */
-static struct pending_irq* get_pending_slot(struct per_cpu *cpu_data,
- struct pending_irq **prev)
-{
- u32 i, pending_idx;
- struct pending_irq *pending = cpu_data->first_pending;
-
- *prev = NULL;
-
- for (i = 0; i < MAX_PENDING_IRQS; i++) {
- pending_idx = pending - cpu_data->pending_irqs;
- if (pending == NULL || i < pending_idx)
- return cpu_data->pending_irqs + i;
+ unsigned int new_tail;

- *prev = pending;
- pending = pending->next;
- }
-
- return NULL;
-}
+ if (try_inject && irqchip.inject_irq(cpu_data, irq_id) != -EBUSY)
+ return;

-int irqchip_insert_pending(struct per_cpu *cpu_data, struct pending_irq *irq)
-{
- struct pending_irq *prev = NULL;
- struct pending_irq *slot;
+ spin_lock(&cpu_data->pending_irqs_lock);

- spin_lock(&cpu_data->gic_lock);
+ new_tail = (cpu_data->pending_irqs_tail + 1) % MAX_PENDING_IRQS;

- slot = get_pending_slot(cpu_data, &prev);
- if (slot == NULL) {
- spin_unlock(&cpu_data->gic_lock);
- return -ENOMEM;
+ /* Queue space available? */
+ if (new_tail != cpu_data->pending_irqs_head) {
+ cpu_data->pending_irqs[cpu_data->pending_irqs_tail] = irq_id;
+ cpu_data->pending_irqs_tail = new_tail;
+ /*
+ * Make the change to pending_irqs_tail visible before the
+ * caller sends SGI_INJECT.
+ */
+ memory_barrier();
}

- /*
- * Don't override the pointers yet, they may be read by the injection
- * loop. Odds are astronomically low, but hey.
- */
- memcpy(slot, irq, sizeof(struct pending_irq) - 2 * sizeof(void *));
- slot->prev = prev;
- if (prev) {
- slot->next = prev->next;
- prev->next = slot;
- } else {
- slot->next = cpu_data->first_pending;
- cpu_data->first_pending = slot;
- }
- if (slot->next)
- slot->next->prev = slot;
-
- spin_unlock(&cpu_data->gic_lock);
-
- return 0;
-}
-
-void irqchip_set_pending(struct per_cpu *cpu_data, u32 irq_id, bool try_inject)
-{
- struct pending_irq pending;
-
- pending.virt_id = irq_id;
-
- if (!try_inject || irqchip.inject_irq(cpu_data, &pending) == -EBUSY)
- irqchip_insert_pending(cpu_data, &pending);
-}
-
-/*
- * Only executed by `irqchip_inject_pending' on a CPU to inject its own stuff.
- */
-int irqchip_remove_pending(struct per_cpu *cpu_data, struct pending_irq *irq)
-{
- spin_lock(&cpu_data->gic_lock);
-
- if (cpu_data->first_pending == irq)
- cpu_data->first_pending = irq->next;
- if (irq->prev)
- irq->prev->next = irq->next;
- if (irq->next)
- irq->next->prev = irq->prev;
-
- spin_unlock(&cpu_data->gic_lock);
-
- return 0;
+ spin_unlock(&cpu_data->pending_irqs_lock);
}

void irqchip_inject_pending(struct per_cpu *cpu_data)
{
- int err;
- struct pending_irq *pending = cpu_data->first_pending;
+ u16 irq_id;

- while (pending != NULL) {
- err = irqchip.inject_irq(cpu_data, pending);
- if (err == -EBUSY) {
+ while (cpu_data->pending_irqs_head != cpu_data->pending_irqs_tail) {
+ irq_id = cpu_data->pending_irqs[cpu_data->pending_irqs_head];
+
+ if (irqchip.inject_irq(cpu_data, irq_id) == -EBUSY) {
/*

* The list registers are full, trigger maintenance

* interrupt and leave.
*/
irqchip.enable_maint_irq(true);
return;
- } else {
- /*
- * Removal only changes the pointers, but does not
- * deallocate anything.
- * Concurrent accesses are avoided with the spinlock,
- * but the `next' pointer of the current pending object
- * may be rewritten by an external insert before or
- * after this removal, which isn't an issue.
- */
- irqchip_remove_pending(cpu_data, pending);
}

- pending = pending->next;
+ cpu_data->pending_irqs_head =
+ (cpu_data->pending_irqs_head + 1) % MAX_PENDING_IRQS;
}

/*
@@ -211,12 +118,6 @@ int irqchip_send_sgi(struct sgi *sgi)

int irqchip_cpu_init(struct per_cpu *cpu_data)
{
- int err;
-
- err = irqchip_init_pending(cpu_data);
- if (err)
- return err;
-
if (irqchip.cpu_init)
return irqchip.cpu_init(cpu_data);

@@ -225,11 +126,7 @@ int irqchip_cpu_init(struct per_cpu *cpu_data)

int irqchip_cpu_reset(struct per_cpu *cpu_data)
{
- int err;
-
- err = irqchip_init_pending(cpu_data);
- if (err)
- return err;
+ cpu_data->pending_irqs_head = cpu_data->pending_irqs_tail = 0;

if (irqchip.cpu_reset)
return irqchip.cpu_reset(cpu_data, false);
--
2.1.4

[Jan Kiszka]

No need for checking them to be NULL, we need them in both
implementations.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

hypervisor/arch/arm/irqchip.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/hypervisor/arch/arm/irqchip.c b/hypervisor/arch/arm/irqchip.c
index 7f81eb8..a177220 100644
--- a/hypervisor/arch/arm/irqchip.c
+++ b/hypervisor/arch/arm/irqchip.c
@@ -127,20 +127,14 @@ int irqchip_send_sgi(struct sgi *sgi)

int irqchip_cpu_init(struct per_cpu *cpu_data)
{
- if (irqchip.cpu_init)
- return irqchip.cpu_init(cpu_data);
-
- return 0;
+ return irqchip.cpu_init(cpu_data);
}

int irqchip_cpu_reset(struct per_cpu *cpu_data)
{

cpu_data->pending_irqs_head = cpu_data->pending_irqs_tail = 0;

- if (irqchip.cpu_reset)
- return irqchip.cpu_reset(cpu_data, false);
-
- return 0;
+ return irqchip.cpu_reset(cpu_data, false);
}

void irqchip_cpu_shutdown(struct per_cpu *cpu_data)
@@ -150,8 +144,7 @@ void irqchip_cpu_shutdown(struct per_cpu *cpu_data)
* it has been initialised: this function may be executed during the
* setup phase.
*/
- if (irqchip.cpu_reset)
- irqchip.cpu_reset(cpu_data, true);
+ irqchip.cpu_reset(cpu_data, true);
}

static const struct jailhouse_irqchip *
--
2.1.4

[Antonios Motakis]

On 16-Jun-16 19:06, Jan Kiszka wrote:
> This is my stack of patches that piled up while walking through the
> interrupt injection logic of arm (and soon also arm64). Besides a lot of
> more or less minor cleanups, it primarily reworks the software queue for
> pending interrupts into a ring and fixes setup, handling and protection
> of the maintenance interrupt.
>
> One open issue, though still a minor one: We should probably bring all
> distributor registers related to hypervisor SGIs and the maintenance
> interrupt into defined state during handover from Linux.
>
> The whole thing is lightly tested so far, only on the Banana Pi (I lost
> my Jetson board, need to reclaim next). That also means, GICv3 is
> compile-tested only.
>
> Tony, if you can reintegrate you patches on top soon, I will refrain
> from doing this with the current wip/arm64 state and will wait for your
> test results.

Rebasing on these, after some minor touches, and it still works on the AMD Seattle.

Cheers
Tony

Antonios Motakis
Virtualization Engineer
Huawei Technologies Duesseldorf GmbH
European Research Center
Riesstrasse 25, 80992 München

[Jan Kiszka]

On 2016-06-17 15:53, Antonios Motakis wrote:
>
>
> On 16-Jun-16 19:06, Jan Kiszka wrote:
>> This is my stack of patches that piled up while walking through the
>> interrupt injection logic of arm (and soon also arm64). Besides a lot of
>> more or less minor cleanups, it primarily reworks the software queue for
>> pending interrupts into a ring and fixes setup, handling and protection
>> of the maintenance interrupt.
>>
>> One open issue, though still a minor one: We should probably bring all
>> distributor registers related to hypervisor SGIs and the maintenance
>> interrupt into defined state during handover from Linux.
>>
>> The whole thing is lightly tested so far, only on the Banana Pi (I lost
>> my Jetson board, need to reclaim next). That also means, GICv3 is
>> compile-tested only.
>>
>> Tony, if you can reintegrate you patches on top soon, I will refrain
>> from doing this with the current wip/arm64 state and will wait for your
>> test results.
>
> Rebasing on these, after some minor touches, and it still works on the AMD Seattle.

Cool, thanks for testing!

Jan

--
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

[Jan Kiszka]

From: Jan Kiszka <jan.k...@siemens.com>

We must not allow the cells to manipulate distributor registers or
register bits related to the hypervisor SGIs or the maintenance
interrupt.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

v1 was neglecting bits_per_irq > 1.

hypervisor/arch/arm/gic-common.c | 21 ++++++++-------------
1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/hypervisor/arch/arm/gic-common.c b/hypervisor/arch/arm/gic-common.c
index 958102c..a8ac4c7 100644
--- a/hypervisor/arch/arm/gic-common.c
+++ b/hypervisor/arch/arm/gic-common.c
@@ -46,29 +46,24 @@ restrict_bitmask_access(struct mmio_access *mmio, unsigned int reg_index,
unsigned int bits_per_irq, bool is_poke)
{
struct cell *cell = this_cell();
- unsigned int spi;
+ unsigned int irq;
unsigned long access_mask = 0;
/*
* In order to avoid division, the number of bits per irq is limited
* to powers of 2 for the moment.
*/
unsigned long irqs_per_reg = 32 >> ffsl(bits_per_irq);
- unsigned long spi_bits = (1 << bits_per_irq) - 1;
+ unsigned long irq_bits = (1 << bits_per_irq) - 1;

/* First, extract the first interrupt affected by this access */
unsigned int first_irq = reg_index * irqs_per_reg;

- /* For SGIs or PPIs, let the caller do the mmio access */

- if (!is_spi(first_irq)) {

- mmio_perform_access(gicd_base, mmio);
- return MMIO_HANDLED;
- }

+ for (irq = first_irq; irq < first_irq + irqs_per_reg; irq++) {
+ unsigned int bit_nr = (irq - first_irq) * bits_per_irq;

- /* For SPIs, compare against the cell config mask */
- first_irq -= 32;
- for (spi = first_irq; spi < first_irq + irqs_per_reg; spi++) {
- unsigned int bit_nr = (spi - first_irq) * bits_per_irq;
- if (spi_in_cell(cell, spi))
- access_mask |= spi_bits << bit_nr;

+ if ((is_spi(irq) && spi_in_cell(cell, irq - 32)) ||
+ irq == SGI_INJECT || irq == SGI_CPU_OFF ||
+ irq == MAINTENANCE_IRQ)
+ access_mask |= irq_bits << bit_nr;
}

if (!mmio->is_write) {