[PATCH v1 1/2] KVM: PPC: Book3S HV: Add support for H_RPT_INVALIDATE (nested case only)

[David Gibson]

On Fri, Dec 11, 2020 at 04:27:44PM +1100, Paul Mackerras wrote:
> On Fri, Dec 11, 2020 at 12:16:39PM +1100, David Gibson wrote:
> > On Thu, Dec 10, 2020 at 09:54:18AM +0530, Bharata B Rao wrote:
> > > On Wed, Dec 09, 2020 at 03:15:42PM +1100, Paul Mackerras wrote:
> > > > On Mon, Oct 19, 2020 at 04:56:41PM +0530, Bharata B Rao wrote:
> > > > > Implements H_RPT_INVALIDATE hcall and supports only nested case
> > > > > currently.
> > > > > 
> > > > > A KVM capability KVM_CAP_RPT_INVALIDATE is added to indicate the
> > > > > support for this hcall.
> > > > 
> > > > I have a couple of questions about this patch:
> > > > 
> > > > 1. Is this something that is useful today, or is it something that may
> > > > become useful in the future depending on future product plans? In
> > > > other words, what advantage is there to forcing L2 guests to use this
> > > > hcall instead of doing tlbie themselves?
> > > 
> > > H_RPT_INVALIDATE will replace the use of the existing H_TLB_INVALIDATE
> > > for nested partition scoped invalidations. Implementations that want to
> > > off-load invalidations to the host (when GTSE=0) would have to bother
> > > about only one hcall (H_RPT_INVALIDATE)
> > > 
> > > > 
> > > > 2. Why does it need to be added to the default-enabled hcall list?
> > > > 
> > > > There is a concern that if this is enabled by default we could get the
> > > > situation where a guest using it gets migrated to a host that doesn't
> > > > support it, which would be bad.  That is the reason that all new
> > > > things like this are disabled by default and only enabled by userspace
> > > > (i.e. QEMU) in situations where we can enforce that it is available on
> > > > all hosts to which the VM might be migrated.
> > > 
> > > As you suggested privately, I am thinking of falling back to
> > > H_TLB_INVALIDATE in case where this new hcall fails due to not being
> > > present. That should address the migration case that you mention
> > > above. With that and leaving the new hcall enabled by default
> > > is good okay?
> > 
> > No.  Assuming that guests will have some fallback is not how the qemu
> > migration compatibility model works.  If we specify an old machine
> > type, we need to provide the same environment that the older host
> > would have.
> 
> I misunderstood what this patchset is about when I first looked at
> it.  H_RPT_INVALIDATE has two separate functions; one is to do
> process-scoped invalidations for a guest when LPCR[GTSE] = 0 (i.e.,
> when the guest is not permitted to do tlbie itself), and the other is
> to do partition-scoped invalidations that an L1 hypervisor needs to do
> on behalf of an L2 guest.  The second function is a replacement and
> standardization of the existing H_TLB_INVALIDATE which was introduced
> with the nested virtualization code (using a hypercall number from the
> platform-specific range).
> 
> This patchset only implements the second function, not the first.  The
> first function remains unimplemented in KVM at present.
> 
> Given that QEMU will need changes for a guest to be able to exploit
> H_RPT_INVALIDATE (at a minimum, adding a device tree property), it
> doesn't seem onerous for QEMU to have to enable the hcall with
> KVM_CAP_PPC_ENABLE_HCALL.  I think that the control on whether the
> hcall is handled in KVM along with the control on nested hypervisor
> function provides adequate control for QEMU without needing a writable
> capability.  The read-only capability to say whether the hcall exists
> does seem useful.
> 
> Given all that, I'm veering towards taking Bharata's patchset pretty
> much as-is, minus the addition of H_RPT_INVALIDATE to the
> default-enabled set.

Yes, that's fine.  I was only the suggestion that it be on the
default-enabled set I was objecting to.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/linuxppc-dev/attachments/20201214/77763b29/attachment.sig>