On Thu, 22 Aug 2019 01:13:32 +0800 Zenghui Yu <yuzenghui@xxxxxxxxxx> wrote: Hi, > On 2019/8/22 1:00, Andre Przywara 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. > > 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 <andre.przywara@xxxxxxx> > > Reported-by: Dave Martin <dave.martin@xxxxxxx> > > --- > > Hi, > > > > this came up here again, I think it fell through the cracks back in > > March: > > http://lists.infradead.org/pipermail/linux-arm-kernel/2019-March/637209.html > > > > Cheers, > > Andre. > > > > 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 80127ca9269f..8bce2f75e0c1 100644 > > --- a/virt/kvm/arm/vgic/vgic-init.c > > +++ b/virt/kvm/arm/vgic/vgic-init.c > > @@ -210,7 +210,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 */ > > @@ -221,10 +220,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) { > > I still think that if user-space create VCPUs before vGIC (like what > Qemu does), the actual vGIC model will be unknown here. The UBSAN > warning will still show up when booting a vGIC-v3 guest (with Qemu). Yes, you are right. I vaguely remembered this issue, but couldn't find anything on the list about it. So thanks for the heads up! So I think I have a solution, where we drop the initialisation part here altogether, instead initialise mpdir/targets together with the group in vgic_init(). Unless there is some code which needs irq->group before that point? Will send a patch shortly. Cheers, Andre. > > 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)) > > > _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
