Re: [PATCH V2 7/8] KVM: PPC: Introduce new hcall H_COPY_TOFROM_GUEST to access quadrants 1 & 2

[Suraj Jitindar Singh <sjitindarsingh@gmail.com>]

On Thu, 2018-12-13 at 16:24 +1100, Paul Mackerras wrote:
> On Mon, Dec 10, 2018 at 02:58:24PM +1100, Suraj Jitindar Singh wrote:
> > A guest cannot access quadrants 1 or 2 as this would result in an
> > exception. Thus introduce the hcall H_COPY_TOFROM_GUEST to be used
> > by a
> > guest when it wants to perform an access to quadrants 1 or 2, for
> > example when it wants to access memory for one of its nested
> > guests.
> > 
> > Also provide an implementation for the kvm-hv module.
> > 
> > Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> 
> [snip]
> 
> >  /*
> > + * Handle the H_COPY_TOFROM_GUEST hcall.
> > + * r4 = L1 lpid of nested guest
> > + * r5 = pid
> > + * r6 = eaddr to access
> > + * r7 = to buffer (L1 gpa)
> > + * r8 = from buffer (L1 gpa)
> 
> Comment says these are GPAs...
> 
> > + * r9 = n bytes to copy
> > + */
> > +long kvmhv_copy_tofrom_guest_nested(struct kvm_vcpu *vcpu)
> > +{
> > +	struct kvm_nested_guest *gp;
> > +	int l1_lpid = kvmppc_get_gpr(vcpu, 4);
> > +	int pid = kvmppc_get_gpr(vcpu, 5);
> > +	gva_t eaddr = kvmppc_get_gpr(vcpu, 6);
> > +	void *gp_to = (void *) kvmppc_get_gpr(vcpu, 7);
> > +	void *gp_from = (void *) kvmppc_get_gpr(vcpu, 8);
> > +	void *buf;
> > +	unsigned long n = kvmppc_get_gpr(vcpu, 9);
> > +	bool is_load = !!gp_to;
> > +	long rc;
> > +
> > +	if (gp_to && gp_from) /* One must be NULL to determine the
> > direction */
> > +		return H_PARAMETER;
> > +
> > +	if (eaddr & (0xFFFUL << 52))
> > +		return H_PARAMETER;
> > +
> > +	buf = kzalloc(n, GFP_KERNEL);
> > +	if (!buf)
> > +		return H_NO_MEM;
> > +
> > +	gp = kvmhv_get_nested(vcpu->kvm, l1_lpid, false);
> > +	if (!gp) {
> > +		rc = H_PARAMETER;
> > +		goto out_free;
> > +	}
> > +
> > +	mutex_lock(&gp->tlb_lock);
> > +
> > +	if (is_load) {
> > +		/* Load from the nested guest into our buffer */
> > +		rc = __kvmhv_copy_tofrom_guest_radix(gp-
> > >shadow_lpid, pid,
> > +						     eaddr, buf,
> > NULL, n);
> > +		if (rc)
> > +			goto not_found;
> > +
> > +		/* Write what was loaded into our buffer back to
> > the L1 guest */
> > +		rc = kvmppc_st(vcpu, (ulong *) &gp_to, n, buf,
> > true);
> 
> but using kvmppc_st implies that it is an EA (and in fact when you
> call it in the next patch you pass an EA).
> 
> It would be more like other hcalls to pass a GPA, meaning that you
> would use kvm_write_guest() here.  On the other hand, with the
> quadrant access, kvmppc_st() might well be faster than
> kvm_write_guest.
> 
> So you need to decide which it is and either fix the comment or
> change
> the code.

Lets stick with gpa for now then for consistency, with room for
optimisation.

> 
> Paul.