On Thu, Oct 27, 2022 at 9:16 PM Yi Liu <yi.l....@intel.com> wrote:
>
> On 2022/10/27 15:50, Jason Wang wrote:
> > We used to have a macro for VTD_PE_GET_FPD_ERR() but it has an
> > internal goto which prevents it from being reused. This patch convert
> > that macro to a dedicated function and let the caller to decide what
> > to do (e.g using goto or not). This makes sure it can be re-used for
> > other function that requires fault reporting.
> >
> > Signed-off-by: Jason Wang <jasow...@redhat.com>
> > ---
> > Changes since V2:
> > - rename vtd_qualify_report_fault() to vtd_report_qualify_fault()
> > ---
> > hw/i386/intel_iommu.c | 42 ++++++++++++++++++++++++++++--------------
> > 1 file changed, 28 insertions(+), 14 deletions(-)
> >
> > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> > index 6abe12a8c5..6c03ecf3cb 100644
> > --- a/hw/i386/intel_iommu.c
> > +++ b/hw/i386/intel_iommu.c
> > @@ -49,17 +49,6 @@
> > /* pe operations */
> > #define VTD_PE_GET_TYPE(pe) ((pe)->val[0] & VTD_SM_PASID_ENTRY_PGTT)
> > #define VTD_PE_GET_LEVEL(pe) (2 + (((pe)->val[0] >> 2) &
> > VTD_SM_PASID_ENTRY_AW))
> > -#define VTD_PE_GET_FPD_ERR(ret_fr, is_fpd_set, s, source_id, addr,
> > is_write) {\
> > - if (ret_fr) {
> > \
> > - ret_fr = -ret_fr;
> > \
> > - if (is_fpd_set && vtd_is_qualified_fault(ret_fr)) {
> > \
> > - trace_vtd_fault_disabled();
> > \
> > - } else {
> > \
> > - vtd_report_dmar_fault(s, source_id, addr, ret_fr, is_write);
> > \
> > - }
> > \
> > - goto error;
> > \
> > - }
> > \
> > -}
> >
> > /*
> > * PCI bus number (or SID) is not reliable since the device is usaully
> > @@ -1718,6 +1707,19 @@ out:
> > trace_vtd_pt_enable_fast_path(source_id, success);
> > }
> >
> > +static void vtd_report_qualify_fault(IntelIOMMUState *s,
> > + int err, bool is_fpd_set,
> > + uint16_t source_id,
> > + hwaddr addr,
> > + bool is_write)
> > +{
> > + if (is_fpd_set && vtd_is_qualified_fault(err)) {
> > + trace_vtd_fault_disabled();
> > + } else {
> > + vtd_report_dmar_fault(s, source_id, addr, err, is_write);
>
> seems like this will report non-qualified fault. so the naming is not
> most suit. :-) Otherwise, I'm ok with the change.
Right, let me rename it to vtd_report_fault().
Thanks
>
> > + }
> > +}
> > +
> > /* Map dev to context-entry then do a paging-structures walk to do a iommu
> > * translation.
> > *
> > @@ -1778,7 +1780,11 @@ static bool vtd_do_iommu_translate(VTDAddressSpace
> > *vtd_as, PCIBus *bus,
> > is_fpd_set = ce.lo & VTD_CONTEXT_ENTRY_FPD;
> > if (!is_fpd_set && s->root_scalable) {
> > ret_fr = vtd_ce_get_pasid_fpd(s, &ce, &is_fpd_set);
> > - VTD_PE_GET_FPD_ERR(ret_fr, is_fpd_set, s, source_id, addr,
> > is_write);
> > + if (ret_fr) {
> > + vtd_report_qualify_fault(s, -ret_fr, is_fpd_set,
> > + source_id, addr, is_write);
> > + goto error;
> > + }
> > }
> > } else {
> > ret_fr = vtd_dev_to_context_entry(s, bus_num, devfn, &ce);
> > @@ -1786,7 +1792,11 @@ static bool vtd_do_iommu_translate(VTDAddressSpace
> > *vtd_as, PCIBus *bus,
> > if (!ret_fr && !is_fpd_set && s->root_scalable) {
> > ret_fr = vtd_ce_get_pasid_fpd(s, &ce, &is_fpd_set);
> > }
> > - VTD_PE_GET_FPD_ERR(ret_fr, is_fpd_set, s, source_id, addr,
> > is_write);
> > + if (ret_fr) {
> > + vtd_report_qualify_fault(s, -ret_fr, is_fpd_set,
> > + source_id, addr, is_write);
> > + goto error;
> > + }
> > /* Update context-cache */
> > trace_vtd_iotlb_cc_update(bus_num, devfn, ce.hi, ce.lo,
> > cc_entry->context_cache_gen,
> > @@ -1822,7 +1832,11 @@ static bool vtd_do_iommu_translate(VTDAddressSpace
> > *vtd_as, PCIBus *bus,
> >
> > ret_fr = vtd_iova_to_slpte(s, &ce, addr, is_write, &slpte, &level,
> > &reads, &writes, s->aw_bits);
> > - VTD_PE_GET_FPD_ERR(ret_fr, is_fpd_set, s, source_id, addr, is_write);
> > + if (ret_fr) {
> > + vtd_report_qualify_fault(s, -ret_fr, is_fpd_set, source_id,
> > + addr, is_write);
> > + goto error;
> > + }
> >
> > page_mask = vtd_slpt_level_page_mask(level);
> > access_flags = IOMMU_ACCESS_FLAG(reads, writes);
>
> --
> Regards,
> Yi Liu
>
