Commit 8a8ff069 authored by Marc Zyngier's avatar Marc Zyngier
Browse files

KVM: arm64: nv: Fix tracking of shadow list registers 



Wei-Lin reports that the tracking of shadow list registers is
majorly broken when resync'ing the L2 state after a run, as
we confuse the guest's LR index with the host's, potentially
losing the interrupt state.

While this could be fixed by adding yet another side index to
track it (Wei-Lin's fix), it may be better to refactor this
code to avoid having a side index altogether, limiting the
risk to introduce this class of bugs.

A key observation is that the shadow index is always the number
of bits in the lr_map bitmap. With that, the parallel indexing
scheme can be completely dropped.

While doing this, introduce a couple of helpers that abstract
the index conversion and some of the LR repainting, making the
whole exercise much simpler.

Reported-by: default avatarWei-Lin Chang <r09922117@csie.ntu.edu.tw>
Reviewed-by: default avatarWei-Lin Chang <r09922117@csie.ntu.edu.tw>
Reviewed-by: default avatarOliver Upton <oliver.upton@linux.dev>
Signed-off-by: default avatarMarc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20250614145721.2504524-1-r09922117@csie.ntu.edu.tw
Link: https://lore.kernel.org/r/86qzzkc5xa.wl-maz@kernel.org
parent e04c78d8

arch/arm64/kvm/vgic/vgic-v3-nested.c

+42 −39
Original line number Diff line number Diff line
@@ -36,6 +36,11 @@ struct shadow_if { 


static DEFINE_PER_CPU(struct shadow_if, shadow_if);



static int lr_map_idx_to_shadow_idx(struct shadow_if *shadow_if, int idx)

{

	return hweight16(shadow_if->lr_map & (BIT(idx) - 1));

}



/*

 * Nesting GICv3 support

 *
@@ -209,6 +214,29 @@ u64 vgic_v3_get_misr(struct kvm_vcpu *vcpu) 
	return reg;

}



static u64 translate_lr_pintid(struct kvm_vcpu *vcpu, u64 lr)

{

	struct vgic_irq *irq;



	if (!(lr & ICH_LR_HW))

		return lr;



	/* We have the HW bit set, check for validity of pINTID */

	irq = vgic_get_vcpu_irq(vcpu, FIELD_GET(ICH_LR_PHYS_ID_MASK, lr));

	/* If there was no real mapping, nuke the HW bit */

	if (!irq || !irq->hw || irq->intid > VGIC_MAX_SPI)

		lr &= ~ICH_LR_HW;



	/* Translate the virtual mapping to the real one, even if invalid */

	if (irq) {

		lr &= ~ICH_LR_PHYS_ID_MASK;

		lr |= FIELD_PREP(ICH_LR_PHYS_ID_MASK, (u64)irq->hwintid);

		vgic_put_irq(vcpu->kvm, irq);

	}



	return lr;

}



/*

 * For LRs which have HW bit set such as timer interrupts, we modify them to

 * have the host hardware interrupt number instead of the virtual one programmed
@@ -217,58 +245,37 @@ u64 vgic_v3_get_misr(struct kvm_vcpu *vcpu) 
static void vgic_v3_create_shadow_lr(struct kvm_vcpu *vcpu,

				     struct vgic_v3_cpu_if *s_cpu_if)

{

	unsigned long lr_map = 0;

	int index = 0;

	struct shadow_if *shadow_if;



	shadow_if = container_of(s_cpu_if, struct shadow_if, cpuif);

	shadow_if->lr_map = 0;



	for (int i = 0; i < kvm_vgic_global_state.nr_lr; i++) {

		u64 lr = __vcpu_sys_reg(vcpu, ICH_LRN(i));

		struct vgic_irq *irq;



		if (!(lr & ICH_LR_STATE))

			lr = 0;



		if (!(lr & ICH_LR_HW))

			goto next;



		/* We have the HW bit set, check for validity of pINTID */

		irq = vgic_get_vcpu_irq(vcpu, FIELD_GET(ICH_LR_PHYS_ID_MASK, lr));

		if (!irq || !irq->hw || irq->intid > VGIC_MAX_SPI ) {

			/* There was no real mapping, so nuke the HW bit */

			lr &= ~ICH_LR_HW;

			if (irq)

				vgic_put_irq(vcpu->kvm, irq);

			goto next;

		}

			continue;



		/* Translate the virtual mapping to the real one */

		lr &= ~ICH_LR_PHYS_ID_MASK;

		lr |= FIELD_PREP(ICH_LR_PHYS_ID_MASK, (u64)irq->hwintid);

		lr = translate_lr_pintid(vcpu, lr);



		vgic_put_irq(vcpu->kvm, irq);



next:

		s_cpu_if->vgic_lr[index] = lr;

		if (lr) {

			lr_map |= BIT(i);

			index++;

		}

		s_cpu_if->vgic_lr[hweight16(shadow_if->lr_map)] = lr;

		shadow_if->lr_map |= BIT(i);

	}



	container_of(s_cpu_if, struct shadow_if, cpuif)->lr_map = lr_map;

	s_cpu_if->used_lrs = index;

	s_cpu_if->used_lrs = hweight16(shadow_if->lr_map);

}



void vgic_v3_sync_nested(struct kvm_vcpu *vcpu)

{

	struct shadow_if *shadow_if = get_shadow_if();

	int i, index = 0;

	int i;



	for_each_set_bit(i, &shadow_if->lr_map, kvm_vgic_global_state.nr_lr) {

		u64 lr = __vcpu_sys_reg(vcpu, ICH_LRN(i));

		struct vgic_irq *irq;



		if (!(lr & ICH_LR_HW) || !(lr & ICH_LR_STATE))

			goto next;

			continue;



		/*

		 * If we had a HW lr programmed by the guest hypervisor, we
@@ -277,15 +284,13 @@ void vgic_v3_sync_nested(struct kvm_vcpu *vcpu) 
		 */

		irq = vgic_get_vcpu_irq(vcpu, FIELD_GET(ICH_LR_PHYS_ID_MASK, lr));

		if (WARN_ON(!irq)) /* Shouldn't happen as we check on load */

			goto next;

			continue;



		lr = __gic_v3_get_lr(index);

		lr = __gic_v3_get_lr(lr_map_idx_to_shadow_idx(shadow_if, i));

		if (!(lr & ICH_LR_STATE))

			irq->active = false;



		vgic_put_irq(vcpu->kvm, irq);

	next:

		index++;

	}

}
@@ -368,13 +373,11 @@ void vgic_v3_put_nested(struct kvm_vcpu *vcpu) 
		val = __vcpu_sys_reg(vcpu, ICH_LRN(i));



		val &= ~ICH_LR_STATE;

		val |= s_cpu_if->vgic_lr[i] & ICH_LR_STATE;

		val |= s_cpu_if->vgic_lr[lr_map_idx_to_shadow_idx(shadow_if, i)] & ICH_LR_STATE;



		__vcpu_assign_sys_reg(vcpu, ICH_LRN(i), val);

		s_cpu_if->vgic_lr[i] = 0;

	}



	shadow_if->lr_map = 0;

	vcpu->arch.vgic_cpu.vgic_v3.used_lrs = 0;

}