From b123a2136a52d378842db445ff77b196f66af225 Mon Sep 17 00:00:00 2001 From: Ivan Velickovic Date: Thu, 22 Feb 2024 13:19:20 +1100 Subject: [PATCH] Fix incorrect assumption in GIC driver Before handling an IRQ, we should first be checking that it has been registered with the GIC by the VMM. --- examples/simple/vmm.c | 2 -- src/arch/aarch64/vgic/vdist.h | 10 +++++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/examples/simple/vmm.c b/examples/simple/vmm.c index 0d674016..7dde4a3c 100644 --- a/examples/simple/vmm.c +++ b/examples/simple/vmm.c @@ -111,8 +111,6 @@ void init(void) { LOG_VMM_ERR("Failed to initialise emulated interrupt controller\n"); return; } - // @ivanv: Note that remove this line causes the VMM to fault if we - // actually get the interrupt. This should be avoided by making the VGIC driver more stable. success = virq_register(GUEST_VCPU_ID, SERIAL_IRQ, &serial_ack, NULL); /* Just in case there is already an interrupt available to handle, we ack it here. */ microkit_irq_ack(SERIAL_IRQ_CH); diff --git a/src/arch/aarch64/vgic/vdist.h b/src/arch/aarch64/vgic/vdist.h index 2b1d2863..a950eee5 100644 --- a/src/arch/aarch64/vgic/vdist.h +++ b/src/arch/aarch64/vgic/vdist.h @@ -172,11 +172,15 @@ static void vgic_dist_disable_irq(vgic_t *vgic, size_t vcpu_id, int irq) static bool vgic_dist_set_pending_irq(vgic_t *vgic, size_t vcpu_id, int irq) { - // @ivanv: I believe this function causes a fault in the VMM if the IRQ has not - // been registered. This is not good. /* STATE c) */ - + /* First check that we find vIRQ data in case the vIRQ has not been + * registered yet. */ struct virq_handle *virq_data = virq_find_irq_data(vgic, vcpu_id, irq); + assert(virq_data); + if (!virq_data) { + LOG_VMM_ERR("could not find vIRQ data for vIRQ 0x%lx on vCPU 0x%lx\n", irq, vcpu_id); + return false; + } struct gic_dist_map *dist = vgic_get_dist(vgic->registers); if (virq_data->virq == VIRQ_INVALID || !vgic_dist_is_enabled(dist) || !is_enabled(dist, irq, vcpu_id)) {