RE: [PATCH v4 14/24] xen/domctl: wrap pci-subset iommu-related domctl op with CONFIG_MGMT_HYPERCALLS

["Penny, Zheng" <penny.zheng@xxxxxxx>]

[Public]

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: Tuesday, November 25, 2025 11:37 PM
> To: Penny, Zheng <penny.zheng@xxxxxxx>
> Cc: Huang, Ray <Ray.Huang@xxxxxxx>; grygorii_strashko@xxxxxxxx; Andrew
> Cooper <andrew.cooper3@xxxxxxxxxx>; Roger Pau Monné <roger.pau@xxxxxxxxxx>;
> Andryuk, Jason <Jason.Andryuk@xxxxxxx>; Daniel P. Smith
> <dpsmith@xxxxxxxxxxxxxxxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH v4 14/24] xen/domctl: wrap pci-subset iommu-related domctl
> op with CONFIG_MGMT_HYPERCALLS
>
> On 21.11.2025 11:57, Penny Zheng wrote:
> >  xen/drivers/passthrough/amd/pci_amd_iommu.c | 20 ++++----
> >  xen/drivers/passthrough/pci.c               | 52 +++++++++++----------
> >  xen/drivers/passthrough/vtd/iommu.c         |  6 ++-
> >  xen/include/xsm/dummy.h                     |  6 ++-
> >  xen/include/xsm/xsm.h                       | 12 +++--
> >  xen/xsm/dummy.c                             |  6 ++-
> >  xen/xsm/flask/hooks.c                       | 12 +++--
> >  7 files changed, 68 insertions(+), 46 deletions(-)
>
> With this diffstat and there being quite a few HAS_PCI under
> xen/drivers/passthrough/arm/, what's the (PCI) deal there?

In arm, we have the following select chain:
HAS_VPCI_GUEST_SUPPORT if PCI_PASSTHROUGH -> HAS_VPCI -> HAS_PCI
So if we make PCI_PASSTHROUGH later depend on MGMT_HYPERCALLS, the PCI-subset 
for arm will also be guarded too.
I'll add description in commit message

> > --- a/xen/include/xsm/dummy.h
> > +++ b/xen/include/xsm/dummy.h
> > @@ -407,7 +407,8 @@ static XSM_INLINE int cf_check xsm_get_vnumainfo(
> >      return xsm_default_action(action, current->domain, d);  }
> >
> > -#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_PCI)
> > +#if defined(CONFIG_HAS_PASSTHROUGH) &&
> > +defined(CONFIG_MGMT_HYPERCALLS) #ifdef CONFIG_HAS_PCI
>
> Why the separate #ifdef? Can't that be folded with the #if? Are there further 
> changes
> to be put inside the outer #if? (Applies again further down as well.)
>

Because later in DT-subset, we want something like the following:
```
#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_MGMT_HYPERCALLS)
#ifdef CONFIG_HAS_PCI
...
#endif
#ifdef CONFIG_HAS_DEVICE_TREE_DISCOVERY
...
#endif
#endif /* HAS_PASSTHROUGH  && MGMT_HYPERCALLS  */
Letting HAS_PASSTHROUGH  && MGMT_HYPERCALLS being outer wrapper and a separate 
#ifdef CONFIG_HAS_PCI here could avoid extra changes in DT commit.

> >  static XSM_INLINE int cf_check xsm_get_device_group(
> >      XSM_DEFAULT_ARG uint32_t machine_bdf)  { @@ -429,7 +430,8 @@
> > static XSM_INLINE int cf_check xsm_deassign_device(
> >      return xsm_default_action(action, current->domain, d);  }
> >
> > -#endif /* HAS_PASSTHROUGH && HAS_PCI */
> > +#endif /* CONFIG_HAS_PCI */
> > +#endif /* HAS_PASSTHROUGH && MGMT_HYPERCALLS */
> >
> >  #if defined(CONFIG_HAS_PASSTHROUGH) &&
> > defined(CONFIG_HAS_DEVICE_TREE_DISCOVERY)
> >  static XSM_INLINE int cf_check xsm_assign_dtdevice(
>
> The DT counterpart, otoh, is separate anyway.
>
> Jan