Re: [PATCH v6 1/4] xen/arm: add generic SCI subsystem

[Oleksii Moisieiev <Oleksii_Moisieiev@xxxxxxxx>]

On 29/08/2025 16:42, Oleksandr Tyshchenko wrote:
>
>
> On 28.08.25 19:40, Oleksii Moisieiev wrote:
>
> Hello Oleksii
>
>
>> This patch adds the basic framework for ARM SCI mediator. SCI is System
>> Control Interface, which is designed to redirect requests from the 
>> Domains
>> to ARM specific Firmware (for example SCMI). This will allow the 
>> devices,
>> passed-through to the different Domains, to access to the System 
>> resources
>> (such as clocks/resets etc) by sending requests to the firmware.
>>
>> ARM SCI subsystem allows to implement different SCI drivers to handle
>> specific ARM firmware interfaces (like ARM SCMI) and mediate requests
>> -between the Domains and the Firmware. Also it allows SCI drivers to 
>> perform
>> proper action during Domain creation/destruction which is vital for
>> handling use cases like Domain reboot.
>>
>> This patch introduces new DEVICE_FIRMWARE device subclass for probing 
>> SCI
>> drivers basing on device tree, SCI drivers register itself with
>> DT_DEVICE_START/END macro. On init - the SCI drivers should register its
>> SCI ops with sci_register(). Only one SCI driver can be supported.
>>
>> At run-time, the following SCI API calls are introduced:
>>
>> - sci_domain_sanitise_config() called from arch_sanitise_domain_config()
>> - sci_domain_init() called from arch_domain_create()
>> - sci_relinquish_resources() called from domain_relinquish_resources()
>> - sci_domain_destroy() called from arch_domain_destroy()
>> - sci_handle_call() called from vsmccc_handle_call()
>> - sci_dt_handle_node()
>> - sci_dt_finalize() called from handle_node() (Dom0 DT)
>>
>> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@xxxxxxxx>
>> Signed-off-by: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
>> Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>> ---
>>
>> Changes in v6:
>> - rebase on top of the latest master
>> - fix return value of sci_dt_finalize() call
>>
>> Changes in v5:
>> - update Maintainers file. Set role as a Reviewer
>> - rebased on the latest master branch
>> - Introduce arch_handle_passthrough_prop call to handle arm specific 
>> nodes
>>
>> Changes in v4:
>> - fix SPDX-License
>> - rename DEVICE_ARM_SCI DT device class to FIRMWARE_DEVICE
>> - move XEN_DOMCTL_assign_device code in separate patch
>> - Add documentation for SCI SCMI drivers
>>
>>   MAINTAINERS                             |   6 +
>>   xen/arch/arm/device.c                   |   5 +
>>   xen/arch/arm/dom0less-build.c           |   8 +
>>   xen/arch/arm/domain.c                   |  12 +-
>>   xen/arch/arm/domain_build.c             |   8 +
>>   xen/arch/arm/firmware/Kconfig           |   8 +
>>   xen/arch/arm/firmware/Makefile          |   1 +
>>   xen/arch/arm/firmware/sci.c             | 154 ++++++++++++++++++
>>   xen/arch/arm/include/asm/domain.h       |   5 +
>>   xen/arch/arm/include/asm/firmware/sci.h | 200 ++++++++++++++++++++++++
>>   xen/arch/arm/vsmc.c                     |   3 +
>>   xen/common/device-tree/dom0less-build.c |   4 +
>>   xen/include/asm-generic/device.h        |   1 +
>>   xen/include/public/arch-arm.h           |   4 +
>>   xen/include/xen/dom0less-build.h        |   9 ++
>>   15 files changed, 427 insertions(+), 1 deletion(-)
>>   create mode 100644 xen/arch/arm/firmware/sci.c
>>   create mode 100644 xen/arch/arm/include/asm/firmware/sci.h
>
> [snip]
>
>> diff --git a/xen/arch/arm/include/asm/firmware/sci.h 
>> b/xen/arch/arm/include/asm/firmware/sci.h
>> new file mode 100644
>> index 0000000000..1b9a5c0c07
>> --- /dev/null
>> +++ b/xen/arch/arm/include/asm/firmware/sci.h
>> @@ -0,0 +1,200 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Generic ARM SCI (System Control Interface) subsystem.
>> + *
>> + * Oleksii Moisieiev <oleksii_moisieiev@xxxxxxxx>
>> + * Copyright (c) 2025 EPAM Systems
>> + */
>> +
>> +#ifndef __ASM_ARM_SCI_H
>> +#define __ASM_ARM_SCI_H
>> +
>> +#include <xen/lib.h>
>> +#include <xen/types.h>
>> +#include <xen/device_tree.h>
>> +#include <xen/errno.h>
>> +#include <xen/sched.h>
>
> NIT: Looks like the headers are not alphabetically sorted
>
> [snip]
>
>>   diff --git a/xen/include/xen/dom0less-build.h 
>> b/xen/include/xen/dom0less-build.h
>> index 408859e325..6fdd0552e0 100644
>> --- a/xen/include/xen/dom0less-build.h
>> +++ b/xen/include/xen/dom0less-build.h
>> @@ -62,6 +62,9 @@ void set_domain_type(struct domain *d, struct 
>> kernel_info *kinfo);
>>   int init_intc_phandle(struct kernel_info *kinfo, const char *name,
>>                         const int node_next, const void *pfdt);
>>   +int arch_handle_passthrough_prop(struct kernel_info *kinfo,
>> +                                 struct dt_device_node *node);
>> +
>>   #else /* !CONFIG_DOM0LESS_BOOT */
>>     static inline void create_domUs(void) {}
>> @@ -71,6 +74,12 @@ static inline bool is_dom0less_mode(void)
>>   }
>>   static inline void set_xs_domain(struct domain *d) {}
>>   +static inline int arch_handle_passthrough_prop(struct kernel_info 
>> *kinfo,
>> +                                               struct dt_device_node 
>> *node)
>> +{
>> +    return 0;
>> +}
>> +
>>   #endif /* CONFIG_DOM0LESS_BOOT */
>>     #endif /* __ASM_GENERIC_DOM0LESS_BUILD_H__ */
>
>
> If I build current patch with # CONFIG_DOM0LESS_BOOT is not set
> I got the following error:
>
> CC      arch/arm/domain_build.o
>   CC      arch/arm/guestcopy.o
> In file included from arch/arm/domain_build.c:5:
> ./include/xen/dom0less-build.h:78:55: error: 'struct dt_device_node' 
> declared inside parameter list will not be visible outside of this 
> definition or declaration [-Werror]
>    78 |                                                struct 
> dt_device_node *node)
>       | ^~~~~~~~~~~~~~
> ./include/xen/dom0less-build.h:77:55: error: 'struct kernel_info' 
> declared inside parameter list will not be visible outside of this 
> definition or declaration [-Werror]
>    77 | static inline int arch_handle_passthrough_prop(struct 
> kernel_info *kinfo,
>       | ^~~~~~~~~~~
>
Good finding. Thanks. Will be fixed.