Re: [PATCH v2 1/7] intel_iommu: Fix mask may be uninitialized in vtd_context_device_invalidate

[Auger Eric]

Hi Philippe,

On 2/25/21 11:08 AM, Philippe Mathieu-Daudé wrote:
> On 2/25/21 10:14 AM, Eric Auger wrote:
>> With -Werror=maybe-uninitialized configuration we get
>> ../hw/i386/intel_iommu.c: In function ‘vtd_context_device_invalidate’:
>> ../hw/i386/intel_iommu.c:1888:10: error: ‘mask’ may be used
>> uninitialized in this function [-Werror=maybe-uninitialized]
>>  1888 |     mask = ~mask;
>>       |     ~~~~~^~~~~~~
>>
>> Add a g_assert_not_reached() to avoid the error.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> ---
>>  hw/i386/intel_iommu.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index b4f5094259..3206f379f8 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -1884,6 +1884,8 @@ static void 
>> vtd_context_device_invalidate(IntelIOMMUState *s,
>>      case 3:
>>          mask = 7;   /* Mask bit 2:0 in the SID field */
>>          break;
>> +    default:
>> +        g_assert_not_reached();
>>      }
>>      mask = ~mask;
> 
> Unrelated to this patch, but I wonder why we don't directly assign the
> correct value of the mask in the switch cases...

After reading the vtd spec again, I think this is aligned with the spec
description.  FM = function mask encodes the bits to mask. Then you
actually compute the mask by ~mask.
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Thanks!

Eric

> 
> set the mask
> diuse the
>>  
>>
>