| Message ID | 20221027075042.16894-4-jasowang@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | PASID support for Intel IOMMU | expand |
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 <jasowang@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. > + } > +} > + > /* 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);
On Thu, Oct 27, 2022 at 9:16 PM Yi Liu <yi.l.liu@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 <jasowang@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 >
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); + } +} + /* 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);
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 <jasowang@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(-)