On 03/26/2015 10:06 PM, Marc Zyngier wrote: > On Thu, 26 Mar 2015 14:39:37 +0000 > Andre Przywara <[email protected]> wrote: > >> Using the framework provided by the recent vgic.c changes, we >> register a kvm_io_bus device on mapping the virtual GICv3 resources. >> The distributor mapping is pretty straight forward, but the >> redistributors need some more love, since they need to be tagged with >> the respective redistributor (read: VCPU) they are connected with. >> We use the kvm_io_bus framework to register one devices per VCPU. >> >> Signed-off-by: Andre Przywara <[email protected]> >> --- >> include/kvm/arm_vgic.h | 1 + >> virt/kvm/arm/vgic-v3-emul.c | 39 ++++++++++++++++++++++++++++++++++++++- >> 2 files changed, 39 insertions(+), 1 deletion(-) >> >> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h >> index 4523984..d6705f4 100644 >> --- a/include/kvm/arm_vgic.h >> +++ b/include/kvm/arm_vgic.h >> @@ -252,6 +252,7 @@ struct vgic_dist { >> >> struct vgic_vm_ops vm_ops; >> struct vgic_io_device dist_iodev; >> + struct vgic_io_device *redist_iodevs; >> }; >> >> struct vgic_v2_cpu_if { >> diff --git a/virt/kvm/arm/vgic-v3-emul.c b/virt/kvm/arm/vgic-v3-emul.c >> index 2f03a36..eb1a797 100644 >> --- a/virt/kvm/arm/vgic-v3-emul.c >> +++ b/virt/kvm/arm/vgic-v3-emul.c >> @@ -758,6 +758,9 @@ static int vgic_v3_map_resources(struct kvm *kvm, >> { >> int ret = 0; >> struct vgic_dist *dist = &kvm->arch.vgic; >> + gpa_t rdbase = dist->vgic_redist_base; >> + struct vgic_io_device *iodevs = NULL; >> + int i; >> >> if (!irqchip_in_kernel(kvm)) >> return 0; >> @@ -783,7 +786,41 @@ static int vgic_v3_map_resources(struct kvm *kvm, >> goto out; >> } >> >> - kvm->arch.vgic.ready = true; >> + ret = vgic_register_kvm_io_dev(kvm, dist->vgic_dist_base, >> + GIC_V3_DIST_SIZE, vgic_v3_dist_ranges, >> + -1, &dist->dist_iodev); > >> + if (ret) >> + goto out; >> + >> + iodevs = kcalloc(dist->nr_cpus, sizeof(iodevs[0]), GFP_KERNEL); >> + if (!iodevs) { >> + ret = -ENOMEM; >> + goto out_unregister; >> + } >> + >> + for (i = 0; i < dist->nr_cpus; i++) { >> + ret = vgic_register_kvm_io_dev(kvm, rdbase, >> + SZ_128K, vgic_redist_ranges, >> + i, &iodevs[i]); > > This looks really weird. You seems to be mapping all redistributors at > the same IPA. Have you actually tested this with an SMP guest?
But the patch continues: + if (ret) + goto out_unregister; + rdbase += GIC_V3_REDIST_SIZE; Is that too confusing to re-use the rdbase variable? The spec speaks of RD_base for each redistributor, so that seemed sane to me. Shall I add a comment or use "rdbase + i * GIC_V3_REDIST_SIZE" for clarity? Cheers, Andre. _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
