| Message ID | 6a99f9b99b419a20e895d54db2e345c80270248c.1658804819.git-series.marmarek@invisiblethingslab.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add Xue - console over USB 3 Debug Capability | expand |
On 26.07.2022 05:23, Marek Marczykowski-Górecki wrote: > --- a/xen/drivers/passthrough/iommu.c > +++ b/xen/drivers/passthrough/iommu.c > @@ -651,6 +651,51 @@ bool_t iommu_has_feature(struct domain *d, enum iommu_feature feature) > return is_iommu_enabled(d) && test_bit(feature, dom_iommu(d)->features); > } > > +#define MAX_EXTRA_RESERVED_RANGES 20 > +struct extra_reserved_range { > + unsigned long start; > + unsigned long nr; > + uint32_t sbdf; It's not easy to judge why this isn't pci_sbdf_t when no callers exist at this point. > +}; > +static unsigned int __initdata nr_extra_reserved_ranges; > +static struct extra_reserved_range __initdata > + extra_reserved_ranges[MAX_EXTRA_RESERVED_RANGES]; With __initdata here, ... > +int iommu_add_extra_reserved_device_memory(unsigned long start, > + unsigned long nr, > + uint32_t sbdf) ... this and the other function want to be __init. Jan
On Thu, Aug 04, 2022 at 04:25:38PM +0200, Jan Beulich wrote: > On 26.07.2022 05:23, Marek Marczykowski-Górecki wrote: > > --- a/xen/drivers/passthrough/iommu.c > > +++ b/xen/drivers/passthrough/iommu.c > > @@ -651,6 +651,51 @@ bool_t iommu_has_feature(struct domain *d, enum iommu_feature feature) > > return is_iommu_enabled(d) && test_bit(feature, dom_iommu(d)->features); > > } > > > > +#define MAX_EXTRA_RESERVED_RANGES 20 > > +struct extra_reserved_range { > > + unsigned long start; > > + unsigned long nr; > > + uint32_t sbdf; > > It's not easy to judge why this isn't pci_sbdf_t when no callers > exist at this point. I'm following here types used in the rest of IOMMU code. Especially, this field is later passed to iommu_grdm_t func, which is: typedef int iommu_grdm_t(xen_pfn_t start, xen_ulong_t nr, u32 id, void *ctxt); ^^^^ I can probably use pci_sbdf_t here, but it will be cast to u32 later anyway... > > +}; > > +static unsigned int __initdata nr_extra_reserved_ranges; > > +static struct extra_reserved_range __initdata > > + extra_reserved_ranges[MAX_EXTRA_RESERVED_RANGES]; > > With __initdata here, ... > > > +int iommu_add_extra_reserved_device_memory(unsigned long start, > > + unsigned long nr, > > + uint32_t sbdf) > > ... this and the other function want to be __init. Ok.
On 04.08.2022 16:38, Marek Marczykowski-Górecki wrote: > On Thu, Aug 04, 2022 at 04:25:38PM +0200, Jan Beulich wrote: >> On 26.07.2022 05:23, Marek Marczykowski-Górecki wrote: >>> --- a/xen/drivers/passthrough/iommu.c >>> +++ b/xen/drivers/passthrough/iommu.c >>> @@ -651,6 +651,51 @@ bool_t iommu_has_feature(struct domain *d, enum iommu_feature feature) >>> return is_iommu_enabled(d) && test_bit(feature, dom_iommu(d)->features); >>> } >>> >>> +#define MAX_EXTRA_RESERVED_RANGES 20 >>> +struct extra_reserved_range { >>> + unsigned long start; >>> + unsigned long nr; >>> + uint32_t sbdf; >> >> It's not easy to judge why this isn't pci_sbdf_t when no callers >> exist at this point. > > I'm following here types used in the rest of IOMMU code. Especially, > this field is later passed to iommu_grdm_t func, which is: > > typedef int iommu_grdm_t(xen_pfn_t start, xen_ulong_t nr, u32 id, void *ctxt); > ^^^^ > > I can probably use pci_sbdf_t here, but it will be cast to u32 later > anyway... No, rather than a cast you'd use the union's sbdf field. And yes, eventually that function typedef you refer to will want switching to pci_sbdf_t as well. Jan
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c index 77f64e61748d..74efd865ab69 100644 --- a/xen/drivers/passthrough/iommu.c +++ b/xen/drivers/passthrough/iommu.c @@ -651,6 +651,51 @@ bool_t iommu_has_feature(struct domain *d, enum iommu_feature feature) return is_iommu_enabled(d) && test_bit(feature, dom_iommu(d)->features); } +#define MAX_EXTRA_RESERVED_RANGES 20 +struct extra_reserved_range { + unsigned long start; + unsigned long nr; + uint32_t sbdf; +}; +static unsigned int __initdata nr_extra_reserved_ranges; +static struct extra_reserved_range __initdata + extra_reserved_ranges[MAX_EXTRA_RESERVED_RANGES]; + +int iommu_add_extra_reserved_device_memory(unsigned long start, + unsigned long nr, + uint32_t sbdf) +{ + unsigned int idx; + + if ( nr_extra_reserved_ranges >= MAX_EXTRA_RESERVED_RANGES ) + return -ENOMEM; + + idx = nr_extra_reserved_ranges++; + extra_reserved_ranges[idx].start = start; + extra_reserved_ranges[idx].nr = nr; + extra_reserved_ranges[idx].sbdf = sbdf; + + return 0; +} + +int iommu_get_extra_reserved_device_memory(iommu_grdm_t *func, void *ctxt) +{ + unsigned int idx; + int ret; + + for ( idx = 0; idx < nr_extra_reserved_ranges; idx++ ) + { + ret = func(extra_reserved_ranges[idx].start, + extra_reserved_ranges[idx].nr, + extra_reserved_ranges[idx].sbdf, + ctxt); + if ( ret < 0 ) + return ret; + } + + return 0; +} + /* * Local variables: * mode: C diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h index 79529adf1fa5..aa87c3fd9ebc 100644 --- a/xen/include/xen/iommu.h +++ b/xen/include/xen/iommu.h @@ -297,6 +297,19 @@ struct iommu_ops { #endif }; +/* + * To be called by Xen internally, to register extra RMRR/IVMD ranges. + * Needs to be called before IOMMU initialization. + */ +extern int iommu_add_extra_reserved_device_memory(unsigned long start, + unsigned long nr, + uint32_t sbdf); +/* + * To be called by specific IOMMU driver during initialization, + * to fetch ranges registered with iommu_add_extra_reserved_device_memory(). + */ +extern int iommu_get_extra_reserved_device_memory(iommu_grdm_t *func, void *ctxt); + #include <asm/iommu.h> #ifndef iommu_call
Add API similar to rmrr= and ivmd= arguments, but in a common code. This will allow drivers to register reserved memory regardless of the IOMMU vendor. The direct reason for this API is xhci-dbc console driver (aka xue), that needs to use DMA. But future change may unify command line arguments for user-supplied reserved memory, and it may be useful for other drivers in the future too. This commit just introduces an API, subsequent patches will plug it in appropriate places. The reserved memory ranges needs to be saved locally, because at the point when they are collected, Xen doesn't know yet which IOMMU driver will be used. Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- Changes in v3: - adjust code style --- xen/drivers/passthrough/iommu.c | 45 ++++++++++++++++++++++++++++++++++- xen/include/xen/iommu.h | 13 ++++++++++- 2 files changed, 58 insertions(+)