[PATCH] powerpc/pseries/vas: Ignore VAS update for DLPAR if copy/paste is not enabled

[PATCH] powerpc/pseries/vas: Ignore VAS update for DLPAR if copy/paste is not enabled

[Haren Myneni]

The hypervisor supports user-mode NX from Power10. pseries_vas_dlpar_cpu()
is called from lparcfg_write() to update VAS windows for DLPAR CPU event
and the kernel gets -ENOTSUPP for HCALLs if the user-mode NX is not
supported.

This patch ignores updating VAS capabilities and returns success if the
copy/paste feature is not enabled.

Fixes: 2147783d6bf0 ("powerpc/pseries: Use lparcfg to reconfig VAS windows for DLPAR CPU")
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 arch/powerpc/platforms/pseries/vas.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/powerpc/platforms/pseries/vas.c b/arch/powerpc/platforms/pseries/vas.c
index 559112312810..dc003849d2c5 100644
--- a/arch/powerpc/platforms/pseries/vas.c
+++ b/arch/powerpc/platforms/pseries/vas.c
@@ -856,6 +856,13 @@ int pseries_vas_dlpar_cpu(void)
 {
 	int new_nr_creds, rc;
 
+	/*
+	 * NX-GZIP is not enabled. Nothing to do for DLPAR event
+	 */
+	if (!copypaste_feat)
+		return 0;
+
+
 	rc = h_query_vas_capabilities(H_QUERY_VAS_CAPABILITIES,
 				      vascaps[VAS_GZIP_DEF_FEAT_TYPE].feat,
 				      (u64)virt_to_phys(&hv_cop_caps));
@@ -1012,6 +1019,7 @@ static int __init pseries_vas_init(void)
 	 * Linux supports user space COPY/PASTE only with Radix
 	 */
 	if (!radix_enabled()) {
+		copypaste_feat = 0;
 		pr_err("API is supported only with radix page tables\n");
 		return -ENOTSUPP;
 	}
-- 
2.26.3

Re: [PATCH] powerpc/pseries/vas: Ignore VAS update for DLPAR if copy/paste is not enabled

[Nathan Lynch]

Haren Myneni <haren@linux.ibm.com> writes:
> The hypervisor supports user-mode NX from Power10. pseries_vas_dlpar_cpu()
> is called from lparcfg_write() to update VAS windows for DLPAR CPU event
> and the kernel gets -ENOTSUPP for HCALLs if the user-mode NX is not
> supported.

The commit text would be improved by more explanation about the higher
level failure mode here. Does lparcfg_write() fail when it shouldn't? If
so, does that cause a processor DLPAR operation to spuriously fail?

pseries_vas_dlpar_cpu() is also called from pseries_vas_notifier() in
dedicated processor mode. Does this problem affect that scenario also?

> This patch ignores updating VAS capabilities and returns success if the
> copy/paste feature is not enabled.
>
> Fixes: 2147783d6bf0 ("powerpc/pseries: Use lparcfg to reconfig VAS windows for DLPAR CPU")
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
>  arch/powerpc/platforms/pseries/vas.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/arch/powerpc/platforms/pseries/vas.c b/arch/powerpc/platforms/pseries/vas.c
> index 559112312810..dc003849d2c5 100644
> --- a/arch/powerpc/platforms/pseries/vas.c
> +++ b/arch/powerpc/platforms/pseries/vas.c
> @@ -856,6 +856,13 @@ int pseries_vas_dlpar_cpu(void)
>  {
>  	int new_nr_creds, rc;
>  
> +	/*
> +	 * NX-GZIP is not enabled. Nothing to do for DLPAR event
> +	 */
> +	if (!copypaste_feat)
> +		return 0;
> +
> +
>  	rc = h_query_vas_capabilities(H_QUERY_VAS_CAPABILITIES,
>  				      vascaps[VAS_GZIP_DEF_FEAT_TYPE].feat,
>  				      (u64)virt_to_phys(&hv_cop_caps));
> @@ -1012,6 +1019,7 @@ static int __init pseries_vas_init(void)
>  	 * Linux supports user space COPY/PASTE only with Radix
>  	 */
>  	if (!radix_enabled()) {
> +		copypaste_feat = 0;

copypaste_feat is a bool, so use false, not 0. But otherwise I think
this looks correct and consistent with the rest of the code in vas.c.

Re: [PATCH] powerpc/pseries/vas: Ignore VAS update for DLPAR if copy/paste is not enabled

[Haren Myneni]

On Tue, 2023-03-07 at 20:55 -0600, Nathan Lynch wrote:
> Haren Myneni <haren@linux.ibm.com> writes:
> > The hypervisor supports user-mode NX from Power10.
> > pseries_vas_dlpar_cpu()
> > is called from lparcfg_write() to update VAS windows for DLPAR CPU
> > event
> > and the kernel gets -ENOTSUPP for HCALLs if the user-mode NX is not
> > supported.
> 
> The commit text would be improved by more explanation about the
> higher
> level failure mode here. Does lparcfg_write() fail when it shouldn't?
> If
> so, does that cause a processor DLPAR operation to spuriously fail?

Thanks for your suggestions, I will add more explanation in the
description.

Even if copy/paste is not enabled, pseries_vas_dlpar_cpu() just returns
0 which allows lparcfg_write() returns success - will not affect DLPAR
operation.

  if (pseries_vas_dlpar_cpu() != 0)
       retval = H_HARDWARE;
> 
> pseries_vas_dlpar_cpu() is also called from pseries_vas_notifier() in
> dedicated processor mode. Does this problem affect that scenario
> also?

It should not affect for dedicated processor
mode. pseries_vas_notifier() is registered if copy/paste is enabled. 

if (!rc && copypaste_feat) {
                if (firmware_has_feature(FW_FEATURE_LPAR))
                        of_reconfig_notifier_register(&pseries_vas_nb);

> 
> > This patch ignores updating VAS capabilities and returns success if
> > the
> > copy/paste feature is not enabled.
> > 
> > Fixes: 2147783d6bf0 ("powerpc/pseries: Use lparcfg to reconfig VAS
> > windows for DLPAR CPU")
> > Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> > ---
> >  arch/powerpc/platforms/pseries/vas.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/arch/powerpc/platforms/pseries/vas.c
> > b/arch/powerpc/platforms/pseries/vas.c
> > index 559112312810..dc003849d2c5 100644
> > --- a/arch/powerpc/platforms/pseries/vas.c
> > +++ b/arch/powerpc/platforms/pseries/vas.c
> > @@ -856,6 +856,13 @@ int pseries_vas_dlpar_cpu(void)
> >  {
> >  	int new_nr_creds, rc;
> >  
> > +	/*
> > +	 * NX-GZIP is not enabled. Nothing to do for DLPAR event
> > +	 */
> > +	if (!copypaste_feat)
> > +		return 0;
> > +
> > +
> >  	rc = h_query_vas_capabilities(H_QUERY_VAS_CAPABILITIES,
> >  				      vascaps[VAS_GZIP_DEF_FEAT_TYPE].f
> > eat,
> >  				      (u64)virt_to_phys(&hv_cop_caps));
> > @@ -1012,6 +1019,7 @@ static int __init pseries_vas_init(void)
> >  	 * Linux supports user space COPY/PASTE only with Radix
> >  	 */
> >  	if (!radix_enabled()) {
> > +		copypaste_feat = 0;
> 
> copypaste_feat is a bool, so use false, not 0. But otherwise I think
> this looks correct and consistent with the rest of the code in vas.c.

Correct my mistake.

Thanks
Haren