On Tue, Sep 03, 2019 at 01:45:22PM +0200, Auger Eric wrote:
> Hi Peter,
>
> On 8/19/19 10:24 AM, Peter Xu wrote:
> > On Tue, Jul 30, 2019 at 07:21:31PM +0200, Eric Auger wrote:
> >> @@ -464,19 +464,75 @@ static IOMMUTLBEntry
> >> virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr,
> >> int iommu_idx)
> >> {
> >> IOMMUDevice *sdev = container_of(mr, IOMMUDevice, iommu_mr);
> >> + VirtIOIOMMU *s = sdev->viommu;
> >> uint32_t sid;
> >> + viommu_endpoint *ep;
> >> + viommu_mapping *mapping;
> >> + viommu_interval interval;
> >> + bool bypass_allowed;
> >> +
> >> + interval.low = addr;
> >> + interval.high = addr + 1;
> >>
> >> IOMMUTLBEntry entry = {
> >> .target_as = &address_space_memory,
> >> .iova = addr,
> >> .translated_addr = addr,
> >> - .addr_mask = ~(hwaddr)0,
> >> + .addr_mask = (1 << ctz32(s->config.page_size_mask)) - 1,
> >> .perm = IOMMU_NONE,
> >> };
> >>
> >> + bypass_allowed = virtio_has_feature(s->acked_features,
> >> + VIRTIO_IOMMU_F_BYPASS);
> >> +
> >> sid = virtio_iommu_get_sid(sdev);
> >>
> >> trace_virtio_iommu_translate(mr->parent_obj.name, sid, addr, flag);
> >> + qemu_mutex_lock(&s->mutex);
> >> +
> >> + ep = g_tree_lookup(s->endpoints, GUINT_TO_POINTER(sid));
> >> + if (!ep) {
> >> + if (!bypass_allowed) {
> >> + error_report("%s sid=%d is not known!!", __func__, sid);
> >
> > Maybe use error_report_once() to avoid DOS attack? Also would it be
> > good to unify the debug prints? I see both error_report() and
> > qemu_log_mask() are used in the whole patchset. Or is that attempted?
>
> I switched to error_report_once()
>
> I understand that qemu_log_mask() should be used whenever the root cause
> is a bad action of the guest OS (in below case, the EP was not attached
> to any domain). Above, there is an EP that attempts to talk through the
> IOMMU and this was not expected (rather a platform description issue or
> a qemu bug).
I see. It's a bit unclear at least to me on how to use these. I have
seen, and used error_report*() to report guest misbehaves as well just
for the debugging and triaging simply because error_report*() will
always be there even without "-d" (because when issue happens most
users are without it...). Then with these information captured by
either libvirt or direct QEMU users we can triage guest bugs easier.
I hope I'm not severly wrong, and please feel free to use
qemu_log_mask() no matter what.
Regards,
--
Peter Xu
