Re: [Xen-devel] [v3 09/15] vt-d: Extend struct iremap_entry to support VT-d Posted-Interrupts

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

> -----Original Message-----
> From: xen-devel-bounces@xxxxxxxxxxxxx
> [mailto:xen-devel-bounces@xxxxxxxxxxxxx] On Behalf Of Andrew Cooper
> Sent: Tuesday, June 30, 2015 12:05 AM
> To: Wu, Feng; xen-devel@xxxxxxxxxxxxx
> Cc: Zhang, Yang Z; george.dunlap@xxxxxxxxxxxxx; Tian, Kevin; keir@xxxxxxx;
> jbeulich@xxxxxxxx
> Subject: Re: [Xen-devel] [v3 09/15] vt-d: Extend struct iremap_entry to 
> support
> VT-d Posted-Interrupts
> 
> On 24/06/15 06:18, Feng Wu wrote:
> > diff --git a/xen/drivers/passthrough/vtd/iommu.h
> b/xen/drivers/passthrough/vtd/iommu.h
> > index e807253..49daa70 100644
> > --- a/xen/drivers/passthrough/vtd/iommu.h
> > +++ b/xen/drivers/passthrough/vtd/iommu.h
> > @@ -289,29 +289,43 @@ struct dma_pte {
> >  /* interrupt remap entry */
> >  struct iremap_entry {
> >    union {
> > -    u64 lo_val;
> > +    struct { u64 lo, hi; };
> >      struct {
> > -        u64 p       : 1,
> > +        u16 p       : 1,
> >              fpd     : 1,
> >              dm      : 1,
> >              rh      : 1,
> >              tm      : 1,
> >              dlm     : 3,
> >              avail   : 4,
> > -            res_1   : 4,
> > -            vector  : 8,
> > -            res_2   : 8,
> > -            dst     : 32;
> > -    }lo;
> > -  };
> > -  union {
> > -    u64 hi_val;
> > +            res_1   : 4;
> > +        u8  vector;
> > +        u8  res_2;
> > +        u32 dst;
> > +        u16 sid;
> > +        u16 sq      : 2,
> > +            svt     : 2,
> > +            res_3   : 12;
> > +        u32 res_4   : 32;
> 
> res_4 does not need to be a bitfield.
> 
> > +    } remap;
> >      struct {
> > -        u64 sid     : 16,
> > -            sq      : 2,
> > +        u16 p       : 1,
> > +            fpd     : 1,
> > +            res_1   : 6,
> > +            avail   : 4,
> > +            res_2   : 2,
> > +            urg     : 1,
> > +            im      : 1;
> 
> I think "im" needs exposing in both the post and remap unions, as it is
> the bit which identifies which representation to use.

Reasonable.

> 
> > +        u8  vector;
> > +        u8  res_3;
> > +        u32 res_4   : 6,
> > +            pda_l   : 26;
> > +        u16 sid;
> > +        u16 sq      : 2,
> >              svt     : 2,
> > -            res_1   : 44;
> > -    }hi;
> > +            res_5   : 12;
> > +        u32 pda_h;
> > +    } post;
> >    };
> >  };
> >
> > diff --git a/xen/drivers/passthrough/vtd/utils.c
> b/xen/drivers/passthrough/vtd/utils.c
> > index bd14c02..a5fe237 100644
> > --- a/xen/drivers/passthrough/vtd/utils.c
> > +++ b/xen/drivers/passthrough/vtd/utils.c
> > @@ -238,14 +238,14 @@ static void dump_iommu_info(unsigned char key)
> >                  else
> >                      p = &iremap_entries[i % (1 <<
> IREMAP_ENTRY_ORDER)];
> >
> > -                if ( !p->lo.p )
> > +                if ( !p->remap.p )
> >                      continue;
> >
> printk("  %04x:  %x   %x  %04x %08x %02x    %x   %x  %x  %x  %x"
> >                      "   %x %x\n", i,
> > -                    (u32)p->hi.svt, (u32)p->hi.sq, (u32)p->hi.sid,
> > -                    (u32)p->lo.dst, (u32)p->lo.vector, (u32)p->lo.avail,
> > -                    (u32)p->lo.dlm, (u32)p->lo.tm, (u32)p->lo.rh,
> > -                    (u32)p->lo.dm, (u32)p->lo.fpd, (u32)p->lo.p);
> > +                    (u32)p->remap.svt, (u32)p->remap.sq,
> (u32)p->remap.sid,
> > +                    (u32)p->remap.dst, (u32)p->remap.vector,
> (u32)p->remap.avail,
> > +                    (u32)p->remap.dlm, (u32)p->remap.tm,
> (u32)p->remap.rh,
> > +                    (u32)p->remap.dm, (u32)p->remap.fpd,
> (u32)p->remap.p);
> 
> This printing is only valid if "im" is 0.  As this series adds support
> for the posted format, I would suggest you extend this debugging here to
> deal with both formats.

Good suggestion, as this patch is only the adjustment for the changes of
'struct iremap_entry', I will add the new logic in a separate patch.

Thanks,
Feng

> 
> ~Andrew
> 
> >                  print_cnt++;
> >              }
> >              if ( iremap_entries )
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

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