diff mbox series

[v3,2/3] intel_iommu: fix type of the mask field in VTDIOTLBPageInvInfo

Message ID 20240705050213.1492515-3-clement.mathieu--drif@eviden.com (mailing list archive)
State New, archived
Headers show
Series VT-d minor fixes | expand

Commit Message

CLEMENT MATHIEU--DRIF July 5, 2024, 5:03 a.m. UTC 
From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>

VTDIOTLBPageInvInfo.mask might not fit in an uint8_t.
Moreover, this field is used in binary operations with 64-bit addresses.

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

Comments

Duan, Zhenzhong July 5, 2024, 8:49 a.m. UTC | #1 
>-----Original Message-----
>From: CLEMENT MATHIEU--DRIF <clement.mathieu--drif@eviden.com>
>Subject: [PATCH v3 2/3] intel_iommu: fix type of the mask field in
>VTDIOTLBPageInvInfo
>
>From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
>
>VTDIOTLBPageInvInfo.mask might not fit in an uint8_t.
>Moreover, this field is used in binary operations with 64-bit addresses.
>
>Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>

Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

Thanks
Zhenzhong

>---
> 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 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
Michael S. Tsirkin July 5, 2024, 8:51 a.m. UTC | #2 
On Fri, Jul 05, 2024 at 05:03:17AM +0000, CLEMENT MATHIEU--DRIF wrote:
> From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
> 
> VTDIOTLBPageInvInfo.mask might not fit in an uint8_t.

I think what you mean is that is assigned values that might not
fit .... it's u8 ATM so of course it fits.

> Moreover, this field is used in binary operations with 64-bit addresses.

So what?

> 
> 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 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
CLEMENT MATHIEU--DRIF July 5, 2024, 9:52 a.m. UTC | #3 
On 05/07/2024 10:51, Michael S. Tsirkin wrote:

Caution: External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe.


On Fri, Jul 05, 2024 at 05:03:17AM +0000, CLEMENT MATHIEU--DRIF wrote:


From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com><mailto:clement.mathieu--drif@eviden.com>

VTDIOTLBPageInvInfo.mask might not fit in an uint8_t.



I think what you mean is that is assigned values that might not
fit .... it's u8 ATM so of course it fits.

What about :
"The mask stored into VTDIOTLBPageInvInfo.mask might not fit in an uint8_t. Use uint64_t to avoid overflows"





Moreover, this field is used in binary operations with 64-bit addresses.



So what?

I thing the first part of the message is enough, the issue comes from the fact that the mask does not fit into the type






Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com><mailto: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 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
Michael S. Tsirkin July 5, 2024, 10:16 a.m. UTC | #4 
On Fri, Jul 05, 2024 at 09:52:48AM +0000, CLEMENT MATHIEU--DRIF wrote:
> 
> 
> On 05/07/2024 10:51, Michael S. Tsirkin wrote:
> 
>     Caution: External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe.
> 
> 
>     On Fri, Jul 05, 2024 at 05:03:17AM +0000, CLEMENT MATHIEU--DRIF wrote:
> 
>         From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
> 
>         VTDIOTLBPageInvInfo.mask might not fit in an uint8_t.
> 
>     I think what you mean is that is assigned values that might not
>     fit .... it's u8 ATM so of course it fits.
> 
> What about :
> "The mask stored into VTDIOTLBPageInvInfo.mask might not fit in an uint8_t. Use
> uint64_t to avoid overflows"

No, the mask stored there is u8.
You mean "that we are trying to store into ".

> 
> 
>         Moreover, this field is used in binary operations with 64-bit addresses.
> 
>     So what?
> 
> I thing the first part of the message is enough, the issue comes from the fact
> that the mask does not fit into the type
> 
> 
> 
>         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 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 mbox series

Patch

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;