Re: [PATCH] KVM: PPC: Enable CAP_SPAPR_TCE_VFIO on pSeries KVM guests

["Ritesh Harjani (IBM)" <ritesh.list@gmail.com>]

Amit Machhiwal <amachhiw@linux.ibm.com> writes:

> Currently, on book3s-hv, the capability KVM_CAP_SPAPR_TCE_VFIO is only
> available for KVM Guests running on PowerNV and not for the KVM guests
> running on pSeries hypervisors. 

IIUC it was said here [1] that this capability is not available on
pSeries, hence it got removed. Could you please give a background on
why this can be enabled now for pSeries? Was there any additional
support added for this? 
[1]:
https://lore.kernel.org/linuxppc-dev/20181214052910.23639-2-sjitindarsingh@gmail.com/

... Ohh thinking back a little, are you saying that after the patch...
f431a8cde7f1 ("powerpc/iommu: Reimplement the iommu_table_group_ops for pSeries")
 ...we can bring back this capability for kvm guest running on pseries
as well. Because all underlying issues in using VFIO on pseries were
fixed. Is this understanding correct? 


> This prevents a pSeries hypervisor from
> leveraging the in-kernel acceleration for H_PUT_TCE_INDIRECT and
> H_STUFF_TCE hcalls that results in slow startup times for large memory
> guests.

By any chance could you share the startup time improvements for above?
IIUC, other than the boot up time, we should also see the performance
improvements while using VFIO device in nested pSeries kvm guest too right?

>
> Fix this by enabling the CAP_SPAPR_TCE_VFIO on the pSeries hosts for the
> nested PAPR guests.
>
> Fixes: f431a8cde7f1 ("powerpc/iommu: Reimplement the iommu_table_group_ops for pSeries")
> Cc: stable@vger.kernel.org
> Signed-off-by: Amit Machhiwal <amachhiw@linux.ibm.com>
> ---
>  arch/powerpc/kvm/powerpc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index ce1d91eed231..9c479c7381e4 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -554,7 +554,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>  		r = 1;
>  		break;
>  	case KVM_CAP_SPAPR_TCE_VFIO:
> -		r = !!cpu_has_feature(CPU_FTR_HVMODE);
> +		r = !!cpu_has_feature(CPU_FTR_HVMODE) || is_kvmppc_hv_enabled(kvm);
>  		break;

In above you said - "Fix this by enabling the CAP_SPAPR_TCE_VFIO on the pSeries hosts for the nested PAPR guests."
So why can't this simply be r = 1? Or maybe you meant only for HV KVM module is it?

-ritesh

>  	case KVM_CAP_PPC_RTAS:
>  	case KVM_CAP_PPC_FIXUP_HCALL:
>
> base-commit: eea6e4b4dfb8859446177c32961c96726d0117be
> -- 
> 2.47.1