1. Patchew
  2. QEMU
  3. [PATCH v4 0/4] VT-d minor fixes
  4. View patch

[PATCH v4 3/4] intel_iommu: fix type of the mask field in VTDIOTLBPageInvInfo

CLEMENT MATHIEU--DRIF posted 4 patches 4 months, 3 weeks ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Jason Wang <jasowang@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
There is a newer version of this series
[PATCH v4 3/4] intel_iommu: fix type of the mask field in VTDIOTLBPageInvInfo
Posted by CLEMENT MATHIEU--DRIF 4 months, 3 weeks ago 
From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>

The mask we are trying to store into VTDIOTLBPageInvInfo.mask might not
fit in an uint8_t. Use uint64_t to avoid overflows

Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
---
 hw/i386/intel_iommu_internal.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
index faea23e8d6..5f32c36943 100644
--- a/hw/i386/intel_iommu_internal.h
+++ b/hw/i386/intel_iommu_internal.h
@@ -436,7 +436,7 @@ struct VTDIOTLBPageInvInfo {
     uint16_t domain_id;
     uint32_t pasid;
     uint64_t addr;
-    uint8_t mask;
+    uint64_t mask;
 };
 typedef struct VTDIOTLBPageInvInfo VTDIOTLBPageInvInfo;
 
-- 
2.45.2
Re: [PATCH v4 3/4] intel_iommu: fix type of the mask field in VTDIOTLBPageInvInfo
Posted by Yi Liu 4 months, 2 weeks ago 
On 2024/7/5 19:01, CLEMENT MATHIEU--DRIF wrote:
> From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
> 
> The mask we are trying to store into VTDIOTLBPageInvInfo.mask might not
> fit in an uint8_t. Use uint64_t to avoid overflows

Per the below code, it can overflow as am can be larger than 8 according
to the CH 6.5.2.3 IOTLB Invalidate. And you may want a fix tag as well.

info.mask = ~((1 << am) - 1);

CH 6.5.2.3 IOTLB Invalidate

Address Mask (AM): For page-selective-within-domain invalidations, the 
Address Mask specifies
the number of low order bits of the ADDR field that must be masked for the 
invalidation operation.
This field enables software to request invalidation of contiguous mappings 
for size-aligned
regions. Refer to Table 19 for encodings of this field. When invalidating a 
large-page translation,
software must use the appropriate Address Mask value (0 for 4KByte page, 9 
for 2-MByte page,
and 18 for 1-GByte page). Hardware implementations report the maximum 
supported address
mask value through the Capability register

> Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
> ---
>   hw/i386/intel_iommu_internal.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
> index faea23e8d6..5f32c36943 100644
> --- a/hw/i386/intel_iommu_internal.h
> +++ b/hw/i386/intel_iommu_internal.h
> @@ -436,7 +436,7 @@ struct VTDIOTLBPageInvInfo {
>       uint16_t domain_id;
>       uint32_t pasid;
>       uint64_t addr;
> -    uint8_t mask;
> +    uint64_t mask;
>   };
>   typedef struct VTDIOTLBPageInvInfo VTDIOTLBPageInvInfo;
>   

-- 
Regards,
Yi Liu
Re: [PATCH v4 3/4] intel_iommu: fix type of the mask field in VTDIOTLBPageInvInfo
Posted by Michael S. Tsirkin 4 months, 1 week ago 
On Mon, Jul 08, 2024 at 03:12:27PM +0800, Yi Liu wrote:
> On 2024/7/5 19:01, CLEMENT MATHIEU--DRIF wrote:
> > From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
> > 
> > The mask we are trying to store into VTDIOTLBPageInvInfo.mask might not
> > fit in an uint8_t. Use uint64_t to avoid overflows
> 
> Per the below code, it can overflow as am can be larger than 8 according
> to the CH 6.5.2.3 IOTLB Invalidate.

I don't understand what you are saying. What can overflow?
Are you suggesting text for commit log here?

> And you may want a fix tag as well.

why not.

> info.mask = ~((1 << am) - 1);
> 
> CH 6.5.2.3 IOTLB Invalidate
> 
> Address Mask (AM): For page-selective-within-domain invalidations, the
> Address Mask specifies
> the number of low order bits of the ADDR field that must be masked for the
> invalidation operation.
> This field enables software to request invalidation of contiguous mappings
> for size-aligned
> regions. Refer to Table 19 for encodings of this field. When invalidating a
> large-page translation,
> software must use the appropriate Address Mask value (0 for 4KByte page, 9
> for 2-MByte page,
> and 18 for 1-GByte page). Hardware implementations report the maximum
> supported address
> mask value through the Capability register
> 
> > Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
> > ---
> >   hw/i386/intel_iommu_internal.h | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
> > index faea23e8d6..5f32c36943 100644
> > --- a/hw/i386/intel_iommu_internal.h
> > +++ b/hw/i386/intel_iommu_internal.h
> > @@ -436,7 +436,7 @@ struct VTDIOTLBPageInvInfo {
> >       uint16_t domain_id;
> >       uint32_t pasid;
> >       uint64_t addr;
> > -    uint8_t mask;
> > +    uint64_t mask;
> >   };
> >   typedef struct VTDIOTLBPageInvInfo VTDIOTLBPageInvInfo;
> 
> -- 
> Regards,
> Yi Liu
Re: [PATCH v4 3/4] intel_iommu: fix type of the mask field in VTDIOTLBPageInvInfo
Posted by Michael S. Tsirkin 4 months, 1 week ago 
On Sat, Jul 20, 2024 at 02:45:29PM -0400, Michael S. Tsirkin wrote:
> On Mon, Jul 08, 2024 at 03:12:27PM +0800, Yi Liu wrote:
> > On 2024/7/5 19:01, CLEMENT MATHIEU--DRIF wrote:
> > > From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
> > > 
> > > The mask we are trying to store into VTDIOTLBPageInvInfo.mask might not
> > > fit in an uint8_t. Use uint64_t to avoid overflows
> > 
> > Per the below code, it can overflow as am can be larger than 8 according
> > to the CH 6.5.2.3 IOTLB Invalidate.
> 
> I don't understand what you are saying. What can overflow?
> Are you suggesting text for commit log here?
> 
> > And you may want a fix tag as well.
> 
> why not.

ignore pls this is on old version.

> > info.mask = ~((1 << am) - 1);
> > 
> > CH 6.5.2.3 IOTLB Invalidate
> > 
> > Address Mask (AM): For page-selective-within-domain invalidations, the
> > Address Mask specifies
> > the number of low order bits of the ADDR field that must be masked for the
> > invalidation operation.
> > This field enables software to request invalidation of contiguous mappings
> > for size-aligned
> > regions. Refer to Table 19 for encodings of this field. When invalidating a
> > large-page translation,
> > software must use the appropriate Address Mask value (0 for 4KByte page, 9
> > for 2-MByte page,
> > and 18 for 1-GByte page). Hardware implementations report the maximum
> > supported address
> > mask value through the Capability register
> > 
> > > Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
> > > ---
> > >   hw/i386/intel_iommu_internal.h | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
> > > index faea23e8d6..5f32c36943 100644
> > > --- a/hw/i386/intel_iommu_internal.h
> > > +++ b/hw/i386/intel_iommu_internal.h
> > > @@ -436,7 +436,7 @@ struct VTDIOTLBPageInvInfo {
> > >       uint16_t domain_id;
> > >       uint32_t pasid;
> > >       uint64_t addr;
> > > -    uint8_t mask;
> > > +    uint64_t mask;
> > >   };
> > >   typedef struct VTDIOTLBPageInvInfo VTDIOTLBPageInvInfo;
> > 
> > -- 
> > Regards,
> > Yi Liu

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.