Re: [PATCH v2 1/2] KVM: PPC: Book3S HV: Add support for H_RPT_INVALIDATE

[Bharata B Rao <bharata@linux.ibm.com>]

On Thu, Dec 17, 2020 at 02:42:15PM +1100, David Gibson wrote:
> On Wed, Dec 16, 2020 at 02:24:46PM +0530, Bharata B Rao wrote:
> > +static void do_tlb_invalidate(unsigned long rs, unsigned long target,
> > +			      unsigned long type, unsigned long page_size,
> > +			      unsigned long ap, unsigned long start,
> > +			      unsigned long end)
> > +{
> > +	unsigned long rb;
> > +	unsigned long addr = start;
> > +
> > +	if ((type & H_RPTI_TYPE_ALL) == H_RPTI_TYPE_ALL) {
> > +		rb = PPC_BIT(53); /* IS = 1 */
> > +		do_tlb_invalidate_all(rb, rs);
> > +		return;
> > +	}
> > +
> > +	if (type & H_RPTI_TYPE_PWC) {
> > +		rb = PPC_BIT(53); /* IS = 1 */
> > +		do_tlb_invalidate_pwc(rb, rs);
> > +	}
> > +
> > +	if (!addr && end == -1) { /* PID */
> > +		rb = PPC_BIT(53); /* IS = 1 */
> > +		do_tlb_invalidate_tlb(rb, rs);
> > +	} else { /* EA */
> > +		do {
> > +			rb = addr & ~(PPC_BITMASK(52, 63));
> > +			rb |= ap << PPC_BITLSHIFT(58);
> > +			do_tlb_invalidate_tlb(rb, rs);
> > +			addr += page_size;
> > +		} while (addr < end);
> > +	}
> > +}
> > +
> > +static long kvmppc_h_rpt_invalidate(struct kvm_vcpu *vcpu,
> > +				    unsigned long pid, unsigned long target,
> > +				    unsigned long type, unsigned long pg_sizes,
> > +				    unsigned long start, unsigned long end)
> > +{
> > +	unsigned long rs, ap, psize;
> > +
> > +	if (!kvm_is_radix(vcpu->kvm))
> > +		return H_FUNCTION;
> 
> IIUC The cover note said this case was H_NOT_SUPPORTED, rather than H_FUNCTION.
> 
> > +
> > +	if (end < start)
> > +		return H_P5;
> > +
> > +	if (type & H_RPTI_TYPE_NESTED) {
> > +		if (!nesting_enabled(vcpu->kvm))
> > +			return H_FUNCTION;
> 
> Likewise, I'm not sure that H_FUNCTION is the right choice here.

Yes to both, will switch to H_FUNCTION in the next iteration.

> 
> > +
> > +		/* Support only cores as target */
> > +		if (target != H_RPTI_TARGET_CMMU)
> > +			return H_P2;
> > +
> > +		return kvmhv_h_rpti_nested(vcpu, pid,
> > +					   (type & ~H_RPTI_TYPE_NESTED),
> > +					    pg_sizes, start, end);
> > +	}
> > +
> > +	rs = pid << PPC_BITLSHIFT(31);
> > +	rs |= vcpu->kvm->arch.lpid;
> > +
> > +	if (pg_sizes & H_RPTI_PAGE_64K) {
> > +		psize = rpti_pgsize_to_psize(pg_sizes & H_RPTI_PAGE_64K);
> > +		ap = mmu_get_ap(psize);
> > +		do_tlb_invalidate(rs, target, type, (1UL << 16), ap, start,
> > +				  end);
> 
> Should these be conditional on the TLB flag in type?

Didn't quite get you. Do you mean that depending on the type flag
we may not need to do invalidations for different page sizes
separately?

Regards,
Bharata.