Re: [PATCH v2] VT-d: Don't assume register-based invalidation is always supported

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.04.2021 17:08, Roger Pau Monné wrote:
> On Thu, Apr 02, 2020 at 04:06:06AM +0800, Chao Gao wrote:
>> --- a/xen/drivers/passthrough/vtd/qinval.c
>> +++ b/xen/drivers/passthrough/vtd/qinval.c
>> @@ -442,6 +442,23 @@ int enable_qinval(struct vtd_iommu *iommu)
>>      return 0;
>>  }
>>  
>> +static int vtd_flush_context_noop(struct vtd_iommu *iommu, uint16_t did,
>> +                                  uint16_t source_id, uint8_t function_mask,
>> +                                  uint64_t type, bool 
>> flush_non_present_entry)
>> +{
>> +    dprintk(XENLOG_ERR VTDPREFIX, "IOMMU: Cannot flush CONTEXT.\n");
>> +    return -EIO;
>> +}
>> +
>> +static int vtd_flush_iotlb_noop(struct vtd_iommu *iommu, uint16_t did,
>> +                                uint64_t addr, unsigned int size_order,
>> +                                uint64_t type, bool flush_non_present_entry,
>> +                                bool flush_dev_iotlb)
>> +{
>> +    dprintk(XENLOG_ERR VTDPREFIX, "IOMMU: Cannot flush IOTLB.\n");
>> +    return -EIO;
>> +}
> 
> I think I would add an ASSERT_UNREACHABLE() to both noop handlers
> above, as I would expect trying to use them without the proper mode
> being configured would point to an error elsewhere?

If such an assertion triggered e.g. during S3 suspend/resume, it may
lead to the box simply not doing anything useful, without there being
any way to know what went wrong. If instead the system at least
managed to resume, the log message could be observed.

Jan