| Message ID | 20210225091435.644762-2-eric.auger@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Some vIOMMU fixes | expand |
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 > >
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>
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 >> >> >
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;
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(+)