Excerpts from Cédric Le Goater's message of April 3, 2021 2:32 am: > On 4/1/21 5:03 PM, Nicholas Piggin wrote: >> Commit f3c18e9342a44 ("KVM: PPC: Book3S HV: Use XICS hypercalls when >> running as a nested hypervisor") added nested HV tests in XICS >> hypercalls, but not all are required. >> >> * icp_eoi is only called by kvmppc_deliver_irq_passthru which is only >> called by kvmppc_check_passthru which is only caled by >> kvmppc_read_one_intr. >> >> * kvmppc_read_one_intr is only called by kvmppc_read_intr which is only >> called by the L0 HV rmhandlers code. >> >> * kvmhv_rm_send_ipi is called by: >> - kvmhv_interrupt_vcore which is only called by kvmhv_commence_exit >> which is only called by the L0 HV rmhandlers code. >> - icp_send_hcore_msg which is only called by icp_rm_set_vcpu_irq. >> - icp_rm_set_vcpu_irq which is only called by icp_rm_try_update >> - icp_rm_set_vcpu_irq is not nested HV safe because it writes to >> LPCR directly without a kvmhv_on_pseries test. Nested handlers >> should not in general be using the rm handlers. >> >> The important test seems to be in kvmppc_ipi_thread, which sends the >> virt-mode H_IPI handler kick to use smp_call_function rather than >> msgsnd. >> >> Cc: Cédric Le Goater <clg@xxxxxxxx> >> Signed-off-by: Nicholas Piggin <npiggin@xxxxxxxxx> >> --- >> arch/powerpc/kvm/book3s_hv_builtin.c | 44 +++++----------------------- >> arch/powerpc/kvm/book3s_hv_rm_xics.c | 15 ---------- >> 2 files changed, 8 insertions(+), 51 deletions(-) > > So, now, the L1 is not limited to XICS anymore and we can use the XIVE > native interrupt mode with an L2 using XICS in KVM or XIVE in QEMU. > Is that correct ? The intention was to only remove dead code and no change. Perhaps I'm missing something or an earlier patch incorrectly made some of these paths dead but I don't see it. > It seems you removed all the XICS hcalls under kvmhv_on_pseries(). >From what I could work out, kvmhv_on_pseries can never be true for the ones I removed. Thanks, Nick > > C. > > >> diff --git a/arch/powerpc/kvm/book3s_hv_builtin.c b/arch/powerpc/kvm/book3s_hv_builtin.c >> index 8d669a0e15f8..259492bb4153 100644 >> --- a/arch/powerpc/kvm/book3s_hv_builtin.c >> +++ b/arch/powerpc/kvm/book3s_hv_builtin.c >> @@ -199,15 +199,6 @@ void kvmhv_rm_send_ipi(int cpu) >> void __iomem *xics_phys; >> unsigned long msg = PPC_DBELL_TYPE(PPC_DBELL_SERVER); >> >> - /* For a nested hypervisor, use the XICS via hcall */ >> - if (kvmhv_on_pseries()) { >> - unsigned long retbuf[PLPAR_HCALL_BUFSIZE]; >> - >> - plpar_hcall_raw(H_IPI, retbuf, get_hard_smp_processor_id(cpu), >> - IPI_PRIORITY); >> - return; >> - } >> - >> /* On POWER9 we can use msgsnd for any destination cpu. */ >> if (cpu_has_feature(CPU_FTR_ARCH_300)) { >> msg |= get_hard_smp_processor_id(cpu); >> @@ -420,19 +411,12 @@ static long kvmppc_read_one_intr(bool *again) >> return 1; >> >> /* Now read the interrupt from the ICP */ >> - if (kvmhv_on_pseries()) { >> - unsigned long retbuf[PLPAR_HCALL_BUFSIZE]; >> - >> - rc = plpar_hcall_raw(H_XIRR, retbuf, 0xFF); >> - xirr = cpu_to_be32(retbuf[0]); >> - } else { >> - xics_phys = local_paca->kvm_hstate.xics_phys; >> - rc = 0; >> - if (!xics_phys) >> - rc = opal_int_get_xirr(&xirr, false); >> - else >> - xirr = __raw_rm_readl(xics_phys + XICS_XIRR); >> - } >> + xics_phys = local_paca->kvm_hstate.xics_phys; >> + rc = 0; >> + if (!xics_phys) >> + rc = opal_int_get_xirr(&xirr, false); >> + else >> + xirr = __raw_rm_readl(xics_phys + XICS_XIRR); >> if (rc < 0) >> return 1; >> >> @@ -461,13 +445,7 @@ static long kvmppc_read_one_intr(bool *again) >> */ >> if (xisr == XICS_IPI) { >> rc = 0; >> - if (kvmhv_on_pseries()) { >> - unsigned long retbuf[PLPAR_HCALL_BUFSIZE]; >> - >> - plpar_hcall_raw(H_IPI, retbuf, >> - hard_smp_processor_id(), 0xff); >> - plpar_hcall_raw(H_EOI, retbuf, h_xirr); >> - } else if (xics_phys) { >> + if (xics_phys) { >> __raw_rm_writeb(0xff, xics_phys + XICS_MFRR); >> __raw_rm_writel(xirr, xics_phys + XICS_XIRR); >> } else { >> @@ -493,13 +471,7 @@ static long kvmppc_read_one_intr(bool *again) >> /* We raced with the host, >> * we need to resend that IPI, bummer >> */ >> - if (kvmhv_on_pseries()) { >> - unsigned long retbuf[PLPAR_HCALL_BUFSIZE]; >> - >> - plpar_hcall_raw(H_IPI, retbuf, >> - hard_smp_processor_id(), >> - IPI_PRIORITY); >> - } else if (xics_phys) >> + if (xics_phys) >> __raw_rm_writeb(IPI_PRIORITY, >> xics_phys + XICS_MFRR); >> else >> diff --git a/arch/powerpc/kvm/book3s_hv_rm_xics.c b/arch/powerpc/kvm/book3s_hv_rm_xics.c >> index c2c9c733f359..0a11ec88a0ae 100644 >> --- a/arch/powerpc/kvm/book3s_hv_rm_xics.c >> +++ b/arch/powerpc/kvm/book3s_hv_rm_xics.c >> @@ -141,13 +141,6 @@ static void icp_rm_set_vcpu_irq(struct kvm_vcpu *vcpu, >> return; >> } >> >> - if (xive_enabled() && kvmhv_on_pseries()) { >> - /* No XICS access or hypercalls available, too hard */ >> - this_icp->rm_action |= XICS_RM_KICK_VCPU; >> - this_icp->rm_kick_target = vcpu; >> - return; >> - } >> - >> /* >> * Check if the core is loaded, >> * if not, find an available host core to post to wake the VCPU, >> @@ -771,14 +764,6 @@ static void icp_eoi(struct irq_chip *c, u32 hwirq, __be32 xirr, bool *again) >> void __iomem *xics_phys; >> int64_t rc; >> >> - if (kvmhv_on_pseries()) { >> - unsigned long retbuf[PLPAR_HCALL_BUFSIZE]; >> - >> - iosync(); >> - plpar_hcall_raw(H_EOI, retbuf, hwirq); >> - return; >> - } >> - >> rc = pnv_opal_pci_msi_eoi(c, hwirq); >> >> if (rc) >> > >