Re: [Qemu-devel] [PATCH v6 08/18] intel_iommu: fix trace for addr translation

[Jason Wang]

On 2017年02月07日 14:25, Peter Xu wrote:
> On Tue, Feb 07, 2017 at 01:40:39PM +0800, Jason Wang wrote:
> > On 2017年02月03日 16:22, Peter Xu wrote:
> > > Another patch to convert the DPRINTF() stuffs. This patch focuses on the
> > > address translation path and caching.
> > >
> > > Signed-off-by: Peter Xu<address@hidden>
> > > ---
> > >   hw/i386/intel_iommu.c | 84 ++++++++++++++++++++-------------------------------
> > >   hw/i386/trace-events  |  7 +++++
> > >   2 files changed, 39 insertions(+), 52 deletions(-)
> > Similar to previous patch, this in fact a conversion not a fix.
> I'll fix the subject.
>
> > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> > > index d7b9a01..c672621 100644
> > > --- a/hw/i386/intel_iommu.c
> > > +++ b/hw/i386/intel_iommu.c
> > > @@ -260,11 +260,9 @@ static void vtd_update_iotlb(IntelIOMMUState *s, uint16_t 
> > > source_id,
> > >       uint64_t *key = g_malloc(sizeof(*key));
> > >       uint64_t gfn = vtd_get_iotlb_gfn(addr, level);
> > > -    VTD_DPRINTF(CACHE, "update iotlb sid 0x%"PRIx16 " iova 0x%"PRIx64
> > > -                " slpte 0x%"PRIx64 " did 0x%"PRIx16, source_id, addr, slpte,
> > > -                domain_id);
> > > +    trace_vtd_iotlb_page_update(source_id, addr, slpte, domain_id);
> > >       if (g_hash_table_size(s->iotlb) >= VTD_IOTLB_MAX_SIZE) {
> > > -        VTD_DPRINTF(CACHE, "iotlb exceeds size limit, forced to reset");
> > > +        trace_vtd_iotlb_reset("iotlb exceeds size limit");
> > >           vtd_reset_iotlb(s);
> > >       }
> > > @@ -505,8 +503,7 @@ static int vtd_get_root_entry(IntelIOMMUState *s, uint8_t 
> > > index,
> > >       addr = s->root + index * sizeof(*re);
> > >       if (dma_memory_read(&address_space_memory, addr, re, sizeof(*re))) {
> > > -        VTD_DPRINTF(GENERAL, "error: fail to access root-entry at 0x%"PRIx64
> > > -                    " + %"PRIu8, s->root, index);
> > > +        trace_vtd_err("Fail to access root-entry");
> > Looks like some information were removed which may be valuable for
> > debugging, any reason for do this?
> I was trying to avoid introducing unnecessary traces, and I did a
> judgement on which one is important.
>
> I'll keep all the fields printed and add new traces for each of them
> if you really want it.

If it does not cause too much trouble, let's keep those informations.

Thanks

> Thanks,
>
> -- peterx