RE: [PATCH] iommu/vt-d: Increase DMAR_UNITS_SUPPORTED

[Tian, Kevin]

> From: David Woodhouse <dwmw2@xxxxxxxxxxxxx>
> Sent: Friday, May 6, 2022 3:17 PM
> 
> On Fri, 2022-05-06 at 06:49 +0000, Tian, Kevin wrote:
> > > From: Baolu Lu <baolu.lu@xxxxxxxxxxxxxxx>
> > >
> > > > --- a/include/linux/dmar.h
> > > > +++ b/include/linux/dmar.h
> > > > @@ -19,7 +19,7 @@
> > > >   struct acpi_dmar_header;
> > > >
> > > >   #ifdef	CONFIG_X86
> > > > -# define	DMAR_UNITS_SUPPORTED	MAX_IO_APICS
> > > > +# define	DMAR_UNITS_SUPPORTED	640
> > > >   #else
> > > >   # define	DMAR_UNITS_SUPPORTED	64
> > > >   #endif
> >
> > ... is it necessary to permanently do 10x increase which wastes memory
> > on most platforms which won't have such need.
> 
> I was just looking at that. It mostly adds about 3½ KiB to each struct
> dmar_domain.
> 
> I think the only actual static array is the dmar_seq_ids bitmap which
> grows to 640 *bits* which is fairly negligible, and the main growth is
> that it adds about 3½ KiB to each struct dmar_domain for the
> iommu_refcnt[] and iommu_did[] arrays.

Thanks for the quick experiment! though the added material is
negligible it's cleaner to me if having a way to configure it as
discussed below.

> 
> > Does it make more sense to have a configurable approach similar to
> > CONFIG_NR_CPUS? or even better can we just replace those static
> > arrays with dynamic allocation so removing this restriction
> > completely?
> 
> Hotplug makes that fun, but I suppose you only need to grow the array
> in a given struct dmar_domain if you actually add a device to it that's
> behind a newly added IOMMU. I don't know if the complexity of making it
> fully dynamic is worth it though. We could make it a config option,
> and/or a command line option (perhaps automatically derived from
> CONFIG_NR_CPUS).

either config option or command line option is OK to me. Probably
the former is simpler given no need to dynamically expand the
static array. btw though deriving from CONFIG_NR_CPUS could work 
in this case it is unclear why tying the two together is necessary in
concept, e.g. is there guarantee that the number of IOMMUs must
be smaller than the number of CPUs in a platform?

> 
> If it wasn't for hotplug, I think we'd know the right number by the
> time we actually need it anyway, wouldn't we? Can we have a heuristic
> for how many DMAR units are likely to be hotplugged? Is it as simple as
> the ratio of present to not-yet-present CPUs in MADT?

Probably. But I don't have enough knowledge on DMAR hotplug to
judge (e.g. whether it's strictly tied to CPU hotplug and if yes whether
there could be multiple IOMMUs hotplugged together with a CPU
socket)...

Thanks
Kevin