| Message ID | 20240704151220.1018104-3-clement.mathieu--drif@eviden.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | VT-d minor fixes | expand |
On Thu, Jul 04, 2024 at 03:12:48PM +0000, CLEMENT MATHIEU--DRIF wrote: > From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com> > > The 'level' field in vtd_iotlb_key is an unsigned integer. > We don't need to store level as an int in vtd_lookup_iotlb. > > VTDIOTLBPageInvInfo.mask is used in binary operations with addresses. this last sentence is a bit opaque. is there a bug ? E.g. can mask ever get so big it does not fit in u8? > Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com> > --- > hw/i386/intel_iommu.c | 2 +- > hw/i386/intel_iommu_internal.h | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c > index 37c21a0aec..be0cb39b5c 100644 > --- a/hw/i386/intel_iommu.c > +++ b/hw/i386/intel_iommu.c > @@ -358,7 +358,7 @@ static VTDIOTLBEntry *vtd_lookup_iotlb(IntelIOMMUState *s, uint16_t source_id, > { > struct vtd_iotlb_key key; > VTDIOTLBEntry *entry; > - int level; > + unsigned level; > > for (level = VTD_SL_PT_LEVEL; level < VTD_SL_PML4_LEVEL; level++) { > key.gfn = vtd_get_iotlb_gfn(addr, level); > diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h > index cbc4030031..5fcbe2744f 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
On 2024/7/5 06:13, Michael S. Tsirkin wrote: > On Thu, Jul 04, 2024 at 03:12:48PM +0000, CLEMENT MATHIEU--DRIF wrote: >> From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com> >> >> The 'level' field in vtd_iotlb_key is an unsigned integer. >> We don't need to store level as an int in vtd_lookup_iotlb. >> >> VTDIOTLBPageInvInfo.mask is used in binary operations with addresses. > > this last sentence is a bit opaque. is there a bug ? E.g. > can mask ever get so big it does not fit in u8? yes, this looks to be a bug. It's initialized and used by below code. The am is a u8. So it may make more sense to split this patch. One is to make type match, another is to fix a bug. info.mask = ~((1 << am) - 1); uint64_t gfn = (info->addr >> VTD_PAGE_SHIFT_4K) & info->mask; >> Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com> >> --- >> hw/i386/intel_iommu.c | 2 +- >> hw/i386/intel_iommu_internal.h | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c >> index 37c21a0aec..be0cb39b5c 100644 >> --- a/hw/i386/intel_iommu.c >> +++ b/hw/i386/intel_iommu.c >> @@ -358,7 +358,7 @@ static VTDIOTLBEntry *vtd_lookup_iotlb(IntelIOMMUState *s, uint16_t source_id, >> { >> struct vtd_iotlb_key key; >> VTDIOTLBEntry *entry; >> - int level; >> + unsigned level; >> >> for (level = VTD_SL_PT_LEVEL; level < VTD_SL_PML4_LEVEL; level++) { >> key.gfn = vtd_get_iotlb_gfn(addr, level); >> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h >> index cbc4030031..5fcbe2744f 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 >
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index 37c21a0aec..be0cb39b5c 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -358,7 +358,7 @@ static VTDIOTLBEntry *vtd_lookup_iotlb(IntelIOMMUState *s, uint16_t source_id, { struct vtd_iotlb_key key; VTDIOTLBEntry *entry; - int level; + unsigned level; for (level = VTD_SL_PT_LEVEL; level < VTD_SL_PML4_LEVEL; level++) { key.gfn = vtd_get_iotlb_gfn(addr, level); diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h index cbc4030031..5fcbe2744f 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;