[v2] xen/arm: Add support for S32CC platforms and LINFlexD UART

[PATCH v2 4/8] xen/arm: vsmc: Enable handling SiP-owned SCMI SMC calls

Posted by Andrei Cherechesu (OSS) 1 month, 3 weeks ago

From: Andrei Cherechesu <andrei.cherechesu@nxp.com>

Change the handling of SiP SMC calls to be more generic,
instead of directly relying on the `platform_smc()` callback
implementation.

Try to handle the SiP SMC first through the `platform_smc()`
callback (if implemented). If not handled, check if the
SCMI layer is available and that the SMC is a valid SCMI
message. Handle it then within the SCMI layer which forwards
it to EL3 FW, only if the SMC comes from Dom0.

Signed-off-by: Andrei Cherechesu <andrei.cherechesu@nxp.com>
---
 xen/arch/arm/vsmc.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
index 7f2f5eb9ce..0de194a132 100644
--- a/xen/arch/arm/vsmc.c
+++ b/xen/arch/arm/vsmc.c
@@ -14,6 +14,7 @@
 #include <asm/cpufeature.h>
 #include <asm/monitor.h>
 #include <asm/regs.h>
+#include <asm/scmi-smc.h>
 #include <asm/smccc.h>
 #include <asm/tee/ffa.h>
 #include <asm/tee/tee.h>
@@ -224,6 +225,22 @@ static bool handle_sssc(struct cpu_user_regs *regs)
     }
 }
 
+/* Secure Calls defined by the Silicon Provider (SiP) */
+static bool handle_sip(struct cpu_user_regs *regs)
+{
+    uint32_t fid = (uint32_t)get_user_reg(regs, 0);
+
+    /* Firstly, let each platform define custom handling for these SMCs */
+    if ( platform_smc(regs) )
+        return true;
+
+    /* Otherwise, if valid SCMI SMC, forward the call to EL3 */
+    if ( scmi_is_enabled() && scmi_is_valid_smc_id(fid) )
+        return scmi_handle_smc(regs);
+
+    return false;
+}
+
 /*
  * vsmccc_handle_call() - handle SMC/HVC call according to ARM SMCCC.
  * returns true if that was valid SMCCC call (even if function number
@@ -288,7 +305,7 @@ static bool vsmccc_handle_call(struct cpu_user_regs *regs)
             handled = handle_sssc(regs);
             break;
         case ARM_SMCCC_OWNER_SIP:
-            handled = platform_smc(regs);
+            handled = handle_sip(regs);
             break;
         case ARM_SMCCC_OWNER_TRUSTED_APP ... ARM_SMCCC_OWNER_TRUSTED_APP_END:
         case ARM_SMCCC_OWNER_TRUSTED_OS ... ARM_SMCCC_OWNER_TRUSTED_OS_END:
-- 
2.45.2

Re: [PATCH v2 4/8] xen/arm: vsmc: Enable handling SiP-owned SCMI SMC calls

Posted by Julien Grall 1 month, 3 weeks ago

Hi,

On 30/09/2024 12:47, Andrei Cherechesu (OSS) wrote:
> From: Andrei Cherechesu <andrei.cherechesu@nxp.com>
> 
> Change the handling of SiP SMC calls to be more generic,
> instead of directly relying on the `platform_smc()` callback
> implementation.
> 
> Try to handle the SiP SMC first through the `platform_smc()`
> callback (if implemented). If not handled, check if the
> SCMI layer is available and that the SMC is a valid SCMI
> message. Handle it then within the SCMI layer which forwards
> it to EL3 FW, only if the SMC comes from Dom0.

NIT: I would remove the last sentence as this is implementation details. 
But if you want to keep it, then s/Dom0/Hardware domain/

> 
> Signed-off-by: Andrei Cherechesu <andrei.cherechesu@nxp.com>
> ---
>   xen/arch/arm/vsmc.c | 19 ++++++++++++++++++-
>   1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
> index 7f2f5eb9ce..0de194a132 100644
> --- a/xen/arch/arm/vsmc.c
> +++ b/xen/arch/arm/vsmc.c
> @@ -14,6 +14,7 @@
>   #include <asm/cpufeature.h>
>   #include <asm/monitor.h>
>   #include <asm/regs.h>
> +#include <asm/scmi-smc.h>
>   #include <asm/smccc.h>
>   #include <asm/tee/ffa.h>
>   #include <asm/tee/tee.h>
> @@ -224,6 +225,22 @@ static bool handle_sssc(struct cpu_user_regs *regs)
>       }
>   }
>   
> +/* Secure Calls defined by the Silicon Provider (SiP) */
> +static bool handle_sip(struct cpu_user_regs *regs)
> +{
> +    uint32_t fid = (uint32_t)get_user_reg(regs, 0);
> +
> +    /* Firstly, let each platform define custom handling for these SMCs */
> +    if ( platform_smc(regs) )
> +        return true;
> +
> +    /* Otherwise, if valid SCMI SMC, forward the call to EL3 */

This comment is likely going to stale. This is up to smci_handle_smc() 
to decide what to do. So I would remove this comment.

With that:

Acked-by: Julien Grall <jgrall@amazon.com>

Cheers,

-- 
Julien Grall

Re: [PATCH v2 4/8] xen/arm: vsmc: Enable handling SiP-owned SCMI SMC calls

Posted by Andrei Cherechesu 1 month, 2 weeks ago

Hi Julien,


On 01/10/2024 12:46, Julien Grall wrote:
> Hi,
>
> On 30/09/2024 12:47, Andrei Cherechesu (OSS) wrote:
>> From: Andrei Cherechesu <andrei.cherechesu@nxp.com>
>>
>> Change the handling of SiP SMC calls to be more generic,
>> instead of directly relying on the `platform_smc()` callback
>> implementation.
>>
>> Try to handle the SiP SMC first through the `platform_smc()`
>> callback (if implemented). If not handled, check if the
>> SCMI layer is available and that the SMC is a valid SCMI
>> message. Handle it then within the SCMI layer which forwards
>> it to EL3 FW, only if the SMC comes from Dom0.
>
> NIT: I would remove the last sentence as this is implementation details. But if you want to keep it, then s/Dom0/Hardware domain/

Agree, I'll remove it.

>
>
>>
>> Signed-off-by: Andrei Cherechesu <andrei.cherechesu@nxp.com>
>> ---
>>   xen/arch/arm/vsmc.c | 19 ++++++++++++++++++-
>>   1 file changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
>> index 7f2f5eb9ce..0de194a132 100644
>> --- a/xen/arch/arm/vsmc.c
>> +++ b/xen/arch/arm/vsmc.c
>> @@ -14,6 +14,7 @@
>>   #include <asm/cpufeature.h>
>>   #include <asm/monitor.h>
>>   #include <asm/regs.h>
>> +#include <asm/scmi-smc.h>
>>   #include <asm/smccc.h>
>>   #include <asm/tee/ffa.h>
>>   #include <asm/tee/tee.h>
>> @@ -224,6 +225,22 @@ static bool handle_sssc(struct cpu_user_regs *regs)
>>       }
>>   }
>>   +/* Secure Calls defined by the Silicon Provider (SiP) */
>> +static bool handle_sip(struct cpu_user_regs *regs)
>> +{
>> +    uint32_t fid = (uint32_t)get_user_reg(regs, 0);
>> +
>> +    /* Firstly, let each platform define custom handling for these SMCs */
>> +    if ( platform_smc(regs) )
>> +        return true;
>> +
>> +    /* Otherwise, if valid SCMI SMC, forward the call to EL3 */
>
> This comment is likely going to stale. This is up to smci_handle_smc() to decide what to do. So I would remove this comment.

I'll remove this, since the behaviour is up to the SCMI SMC layer.

>
>
> With that:
>
> Acked-by: Julien Grall <jgrall@amazon.com>

Thanks for the review.

>
> Cheers,
>

Regards,
Andrei C

Re: [PATCH v2 4/8] xen/arm: vsmc: Enable handling SiP-owned SCMI SMC calls

Posted by Stefano Stabellini 1 month, 3 weeks ago

On Mon, 30 Sep 2024, Andrei Cherechesu (OSS) wrote:
> From: Andrei Cherechesu <andrei.cherechesu@nxp.com>
> 
> Change the handling of SiP SMC calls to be more generic,
> instead of directly relying on the `platform_smc()` callback
> implementation.
> 
> Try to handle the SiP SMC first through the `platform_smc()`
> callback (if implemented). If not handled, check if the
> SCMI layer is available and that the SMC is a valid SCMI
> message. Handle it then within the SCMI layer which forwards
> it to EL3 FW, only if the SMC comes from Dom0.
> 
> Signed-off-by: Andrei Cherechesu <andrei.cherechesu@nxp.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/arch/arm/vsmc.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
> index 7f2f5eb9ce..0de194a132 100644
> --- a/xen/arch/arm/vsmc.c
> +++ b/xen/arch/arm/vsmc.c
> @@ -14,6 +14,7 @@
>  #include <asm/cpufeature.h>
>  #include <asm/monitor.h>
>  #include <asm/regs.h>
> +#include <asm/scmi-smc.h>
>  #include <asm/smccc.h>
>  #include <asm/tee/ffa.h>
>  #include <asm/tee/tee.h>
> @@ -224,6 +225,22 @@ static bool handle_sssc(struct cpu_user_regs *regs)
>      }
>  }
>  
> +/* Secure Calls defined by the Silicon Provider (SiP) */
> +static bool handle_sip(struct cpu_user_regs *regs)
> +{
> +    uint32_t fid = (uint32_t)get_user_reg(regs, 0);
> +
> +    /* Firstly, let each platform define custom handling for these SMCs */
> +    if ( platform_smc(regs) )
> +        return true;
> +
> +    /* Otherwise, if valid SCMI SMC, forward the call to EL3 */
> +    if ( scmi_is_enabled() && scmi_is_valid_smc_id(fid) )
> +        return scmi_handle_smc(regs);
> +
> +    return false;
> +}
> +
>  /*
>   * vsmccc_handle_call() - handle SMC/HVC call according to ARM SMCCC.
>   * returns true if that was valid SMCCC call (even if function number
> @@ -288,7 +305,7 @@ static bool vsmccc_handle_call(struct cpu_user_regs *regs)
>              handled = handle_sssc(regs);
>              break;
>          case ARM_SMCCC_OWNER_SIP:
> -            handled = platform_smc(regs);
> +            handled = handle_sip(regs);
>              break;
>          case ARM_SMCCC_OWNER_TRUSTED_APP ... ARM_SMCCC_OWNER_TRUSTED_APP_END:
>          case ARM_SMCCC_OWNER_TRUSTED_OS ... ARM_SMCCC_OWNER_TRUSTED_OS_END:
> -- 
> 2.45.2
>

[PATCH v2 1/8] xen/arm: Add NXP LINFlexD UART Driver
[PATCH v2 2/8] xen/arm: Add NXP LINFlexD UART early printk support
[PATCH v2 3/8] xen/arm: Add SCMI over SMC calls handling layer
[PATCH v2 4/8] xen/arm: vsmc: Enable handling SiP-owned SCMI SMC calls
[PATCH v2 5/8] xen/arm: platforms: Add NXP S32CC platform code
[PATCH v2 6/8] CHANGELOG.md: Add NXP S32CC and SCMI-SMC layer support mentions
[PATCH v2 7/8] SUPPORT.md: Describe SCMI-SMC layer feature
[PATCH v2 8/8] MAINTAINERS: Add myself as maintainer for NXP S32CC