On Wed, 6 Mar 2019 12:42:21 +0100 Christophe de Dinechin <[email protected]> wrote:
> > On 6 Mar 2019, at 11:40, Andre Przywara <[email protected]> wrote: > > > > At the moment we initialise the target *mask* of a virtual IRQ to the > > VCPU it belongs to, even though this mask is only defined for GICv2 and > > quickly runs out of bits for many GICv3 guests. > > Just for my education, “targets” seems defined as an u8, > so it looks like you were silently running out of bits before, no? Yes, but UBSAN only complained about the shift >=32. If you look at /sys/kernel/debug/kvm/<pid-vcpus>/vgic-state, you will see that the targets mask for a VCPU ID > 7 is represented as 0 due to the u8 type: VCPU 0: 1 VCPU 1: 2 VCPU 2: 4 VCPU 3: 8 VCPU 4: 10 VCPU 5: 20 VCPU 6: 40 VCPU 7: 80 VCPU 8: 0 VCPU 9: 0 VCPU 10: 0 ... VCPU 32: 0 VCPU 33: 1 VCPU 34: 2 So this is quite bogus at the moment. This patch should fix the UBSAN splat and at least avoids the weird representation, by making target "0" for all private IRQs on a vGICv3. Not sure if there is an easy way to put in the actual virtual MPIDR there. Cheers, Andre. > > This behaviour triggers an UBSAN complaint for more than 32 VCPUs: > > ------ > > [ 5659.462377] UBSAN: Undefined behaviour in > > virt/kvm/arm/vgic/vgic-init.c:223:21 > > [ 5659.471689] shift exponent 32 is too large for 32-bit type 'unsigned int' > > ------ > > Also for GICv3 guests the reporting of TARGET in the "vgic-state" debugfs > > dump is wrong, due to this very same problem. > > > > Fix both issues by only initialising vgic_irq->targets for a vGICv2 guest, > > and by initialising vgic_irq->mpdir for vGICv3 guests instead. We can't > > use the actual MPIDR for that, as the VCPU's system register is not > > initialised at this point yet. This is not really an issue, as ->mpidr > > is just used for the debugfs output and the IROUTER MMIO register, which > > does not exist in redistributors (dealing with SGIs and PPIs). > > > > Signed-off-by: Andre Przywara <[email protected]> > > Reported-by: Dave Martin <[email protected]> > > --- > > virt/kvm/arm/vgic/vgic-init.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c > > index 3bdb31eaed64..3172b2c916f1 100644 > > --- a/virt/kvm/arm/vgic/vgic-init.c > > +++ b/virt/kvm/arm/vgic/vgic-init.c > > @@ -220,7 +220,6 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu) > > irq->intid = i; > > irq->vcpu = NULL; > > irq->target_vcpu = vcpu; > > - irq->targets = 1U << vcpu->vcpu_id; > > kref_init(&irq->refcount); > > if (vgic_irq_is_sgi(i)) { > > /* SGIs */ > > @@ -231,10 +230,14 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu) > > irq->config = VGIC_CONFIG_LEVEL; > > } > > > > - if (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) > > + if (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) { > > irq->group = 1; > > - else > > + /* The actual MPIDR is not initialised at this point. */ > > + irq->mpidr = 0; > > + } else { > > irq->group = 0; > > + irq->targets = 1U << vcpu->vcpu_id; > > + } > > } > > > > if (!irqchip_in_kernel(vcpu->kvm)) > > -- > > 2.17.1 > > > _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
