Re: [Xen-devel] [PATCH v4 12/12] AMD/IOMMU: miscellaneous DTE handling adjustments

[Jan Beulich <JBeulich@suse.com>]

On 30.07.2019 15:42, Andrew Cooper wrote:
> On 25/07/2019 14:33, Jan Beulich wrote:
>> --- a/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h
>> +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h
>> @@ -107,57 +107,60 @@
>>    #define IOMMU_DEV_TABLE_INT_CONTROL_FORWARDED	0x1
>>    #define IOMMU_DEV_TABLE_INT_CONTROL_TRANSLATED	0x2
>>    
>> +/* For now we always allocate maximum possible interrupt remapping tables. */
> 
> /* For now, we always allocate the maximum.  2048 remap entries. */
> 
> ?

Sure, done.

>> +#define IOMMU_INTREMAP_LENGTH			0xB
> 
> Also, LENGTH isn't an appropriate name.  This is actually the order of
> the number of entries.  As you're already changing the name, how about
> s/LENGTH/ORDER/ here?

I did consider this (and will change), but I didn't change it right
away because of the resulting inconsistency on this line

     dte->int_tab_len = IOMMU_INTREMAP_ORDER;

I had taken "length" to mean "encoded length" here, not "actual length".

> If so, Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

Thanks.

> [Not related to this patch...]
> 
> It has always occurred to me that we allocate silly quantities of memory
> for interrupt remapping tables.  If I've done my sums right, for Intel
> we allocate 64k entries per IOMMU (256k RAM), whereas for AMD we
> allocate 2048 entries per PCI function (32k RAM, now with the larger
> format).

Right, that's another thing I wanted to look into as a follow-on. I
too did notice this. Depending what you mean by "PCI function" it
may actually be worse than what you describe: It's not per PCI
function of present devices, but per PCI function enumerated by the
ACPI tables. On my box this means everything from 00:00.0 to
ff:1f.7, which amounts to almost 2Gb if I'm not mistaken ("almost"
because of some aliasing of devices, where only one table gets
allocated for all the aliases).

> The largest Intel system I've encountered (interrupt wise) is a few
> thousand interrupts, split fairly evenly across the root-complex IOMMUs
> (the PCH IOMMU not, because its mostly legacy IO behind there).
> 
> For individual functions, I have never encountered a PCI function with
> more than a dozen interrupts or so, so I think in practice we can get
> away with allocating a 4k (32 entry) interrupt remap table in all cases.

That's clearly a possibility. (I think you meant 256 entries per 4k
though.)

> It would probably make sense to default to allocating less space, and
> providing a command line option to allocate max.  Alternatively, we
> could work this out as we walk the PCI topology, as it is encoded in
> standards compliant ways in config space.

To be honest, first of all I'd like to avoid allocating tables for
devices which don't even exist.

Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel