On Fri, Jan 13, 2017 at 03:46:31PM +0800, Jason Wang wrote: > > > On 2017年01月13日 11:06, Peter Xu wrote: > >VT-d codes are still using static DEBUG_INTEL_IOMMU macro. That's not > >good, and we should end the day when we need to recompile the code > >before getting useful debugging information for vt-d. Time to switch to > >the trace system. > > > >This is the first patch to do it. > > > >Generally, the rule of mine is: > > > >- for the old GENERAL typed message, I use error_report() directly if > > apply. Those are something shouldn't happen, and we should print those > > errors in all cases, even without enabling debug and tracing. > > Looks like some were guest trigger-able. If yes, let's try not use > error_report() for not being flooded.
Yes, it's intended. Most of the error_report()s in this patch can be triggered by guest, but only by illegal guest behaviors (e.g., non-zero reserved fields, or illegal descriptors, etc.). In that sense, shall we keep them even guest can trigger them? Since people will never see them if they are running generic and good kernels. More importantly, these error_report()s can be good hints when guest encounters issues, for better debugging and triaging. Actually we have such usage in existing QEMU as well. For example, when we maintain the DMA mapping in vfio-pci, it's possible that the shadow page table is mapped illegally due to some reason (that depends on the guest as well, may not be guest kernel, but DPDK applications inside guest), and the map() can fail. Here we have: ret = vfio_dma_map(container, iova, iotlb->addr_mask + 1, vaddr, !(iotlb->perm & IOMMU_WO) || mr->readonly); if (ret) { error_report("vfio_dma_map(%p, 0x%"HWADDR_PRIx", " "0x%"HWADDR_PRIx", %p) = %d (%m)", container, iova, iotlb->addr_mask + 1, vaddr, ret); } Which I think is playing the same role here - we will never see these lines if the guest is normal, and these lines will be useful when bad things happen. So I would slightly prefer that we keep these error_reports() for now, as long as they won't flush the screen for most of the users. (during the time I played with this series, none of them jumped out :) Thanks, -- peterx