1. Patchew
  2. QEMU
  3. [PATCH v2 0/2] intel_iommu minor fixes
  4. View patch

[PATCH v2 2/2] intel_iommu: Make PASID-cache and PIOTLB type invalid in legacy mode

Zhenzhong Duan posted 2 patches 3 months, 1 week ago
There is a newer version of this series
[PATCH v2 2/2] intel_iommu: Make PASID-cache and PIOTLB type invalid in legacy mode
Posted by Zhenzhong Duan 3 months, 1 week ago 
In vtd_process_inv_desc(), VTD_INV_DESC_PC and VTD_INV_DESC_PIOTLB are
bypassed without scalable mode check. These two types are not valid
in legacy mode and we should report error.

Suggested-by: Yi Liu <yi.l.liu@intel.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/i386/intel_iommu.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 68cb72a481..90cd4e5044 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -2763,17 +2763,6 @@ static bool vtd_process_inv_desc(IntelIOMMUState *s)
         }
         break;
 
-    /*
-     * TODO: the entity of below two cases will be implemented in future series.
-     * To make guest (which integrates scalable mode support patch set in
-     * iommu driver) work, just return true is enough so far.
-     */
-    case VTD_INV_DESC_PC:
-        break;
-
-    case VTD_INV_DESC_PIOTLB:
-        break;
-
     case VTD_INV_DESC_WAIT:
         trace_vtd_inv_desc("wait", inv_desc.hi, inv_desc.lo);
         if (!vtd_process_wait_desc(s, &inv_desc)) {
@@ -2795,6 +2784,17 @@ static bool vtd_process_inv_desc(IntelIOMMUState *s)
         }
         break;
 
+    /*
+     * TODO: the entity of below two cases will be implemented in future series.
+     * To make guest (which integrates scalable mode support patch set in
+     * iommu driver) work, just return true is enough so far.
+     */
+    case VTD_INV_DESC_PC:
+    case VTD_INV_DESC_PIOTLB:
+        if (s->scalable_mode) {
+            break;
+        }
+    /* fallthrough */
     default:
         error_report_once("%s: invalid inv desc: hi=%"PRIx64", lo=%"PRIx64
                           " (unknown type)", __func__, inv_desc.hi,
-- 
2.34.1
Re: [PATCH v2 2/2] intel_iommu: Make PASID-cache and PIOTLB type invalid in legacy mode
Posted by Yi Liu 3 months, 1 week ago 
On 2024/8/13 15:44, Zhenzhong Duan wrote:
> In vtd_process_inv_desc(), VTD_INV_DESC_PC and VTD_INV_DESC_PIOTLB are
> bypassed without scalable mode check. These two types are not valid
> in legacy mode and we should report error.
> 
> Suggested-by: Yi Liu <yi.l.liu@intel.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   hw/i386/intel_iommu.c | 22 +++++++++++-----------
>   1 file changed, 11 insertions(+), 11 deletions(-)

Reviewed-by: Yi Liu <yi.l.liu@intel.com>

Do you think a fix tag is needed or not? @Jason

> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 68cb72a481..90cd4e5044 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -2763,17 +2763,6 @@ static bool vtd_process_inv_desc(IntelIOMMUState *s)
>           }
>           break;
>   
> -    /*
> -     * TODO: the entity of below two cases will be implemented in future series.
> -     * To make guest (which integrates scalable mode support patch set in
> -     * iommu driver) work, just return true is enough so far.
> -     */
> -    case VTD_INV_DESC_PC:
> -        break;
> -
> -    case VTD_INV_DESC_PIOTLB:
> -        break;
> -
>       case VTD_INV_DESC_WAIT:
>           trace_vtd_inv_desc("wait", inv_desc.hi, inv_desc.lo);
>           if (!vtd_process_wait_desc(s, &inv_desc)) {
> @@ -2795,6 +2784,17 @@ static bool vtd_process_inv_desc(IntelIOMMUState *s)
>           }
>           break;
>   
> +    /*
> +     * TODO: the entity of below two cases will be implemented in future series.
> +     * To make guest (which integrates scalable mode support patch set in
> +     * iommu driver) work, just return true is enough so far.
> +     */
> +    case VTD_INV_DESC_PC:
> +    case VTD_INV_DESC_PIOTLB:
> +        if (s->scalable_mode) {
> +            break;
> +        }
> +    /* fallthrough */
>       default:
>           error_report_once("%s: invalid inv desc: hi=%"PRIx64", lo=%"PRIx64
>                             " (unknown type)", __func__, inv_desc.hi,

-- 
Regards,
Yi Liu
Re: [PATCH v2 2/2] intel_iommu: Make PASID-cache and PIOTLB type invalid in legacy mode
Posted by Jason Wang 3 months, 1 week ago 
On Tue, Aug 13, 2024 at 4:31 PM Yi Liu <yi.l.liu@intel.com> wrote:
>
> On 2024/8/13 15:44, Zhenzhong Duan wrote:
> > In vtd_process_inv_desc(), VTD_INV_DESC_PC and VTD_INV_DESC_PIOTLB are
> > bypassed without scalable mode check. These two types are not valid
> > in legacy mode and we should report error.
> >
> > Suggested-by: Yi Liu <yi.l.liu@intel.com>
> > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> > ---
> >   hw/i386/intel_iommu.c | 22 +++++++++++-----------
> >   1 file changed, 11 insertions(+), 11 deletions(-)
>
> Reviewed-by: Yi Liu <yi.l.liu@intel.com>
>
> Do you think a fix tag is needed or not? @Jason

I think so (as it might have a guest triggerable issue).

Thanks
Re: [PATCH v2 2/2] intel_iommu: Make PASID-cache and PIOTLB type invalid in legacy mode
Posted by CLEMENT MATHIEU--DRIF 3 months, 1 week ago 
Reviewed-by: Clément Mathieu--Drif<clement.mathieu--drif@eviden.com>

Super reactive!

Maybe we can continue along this path after the handlers are implemented.
It would be great to make sure we don't process PASID related descriptors when not in scalable mode.
What are your thoughts?

Thanks
>cmd



On 13/08/2024 09:44, Zhenzhong Duan 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.
>
>
> In vtd_process_inv_desc(), VTD_INV_DESC_PC and VTD_INV_DESC_PIOTLB are
> bypassed without scalable mode check. These two types are not valid
> in legacy mode and we should report error.
>
> Suggested-by: Yi Liu <yi.l.liu@intel.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   hw/i386/intel_iommu.c | 22 +++++++++++-----------
>   1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 68cb72a481..90cd4e5044 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -2763,17 +2763,6 @@ static bool vtd_process_inv_desc(IntelIOMMUState *s)
>           }
>           break;
>
> -    /*
> -     * TODO: the entity of below two cases will be implemented in future series.
> -     * To make guest (which integrates scalable mode support patch set in
> -     * iommu driver) work, just return true is enough so far.
> -     */
> -    case VTD_INV_DESC_PC:
> -        break;
> -
> -    case VTD_INV_DESC_PIOTLB:
> -        break;
> -
>       case VTD_INV_DESC_WAIT:
>           trace_vtd_inv_desc("wait", inv_desc.hi, inv_desc.lo);
>           if (!vtd_process_wait_desc(s, &inv_desc)) {
> @@ -2795,6 +2784,17 @@ static bool vtd_process_inv_desc(IntelIOMMUState *s)
>           }
>           break;
>
> +    /*
> +     * TODO: the entity of below two cases will be implemented in future series.
> +     * To make guest (which integrates scalable mode support patch set in
> +     * iommu driver) work, just return true is enough so far.
> +     */
> +    case VTD_INV_DESC_PC:
> +    case VTD_INV_DESC_PIOTLB:
> +        if (s->scalable_mode) {
> +            break;
> +        }
> +    /* fallthrough */
>       default:
>           error_report_once("%s: invalid inv desc: hi=%"PRIx64", lo=%"PRIx64
>                             " (unknown type)", __func__, inv_desc.hi,
> --
> 2.34.1
>
>
Re: [PATCH v2 2/2] intel_iommu: Make PASID-cache and PIOTLB type invalid in legacy mode
Posted by Yi Liu 3 months, 1 week ago 
On 2024/8/13 16:00, CLEMENT MATHIEU--DRIF wrote:
> Reviewed-by: Clément Mathieu--Drif<clement.mathieu--drif@eviden.com>
> 
> Super reactive!
> 
> Maybe we can continue along this path after the handlers are implemented.
> It would be great to make sure we don't process PASID related descriptors when not in scalable mode.
> What are your thoughts?

yeah. let's keep it in mind when reviewing the stage-1 translation series.

> Thanks
>> cmd
> 
> 
> 
> On 13/08/2024 09:44, Zhenzhong Duan 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.
>>
>>
>> In vtd_process_inv_desc(), VTD_INV_DESC_PC and VTD_INV_DESC_PIOTLB are
>> bypassed without scalable mode check. These two types are not valid
>> in legacy mode and we should report error.
>>
>> Suggested-by: Yi Liu <yi.l.liu@intel.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>    hw/i386/intel_iommu.c | 22 +++++++++++-----------
>>    1 file changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index 68cb72a481..90cd4e5044 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -2763,17 +2763,6 @@ static bool vtd_process_inv_desc(IntelIOMMUState *s)
>>            }
>>            break;
>>
>> -    /*
>> -     * TODO: the entity of below two cases will be implemented in future series.
>> -     * To make guest (which integrates scalable mode support patch set in
>> -     * iommu driver) work, just return true is enough so far.
>> -     */
>> -    case VTD_INV_DESC_PC:
>> -        break;
>> -
>> -    case VTD_INV_DESC_PIOTLB:
>> -        break;
>> -
>>        case VTD_INV_DESC_WAIT:
>>            trace_vtd_inv_desc("wait", inv_desc.hi, inv_desc.lo);
>>            if (!vtd_process_wait_desc(s, &inv_desc)) {
>> @@ -2795,6 +2784,17 @@ static bool vtd_process_inv_desc(IntelIOMMUState *s)
>>            }
>>            break;
>>
>> +    /*
>> +     * TODO: the entity of below two cases will be implemented in future series.
>> +     * To make guest (which integrates scalable mode support patch set in
>> +     * iommu driver) work, just return true is enough so far.
>> +     */
>> +    case VTD_INV_DESC_PC:
>> +    case VTD_INV_DESC_PIOTLB:
>> +        if (s->scalable_mode) {
>> +            break;
>> +        }
>> +    /* fallthrough */
>>        default:
>>            error_report_once("%s: invalid inv desc: hi=%"PRIx64", lo=%"PRIx64
>>                              " (unknown type)", __func__, inv_desc.hi,
>> --
>> 2.34.1
>>
>>

-- 
Regards,
Yi Liu

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.