Re: [PATCH v5 2/3] KVM: PPC: Book3S HV: Add support for H_RPT_INVALIDATE

On Wed, Feb 24, 2021 at 12:58:02PM -0300, Fabiano Rosas wrote:

quoted

@@ -1590,6 +1662,24 @@ static int kvmppc_handle_nested_exit(struct kvm_vcpu *vcpu)
 		if (!xics_on_xive())
 			kvmppc_xics_rm_complete(vcpu, 0);
 		break;
+	case BOOK3S_INTERRUPT_SYSCALL:
+	{
+		unsigned long req = kvmppc_get_gpr(vcpu, 3);
+
+		/*
+		 * The H_RPT_INVALIDATE hcalls issued by nested
+		 * guests for process scoped invalidations when
+		 * GTSE=0, are handled here in L0.
+		 */
+		if (req == H_RPT_INVALIDATE) {
+			kvmppc_nested_rpt_invalidate(vcpu);
+			r = RESUME_GUEST;
+			break;
+		}

I'm inclined to say this is a bit too early. We're handling the hcall
before kvmhv_run_single_vcpu has fully finished and we'll skip some
code that has been running in all guest exits:

	if (trap) {
		if (!nested)
			r = kvmppc_handle_exit_hv(vcpu, current);
		else
			r = kvmppc_handle_nested_exit(vcpu);  <--- we're here
	}
	vcpu->arch.ret = r;

        (...)

	vcpu->arch.ceded = 0;

	vc->vcore_state = VCORE_INACTIVE;
	trace_kvmppc_run_core(vc, 1);

 done:
	kvmppc_remove_runnable(vc, vcpu);
	trace_kvmppc_run_vcpu_exit(vcpu);

	return vcpu->arch.ret;

Especially the kvmppc_remove_runnable function because it sets the
vcpu state:

    	vcpu->arch.state = KVMPPC_VCPU_BUSY_IN_HOST;

which should be the case if we're handling a hypercall.

I suggest we do similarly to the L1 exit code and defer the hcall
handling until after kvmppc_run_single_vcpu has exited, still inside the
is_kvmppc_resume_guest(r) loop.

So we'd set:
case BOOK3S_INTERRUPT_SYSCALL:
	vcpu->run->exit_reason = KVM_EXIT_PAPR_HCALL;
	r = RESUME_HOST;
        break;

and perhaps introduce a new kvmppc_pseries_do_nested_hcall that's called
after kvmppc_run_single_vcpu.

Yes, looks like we should, but I wasn't sure if an exit similar to L1
exit for hcall handling is needed here too, hence took this approach.

Paul, could you please clarify?

Regards,
Bharata.