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

[Michael S. Tsirkin]

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