Re: [PATCH V2 1/4] intel-iommu: don't warn guest errors when getting rid2pasid entry

[Yi Liu]

On 2022/3/28 10:27, Jason Wang wrote:
> On Thu, Mar 24, 2022 at 4:21 PM Tian, Kevin <kevin.tian@intel.com> wrote:
> > > From: Jason Wang
> > > Sent: Monday, March 21, 2022 1:54 PM
> > >
> > > We use to warn on wrong rid2pasid entry. But this error could be
> > > triggered by the guest and could happens during initialization. So
> > > let's don't warn in this case.
> > >
> > > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > > ---
> > >   hw/i386/intel_iommu.c | 6 ++++--
> > >   1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> > > index 874d01c162..90964b201c 100644
> > > --- a/hw/i386/intel_iommu.c
> > > +++ b/hw/i386/intel_iommu.c
> > > @@ -1554,8 +1554,10 @@ static bool vtd_dev_pt_enabled(IntelIOMMUState
> > > *s, VTDContextEntry *ce)
> > >       if (s->root_scalable) {
> > >           ret = vtd_ce_get_rid2pasid_entry(s, ce, &pe);
> > >           if (ret) {
> > > -            error_report_once("%s: vtd_ce_get_rid2pasid_entry error: %"PRId32,
> > > -                              __func__, ret);
> > > +            /*
> > > +             * This error is guest triggerable. We should assumt PT
> > > +             * not enabled for safety.
> > > +             */
> >
> > suppose a VT-d fault should be queued in this case besides returning false:
> >
> > SPD.1: A hardware attempt to access the scalable-mode PASID-directory
> > entry referenced through the PASIDDIRPTR field in scalable-mode
> > context-entry resulted in an error
> >
> > SPT.1: A hardware attempt to access a scalable-mode PASID-table entry
> > referenced through the SMPTBLPTR field in a scalable-mode PASID-directory
> > entry resulted in an error.
>
> Probably, but this issue is not introduced in this patch. We can fix
> it on top if necessary.

agreed.

> > Currently the implementation of vtd_ce_get_rid2pasid_entry() is also
> > problematic. According to VT-d spec, RID2PASID field is effective only
> > when ecap.rps is true otherwise PASID#0 is used for RID2PASID. I didn't
> > see ecap.rps is set, neither is it checked in that function. It works possibly
> > just because Linux currently programs 0 to RID2PASID...
>
> This seems to be another issue since the introduction of scalable mode.

yes. this is not introduced in this series. The current scalable mode 
vIOMMU support was following 3.0 spec, while RPS is added in 3.1. Needs
to be fixed.

> Thanks
>
> > >               return false;
> > >           }
> > >           return (VTD_PE_GET_TYPE(&pe) == VTD_SM_PASID_ENTRY_PT);
> > > --
> > > 2.25.1

--
Regards,
Yi Liu