* [RFC PATCH 0/2] Fix doorbell emulation for nested KVM guests in V1 API @ 2024-06-27 18:03 Gautam Menghani 2024-06-27 18:03 ` [RFC PATCH 1/2] Revert "KVM: PPC: Book3S HV Nested: Stop forwarding all HFUs to L1" Gautam Menghani 2024-06-27 18:03 ` [RFC PATCH 2/2] arch/powerpc/kvm: Fix doorbells for nested KVM guests on PowerNV Gautam Menghani 0 siblings, 2 replies; 6+ messages in thread From: Gautam Menghani @ 2024-06-27 18:03 UTC (permalink / raw) To: mpe, npiggin, christophe.leroy, naveen.n.rao Cc: Gautam Menghani, linuxppc-dev, linux-kernel, kvm Doorbell emulation for nested KVM guests in V1 API is broken because of 2 reasons: 1. L0 presenting H_EMUL_ASSIST to L1 instead of H_FAC_UNAVAIL 2. Broken plumbing for passing around doorbell state. Fix the trap passed to L1 and the plumbing for maintaining doorbell state. Gautam Menghani (2): Revert "KVM: PPC: Book3S HV Nested: Stop forwarding all HFUs to L1" arch/powerpc/kvm: Fix doorbells for nested KVM guests on PowerNV arch/powerpc/kvm/book3s_hv.c | 40 ++++++++--------------------- arch/powerpc/kvm/book3s_hv_nested.c | 20 ++++++++++++--- 2 files changed, 26 insertions(+), 34 deletions(-) -- 2.45.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC PATCH 1/2] Revert "KVM: PPC: Book3S HV Nested: Stop forwarding all HFUs to L1" 2024-06-27 18:03 [RFC PATCH 0/2] Fix doorbell emulation for nested KVM guests in V1 API Gautam Menghani @ 2024-06-27 18:03 ` Gautam Menghani 2024-07-04 12:29 ` Nicholas Piggin 2024-06-27 18:03 ` [RFC PATCH 2/2] arch/powerpc/kvm: Fix doorbells for nested KVM guests on PowerNV Gautam Menghani 1 sibling, 1 reply; 6+ messages in thread From: Gautam Menghani @ 2024-06-27 18:03 UTC (permalink / raw) To: mpe, npiggin, christophe.leroy, naveen.n.rao Cc: Gautam Menghani, linuxppc-dev, linux-kernel, kvm This reverts commit 7c3ded5735141ff4d049747c9f76672a8b737c49. On PowerNV, when a nested guest tries to use a feature prohibited by HFSCR, the nested hypervisor (L1) should get a H_FAC_UNAVAILABLE trap and then L1 can emulate the feature. But with the change introduced by commit 7c3ded573514 ("KVM: PPC: Book3S HV Nested: Stop forwarding all HFUs to L1") the L1 ends up getting a H_EMUL_ASSIST because of which, the L1 ends up injecting a SIGILL when L2 (nested guest) tries to use doorbells. Signed-off-by: Gautam Menghani <gautam@linux.ibm.com> --- arch/powerpc/kvm/book3s_hv.c | 31 ++----------------------------- 1 file changed, 2 insertions(+), 29 deletions(-) diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c index daaf7faf21a5..cea28ac05923 100644 --- a/arch/powerpc/kvm/book3s_hv.c +++ b/arch/powerpc/kvm/book3s_hv.c @@ -2052,36 +2052,9 @@ static int kvmppc_handle_nested_exit(struct kvm_vcpu *vcpu) fallthrough; /* go to facility unavailable handler */ #endif - case BOOK3S_INTERRUPT_H_FAC_UNAVAIL: { - u64 cause = vcpu->arch.hfscr >> 56; - - /* - * Only pass HFU interrupts to the L1 if the facility is - * permitted but disabled by the L1's HFSCR, otherwise - * the interrupt does not make sense to the L1 so turn - * it into a HEAI. - */ - if (!(vcpu->arch.hfscr_permitted & (1UL << cause)) || - (vcpu->arch.nested_hfscr & (1UL << cause))) { - ppc_inst_t pinst; - vcpu->arch.trap = BOOK3S_INTERRUPT_H_EMUL_ASSIST; - - /* - * If the fetch failed, return to guest and - * try executing it again. - */ - r = kvmppc_get_last_inst(vcpu, INST_GENERIC, &pinst); - vcpu->arch.emul_inst = ppc_inst_val(pinst); - if (r != EMULATE_DONE) - r = RESUME_GUEST; - else - r = RESUME_HOST; - } else { - r = RESUME_HOST; - } - + case BOOK3S_INTERRUPT_H_FAC_UNAVAIL: + r = RESUME_HOST; break; - } case BOOK3S_INTERRUPT_HV_RM_HARD: vcpu->arch.trap = 0; -- 2.45.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 1/2] Revert "KVM: PPC: Book3S HV Nested: Stop forwarding all HFUs to L1" 2024-06-27 18:03 ` [RFC PATCH 1/2] Revert "KVM: PPC: Book3S HV Nested: Stop forwarding all HFUs to L1" Gautam Menghani @ 2024-07-04 12:29 ` Nicholas Piggin 0 siblings, 0 replies; 6+ messages in thread From: Nicholas Piggin @ 2024-07-04 12:29 UTC (permalink / raw) To: Gautam Menghani, mpe, christophe.leroy, naveen.n.rao Cc: linuxppc-dev, linux-kernel, kvm On Fri Jun 28, 2024 at 4:03 AM AEST, Gautam Menghani wrote: > This reverts commit 7c3ded5735141ff4d049747c9f76672a8b737c49. > > On PowerNV, when a nested guest tries to use a feature prohibited by > HFSCR, the nested hypervisor (L1) should get a H_FAC_UNAVAILABLE trap > and then L1 can emulate the feature. But with the change introduced by > commit 7c3ded573514 ("KVM: PPC: Book3S HV Nested: Stop forwarding all HFUs to L1") > the L1 ends up getting a H_EMUL_ASSIST because of which, the L1 ends up > injecting a SIGILL when L2 (nested guest) tries to use doorbells. Yeah, we struggled to come up with a coherent story for this kind of compatibility and mismatched feature handling between L0 and L1. The L1 doorbell emulation shows a legitimate case the L1 wants to see the HFAC to emulate it and the L0 does not permit the L1 to set it for the L2. Actually the L0 could just permit it (even if the L0 wanted to emulate doorbells for the L1, it could still allow the L2 to run with doorbells if that's what the L1 asked for). That would also solve this problem, but there is a potential future hardware change where doorbells will be able to address any thread in the core even in "LPAR-per-thread" mode and the hypervisor *must* disable the doorbell HFSCR to the guest if it runs in KVM style that schedules LPARs on a per-thread basis instead of per-core. In that case the L0 must not permit the L2 to run with HFSCR set. So this approach actually works better there. In other cases where the L0 might deliberately prohibit some facility in a way that we don't want the L1 to see HFAC. I think we just cross that bridge when it comes. I'm sure the L0 would really need to advertise that to the L1 properly via device-tree or similar, and we could special case the HFAC->HEAI if necessary then. Reviewed-by: Nicholas Piggin <npiggin@gmail.com> > > Signed-off-by: Gautam Menghani <gautam@linux.ibm.com> > --- > arch/powerpc/kvm/book3s_hv.c | 31 ++----------------------------- > 1 file changed, 2 insertions(+), 29 deletions(-) > > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c > index daaf7faf21a5..cea28ac05923 100644 > --- a/arch/powerpc/kvm/book3s_hv.c > +++ b/arch/powerpc/kvm/book3s_hv.c > @@ -2052,36 +2052,9 @@ static int kvmppc_handle_nested_exit(struct kvm_vcpu *vcpu) > fallthrough; /* go to facility unavailable handler */ > #endif > > - case BOOK3S_INTERRUPT_H_FAC_UNAVAIL: { > - u64 cause = vcpu->arch.hfscr >> 56; > - > - /* > - * Only pass HFU interrupts to the L1 if the facility is > - * permitted but disabled by the L1's HFSCR, otherwise > - * the interrupt does not make sense to the L1 so turn > - * it into a HEAI. > - */ > - if (!(vcpu->arch.hfscr_permitted & (1UL << cause)) || > - (vcpu->arch.nested_hfscr & (1UL << cause))) { > - ppc_inst_t pinst; > - vcpu->arch.trap = BOOK3S_INTERRUPT_H_EMUL_ASSIST; > - > - /* > - * If the fetch failed, return to guest and > - * try executing it again. > - */ > - r = kvmppc_get_last_inst(vcpu, INST_GENERIC, &pinst); > - vcpu->arch.emul_inst = ppc_inst_val(pinst); > - if (r != EMULATE_DONE) > - r = RESUME_GUEST; > - else > - r = RESUME_HOST; > - } else { > - r = RESUME_HOST; > - } > - > + case BOOK3S_INTERRUPT_H_FAC_UNAVAIL: > + r = RESUME_HOST; > break; > - } > > case BOOK3S_INTERRUPT_HV_RM_HARD: > vcpu->arch.trap = 0; ^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC PATCH 2/2] arch/powerpc/kvm: Fix doorbells for nested KVM guests on PowerNV 2024-06-27 18:03 [RFC PATCH 0/2] Fix doorbell emulation for nested KVM guests in V1 API Gautam Menghani 2024-06-27 18:03 ` [RFC PATCH 1/2] Revert "KVM: PPC: Book3S HV Nested: Stop forwarding all HFUs to L1" Gautam Menghani @ 2024-06-27 18:03 ` Gautam Menghani 2024-07-04 12:10 ` Nicholas Piggin 1 sibling, 1 reply; 6+ messages in thread From: Gautam Menghani @ 2024-06-27 18:03 UTC (permalink / raw) To: mpe, npiggin, christophe.leroy, naveen.n.rao Cc: Gautam Menghani, linuxppc-dev, linux-kernel, kvm commit 6398326b9ba1("KVM: PPC: Book3S HV P9: Stop using vc->dpdes") introduced an optimization to use only vcpu->doorbell_request for SMT emulation for Power9 and above guests, but the code for nested guests still relies on the old way of handling doorbells, due to which an L2 guest cannot be booted with XICS with SMT>1. The command to repro this issue is: qemu-system-ppc64 \ -drive file=rhel.qcow2,format=qcow2 \ -m 20G \ -smp 8,cores=1,threads=8 \ -cpu host \ -nographic \ -machine pseries,ic-mode=xics -accel kvm Fix the plumbing to utilize vcpu->doorbell_request instead of vcore->dpdes on P9 and above. Fixes: 6398326b9ba1 ("KVM: PPC: Book3S HV P9: Stop using vc->dpdes") Signed-off-by: Gautam Menghani <gautam@linux.ibm.com> --- arch/powerpc/kvm/book3s_hv.c | 9 ++++++++- arch/powerpc/kvm/book3s_hv_nested.c | 20 ++++++++++++++++---- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c index cea28ac05923..0586fa636707 100644 --- a/arch/powerpc/kvm/book3s_hv.c +++ b/arch/powerpc/kvm/book3s_hv.c @@ -4178,6 +4178,9 @@ static int kvmhv_vcpu_entry_p9_nested(struct kvm_vcpu *vcpu, u64 time_limit, uns } hvregs.hdec_expiry = time_limit; + // clear doorbell bit as hvregs already has the info + vcpu->arch.doorbell_request = 0; + /* * When setting DEC, we must always deal with irq_work_raise * via NMI vs setting DEC. The problem occurs right as we @@ -4694,6 +4697,7 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu, u64 time_limit, struct kvm_nested_guest *nested = vcpu->arch.nested; unsigned long flags; u64 tb; + bool doorbell_pending; trace_kvmppc_run_vcpu_enter(vcpu); @@ -4752,6 +4756,9 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu, u64 time_limit, */ smp_mb(); + doorbell_pending = !cpu_has_feature(CPU_FTR_ARCH_300) && + vcpu->arch.doorbell_request; + if (!nested) { kvmppc_core_prepare_to_enter(vcpu); if (test_bit(BOOK3S_IRQPRIO_EXTERNAL, @@ -4769,7 +4776,7 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu, u64 time_limit, lpcr |= LPCR_MER; } } else if (vcpu->arch.pending_exceptions || - vcpu->arch.doorbell_request || + doorbell_pending || xive_interrupt_pending(vcpu)) { vcpu->arch.ret = RESUME_HOST; goto out; diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c index 05f5220960c6..b34eefa6b268 100644 --- a/arch/powerpc/kvm/book3s_hv_nested.c +++ b/arch/powerpc/kvm/book3s_hv_nested.c @@ -32,7 +32,10 @@ void kvmhv_save_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr) struct kvmppc_vcore *vc = vcpu->arch.vcore; hr->pcr = vc->pcr | PCR_MASK; - hr->dpdes = vc->dpdes; + if (cpu_has_feature(CPU_FTR_ARCH_300)) + hr->dpdes = vcpu->arch.doorbell_request; + else + hr->dpdes = vc->dpdes; hr->hfscr = vcpu->arch.hfscr; hr->tb_offset = vc->tb_offset; hr->dawr0 = vcpu->arch.dawr0; @@ -105,7 +108,10 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu, { struct kvmppc_vcore *vc = vcpu->arch.vcore; - hr->dpdes = vc->dpdes; + if (cpu_has_feature(CPU_FTR_ARCH_300)) + hr->dpdes = vcpu->arch.doorbell_request; + else + hr->dpdes = vc->dpdes; hr->purr = vcpu->arch.purr; hr->spurr = vcpu->arch.spurr; hr->ic = vcpu->arch.ic; @@ -143,7 +149,10 @@ static void restore_hv_regs(struct kvm_vcpu *vcpu, const struct hv_guest_state * struct kvmppc_vcore *vc = vcpu->arch.vcore; vc->pcr = hr->pcr | PCR_MASK; - vc->dpdes = hr->dpdes; + if (cpu_has_feature(CPU_FTR_ARCH_300)) + vcpu->arch.doorbell_request = hr->dpdes; + else + vc->dpdes = hr->dpdes; vcpu->arch.hfscr = hr->hfscr; vcpu->arch.dawr0 = hr->dawr0; vcpu->arch.dawrx0 = hr->dawrx0; @@ -170,7 +179,10 @@ void kvmhv_restore_hv_return_state(struct kvm_vcpu *vcpu, { struct kvmppc_vcore *vc = vcpu->arch.vcore; - vc->dpdes = hr->dpdes; + if (cpu_has_feature(CPU_FTR_ARCH_300) && !vcpu->arch.doorbell_request) + vcpu->arch.doorbell_request = hr->dpdes; + else + vc->dpdes = hr->dpdes; vcpu->arch.hfscr = hr->hfscr; vcpu->arch.purr = hr->purr; vcpu->arch.spurr = hr->spurr; -- 2.45.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 2/2] arch/powerpc/kvm: Fix doorbells for nested KVM guests on PowerNV 2024-06-27 18:03 ` [RFC PATCH 2/2] arch/powerpc/kvm: Fix doorbells for nested KVM guests on PowerNV Gautam Menghani @ 2024-07-04 12:10 ` Nicholas Piggin 2024-10-30 13:33 ` Gautam Menghani 0 siblings, 1 reply; 6+ messages in thread From: Nicholas Piggin @ 2024-07-04 12:10 UTC (permalink / raw) To: Gautam Menghani, mpe, christophe.leroy, naveen.n.rao Cc: linuxppc-dev, linux-kernel, kvm On Fri Jun 28, 2024 at 4:03 AM AEST, Gautam Menghani wrote: > commit 6398326b9ba1("KVM: PPC: Book3S HV P9: Stop using vc->dpdes") > introduced an optimization to use only vcpu->doorbell_request for SMT > emulation for Power9 and above guests, but the code for nested guests > still relies on the old way of handling doorbells, due to which an L2 > guest cannot be booted with XICS with SMT>1. The command to repro > this issue is: > > qemu-system-ppc64 \ > -drive file=rhel.qcow2,format=qcow2 \ > -m 20G \ > -smp 8,cores=1,threads=8 \ > -cpu host \ > -nographic \ > -machine pseries,ic-mode=xics -accel kvm > > Fix the plumbing to utilize vcpu->doorbell_request instead of vcore->dpdes > on P9 and above. > > Fixes: 6398326b9ba1 ("KVM: PPC: Book3S HV P9: Stop using vc->dpdes") > Signed-off-by: Gautam Menghani <gautam@linux.ibm.com> > --- > arch/powerpc/kvm/book3s_hv.c | 9 ++++++++- > arch/powerpc/kvm/book3s_hv_nested.c | 20 ++++++++++++++++---- > 2 files changed, 24 insertions(+), 5 deletions(-) > > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c > index cea28ac05923..0586fa636707 100644 > --- a/arch/powerpc/kvm/book3s_hv.c > +++ b/arch/powerpc/kvm/book3s_hv.c > @@ -4178,6 +4178,9 @@ static int kvmhv_vcpu_entry_p9_nested(struct kvm_vcpu *vcpu, u64 time_limit, uns > } > hvregs.hdec_expiry = time_limit; > > + // clear doorbell bit as hvregs already has the info > + vcpu->arch.doorbell_request = 0; > + > /* > * When setting DEC, we must always deal with irq_work_raise > * via NMI vs setting DEC. The problem occurs right as we > @@ -4694,6 +4697,7 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu, u64 time_limit, > struct kvm_nested_guest *nested = vcpu->arch.nested; > unsigned long flags; > u64 tb; > + bool doorbell_pending; > > trace_kvmppc_run_vcpu_enter(vcpu); > > @@ -4752,6 +4756,9 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu, u64 time_limit, > */ > smp_mb(); > > + doorbell_pending = !cpu_has_feature(CPU_FTR_ARCH_300) && > + vcpu->arch.doorbell_request; Hmm... is the feature test flipped here? > + > if (!nested) { > kvmppc_core_prepare_to_enter(vcpu); > if (test_bit(BOOK3S_IRQPRIO_EXTERNAL, > @@ -4769,7 +4776,7 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu, u64 time_limit, > lpcr |= LPCR_MER; > } > } else if (vcpu->arch.pending_exceptions || > - vcpu->arch.doorbell_request || > + doorbell_pending || > xive_interrupt_pending(vcpu)) { > vcpu->arch.ret = RESUME_HOST; > goto out; > diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c > index 05f5220960c6..b34eefa6b268 100644 > --- a/arch/powerpc/kvm/book3s_hv_nested.c > +++ b/arch/powerpc/kvm/book3s_hv_nested.c > @@ -32,7 +32,10 @@ void kvmhv_save_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr) > struct kvmppc_vcore *vc = vcpu->arch.vcore; > > hr->pcr = vc->pcr | PCR_MASK; > - hr->dpdes = vc->dpdes; > + if (cpu_has_feature(CPU_FTR_ARCH_300)) > + hr->dpdes = vcpu->arch.doorbell_request; > + else > + hr->dpdes = vc->dpdes; > hr->hfscr = vcpu->arch.hfscr; > hr->tb_offset = vc->tb_offset; > hr->dawr0 = vcpu->arch.dawr0; Great find. Nested is all POWER9 and later only, so I think you can just change to using doorbell_request always. And probably don't have to do anything for book3s_hv.c unless I'm mistaken about the feature test. Thanks, Nick > @@ -105,7 +108,10 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu, > { > struct kvmppc_vcore *vc = vcpu->arch.vcore; > > - hr->dpdes = vc->dpdes; > + if (cpu_has_feature(CPU_FTR_ARCH_300)) > + hr->dpdes = vcpu->arch.doorbell_request; > + else > + hr->dpdes = vc->dpdes; > hr->purr = vcpu->arch.purr; > hr->spurr = vcpu->arch.spurr; > hr->ic = vcpu->arch.ic; > @@ -143,7 +149,10 @@ static void restore_hv_regs(struct kvm_vcpu *vcpu, const struct hv_guest_state * > struct kvmppc_vcore *vc = vcpu->arch.vcore; > > vc->pcr = hr->pcr | PCR_MASK; > - vc->dpdes = hr->dpdes; > + if (cpu_has_feature(CPU_FTR_ARCH_300)) > + vcpu->arch.doorbell_request = hr->dpdes; > + else > + vc->dpdes = hr->dpdes; > vcpu->arch.hfscr = hr->hfscr; > vcpu->arch.dawr0 = hr->dawr0; > vcpu->arch.dawrx0 = hr->dawrx0; > @@ -170,7 +179,10 @@ void kvmhv_restore_hv_return_state(struct kvm_vcpu *vcpu, > { > struct kvmppc_vcore *vc = vcpu->arch.vcore; > > - vc->dpdes = hr->dpdes; > + if (cpu_has_feature(CPU_FTR_ARCH_300) && !vcpu->arch.doorbell_request) > + vcpu->arch.doorbell_request = hr->dpdes; > + else > + vc->dpdes = hr->dpdes; > vcpu->arch.hfscr = hr->hfscr; > vcpu->arch.purr = hr->purr; > vcpu->arch.spurr = hr->spurr; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 2/2] arch/powerpc/kvm: Fix doorbells for nested KVM guests on PowerNV 2024-07-04 12:10 ` Nicholas Piggin @ 2024-10-30 13:33 ` Gautam Menghani 0 siblings, 0 replies; 6+ messages in thread From: Gautam Menghani @ 2024-10-30 13:33 UTC (permalink / raw) To: Nicholas Piggin Cc: mpe, christophe.leroy, naveen.n.rao, linuxppc-dev, kvm, linux-kernel On Thu, Jul 04, 2024 at 10:10:05PM +1000, Nicholas Piggin wrote: > On Fri Jun 28, 2024 at 4:03 AM AEST, Gautam Menghani wrote: > > commit 6398326b9ba1("KVM: PPC: Book3S HV P9: Stop using vc->dpdes") > > introduced an optimization to use only vcpu->doorbell_request for SMT > > emulation for Power9 and above guests, but the code for nested guests > > still relies on the old way of handling doorbells, due to which an L2 > > guest cannot be booted with XICS with SMT>1. The command to repro > > this issue is: > > > > qemu-system-ppc64 \ > > -drive file=rhel.qcow2,format=qcow2 \ > > -m 20G \ > > -smp 8,cores=1,threads=8 \ > > -cpu host \ > > -nographic \ > > -machine pseries,ic-mode=xics -accel kvm > > > > Fix the plumbing to utilize vcpu->doorbell_request instead of vcore->dpdes > > on P9 and above. > > > > Fixes: 6398326b9ba1 ("KVM: PPC: Book3S HV P9: Stop using vc->dpdes") > > Signed-off-by: Gautam Menghani <gautam@linux.ibm.com> > > --- > > arch/powerpc/kvm/book3s_hv.c | 9 ++++++++- > > arch/powerpc/kvm/book3s_hv_nested.c | 20 ++++++++++++++++---- > > 2 files changed, 24 insertions(+), 5 deletions(-) > > > > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c > > index cea28ac05923..0586fa636707 100644 > > --- a/arch/powerpc/kvm/book3s_hv.c > > +++ b/arch/powerpc/kvm/book3s_hv.c > > @@ -4178,6 +4178,9 @@ static int kvmhv_vcpu_entry_p9_nested(struct kvm_vcpu *vcpu, u64 time_limit, uns > > } > > hvregs.hdec_expiry = time_limit; > > > > + // clear doorbell bit as hvregs already has the info > > + vcpu->arch.doorbell_request = 0; > > + > > /* > > * When setting DEC, we must always deal with irq_work_raise > > * via NMI vs setting DEC. The problem occurs right as we > > @@ -4694,6 +4697,7 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu, u64 time_limit, > > struct kvm_nested_guest *nested = vcpu->arch.nested; > > unsigned long flags; > > u64 tb; > > + bool doorbell_pending; > > > > trace_kvmppc_run_vcpu_enter(vcpu); > > > > @@ -4752,6 +4756,9 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu, u64 time_limit, > > */ > > smp_mb(); > > > > + doorbell_pending = !cpu_has_feature(CPU_FTR_ARCH_300) && > > + vcpu->arch.doorbell_request; > > Hmm... is the feature test flipped here? Sorry for responding late, I got involved in some other things. Yes I think I got that part wrong, I guess it should've been doorbell_pending = !cpu_has_feature(CPU_FTR_HVMODE) && vcpu->arch.doorbell_request; The objective of introducing this is to avoid returning to L1 midway when L0 is about to run L2. The issue is that if L1 does H_ENTER_NESTED and there is a doorbell for L2, this condition in kvmhv_run_single_vcpu will cause L0 to abort and go back to L1: } else if (vcpu->arch.pending_exceptions || vcpu->arch.doorbell_request || xive_interrupt_pending(vcpu)) { vcpu->arch.ret = RESUME_HOST; goto out; } Earlier, vc->dpdes was used to pass around doorbell state, that's why this condition did not cause problems, until 6398326b9ba1 ("KVM: PPC: Book3S HV P9: Stop using vc->dpdes") > > > + > > if (!nested) { > > kvmppc_core_prepare_to_enter(vcpu); > > if (test_bit(BOOK3S_IRQPRIO_EXTERNAL, > > @@ -4769,7 +4776,7 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu, u64 time_limit, > > lpcr |= LPCR_MER; > > } > > } else if (vcpu->arch.pending_exceptions || > > - vcpu->arch.doorbell_request || > > + doorbell_pending || > > xive_interrupt_pending(vcpu)) { > > vcpu->arch.ret = RESUME_HOST; > > goto out; > > diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c > > index 05f5220960c6..b34eefa6b268 100644 > > --- a/arch/powerpc/kvm/book3s_hv_nested.c > > +++ b/arch/powerpc/kvm/book3s_hv_nested.c > > @@ -32,7 +32,10 @@ void kvmhv_save_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr) > > struct kvmppc_vcore *vc = vcpu->arch.vcore; > > > > hr->pcr = vc->pcr | PCR_MASK; > > - hr->dpdes = vc->dpdes; > > + if (cpu_has_feature(CPU_FTR_ARCH_300)) > > + hr->dpdes = vcpu->arch.doorbell_request; > > + else > > + hr->dpdes = vc->dpdes; > > hr->hfscr = vcpu->arch.hfscr; > > hr->tb_offset = vc->tb_offset; > > hr->dawr0 = vcpu->arch.dawr0; > > Great find. > > Nested is all POWER9 and later only, so I think you can just > change to using doorbell_request always. Noted. > > And probably don't have to do anything for book3s_hv.c unless > I'm mistaken about the feature test. > As pointed out above, the intention was to avoid the "else if" part in kvmhv_run_single_vcpu(). Please do point out if I missed something here. Thanks, Gautam ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-10-30 13:33 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-06-27 18:03 [RFC PATCH 0/2] Fix doorbell emulation for nested KVM guests in V1 API Gautam Menghani 2024-06-27 18:03 ` [RFC PATCH 1/2] Revert "KVM: PPC: Book3S HV Nested: Stop forwarding all HFUs to L1" Gautam Menghani 2024-07-04 12:29 ` Nicholas Piggin 2024-06-27 18:03 ` [RFC PATCH 2/2] arch/powerpc/kvm: Fix doorbells for nested KVM guests on PowerNV Gautam Menghani 2024-07-04 12:10 ` Nicholas Piggin 2024-10-30 13:33 ` Gautam Menghani
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).