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

[Yi Liu]

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