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

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

On Wed, Dec 16, 2020 at 07:47:29PM -0300, Fabiano Rosas 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);
> > +	}
> > +}
> 
> This is all quite similar to _tlbie_pid in mm/book3s64/radix_tlb.c so:
> 
> 1) Shouldn't do_tlb_invalidate be in that file so we could reuse
> __tlbie_pid and __tlbie_va? There are also the tracepoints in that file
> that we might want to reuse.

Will see how much reuse is possible.

> 
> 2) For my own understanding, don't the "fixups" in _tlbie_pid apply to
> this scenario as well?

Yes, I think, will add fixups.

> > +long kvmhv_h_rpti_nested(struct kvm_vcpu *vcpu, unsigned long lpid,
> > +			 unsigned long type, unsigned long pg_sizes,
> > +			 unsigned long start, unsigned long end)
> > +{
> > +	struct kvm_nested_guest *gp;
> > +	long ret;
> > +	unsigned long psize, ap;
> > +
> > +	/*
> > +	 * If L2 lpid isn't valid, we need to return H_PARAMETER.
> > +	 * Nested KVM issues a L2 lpid flush call when creating
> > +	 * partition table entries for L2. This happens even before
> > +	 * the corresponding shadow lpid is created in HV. Until
> > +	 * this is fixed, ignore such flush requests.
> 
> >From the text, it seems that you are talking about kvmhv_set_ptbl_entry
> in L1 calling kvmhv_flush_lpid, but I'm not sure. Could you clarify that
> scenario a bit?

Yes this is the scenario which I am talking about here.

> 
> Maybe it would be good to have a more concrete hint of the issue here or
> in the commit message, since you mentioned this is something that needs
> fixing.

Hmm let me see if I can make the comment more verbose/concrete in the
next version.

Thanks for your review.

Regards,
Bharata.