Message ID | 1688720145-37923-1-git-send-email-chenxiang66@hisilicon.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | KVM: arm64: Keep need_db always true in vgic_v4_put() when emulating WFI | expand |
Hi Xiang, Thanks for reporting this. On Fri, Jul 07, 2023 at 04:55:45PM +0800, chenxiang wrote: > When enabled GICv4.1 on Kunpeng 920 platform with 6.4 kernel (preemptible > kernel), running a vm with 128 vcpus on 64 pcpu, sometimes vm is not booted > successfully, and we find there is a situation that doorbell interrupt will > not occur event if there is a pending interrupt. Oh, that's no good. > --- > arch/arm64/kvm/vgic/vgic-v4.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/arch/arm64/kvm/vgic/vgic-v4.c b/arch/arm64/kvm/vgic/vgic-v4.c > index c1c28fe..37152cf8 100644 > --- a/arch/arm64/kvm/vgic/vgic-v4.c > +++ b/arch/arm64/kvm/vgic/vgic-v4.c > @@ -343,6 +343,9 @@ int vgic_v4_put(struct kvm_vcpu *vcpu, bool need_db) > if (!vgic_supports_direct_msis(vcpu->kvm) || !vpe->resident) > return 0; > > + if (vcpu->stat.generic.blocking == 1) > + need_db = true; > + As I understand it, the issue really comes from the fact that @need_db is always false when called from vgic_v3_put(). I'd rather we not override the argument, as that could have unintended consequences in the future. You could also use the helper we already have for determining if a vCPU is blocking, which we could use as a hint for requesting a doorbell interrupt on sched out. Putting the two comments together, I arrived at the following (untested) diff. I don't have a GICv4 system on hand, sadly. Mind taking it for a spin? -- Thanks, Oliver diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c index 93a47a515c13..8c743643b122 100644 --- a/arch/arm64/kvm/vgic/vgic-v3.c +++ b/arch/arm64/kvm/vgic/vgic-v3.c @@ -756,7 +756,7 @@ void vgic_v3_put(struct kvm_vcpu *vcpu) { struct vgic_v3_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v3; - WARN_ON(vgic_v4_put(vcpu, false)); + WARN_ON(vgic_v4_put(vcpu, kvm_vcpu_is_blocking(vcpu))); vgic_v3_vmcr_sync(vcpu);
Hi Oliver, 在 2023/7/8 星期六 1:16, Oliver Upton 写道: > Hi Xiang, > > Thanks for reporting this. > > On Fri, Jul 07, 2023 at 04:55:45PM +0800, chenxiang wrote: >> When enabled GICv4.1 on Kunpeng 920 platform with 6.4 kernel (preemptible >> kernel), running a vm with 128 vcpus on 64 pcpu, sometimes vm is not booted >> successfully, and we find there is a situation that doorbell interrupt will >> not occur event if there is a pending interrupt. > Oh, that's no good. > >> --- >> arch/arm64/kvm/vgic/vgic-v4.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/arch/arm64/kvm/vgic/vgic-v4.c b/arch/arm64/kvm/vgic/vgic-v4.c >> index c1c28fe..37152cf8 100644 >> --- a/arch/arm64/kvm/vgic/vgic-v4.c >> +++ b/arch/arm64/kvm/vgic/vgic-v4.c >> @@ -343,6 +343,9 @@ int vgic_v4_put(struct kvm_vcpu *vcpu, bool need_db) >> if (!vgic_supports_direct_msis(vcpu->kvm) || !vpe->resident) >> return 0; >> >> + if (vcpu->stat.generic.blocking == 1) >> + need_db = true; >> + > As I understand it, the issue really comes from the fact that @need_db > is always false when called from vgic_v3_put(). I'd rather we not > override the argument, as that could have unintended consequences in the > future. > > You could also use the helper we already have for determining if a vCPU > is blocking, which we could use as a hint for requesting a doorbell > interrupt on sched out. > > Putting the two comments together, I arrived at the following (untested) > diff. I don't have a GICv4 system on hand, sadly. Mind taking it for a > spin? Right, it is more suitable to use the helper we already have for determining if a vCPU is blocking. We have tested the following diff you provide, and it solves the issue, so please feel free to add for the change: Tested-by: Xiang Chen <chenxiang66@hisilicon.com> > > -- > Thanks, > Oliver > > diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c > index 93a47a515c13..8c743643b122 100644 > --- a/arch/arm64/kvm/vgic/vgic-v3.c > +++ b/arch/arm64/kvm/vgic/vgic-v3.c > @@ -756,7 +756,7 @@ void vgic_v3_put(struct kvm_vcpu *vcpu) > { > struct vgic_v3_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v3; > > - WARN_ON(vgic_v4_put(vcpu, false)); > + WARN_ON(vgic_v4_put(vcpu, kvm_vcpu_is_blocking(vcpu))); > > vgic_v3_vmcr_sync(vcpu); > > . >
diff --git a/arch/arm64/kvm/vgic/vgic-v4.c b/arch/arm64/kvm/vgic/vgic-v4.c index c1c28fe..37152cf8 100644 --- a/arch/arm64/kvm/vgic/vgic-v4.c +++ b/arch/arm64/kvm/vgic/vgic-v4.c @@ -343,6 +343,9 @@ int vgic_v4_put(struct kvm_vcpu *vcpu, bool need_db) if (!vgic_supports_direct_msis(vcpu->kvm) || !vpe->resident) return 0; + if (vcpu->stat.generic.blocking == 1) + need_db = true; + return its_make_vpe_non_resident(vpe, need_db); }