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

[Nicholas Piggin]

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 at kaod.org>
>> Signed-off-by: Nicholas Piggin <npiggin at 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)
>> 
> 
>