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. Paul.
