Peter Xu <pet...@redhat.com> writes:
> Replace existing trace_vtd_err() with error_report_once() then stderr
> will capture something if any of the error happens, meanwhile we don't
> suffer from any DDOS. Then remove the trace point. Since at it,
> provide more information where proper (now we can pass parameters into
> the report function).
>
> Signed-off-by: Peter Xu <pet...@redhat.com>
> ---
> hw/i386/intel_iommu.c | 64 ++++++++++++++++++++++++-------------------
> hw/i386/trace-events | 1 -
> 2 files changed, 36 insertions(+), 29 deletions(-)
>
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 0a8cd4e9cc..ed66ca78f5 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -311,14 +311,14 @@ static void vtd_generate_fault_event(IntelIOMMUState
> *s, uint32_t pre_fsts)
> {
> if (pre_fsts & VTD_FSTS_PPF || pre_fsts & VTD_FSTS_PFO ||
> pre_fsts & VTD_FSTS_IQE) {
> - trace_vtd_err("There are previous interrupt conditions "
> - "to be serviced by software, fault event "
> - "is not generated.");
> + error_report_once("There are previous interrupt conditions "
> + "to be serviced by software, fault event "
> + "is not generated");
> return;
> }
> vtd_set_clear_mask_long(s, DMAR_FECTL_REG, 0, VTD_FECTL_IP);
> if (vtd_get_long_raw(s, DMAR_FECTL_REG) & VTD_FECTL_IM) {
> - trace_vtd_err("Interrupt Mask set, irq is not generated.");
> + error_report_once("Interrupt Mask set, irq is not generated");
> } else {
> vtd_generate_interrupt(s, DMAR_FEADDR_REG, DMAR_FEDATA_REG);
> vtd_set_clear_mask_long(s, DMAR_FECTL_REG, VTD_FECTL_IP, 0);
> @@ -426,20 +426,20 @@ static void vtd_report_dmar_fault(IntelIOMMUState *s,
> uint16_t source_id,
> trace_vtd_dmar_fault(source_id, fault, addr, is_write);
>
> if (fsts_reg & VTD_FSTS_PFO) {
> - trace_vtd_err("New fault is not recorded due to "
> - "Primary Fault Overflow.");
> + error_report_once("New fault is not recorded due to "
> + "Primary Fault Overflow");
> return;
> }
>
> if (vtd_try_collapse_fault(s, source_id)) {
> - trace_vtd_err("New fault is not recorded due to "
> - "compression of faults.");
> + error_report_once("New fault is not recorded due to "
> + "compression of faults");
> return;
> }
>
> if (vtd_is_frcd_set(s, s->next_frcd_reg)) {
> - trace_vtd_err("Next Fault Recording Reg is used, "
> - "new fault is not recorded, set PFO field.");
> + error_report_once("Next Fault Recording Reg is used, "
> + "new fault is not recorded, set PFO field");
> vtd_set_clear_mask_long(s, DMAR_FSTS_REG, 0, VTD_FSTS_PFO);
> return;
> }
> @@ -447,8 +447,8 @@ static void vtd_report_dmar_fault(IntelIOMMUState *s,
> uint16_t source_id,
> vtd_record_frcd(s, s->next_frcd_reg, source_id, addr, fault, is_write);
>
> if (fsts_reg & VTD_FSTS_PPF) {
> - trace_vtd_err("There are pending faults already, "
> - "fault event is not generated.");
> + error_report_once("There are pending faults already, "
> + "fault event is not generated");
> vtd_set_frcd_and_update_ppf(s, s->next_frcd_reg);
> s->next_frcd_reg++;
> if (s->next_frcd_reg == DMAR_FRCD_REG_NR) {
> @@ -1056,8 +1056,9 @@ static int
> vtd_sync_shadow_page_table_range(VTDAddressSpace *vtd_as,
> * we just skip the sync for this time. After all we even
> * don't have the root table pointer!
> */
> - trace_vtd_err("Detected invalid context entry when "
> - "trying to sync shadow page table");
> + error_report_once("%s: invalid context entry for bus 0x%" PRIx16
> + "devfn 0x%" PRIx16, __func__,
> + pci_bus_num(vtd_as->bus), vtd_as->devfn);
"%" PRIx16 is wrong both times: pci_bus_num() returns int, and devfn is
uint8_t. Plain "%x" works for anything narrower than int, so let's use
that.
> return 0;
> }
> }
> @@ -1514,7 +1515,8 @@ static uint64_t
> vtd_context_cache_invalidate(IntelIOMMUState *s, uint64_t val)
> break;
>
> default:
> - trace_vtd_err("Context cache invalidate type error.");
> + error_report_once("%s: invalid context: 0x%"PRIx64,
While there, let's add spaces around PRIx64 & friends.
> + __func__, val);
> caig = 0;
> }
> return caig;
[...]
Squashing in:
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index ed66ca78f5..9c0f525408 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -1056,9 +1056,10 @@ static int
vtd_sync_shadow_page_table_range(VTDAddressSpace *vtd_as,
* we just skip the sync for this time. After all we even
* don't have the root table pointer!
*/
- error_report_once("%s: invalid context entry for bus 0x%" PRIx16
- "devfn 0x%" PRIx16, __func__,
- pci_bus_num(vtd_as->bus), vtd_as->devfn);
+ error_report_once("%s: invalid context entry for bus 0x%x"
+ " devfn 0x%x",
+ __func__, pci_bus_num(vtd_as->bus),
+ vtd_as->devfn);
return 0;
}
}
@@ -1515,7 +1516,7 @@ static uint64_t
vtd_context_cache_invalidate(IntelIOMMUState *s, uint64_t val)
break;
default:
- error_report_once("%s: invalid context: 0x%"PRIx64,
+ error_report_once("%s: invalid context: 0x%" PRIx64,
__func__, val);
caig = 0;
}
@@ -1636,7 +1637,7 @@ static uint64_t vtd_iotlb_flush(IntelIOMMUState *s,
uint64_t val)
am = VTD_IVA_AM(addr);
addr = VTD_IVA_ADDR(addr);
if (am > VTD_MAMV) {
- error_report_once("%s: address mask overflow: 0x%"PRIx64,
+ error_report_once("%s: address mask overflow: 0x%" PRIx64,
__func__, vtd_get_quad_raw(s, DMAR_IVA_REG));
iaig = 0;
break;
@@ -1646,7 +1647,7 @@ static uint64_t vtd_iotlb_flush(IntelIOMMUState *s,
uint64_t val)
break;
default:
- error_report_once("%s: invalid granularity: 0x%"PRIx64,
+ error_report_once("%s: invalid granularity: 0x%" PRIx64,
__func__, val);
iaig = 0;
}
@@ -2192,7 +2193,7 @@ static uint64_t vtd_mem_read(void *opaque, hwaddr addr,
unsigned size)
trace_vtd_reg_read(addr, size);
if (addr + size > DMAR_REG_SIZE) {
- error_report_once("%s: MMIO over range: addr=0x%"PRIx64
+ error_report_once("%s: MMIO over range: addr=0x%" PRIx64
" size=0x%u", __func__, addr, size);
return (uint64_t)-1;
}
@@ -2244,7 +2245,7 @@ static void vtd_mem_write(void *opaque, hwaddr addr,
trace_vtd_reg_write(addr, size, val);
if (addr + size > DMAR_REG_SIZE) {
- error_report_once("%s: MMIO over range: addr=0x%"PRIx64
+ error_report_once("%s: MMIO over range: addr=0x%" PRIx64
" size=0x%u", __func__, addr, size);
return;
}
@@ -2616,8 +2617,8 @@ static int vtd_irte_get(IntelIOMMUState *iommu, uint16_t
index,
addr = iommu->intr_root + index * sizeof(*entry);
if (dma_memory_read(&address_space_memory, addr, entry,
sizeof(*entry))) {
- error_report_once("%s: read failed: ind=0x%"PRIu16
- " addr=0x%"PRIx64, __func__, index, addr);
+ error_report_once("%s: read failed: ind=0x%x addr=0x%" PRIx64,
+ __func__, index, addr);
return -VTD_FR_IR_ROOT_INVAL;
}
@@ -2750,13 +2751,13 @@ static int vtd_interrupt_remap_msi(IntelIOMMUState
*iommu,
if (origin->address & VTD_MSI_ADDR_HI_MASK) {
error_report_once("%s: MSI address high 32 bits non-zero detected: "
- "address=0x%"PRIx64, __func__, origin->address);
+ "address=0x%" PRIx64, __func__, origin->address);
return -VTD_FR_IR_REQ_RSVD;
}
addr.data = origin->address & VTD_MSI_ADDR_LO_MASK;
if (addr.addr.__head != 0xfee) {
- error_report_once("%s: MSI address low 32 bit invalid: 0x%"PRIx32,
+ error_report_once("%s: MSI address low 32 bit invalid: 0x%" PRIx32,
__func__, addr.data);
return -VTD_FR_IR_REQ_RSVD;
}
