Hi Ritesh,
Thanks for reviewing the patch. My response is inline:
On 2025/02/17 09:30 AM, Ritesh Harjani (IBM) wrote:
> Amit Machhiwal <amachhiw@xxxxxxxxxxxxx> 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. This prevents a pSeries L2 guest 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.
> >
> > Fix this by enabling the CAP_SPAPR_TCE_VFIO on the pSeries hosts as well
> > for the nested PAPR guests. With the patch, booting an L2 guest with
> > 128G memory results in an average improvement of 11% in the startup
> > times.
> >
> > Fixes: f431a8cde7f1 ("powerpc/iommu: Reimplement the iommu_table_group_ops for pSeries")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Amit Machhiwal <amachhiw@xxxxxxxxxxxxx>
> > ---
> > Changes since v1:
> > * Addressed review comments from Ritesh
> > * v1: https://lore.kernel.org/all/20250109132053.158436-1-amachhiw@xxxxxxxxxxxxx/
>
> Thanks Amit for v2. However we still didn't answer one important
> question regarding the context / background of this patch asked here [2]
>
> [2]: https://lore.kernel.org/linuxppc-dev/87r059vpmi.fsf@xxxxxxxxx/
>
> <copy paste from v1>
> 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@xxxxxxxxx/
>
> ... 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?
>
Yes, your understanding is correct.
>
> Please also update the commit message with the required context of why we can
> enable this capability now while it was explicitely marked as disabled
> earlier in [1].
>
Sure, I'll update the patch description and send a v3 soon.
> But looks good otherwise. With that addressed in the commit message,
> please feel free to add -
>
> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@xxxxxxxxx>
>
Thanks!
~ Amit
> -ritesh
>
> >
> > arch/powerpc/kvm/powerpc.c | 5 +----
> > 1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> > index ce1d91eed231..a7138eb18d59 100644
> > --- a/arch/powerpc/kvm/powerpc.c
> > +++ b/arch/powerpc/kvm/powerpc.c
> > @@ -543,26 +543,23 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> > r = !hv_enabled;
> > break;
> > #ifdef CONFIG_KVM_MPIC
> > case KVM_CAP_IRQ_MPIC:
> > r = 1;
> > break;
> > #endif
> >
> > #ifdef CONFIG_PPC_BOOK3S_64
> > case KVM_CAP_SPAPR_TCE:
> > + fallthrough;
> > case KVM_CAP_SPAPR_TCE_64:
> > - r = 1;
> > - break;
> > case KVM_CAP_SPAPR_TCE_VFIO:
> > - r = !!cpu_has_feature(CPU_FTR_HVMODE);
> > - break;
> > case KVM_CAP_PPC_RTAS:
> > case KVM_CAP_PPC_FIXUP_HCALL:
> > case KVM_CAP_PPC_ENABLE_HCALL:
> > #ifdef CONFIG_KVM_XICS
> > case KVM_CAP_IRQ_XICS:
> > #endif
> > case KVM_CAP_PPC_GET_CPU_CHAR:
> > r = 1;
> > break;
> > #ifdef CONFIG_KVM_XIVE
> >
> > base-commit: 6d61a53dd6f55405ebcaea6ee38d1ab5a8856c2c
> > --
> > 2.48.1
