lkml.org 
[lkml]   [2017]   [Nov]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH v5 09/26] KVM: arm/arm64: GICv4: Add init/teardown of the per-VM vPE irq domain
    On Tue, Nov 07, 2017 at 02:08:23PM +0100, Auger Eric wrote:
    > Hi Marc,
    >
    > On 27/10/2017 16:28, Marc Zyngier wrote:
    > > In order to control the GICv4 view of virtual CPUs, we rely
    > > on an irqdomain allocated for that purpose. Let's add a couple
    > > of helpers to that effect.
    > >
    > > At the same time, the vgic data structures gain new fields to
    > > track all this... erm... wonderful stuff.
    > >
    > > The way we hook into the vgic init is slightly convoluted. We
    > > need the vgic to be initialized (in order to guarantee that
    > > the number of vcpus is now fixed), and we must have a vITS
    > > (otherwise this is all very pointless). So we end-up calling
    > > the init from both vgic_init and vgic_its_create.
    > >
    > > Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
    > > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
    > > ---
    > > arch/arm/kvm/Makefile | 1 +
    > > arch/arm64/kvm/Makefile | 1 +
    > > include/kvm/arm_vgic.h | 19 ++++++++++
    > > virt/kvm/arm/vgic/vgic-init.c | 9 +++++
    > > virt/kvm/arm/vgic/vgic-its.c | 8 +++++
    > > virt/kvm/arm/vgic/vgic-v4.c | 83 +++++++++++++++++++++++++++++++++++++++++++
    > > virt/kvm/arm/vgic/vgic.h | 2 ++
    > > 7 files changed, 123 insertions(+)
    > > create mode 100644 virt/kvm/arm/vgic/vgic-v4.c
    > >
    > > diff --git a/arch/arm/kvm/Makefile b/arch/arm/kvm/Makefile
    > > index d9beee652d36..0a1dd2cdb928 100644
    > > --- a/arch/arm/kvm/Makefile
    > > +++ b/arch/arm/kvm/Makefile
    > > @@ -31,6 +31,7 @@ obj-y += $(KVM)/arm/vgic/vgic-init.o
    > > obj-y += $(KVM)/arm/vgic/vgic-irqfd.o
    > > obj-y += $(KVM)/arm/vgic/vgic-v2.o
    > > obj-y += $(KVM)/arm/vgic/vgic-v3.o
    > > +obj-y += $(KVM)/arm/vgic/vgic-v4.o
    > > obj-y += $(KVM)/arm/vgic/vgic-mmio.o
    > > obj-y += $(KVM)/arm/vgic/vgic-mmio-v2.o
    > > obj-y += $(KVM)/arm/vgic/vgic-mmio-v3.o
    > > diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
    > > index 5d9810086c25..c30fd388ef80 100644
    > > --- a/arch/arm64/kvm/Makefile
    > > +++ b/arch/arm64/kvm/Makefile
    > > @@ -26,6 +26,7 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-init.o
    > > kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-irqfd.o
    > > kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-v2.o
    > > kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-v3.o
    > > +kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-v4.o
    > > kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio.o
    > > kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio-v2.o
    > > kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio-v3.o
    > > diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
    > > index ba9fb450aa1b..7eeb6c2a2f9c 100644
    > > --- a/include/kvm/arm_vgic.h
    > > +++ b/include/kvm/arm_vgic.h
    > > @@ -26,6 +26,8 @@
    > > #include <linux/list.h>
    > > #include <linux/jump_label.h>
    > >
    > > +#include <linux/irqchip/arm-gic-v4.h>
    > > +
    > > #define VGIC_V3_MAX_CPUS 255
    > > #define VGIC_V2_MAX_CPUS 8
    > > #define VGIC_NR_IRQS_LEGACY 256
    > > @@ -236,6 +238,15 @@ struct vgic_dist {
    > >
    > > /* used by vgic-debug */
    > > struct vgic_state_iter *iter;
    > > +
    > > + /*
    > > + * GICv4 ITS per-VM data, containing the IRQ domain, the VPE
    > > + * array, the property table pointer as well as allocation
    > > + * data. This essentially ties the Linux IRQ core and ITS
    > > + * together, and avoids leaking KVM's data structures anywhere
    > > + * else.
    > > + */
    > > + struct its_vm its_vm;
    > > };
    > >
    > > struct vgic_v2_cpu_if {
    > > @@ -254,6 +265,14 @@ struct vgic_v3_cpu_if {
    > > u32 vgic_ap0r[4];
    > > u32 vgic_ap1r[4];
    > > u64 vgic_lr[VGIC_V3_MAX_LRS];
    > > +
    > > + /*
    > > + * GICv4 ITS per-VPE data, containing the doorbell IRQ, the
    > > + * pending table pointer, the its_vm pointer and a few other
    > > + * HW specific things. As for the its_vm structure, this is
    > > + * linking the Linux IRQ subsystem and the ITS together.
    > > + */
    > > + struct its_vpe its_vpe;
    > > };
    > >
    > > struct vgic_cpu {
    > > diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
    > > index 5801261f3add..40be908da238 100644
    > > --- a/virt/kvm/arm/vgic/vgic-init.c
    > > +++ b/virt/kvm/arm/vgic/vgic-init.c
    > > @@ -285,6 +285,12 @@ int vgic_init(struct kvm *kvm)
    > > if (ret)
    > > goto out;
    > >
    > > + if (vgic_supports_direct_msis(kvm)) {
    > > + ret = vgic_v4_init(kvm);
    > > + if (ret)
    > > + goto out;
    > > + }
    > > +
    > > kvm_for_each_vcpu(i, vcpu, kvm)
    > > kvm_vgic_vcpu_enable(vcpu);
    > >
    > > @@ -320,6 +326,9 @@ static void kvm_vgic_dist_destroy(struct kvm *kvm)
    > >
    > > kfree(dist->spis);
    > > dist->nr_spis = 0;
    > > +
    > > + if (vgic_supports_direct_msis(kvm))
    > > + vgic_v4_teardown(kvm);
    > > }
    > >
    > > void kvm_vgic_vcpu_destroy(struct kvm_vcpu *vcpu)
    > > diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
    > > index 8ee03f1e89fc..89768d2b6a91 100644
    > > --- a/virt/kvm/arm/vgic/vgic-its.c
    > > +++ b/virt/kvm/arm/vgic/vgic-its.c
    > > @@ -1603,6 +1603,14 @@ static int vgic_its_create(struct kvm_device *dev, u32 type)
    > > if (!its)
    > > return -ENOMEM;
    > >
    > > + if (vgic_initialized(dev->kvm)) {
    > Don't we need to test vgic_supports_direct_msis() on this path too?
    >

    Seems to me that we should, otherwise creating an ITS after the VGIC has
    been initialized would fail on non-GICv4 compatible systems, right?

    How about this patch as a follow-up to the series:

    commit 48ec1662d0f10d6468907cdc7e12c46ca1ef497c (HEAD -> next-gicv4)
    Author: Christoffer Dall <christoffer.dall@linaro.org>
    Date: Fri Nov 10 09:16:23 2017 +0100

    KVM: arm/arm64: Fix GICv4 ITS initialization issues

    We should only try to initialize GICv4 data structures on a GICv4
    capable system. Move the vgic_supports_direct_msis() check inito
    vgic_v4_init() so that any KVM VGIC initialization path does not fail
    on non-GICv4 systems.

    Also be slightly more strict in the checking of the return value in
    vgic_its_create, and only error out on negative return values from the
    vgic_v4_init() function. This is important because the kvm device code
    only treats negative values as errors and only cleans up in this case.
    Errornously treating a positive return value as an error from the
    vgic_v4_init() function can lead to NULL pointer dereferences, as has
    recently been observed.

    Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>

    diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
    index 40be908da238..62310122ee78 100644
    --- a/virt/kvm/arm/vgic/vgic-init.c
    +++ b/virt/kvm/arm/vgic/vgic-init.c
    @@ -285,11 +285,9 @@ int vgic_init(struct kvm *kvm)
    if (ret)
    goto out;

    - if (vgic_supports_direct_msis(kvm)) {
    - ret = vgic_v4_init(kvm);
    - if (ret)
    - goto out;
    - }
    + ret = vgic_v4_init(kvm);
    + if (ret)
    + goto out;

    kvm_for_each_vcpu(i, vcpu, kvm)
    kvm_vgic_vcpu_enable(vcpu);
    diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
    index b8c1b724ba3e..c93ecd4a903b 100644
    --- a/virt/kvm/arm/vgic/vgic-its.c
    +++ b/virt/kvm/arm/vgic/vgic-its.c
    @@ -1673,7 +1673,7 @@ static int vgic_its_create(struct kvm_device *dev, u32 type)

    if (vgic_initialized(dev->kvm)) {
    int ret = vgic_v4_init(dev->kvm);
    - if (ret) {
    + if (ret < 0) {
    kfree(its);
    return ret;
    }
    diff --git a/virt/kvm/arm/vgic/vgic-v4.c b/virt/kvm/arm/vgic/vgic-v4.c
    index e367d65a0ebe..bb7e31fcee35 100644
    --- a/virt/kvm/arm/vgic/vgic-v4.c
    +++ b/virt/kvm/arm/vgic/vgic-v4.c
    @@ -118,6 +118,9 @@ int vgic_v4_init(struct kvm *kvm)
    struct kvm_vcpu *vcpu;
    int i, nr_vcpus, ret;

    + if (!vgic_supports_direct_msis(kvm))
    + return 0; /* Nothing to see here... move along. */
    +
    if (dist->its_vm.vpes)
    return 0;

    \
     
     \ /
      Last update: 2017-11-10 09:21    [W:2.336 / U:0.148 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site