diff mbox series

[1/1] firmware: smccc: Fix check for ARCH_SOC_ID not implemented

Message ID 1637365141-16823-1-git-send-email-mikelley@microsoft.com (mailing list archive)
State New, archived
Headers show
Series [1/1] firmware: smccc: Fix check for ARCH_SOC_ID not implemented | expand

Commit Message

Michael Kelley (LINUX) Nov. 19, 2021, 11:39 p.m. UTC
The ARCH_FEATURES function ID is a 32-bit SMC call, which returns
a 32-bit result per the SMCCC spec.  Current code is doing a 64-bit
comparison against -1 (SMCCC_RET_NOT_SUPPORTED) to detect that the
feature is unimplemented.  That check doesn't work in a Hyper-V VM,
where the upper 32-bits are zero as allowed by the spec.

Cast the result as an 'int' so the comparison works. The change also
makes the code consistent with other similar checks in this file.

Fixes: 821b67fa4639 ("firmware: smccc: Add ARCH_SOC_ID support")
Signed-off-by: Michael Kelley <mikelley@microsoft.com>
---
 drivers/firmware/smccc/soc_id.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Sudeep Holla Nov. 22, 2021, 8:35 a.m. UTC | #1
On Fri, Nov 19, 2021 at 03:39:01PM -0800, Michael Kelley wrote:
> The ARCH_FEATURES function ID is a 32-bit SMC call, which returns
> a 32-bit result per the SMCCC spec.  Current code is doing a 64-bit
> comparison against -1 (SMCCC_RET_NOT_SUPPORTED) to detect that the
> feature is unimplemented.  That check doesn't work in a Hyper-V VM,
> where the upper 32-bits are zero as allowed by the spec.
> 
> Cast the result as an 'int' so the comparison works. The change also
> makes the code consistent with other similar checks in this file.
> 
> Fixes: 821b67fa4639 ("firmware: smccc: Add ARCH_SOC_ID support")

Good catch.

Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>

Can you please re-post with my review tag keeping arm@kernel.org and
soc@kernel.org ? I don't have any other fixes at the moment, we can
ask SoC maintainers to fix this up directly.
Arnd Bergmann Nov. 22, 2021, 10:56 a.m. UTC | #2
On Mon, Nov 22, 2021 at 9:35 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
> On Fri, Nov 19, 2021 at 03:39:01PM -0800, Michael Kelley wrote:
> > The ARCH_FEATURES function ID is a 32-bit SMC call, which returns
> > a 32-bit result per the SMCCC spec.  Current code is doing a 64-bit
> > comparison against -1 (SMCCC_RET_NOT_SUPPORTED) to detect that the
> > feature is unimplemented.  That check doesn't work in a Hyper-V VM,
> > where the upper 32-bits are zero as allowed by the spec.
> >
> > Cast the result as an 'int' so the comparison works. The change also
> > makes the code consistent with other similar checks in this file.
> >
> > Fixes: 821b67fa4639 ("firmware: smccc: Add ARCH_SOC_ID support")
>
> Good catch.
>
> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
>
> Can you please re-post with my review tag keeping arm@kernel.org and
> soc@kernel.org ? I don't have any other fixes at the moment, we can
> ask SoC maintainers to fix this up directly.

In general, I think the easiest way would be for you to forward the
patch with your Signed-off-by, when you don't have other material
for a pull request.

I've applied this patch from the list now.

       Arnd
Sudeep Holla Nov. 22, 2021, 11:52 a.m. UTC | #3
On Mon, Nov 22, 2021 at 11:56:16AM +0100, Arnd Bergmann wrote:
> On Mon, Nov 22, 2021 at 9:35 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
> > On Fri, Nov 19, 2021 at 03:39:01PM -0800, Michael Kelley wrote:
> > > The ARCH_FEATURES function ID is a 32-bit SMC call, which returns
> > > a 32-bit result per the SMCCC spec.  Current code is doing a 64-bit
> > > comparison against -1 (SMCCC_RET_NOT_SUPPORTED) to detect that the
> > > feature is unimplemented.  That check doesn't work in a Hyper-V VM,
> > > where the upper 32-bits are zero as allowed by the spec.
> > >
> > > Cast the result as an 'int' so the comparison works. The change also
> > > makes the code consistent with other similar checks in this file.
> > >
> > > Fixes: 821b67fa4639 ("firmware: smccc: Add ARCH_SOC_ID support")
> >
> > Good catch.
> >
> > Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> >
> > Can you please re-post with my review tag keeping arm@kernel.org and
> > soc@kernel.org ? I don't have any other fixes at the moment, we can
> > ask SoC maintainers to fix this up directly.
>
> In general, I think the easiest way would be for you to forward the
> patch with your Signed-off-by, when you don't have other material
> for a pull request.
>

Thanks for the tip, I will follow that in future.

> I've applied this patch from the list now.
>

Thanks for that.

--
Regards,
Sudeep
diff mbox series

Patch

diff --git a/drivers/firmware/smccc/soc_id.c b/drivers/firmware/smccc/soc_id.c
index 581aa5e..dd7c3d5 100644
--- a/drivers/firmware/smccc/soc_id.c
+++ b/drivers/firmware/smccc/soc_id.c
@@ -50,7 +50,7 @@  static int __init smccc_soc_init(void)
 	arm_smccc_1_1_invoke(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
 			     ARM_SMCCC_ARCH_SOC_ID, &res);
 
-	if (res.a0 == SMCCC_RET_NOT_SUPPORTED) {
+	if ((int)res.a0 == SMCCC_RET_NOT_SUPPORTED) {
 		pr_info("ARCH_SOC_ID not implemented, skipping ....\n");
 		return 0;
 	}