Re: [Xen-devel] [PATCH v6 17/18] VT-d: Dump the posted format IRTE

["Wu, Feng" <feng.wu@xxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Monday, September 07, 2015 9:05 PM
> To: Wu, Feng
> Cc: Tian, Kevin; Zhang, Yang Z; xen-devel@xxxxxxxxxxxxx
> Subject: Re: [Xen-devel] [PATCH v6 17/18] VT-d: Dump the posted format IRTE
> 
> >>> On 25.08.15 at 03:57, <feng.wu@xxxxxxxxx> wrote:
> > @@ -220,7 +224,7 @@ static void dump_iommu_info(unsigned char key)
> >                  struct iremap_entry *p;
> >                  if ( i % (1 << IREMAP_ENTRY_ORDER) == 0 )
> >                  {
> > -                    /* This entry across page boundry */
> > +                    /* This entry across page boundary. */
> 
> Either leave the comment alone, or fix its content along with its format.
> 
> > @@ -230,7 +234,7 @@ static void dump_iommu_info(unsigned char key)
> >                  else
> >                      p = &iremap_entries[i % (1 <<
> IREMAP_ENTRY_ORDER)];
> >
> > -                if ( !p->remap.p )
> > +                if ( !p->remap.p || p->remap.im )
> >                      continue;
> >
> printk("  %04x:  %x   %x  %04x %08x %02x    %x   %x  %x  %x  %x"
> >                      "   %x %x\n", i,
> > @@ -239,6 +243,41 @@ static void dump_iommu_info(unsigned char key)
> >                      p->remap.rh, p->remap.dm, p->remap.fpd,
> p->remap.p);
> >                  print_cnt++;
> >              }
> > +
> > +            if ( iremap_entries )
> > +                unmap_vtd_domain_page(iremap_entries);
> > +
> > +            iremap_entries = NULL;
> > +            printk ("\nEntries for posted format:\n");
> > +            printk("       SVT  SQ   SID              PDA  V
> URG AVL FPD P\n");
> > +            for ( i = 0; i < nr_entry; i++ )
> > +            {
> > +                struct iremap_entry *p;
> > +                if ( i % (1 << IREMAP_ENTRY_ORDER) == 0 )
> > +                {
> > +                    /* This entry across page boundry */
> > +                    if ( iremap_entries )
> > +                        unmap_vtd_domain_page(iremap_entries);
> > +
> > +                    GET_IREMAP_ENTRY(iremap_maddr, i,
> > +                                     iremap_entries, p);
> > +                }
> > +                else
> > +                    p = &iremap_entries[i % (1 <<
> IREMAP_ENTRY_ORDER)];
> > +
> > +                if ( !p->post.p || !p->post.im )
> > +                    continue;
> > +
> > +
> printk("  %04x:  %x   %x  %04x %16lx %02x    %x   %x  %x  %x\n",
> > +                    i,
> > +                    p->post.svt, p->post.sq, p->post.sid,
> > +                    ((u64)p->post.pda_h << 32) | (p->post.pda_l << 6),
> > +                    p->post.vector, p->post.urg, p->post.avail,
> p->post.fpd,
> > +                    p->post.p);
> > +
> > +                print_cnt++;
> > +            }
> 
> Hmm, this two stage approach seems to imply more overhead in
> terms of execution time and in terms of analysis (of the output)
> time. Can you see to make this a single loop with the output
> formatted in a way such that
> - one can immediately recognize the kind that each line represents,
This is easy, I can add a prefix in each line, such as "posted", "remapped".

> - mixed output is still sufficiently aligned to retain overall readability?
I think this is a little hard, there are many different fields between posted
format and remapped format, and the number of filed for each format
is different, mixing them total will hurt the readability very big.

And consider this is just a debug function, is it that bad to add a
two stage search here? The output is clear and nice in this way.

Thanks,
Feng

> 
> Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel