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;
 
-- 
2.26.2

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...

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

set the mask
diuse the
>  
> 

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
>>  
>>
> 

On Thu, Feb 25, 2021 at 10:14:29AM +0100, 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>

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu