| Message ID | 20240813074410.2571369-3-zhenzhong.duan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | intel_iommu minor fixes | expand |
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 > >
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,
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
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 >> >>
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,
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(-)