Re: [PATCH v4 15/24] xen/domctl: wrap device-tree-subset iommu-related domctl op with CONFIG_MGMT_HYPERCALLS

[Grygorii Strashko <grygorii_strashko@xxxxxxxx>]

Hi Penny,

On 21.11.25 12:57, Penny Zheng wrote:
> Function iommu_do_dt_domctl() is the main entry for all device-tree-subset
> iommu-related domctl-op, and shall be wrapped with CONFIG_MGMT_HYPERCALLS.
> Tracking its calling chain, the following functions shall all be wrapped
> with CONFIG_MGMT_HYPERCALLS:
> - make PCI_PASSTHROUGH depend on MGMT_HYPERCALLS
> - iommu_do_dt_domctl
>    - xsm_assign_dtdevice
>    - xsm_deassign_dtdevice
>    - iommu_deassign_dt_device
>      - arm_smmu_reassign_dev
>        - arm_smmu_deassign_dev
>          - arm_smmu_detach_dev
>            - arm_smmu_domain_remove_master
>      - ipmmu_reassign_device
>        - ipmmu_deassign_device
>          - ipmmu_detach_device
>    - iommu_remove_dt_device
>      - iommu_dt_device_is_assigned_locked
>    - dt_find_node_by_gpath
> Otherwise all the functions will become unreachable when MGMT_HYPERCALLS=n,
> and hence violating Misra rule 2.1
> Move codes closer to avoid scattering #ifdef
>
> Signed-off-by: Penny Zheng <Penny.Zheng@xxxxxxx>
> ---
> v3 -> v4
> - split into PCI related subset and DT subset
> - Move codes closer to avoid scattering #ifdef
> ---
>   xen/arch/arm/Kconfig                     |   2 +-
>   xen/common/device-tree/device-tree.c     |   2 +
>   xen/drivers/passthrough/arm/ipmmu-vmsa.c |  26 +++---
>   xen/drivers/passthrough/arm/smmu-v3.c    |   4 +
>   xen/drivers/passthrough/arm/smmu.c       |  55 ++++++------
>   xen/drivers/passthrough/device_tree.c    | 108 ++++++++++++-----------
>   xen/include/xsm/dummy.h                  |   6 +-
>   xen/include/xsm/xsm.h                    |  12 +--
>   xen/xsm/dummy.c                          |   6 +-
>   xen/xsm/flask/hooks.c                    |  12 +--
>   10 files changed, 126 insertions(+), 107 deletions(-)
>
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index cf6af68299..5a5d7810c8 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -270,7 +270,7 @@ source "arch/arm/firmware/Kconfig"
>   
>   config PCI_PASSTHROUGH
>         bool "PCI passthrough" if EXPERT
> -       depends on ARM_64 && HAS_PASSTHROUGH
> +       depends on ARM_64 && HAS_PASSTHROUGH && MGMT_HYPERCALLS
>         help
>           This option enables PCI device passthrough
>   
> diff --git a/xen/common/device-tree/device-tree.c b/xen/common/device-tree/device-tree.c
> index 0b5375f151..70bd8e7da5 100644
> --- a/xen/common/device-tree/device-tree.c
> +++ b/xen/common/device-tree/device-tree.c
> @@ -371,6 +371,7 @@ struct dt_device_node *dt_find_node_by_path_from(struct 
> dt_device_node *from,
>       return np;
>   }
>   
> +#ifdef CONFIG_MGMT_HYPERCALLS
>   int dt_find_node_by_gpath(XEN_GUEST_HANDLE(char) u_path, uint32_t u_plen,
>                             struct dt_device_node **node)
>   {
> @@ -386,6 +387,7 @@ int dt_find_node_by_gpath(XEN_GUEST_HANDLE(char) u_path, 
> uint32_t u_plen,
>   
>       return (*node == NULL) ? -ESRCH : 0;
>   }
> +#endif /* CONFIG_MGMT_HYPERCALLS */
The common/device-tree/device-tree.c contains generic unflatten DT helpers API,
while dt_find_node_by_gpath() is specific for domctl (iommu) processing.

Therefore I'd like to propose to move this function into 
drivers\passthrough\device_tree.c
as it is used only there now.

>   
>   struct dt_device_node *dt_find_node_by_alias(const char *alias)
>   {
[...]

> diff --git a/xen/drivers/passthrough/arm/smmu-v3.c 
> b/xen/drivers/passthrough/arm/smmu-v3.c
> index bf153227db..22def57b03 100644
> --- a/xen/drivers/passthrough/arm/smmu-v3.c
> +++ b/xen/drivers/passthrough/arm/smmu-v3.c
> @@ -2759,6 +2759,7 @@ out:
>         return ret;
>   }
There is not wrapped arm_smmu_deassign_dev() at the beginning of the file which 
causes
ARM64 build failure:

drivers/passthrough/arm/smmu-v3.c:1474:12: error: 'arm_smmu_deassign_dev' 
declared 'static' but never defined [-Werror=unused-function]
 1474 | static int arm_smmu_deassign_dev(struct domain *d, uint8_t devfn,


>   
> +#ifdef CONFIG_MGMT_HYPERCALLS
>   static int arm_smmu_deassign_dev(struct domain *d, uint8_t devfn, struct 
> device *dev)
>   {
>         struct iommu_domain *io_domain = arm_smmu_get_domain(d, dev);
> @@ -2826,6 +2827,7 @@ static int arm_smmu_reassign_dev(struct domain *s, struct 
> domain *t,
>   
>   	return 0;
>   }
> +#endif /* CONFIG_MGMT_HYPERCALLS */
>   
>   static int arm_smmu_iommu_xen_domain_init(struct domain *d)
>   {
> @@ -2862,7 +2864,9 @@ static const struct iommu_ops arm_smmu_iommu_ops = {
>         .teardown               = arm_smmu_iommu_xen_domain_teardown,
>         .iotlb_flush            = arm_smmu_iotlb_flush,
>         .assign_device          = arm_smmu_assign_dev,
> +#ifdef CONFIG_MGMT_HYPERCALLS
>         .reassign_device        = arm_smmu_reassign_dev,
> +#endif
>         .map_page               = arm_iommu_map_page,
>         .unmap_page             = arm_iommu_unmap_page,
>         .dt_xlate               = arm_smmu_dt_xlate,
> diff --git a/xen/drivers/passthrough/arm/smmu.c 
> b/xen/drivers/passthrough/arm/smmu.c
> index 22d306d0cb..a75ec08633 100644
[...]

--
Best regards,
-grygorii