Re: [PATCH 6/6] VT-d: avoid allocating domid_{bit,}map[] when possible

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.11.2021 07:18, Tian, Kevin wrote:
>> From: Jan Beulich <jbeulich@xxxxxxxx>
>> Sent: Friday, November 12, 2021 5:50 PM
>>
>> When an IOMMU implements the full 16 bits worth of DID in context
>> entries, there's no point going through a memory base translation table.
>> For IOMMUs not using Caching Mode we can simply use the domain IDs
>> verbatim, while for Caching Mode we need to avoid DID 0.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> ---
>> For the case where the memory tables are needed, xvzalloc_array() would
>> of course be an option to use here as well, despite this being boot time
>> allocations. Yet the introduction of xvmalloc() et al continues to be
>> stuck ...
>>
>> --- a/xen/drivers/passthrough/vtd/iommu.c
>> +++ b/xen/drivers/passthrough/vtd/iommu.c
>> @@ -62,11 +62,32 @@ static struct tasklet vtd_fault_tasklet;
>>  static int setup_hwdom_device(u8 devfn, struct pci_dev *);
>>  static void setup_hwdom_rmrr(struct domain *d);
>>
>> +static bool domid_mapping(const struct vtd_iommu *iommu)
>> +{
>> +    return (const void *)iommu->domid_bitmap != (const void *)iommu-
>>> domid_map;
>> +}
>> +
>> +static domid_t convert_domid(const struct vtd_iommu *iommu, domid_t
>> domid)
>> +{
>> +    /*
>> +     * While we need to avoid DID 0 for caching-mode IOMMUs, maintain
>> +     * the property of the transformation being the same in either
>> +     * direction. By clipping to 16 bits we ensure that the resulting
>> +     * DID will fit in the respective context entry field.
>> +     */
>> +    BUILD_BUG_ON(sizeof(domid_t) > sizeof(uint16_t));
>> +
>> +    return !cap_caching_mode(iommu->cap) ? domid : ~domid;
> 
> If DOMID_MASK grows to 0xFFFF (though unlikely),

I did consider this too unlikely to warrant taking care of. Now that
you ask for it anyway, ...

> then it translates 
> to '0' when caching mode is true. We need extend BUILD_BUG_ON() to 
> check DOMID_MASK in this case, since caching mode implies
> total_size minus one for available domain IDs 

... I guess I'd rather replace the BUILD_BUG_ON() than extend it or
add a 2nd one.

Jan