Re: [PATCH ats_vtd v5 04/22] intel_iommu: do not consider wait_desc as an invalid descriptor

[CLEMENT MATHIEU--DRIF]

On 02/07/2024 15:33, Yi Liu wrote:
> Caution: External email. Do not open attachments or click links, 
> unless this email comes from a known sender and you know the content 
> is safe.
>
>
> On 2024/7/2 13:52, CLEMENT MATHIEU--DRIF wrote:
>> From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
>>
>> Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
>> Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>   hw/i386/intel_iommu.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index 98996ededc..71cebe2fd3 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -3500,6 +3500,11 @@ static bool 
>> vtd_process_wait_desc(IntelIOMMUState *s, VTDInvDesc *inv_desc)
>>       } else if (inv_desc->lo & VTD_INV_DESC_WAIT_IF) {
>>           /* Interrupt flag */
>>           vtd_generate_completion_event(s);
>> +    } else if (inv_desc->lo & VTD_INV_DESC_WAIT_FN) {
>> +        /*
>> +         * SW = 0, IF = 0, FN = 1
>> +         * Nothing to do as we process the events sequentially
>> +         */
>
> This code looks a bit weird. SW field does not co-exist with IF. But 
> either
> SW or IF can co-exist with FN flag. Is it? Have you already seen a wait
> descriptor that only has FN flag set but no SW nor IF flag?
Yes, my test suite triggers that condition
>
>>       } else {
>>           error_report_once("%s: invalid wait desc: hi=%"PRIx64", 
>> lo=%"PRIx64
>>                             " (unknown type)", __func__, inv_desc->hi,
>
> -- 
> Regards,
> Yi Liu