[PATCH v4 1/4] intel_iommu: fix FRCD construction macro

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 1/4] intel_iommu: fix FRCD construction macro
Posted by CLEMENT MATHIEU--DRIF 4 months, 3 weeks ago
From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>

The constant must be unsigned, otherwise the two's complement
overrides the other fields when a PASID is present.

Fixes: 1b2b12376c8a ("intel-iommu: PASID support")

Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
Reviewed-by: Yi Liu <yi.l.liu@intel.com>
Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.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 f8cf99bddf..cbc4030031 100644
--- a/hw/i386/intel_iommu_internal.h
+++ b/hw/i386/intel_iommu_internal.h
@@ -267,7 +267,7 @@
 /* For the low 64-bit of 128-bit */
 #define VTD_FRCD_FI(val)        ((val) & ~0xfffULL)
 #define VTD_FRCD_PV(val)        (((val) & 0xffffULL) << 40)
-#define VTD_FRCD_PP(val)        (((val) & 0x1) << 31)
+#define VTD_FRCD_PP(val)        (((val) & 0x1ULL) << 31)
 #define VTD_FRCD_IR_IDX(val)    (((val) & 0xffffULL) << 48)
 
 /* DMA Remapping Fault Conditions */
-- 
2.45.2
Re: [PATCH v4 1/4] intel_iommu: fix FRCD construction macro
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 constant must be unsigned, otherwise the two's complement
> overrides the other fields when a PASID is present.
> 
> Fixes: 1b2b12376c8a ("intel-iommu: PASID support")
> 

The extra line behind the "Fixes tag" is not needed.

> Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
> Reviewed-by: Yi Liu <yi.l.liu@intel.com>
> Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.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 f8cf99bddf..cbc4030031 100644
> --- a/hw/i386/intel_iommu_internal.h
> +++ b/hw/i386/intel_iommu_internal.h
> @@ -267,7 +267,7 @@
>   /* For the low 64-bit of 128-bit */
>   #define VTD_FRCD_FI(val)        ((val) & ~0xfffULL)
>   #define VTD_FRCD_PV(val)        (((val) & 0xffffULL) << 40)
> -#define VTD_FRCD_PP(val)        (((val) & 0x1) << 31)
> +#define VTD_FRCD_PP(val)        (((val) & 0x1ULL) << 31)
>   #define VTD_FRCD_IR_IDX(val)    (((val) & 0xffffULL) << 48)
>   
>   /* DMA Remapping Fault Conditions */

It might be fine to squash patch 02 of this series into this one. @Jason?

-- 
Regards,
Yi Liu

Re: [PATCH v4 1/4] intel_iommu: fix FRCD construction macro
Posted by Jason Wang 4 months, 2 weeks ago
On Mon, Jul 8, 2024 at 3:04 PM Yi Liu <yi.l.liu@intel.com> wrote:
>
> On 2024/7/5 19:01, CLEMENT MATHIEU--DRIF wrote:
> > From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
> >
> > The constant must be unsigned, otherwise the two's complement
> > overrides the other fields when a PASID is present.
> >
> > Fixes: 1b2b12376c8a ("intel-iommu: PASID support")
> >
>
> The extra line behind the "Fixes tag" is not needed.
>
> > Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
> > Reviewed-by: Yi Liu <yi.l.liu@intel.com>
> > Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.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 f8cf99bddf..cbc4030031 100644
> > --- a/hw/i386/intel_iommu_internal.h
> > +++ b/hw/i386/intel_iommu_internal.h
> > @@ -267,7 +267,7 @@
> >   /* For the low 64-bit of 128-bit */
> >   #define VTD_FRCD_FI(val)        ((val) & ~0xfffULL)
> >   #define VTD_FRCD_PV(val)        (((val) & 0xffffULL) << 40)
> > -#define VTD_FRCD_PP(val)        (((val) & 0x1) << 31)
> > +#define VTD_FRCD_PP(val)        (((val) & 0x1ULL) << 31)
> >   #define VTD_FRCD_IR_IDX(val)    (((val) & 0xffffULL) << 48)
> >
> >   /* DMA Remapping Fault Conditions */
>
> It might be fine to squash patch 02 of this series into this one. @Jason?

Not sure, we may need this for -stable. So having a standalone patch
doesn't hurt.

Thanks

>
> --
> Regards,
> Yi Liu
>
Re: [PATCH v4 1/4] intel_iommu: fix FRCD construction macro
Posted by Yi Liu 4 months, 2 weeks ago
On 2024/7/9 10:51, Jason Wang wrote:
> On Mon, Jul 8, 2024 at 3:04 PM Yi Liu <yi.l.liu@intel.com> wrote:
>>
>> On 2024/7/5 19:01, CLEMENT MATHIEU--DRIF wrote:
>>> From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
>>>
>>> The constant must be unsigned, otherwise the two's complement
>>> overrides the other fields when a PASID is present.
>>>
>>> Fixes: 1b2b12376c8a ("intel-iommu: PASID support")
>>>
>>
>> The extra line behind the "Fixes tag" is not needed.
>>
>>> Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
>>> Reviewed-by: Yi Liu <yi.l.liu@intel.com>
>>> Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.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 f8cf99bddf..cbc4030031 100644
>>> --- a/hw/i386/intel_iommu_internal.h
>>> +++ b/hw/i386/intel_iommu_internal.h
>>> @@ -267,7 +267,7 @@
>>>    /* For the low 64-bit of 128-bit */
>>>    #define VTD_FRCD_FI(val)        ((val) & ~0xfffULL)
>>>    #define VTD_FRCD_PV(val)        (((val) & 0xffffULL) << 40)
>>> -#define VTD_FRCD_PP(val)        (((val) & 0x1) << 31)
>>> +#define VTD_FRCD_PP(val)        (((val) & 0x1ULL) << 31)
>>>    #define VTD_FRCD_IR_IDX(val)    (((val) & 0xffffULL) << 48)
>>>
>>>    /* DMA Remapping Fault Conditions */
>>
>> It might be fine to squash patch 02 of this series into this one. @Jason?
> 
> Not sure, we may need this for -stable. So having a standalone patch
> doesn't hurt.

I see. :)

-- 
Regards,
Yi Liu