> -----Original Message-----
> From: Julien Grall <jul...@xen.org>
> Sent: 16 October 2020 16:47
> To: Paul Durrant <p...@xen.org>; xen-devel@lists.xenproject.org
> Cc: Paul Durrant <pdurr...@amazon.com>; Daniel De Graaf
> <dgde...@tycho.nsa.gov>; Ian Jackson
> <i...@xenproject.org>; Wei Liu <w...@xen.org>; Andrew Cooper
> <andrew.coop...@citrix.com>; George Dunlap
> <george.dun...@citrix.com>; Jan Beulich <jbeul...@suse.com>; Stefano
> Stabellini
> <sstabell...@kernel.org>
> Subject: Re: [PATCH 2/5] iommu / domctl: introduce XEN_DOMCTL_iommu_ctl
>
> Hi Paul,
>
> On 05/10/2020 10:49, Paul Durrant wrote:
> > diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> > index 791f0a2592..75e855625a 100644
> > --- a/xen/include/public/domctl.h
> > +++ b/xen/include/public/domctl.h
> > @@ -1130,6 +1130,18 @@ struct xen_domctl_vuart_op {
> > */
> > };
> >
> > +/*
> > + * XEN_DOMCTL_iommu_ctl
> > + *
> > + * Control of VM IOMMU settings
> > + */
> > +
> > +#define XEN_DOMCTL_IOMMU_INVALID 0
>
> I can't find any user of XEN_DOMCTL_IOMMU_INVALID. What's the purpose
> for it?
>
It's just a placeholder. I think it's generally safer that a zero opcode value
is invalid.
> [...]
>
> > diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
> > index b21c3783d3..1a96d3502c 100644
> > --- a/xen/include/xsm/xsm.h
> > +++ b/xen/include/xsm/xsm.h
> > @@ -106,17 +106,19 @@ struct xsm_operations {
> > int (*irq_permission) (struct domain *d, int pirq, uint8_t allow);
> > int (*iomem_permission) (struct domain *d, uint64_t s, uint64_t e,
> > uint8_t allow);
> > int (*iomem_mapping) (struct domain *d, uint64_t s, uint64_t e,
> > uint8_t allow);
> > - int (*pci_config_permission) (struct domain *d, uint32_t machine_bdf,
> > uint16_t start, uint16_t
> end, uint8_t access);
> > + int (*pci_config_permission) (struct domain *d, uint32_t machine_bdf,
> > uint16_t start, uint16_t
> end, uint8_t access);
>
> I can't figure out what changed here. Is it an intended change?
>
No, I'll check and get rid of it.
Paul
> >
> > -#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_PCI)
> > +#if defined(CONFIG_HAS_PASSTHROUGH)
> > + int (*iommu_ctl) (struct domain *d, unsigned int op);
> > +#if defined(CONFIG_HAS_PCI)
> > int (*get_device_group) (uint32_t machine_bdf);
> > int (*assign_device) (struct domain *d, uint32_t machine_bdf);
> > int (*deassign_device) (struct domain *d, uint32_t machine_bdf);
> > #endif
> > -
> > -#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_DEVICE_TREE)
> > +#if defined(CONFIG_HAS_DEVICE_TREE)
> > int (*assign_dtdevice) (struct domain *d, const char *dtpath);
> > int (*deassign_dtdevice) (struct domain *d, const char *dtpath);
> > +#endif
> > #endif
>
> Cheers,
>
> --
> Julien Grall
