[PATCH] powerpc/pseries/vas: Don't print an error when VAS is unavailable

KVM does not support VAS so guests always print a useless error on boot

    vas: HCALL(398) error -2, query_type 0, result buffer 0x57f2000

Change this to only print the message if the error is not H_FUNCTION.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/platforms/pseries/vas.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/vas.c b/arch/powerpc/platforms/pseries/vas.c
index b043e3936d21..734523e2272f 100644
--- a/arch/powerpc/platforms/pseries/vas.c
+++ b/arch/powerpc/platforms/pseries/vas.c

@@ -151,8 +151,15 @@ int h_query_vas_capabilities(const u64 hcall, u8 query_type, u64 result)
 	if (rc == H_SUCCESS)
 		return 0;
 
-	pr_err("HCALL(%llx) error %ld, query_type %u, result buffer 0x%llx\n",
-			hcall, rc, query_type, result);
+	/* H_FUNCTION means HV does not support VAS so don't print an error */
+	if (rc != H_FUNCTION) {
+		pr_err("%s error %ld, query_type %u, result buffer 0x%llx\n",
+			(hcall == H_QUERY_VAS_CAPABILITIES) ?
+				"H_QUERY_VAS_CAPABILITIES" :
+				"H_QUERY_NX_CAPABILITIES",
+			rc, query_type, result);
+	}
+
 	return -EIO;
 }
 EXPORT_SYMBOL_GPL(h_query_vas_capabilities);

-- 
2.23.0

On 11/26/21 06:21, Nicholas Piggin wrote:

KVM does not support VAS so guests always print a useless error on boot

     vas: HCALL(398) error -2, query_type 0, result buffer 0x57f2000

Change this to only print the message if the error is not H_FUNCTION.

Just being curious, why is it even called since "ibm,compression" should
not be exposed in the DT ?

C.

Excerpts from Cédric Le Goater's message of November 26, 2021 5:13 pm:

On 11/26/21 06:21, Nicholas Piggin wrote:

quoted

KVM does not support VAS so guests always print a useless error on boot

     vas: HCALL(398) error -2, query_type 0, result buffer 0x57f2000

Change this to only print the message if the error is not H_FUNCTION.

Just being curious, why is it even called since "ibm,compression" should
not be exposed in the DT ?

It looks like vas does not test for it. I guess in theory there can be 
other functions than compression implemented as an accelerator. Maybe
that's why?

Thanks,
Nick

On 11/26/21 2:31 AM, Nicholas Piggin wrote:

Excerpts from Cédric Le Goater's message of November 26, 2021 5:13 pm:

quoted

On 11/26/21 06:21, Nicholas Piggin wrote:

quoted

KVM does not support VAS so guests always print a useless error on boot

     vas: HCALL(398) error -2, query_type 0, result buffer 0x57f2000

Change this to only print the message if the error is not H_FUNCTION.

Just being curious, why is it even called since "ibm,compression" should
not be exposed in the DT ?

It looks like vas does not test for it. I guess in theory there can be 
other functions than compression implemented as an accelerator. Maybe
that's why?

Thanks,
Nick

Looks like pseries_vas_init() simply calls h_query_vas_capabilities() to test
for VAS coprocessor support. I would assume KVM doesn't expose hcall-vas or
hcall-nx in /rtas/ibm,hypertas-functions? Doesn't look like hcall-vas or
hcall-nx have been added to the hypertas_fw_feature matching, but maybe they
should and we can gate VAS initialization on those, or at the minimum
FW_FEATURE_VAS?

-Tyrel

Nicholas Piggin [off-list ref] writes:

Excerpts from Cédric Le Goater's message of November 26, 2021 5:13 pm:

quoted

On 11/26/21 06:21, Nicholas Piggin wrote:

quoted

KVM does not support VAS so guests always print a useless error on boot

     vas: HCALL(398) error -2, query_type 0, result buffer 0x57f2000

Change this to only print the message if the error is not H_FUNCTION.

Just being curious, why is it even called since "ibm,compression" should
not be exposed in the DT ?

It looks like vas does not test for it. I guess in theory there can be 
other functions than compression implemented as an accelerator. Maybe
that's why?

Yeah I guess, or it's just not structured that well. The vas platform
code is a bit awkward, it's there to support drivers, but it's not
actually driver code.

I think we can probably rework it so the vas code does nothing until a
driver calls in to it.

eg. something like below.

cheers

diff --git a/arch/powerpc/platforms/pseries/vas.c b/arch/powerpc/platforms/pseries/vas.c
index b043e3936d21..dc3491fc919d 100644
--- a/arch/powerpc/platforms/pseries/vas.c
+++ b/arch/powerpc/platforms/pseries/vas.c

@@ -454,6 +454,8 @@ static const struct vas_user_win_ops vops_pseries = {
 	.close_win	= vas_deallocate_window, /* Close window */
 };
 
+static int pseries_vas_init(void);
+
 /*
  * Supporting only nx-gzip coprocessor type now, but this API code
  * extended to other coprocessor types later.

@@ -463,7 +465,8 @@ int vas_register_api_pseries(struct module *mod, enum vas_cop_type cop_type,
 {
 	int rc;
 
-	if (!copypaste_feat)
+	rc = pseries_vas_init();
+	if (rc || !copypaste_feat)
 		return -ENOTSUPP;
 
 	rc = vas_register_coproc_api(mod, cop_type, name, &vops_pseries);

@@ -531,7 +534,7 @@ static int get_vas_capabilities(u8 feat, enum vas_cop_feat_type type,
 	return 0;
 }
 
-static int __init pseries_vas_init(void)
+static int pseries_vas_init(void)
 {
 	struct hv_vas_cop_feat_caps *hv_cop_caps;
 	struct hv_vas_all_caps *hv_caps;

@@ -592,4 +595,3 @@ static int __init pseries_vas_init(void)
 	kfree(hv_caps);
 	return rc;
 }
-machine_device_initcall(pseries, pseries_vas_init);

On Tue, 2021-11-30 at 10:25 +1100, Michael Ellerman wrote:

Nicholas Piggin [off-list ref] writes:

quoted

Excerpts from Cédric Le Goater's message of November 26, 2021 5:13
pm:

quoted

On 11/26/21 06:21, Nicholas Piggin wrote:

quoted

KVM does not support VAS so guests always print a useless error
on boot

     vas: HCALL(398) error -2, query_type 0, result buffer
0x57f2000

Change this to only print the message if the error is not
H_FUNCTION.

Just being curious, why is it even called since "ibm,compression"
should
not be exposed in the DT ?

It looks like vas does not test for it. I guess in theory there can
be 
other functions than compression implemented as an accelerator.
Maybe
that's why?

Yeah I guess, or it's just not structured that well. The vas platform
code is a bit awkward, it's there to support drivers, but it's not
actually driver code.

I think we can probably rework it so the vas code does nothing until
a
driver calls in to it.

eg. something like below.

Correct, Even though NXGZIP is the only usage right now, VAS is
accelerator switchboard which should support other coprocessor types
such as GZIP and 842 or SW type solutions such as fast thread wakeup
and fast memory copy. 

So can we leave VAS initialization separate from drivers and use some
feature such as FW_FEATURE_LPAR to differentiate from KVM guests?

Thanks
Haren

Haren Myneni [off-list ref] writes:

On Tue, 2021-11-30 at 10:25 +1100, Michael Ellerman wrote:

quoted

Nicholas Piggin [off-list ref] writes:

quoted

Excerpts from Cédric Le Goater's message of November 26, 2021 5:13
pm:

quoted

On 11/26/21 06:21, Nicholas Piggin wrote:

quoted

KVM does not support VAS so guests always print a useless error
on boot

     vas: HCALL(398) error -2, query_type 0, result buffer
0x57f2000

Change this to only print the message if the error is not
H_FUNCTION.

Just being curious, why is it even called since "ibm,compression"
should
not be exposed in the DT ?

It looks like vas does not test for it. I guess in theory there can
be 
other functions than compression implemented as an accelerator.
Maybe
that's why?

Yeah I guess, or it's just not structured that well. The vas platform
code is a bit awkward, it's there to support drivers, but it's not
actually driver code.

I think we can probably rework it so the vas code does nothing until
a
driver calls in to it.

eg. something like below.

Correct, Even though NXGZIP is the only usage right now, VAS is
accelerator switchboard which should support other coprocessor types
such as GZIP and 842 or SW type solutions such as fast thread wakeup
and fast memory copy. 

So can we leave VAS initialization separate from drivers and use some
feature such as FW_FEATURE_LPAR to differentiate from KVM guests?

FW_FEATURE_LPAR is true on KVM guests as well.

As Tyrel pointed out, you should be looking for "hcall-vas" in
"ibm,hypertas-functions" and setting a new FW_FEATURE_VAS based on that.
Then use that to gate the vas init routine.

cheers

On Fri, 26 Nov 2021 15:21:33 +1000, Nicholas Piggin wrote:

KVM does not support VAS so guests always print a useless error on boot

    vas: HCALL(398) error -2, query_type 0, result buffer 0x57f2000

Change this to only print the message if the error is not H_FUNCTION.


[...]

Applied to powerpc/next.

[1/1] powerpc/pseries/vas: Don't print an error when VAS is unavailable
      https://git.kernel.org/powerpc/c/0a006ace634dcaf1bbf9125fb8089a4a50bf33d6

cheers