diff mbox series

[v5,41/48] KVM: PPC: Book3S HV: Remove unused nested HV tests in XICS emulation

Message ID 20210401150325.442125-42-npiggin@gmail.com
State New
Headers show
Series KVM: PPC: Book3S: C-ify the P9 entry/exit code | expand

Commit Message

Nicholas Piggin April 1, 2021, 3:03 p.m. UTC 
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@kaod.org>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kvm/book3s_hv_builtin.c | 44 +++++-----------------------
 arch/powerpc/kvm/book3s_hv_rm_xics.c | 15 ----------
 2 files changed, 8 insertions(+), 51 deletions(-)

Comments

kernel test robot April 2, 2021, 3:28 p.m. UTC | #1 
Hi Nicholas,

I love your patch! Perhaps something to improve:

[auto build test WARNING on powerpc/next]
[also build test WARNING on v5.12-rc5 next-20210401]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Nicholas-Piggin/KVM-PPC-Book3S-C-ify-the-P9-entry-exit-code/20210401-232743
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc64-randconfig-s032-20210402 (attached as .config)
compiler: powerpc64le-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.3-279-g6d5d9b42-dirty
        # https://github.com/0day-ci/linux/commit/53519e6ae0f84e2742b886a08598648b424e6f08
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Nicholas-Piggin/KVM-PPC-Book3S-C-ify-the-P9-entry-exit-code/20210401-232743
        git checkout 53519e6ae0f84e2742b886a08598648b424e6f08
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=powerpc64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
   arch/powerpc/kvm/book3s_hv_builtin.c:417:41: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int [usertype] *out_xirr @@     got restricted __be32 * @@
   arch/powerpc/kvm/book3s_hv_builtin.c:417:41: sparse:     expected unsigned int [usertype] *out_xirr
   arch/powerpc/kvm/book3s_hv_builtin.c:417:41: sparse:     got restricted __be32 *
>> arch/powerpc/kvm/book3s_hv_builtin.c:419:22: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __be32 [addressable] [usertype] xirr @@     got unsigned int @@
   arch/powerpc/kvm/book3s_hv_builtin.c:419:22: sparse:     expected restricted __be32 [addressable] [usertype] xirr
   arch/powerpc/kvm/book3s_hv_builtin.c:419:22: sparse:     got unsigned int
>> arch/powerpc/kvm/book3s_hv_builtin.c:450:41: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int [usertype] val @@     got restricted __be32 [addressable] [usertype] xirr @@
   arch/powerpc/kvm/book3s_hv_builtin.c:450:41: sparse:     expected unsigned int [usertype] val
   arch/powerpc/kvm/book3s_hv_builtin.c:450:41: sparse:     got restricted __be32 [addressable] [usertype] xirr
   arch/powerpc/kvm/book3s_hv_builtin.c: note: in included file:
   arch/powerpc/include/asm/kvm_ppc.h:967:1: sparse: sparse: cast to restricted __be64
   arch/powerpc/include/asm/kvm_ppc.h:967:1: sparse: sparse: cast to restricted __be64
   arch/powerpc/include/asm/kvm_ppc.h:967:1: sparse: sparse: cast to restricted __be64
   arch/powerpc/include/asm/kvm_ppc.h:967:1: sparse: sparse: cast to restricted __be64
   arch/powerpc/include/asm/kvm_ppc.h:967:1: sparse: sparse: cast to restricted __be64
   arch/powerpc/include/asm/kvm_ppc.h:967:1: sparse: sparse: cast to restricted __be64
   arch/powerpc/include/asm/kvm_ppc.h:967:1: sparse: sparse: cast to restricted __be64
   arch/powerpc/include/asm/kvm_ppc.h:967:1: sparse: sparse: cast to restricted __be64
   arch/powerpc/include/asm/kvm_ppc.h:967:1: sparse: sparse: cast to restricted __be64
   arch/powerpc/include/asm/kvm_ppc.h:967:1: sparse: sparse: cast to restricted __be64
   arch/powerpc/include/asm/kvm_ppc.h:967:1: sparse: sparse: cast to restricted __le64
   arch/powerpc/include/asm/kvm_ppc.h:963:1: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned long long [usertype] srr0 @@     got restricted __be64 [usertype] @@
   arch/powerpc/include/asm/kvm_ppc.h:963:1: sparse:     expected unsigned long long [usertype] srr0
   arch/powerpc/include/asm/kvm_ppc.h:963:1: sparse:     got restricted __be64 [usertype]
   arch/powerpc/include/asm/kvm_ppc.h:963:1: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned long long [usertype] srr0 @@     got restricted __le64 [usertype] @@
   arch/powerpc/include/asm/kvm_ppc.h:963:1: sparse:     expected unsigned long long [usertype] srr0
   arch/powerpc/include/asm/kvm_ppc.h:963:1: sparse:     got restricted __le64 [usertype]
   arch/powerpc/include/asm/kvm_ppc.h:964:1: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned long long [usertype] srr1 @@     got restricted __be64 [usertype] @@
   arch/powerpc/include/asm/kvm_ppc.h:964:1: sparse:     expected unsigned long long [usertype] srr1
   arch/powerpc/include/asm/kvm_ppc.h:964:1: sparse:     got restricted __be64 [usertype]
   arch/powerpc/include/asm/kvm_ppc.h:964:1: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned long long [usertype] srr1 @@     got restricted __le64 [usertype] @@
   arch/powerpc/include/asm/kvm_ppc.h:964:1: sparse:     expected unsigned long long [usertype] srr1
   arch/powerpc/include/asm/kvm_ppc.h:964:1: sparse:     got restricted __le64 [usertype]

vim +419 arch/powerpc/kvm/book3s_hv_builtin.c

f725758b899f11 Paul Mackerras         2016-11-18  395  
f725758b899f11 Paul Mackerras         2016-11-18  396  static long kvmppc_read_one_intr(bool *again)
37f55d30df2eef Suresh Warrier         2016-08-19  397  {
d381d7caf812f7 Benjamin Herrenschmidt 2017-04-05  398  	void __iomem *xics_phys;
37f55d30df2eef Suresh Warrier         2016-08-19  399  	u32 h_xirr;
37f55d30df2eef Suresh Warrier         2016-08-19  400  	__be32 xirr;
37f55d30df2eef Suresh Warrier         2016-08-19  401  	u32 xisr;
37f55d30df2eef Suresh Warrier         2016-08-19  402  	u8 host_ipi;
f725758b899f11 Paul Mackerras         2016-11-18  403  	int64_t rc;
37f55d30df2eef Suresh Warrier         2016-08-19  404  
5af50993850a48 Benjamin Herrenschmidt 2017-04-05  405  	if (xive_enabled())
5af50993850a48 Benjamin Herrenschmidt 2017-04-05  406  		return 1;
5af50993850a48 Benjamin Herrenschmidt 2017-04-05  407  
37f55d30df2eef Suresh Warrier         2016-08-19  408  	/* see if a host IPI is pending */
37f55d30df2eef Suresh Warrier         2016-08-19  409  	host_ipi = local_paca->kvm_hstate.host_ipi;
37f55d30df2eef Suresh Warrier         2016-08-19  410  	if (host_ipi)
37f55d30df2eef Suresh Warrier         2016-08-19  411  		return 1;
37f55d30df2eef Suresh Warrier         2016-08-19  412  
37f55d30df2eef Suresh Warrier         2016-08-19  413  	/* Now read the interrupt from the ICP */
37f55d30df2eef Suresh Warrier         2016-08-19  414  	xics_phys = local_paca->kvm_hstate.xics_phys;
53af3ba2e8195f Paul Mackerras         2017-01-30  415  	rc = 0;
ab9bad0ead9ab1 Benjamin Herrenschmidt 2017-02-07  416  	if (!xics_phys)
53af3ba2e8195f Paul Mackerras         2017-01-30  417  		rc = opal_int_get_xirr(&xirr, false);
53af3ba2e8195f Paul Mackerras         2017-01-30  418  	else
d381d7caf812f7 Benjamin Herrenschmidt 2017-04-05 @419  		xirr = __raw_rm_readl(xics_phys + XICS_XIRR);
f725758b899f11 Paul Mackerras         2016-11-18  420  	if (rc < 0)
37f55d30df2eef Suresh Warrier         2016-08-19  421  		return 1;
37f55d30df2eef Suresh Warrier         2016-08-19  422  
37f55d30df2eef Suresh Warrier         2016-08-19  423  	/*
37f55d30df2eef Suresh Warrier         2016-08-19  424  	 * Save XIRR for later. Since we get control in reverse endian
37f55d30df2eef Suresh Warrier         2016-08-19  425  	 * on LE systems, save it byte reversed and fetch it back in
37f55d30df2eef Suresh Warrier         2016-08-19  426  	 * host endian. Note that xirr is the value read from the
37f55d30df2eef Suresh Warrier         2016-08-19  427  	 * XIRR register, while h_xirr is the host endian version.
37f55d30df2eef Suresh Warrier         2016-08-19  428  	 */
37f55d30df2eef Suresh Warrier         2016-08-19  429  	h_xirr = be32_to_cpu(xirr);
37f55d30df2eef Suresh Warrier         2016-08-19  430  	local_paca->kvm_hstate.saved_xirr = h_xirr;
37f55d30df2eef Suresh Warrier         2016-08-19  431  	xisr = h_xirr & 0xffffff;
37f55d30df2eef Suresh Warrier         2016-08-19  432  	/*
37f55d30df2eef Suresh Warrier         2016-08-19  433  	 * Ensure that the store/load complete to guarantee all side
37f55d30df2eef Suresh Warrier         2016-08-19  434  	 * effects of loading from XIRR has completed
37f55d30df2eef Suresh Warrier         2016-08-19  435  	 */
37f55d30df2eef Suresh Warrier         2016-08-19  436  	smp_mb();
37f55d30df2eef Suresh Warrier         2016-08-19  437  
37f55d30df2eef Suresh Warrier         2016-08-19  438  	/* if nothing pending in the ICP */
37f55d30df2eef Suresh Warrier         2016-08-19  439  	if (!xisr)
37f55d30df2eef Suresh Warrier         2016-08-19  440  		return 0;
37f55d30df2eef Suresh Warrier         2016-08-19  441  
37f55d30df2eef Suresh Warrier         2016-08-19  442  	/* We found something in the ICP...
37f55d30df2eef Suresh Warrier         2016-08-19  443  	 *
37f55d30df2eef Suresh Warrier         2016-08-19  444  	 * If it is an IPI, clear the MFRR and EOI it.
37f55d30df2eef Suresh Warrier         2016-08-19  445  	 */
37f55d30df2eef Suresh Warrier         2016-08-19  446  	if (xisr == XICS_IPI) {
53af3ba2e8195f Paul Mackerras         2017-01-30  447  		rc = 0;
53519e6ae0f84e Nicholas Piggin        2021-04-02  448  		if (xics_phys) {
d381d7caf812f7 Benjamin Herrenschmidt 2017-04-05  449  			__raw_rm_writeb(0xff, xics_phys + XICS_MFRR);
d381d7caf812f7 Benjamin Herrenschmidt 2017-04-05 @450  			__raw_rm_writel(xirr, xics_phys + XICS_XIRR);
f725758b899f11 Paul Mackerras         2016-11-18  451  		} else {
ab9bad0ead9ab1 Benjamin Herrenschmidt 2017-02-07  452  			opal_int_set_mfrr(hard_smp_processor_id(), 0xff);
ab9bad0ead9ab1 Benjamin Herrenschmidt 2017-02-07  453  			rc = opal_int_eoi(h_xirr);
53af3ba2e8195f Paul Mackerras         2017-01-30  454  		}
f725758b899f11 Paul Mackerras         2016-11-18  455  		/* If rc > 0, there is another interrupt pending */
f725758b899f11 Paul Mackerras         2016-11-18  456  		*again = rc > 0;
f725758b899f11 Paul Mackerras         2016-11-18  457  
37f55d30df2eef Suresh Warrier         2016-08-19  458  		/*
37f55d30df2eef Suresh Warrier         2016-08-19  459  		 * Need to ensure side effects of above stores
37f55d30df2eef Suresh Warrier         2016-08-19  460  		 * complete before proceeding.
37f55d30df2eef Suresh Warrier         2016-08-19  461  		 */
37f55d30df2eef Suresh Warrier         2016-08-19  462  		smp_mb();
37f55d30df2eef Suresh Warrier         2016-08-19  463  
37f55d30df2eef Suresh Warrier         2016-08-19  464  		/*
37f55d30df2eef Suresh Warrier         2016-08-19  465  		 * We need to re-check host IPI now in case it got set in the
37f55d30df2eef Suresh Warrier         2016-08-19  466  		 * meantime. If it's clear, we bounce the interrupt to the
37f55d30df2eef Suresh Warrier         2016-08-19  467  		 * guest
37f55d30df2eef Suresh Warrier         2016-08-19  468  		 */
37f55d30df2eef Suresh Warrier         2016-08-19  469  		host_ipi = local_paca->kvm_hstate.host_ipi;
37f55d30df2eef Suresh Warrier         2016-08-19  470  		if (unlikely(host_ipi != 0)) {
37f55d30df2eef Suresh Warrier         2016-08-19  471  			/* We raced with the host,
37f55d30df2eef Suresh Warrier         2016-08-19  472  			 * we need to resend that IPI, bummer
37f55d30df2eef Suresh Warrier         2016-08-19  473  			 */
53519e6ae0f84e Nicholas Piggin        2021-04-02  474  			if (xics_phys)
d381d7caf812f7 Benjamin Herrenschmidt 2017-04-05  475  				__raw_rm_writeb(IPI_PRIORITY,
d381d7caf812f7 Benjamin Herrenschmidt 2017-04-05  476  						xics_phys + XICS_MFRR);
f725758b899f11 Paul Mackerras         2016-11-18  477  			else
ab9bad0ead9ab1 Benjamin Herrenschmidt 2017-02-07  478  				opal_int_set_mfrr(hard_smp_processor_id(),
f725758b899f11 Paul Mackerras         2016-11-18  479  						  IPI_PRIORITY);
37f55d30df2eef Suresh Warrier         2016-08-19  480  			/* Let side effects complete */
37f55d30df2eef Suresh Warrier         2016-08-19  481  			smp_mb();
37f55d30df2eef Suresh Warrier         2016-08-19  482  			return 1;
37f55d30df2eef Suresh Warrier         2016-08-19  483  		}
37f55d30df2eef Suresh Warrier         2016-08-19  484  
37f55d30df2eef Suresh Warrier         2016-08-19  485  		/* OK, it's an IPI for us */
37f55d30df2eef Suresh Warrier         2016-08-19  486  		local_paca->kvm_hstate.saved_xirr = 0;
37f55d30df2eef Suresh Warrier         2016-08-19  487  		return -1;
37f55d30df2eef Suresh Warrier         2016-08-19  488  	}
37f55d30df2eef Suresh Warrier         2016-08-19  489  
f725758b899f11 Paul Mackerras         2016-11-18  490  	return kvmppc_check_passthru(xisr, xirr, again);
37f55d30df2eef Suresh Warrier         2016-08-19  491  }
5af50993850a48 Benjamin Herrenschmidt 2017-04-05  492  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Cédric Le Goater April 2, 2021, 4:32 p.m. UTC | #2 
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@kaod.org>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  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 ?   

It seems you removed all the XICS hcalls under kvmhv_on_pseries().

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)
>
Nicholas Piggin April 4, 2021, 12:48 a.m. UTC | #3 
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@kaod.org>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>>  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)
>> 
> 
>
diff mbox series

Patch

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)