[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [v3 06/15] vmx: Extend struct pi_desc to support VT-d Posted-Interrupts




> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Friday, July 10, 2015 9:08 PM
> To: Wu, Feng
> Cc: andrew.cooper3@xxxxxxxxxx; george.dunlap@xxxxxxxxxxxxx; Tian, Kevin;
> Zhang, Yang Z; xen-devel@xxxxxxxxxxxxx; keir@xxxxxxx
> Subject: Re: [v3 06/15] vmx: Extend struct pi_desc to support VT-d
> Posted-Interrupts
> 
> >>> On 24.06.15 at 07:18, <feng.wu@xxxxxxxxx> wrote:
> > @@ -81,8 +81,19 @@ struct vmx_domain {
> >
> >  struct pi_desc {
> >      DECLARE_BITMAP(pir, NR_VECTORS);
> > -    u32 control;
> > -    u32 rsvd[7];
> > +    union {
> > +        struct
> > +        {
> > +        u16 on     : 1,  /* bit 256 - Outstanding Notification */
> > +            sn     : 1,  /* bit 257 - Suppress Notification */
> > +            rsvd_1 : 14; /* bit 271:258 - Reserved */
> > +        u8  nv;          /* bit 279:272 - Notification Vector */
> > +        u8  rsvd_2;      /* bit 287:280 - Reserved */
> > +        u32 ndst;        /* bit 319:288 - Notification Destination */
> > +        };
> > +        u64 control;
> > +    };
> 
> So current code, afaics, uses e.g. test_and_set_bit() to set ON.
> By also declaring this as a bitfield you're opening the structure for
> non-atomic accesses. If that's correct, why is other code not
> being changed to _only_ use the bitfield mechanism (likely also
> eliminating the need for it being a union with the now 64-bit
> "control"? If atomic accesses are required, then I'd strongly
> suggest against making this a bit field.

All this fields are defined in the hardware Spec, if we define nv,
ndst, but not define on and sn explicitly, it is a little strange.
But you are right, I should use the same type to access them,
e.g, bitfield mechanism.

Thanks,
Feng

> 
> And in no event can I see why "ndst" needs to be union-ized
> with "control" if it doesn't need to be updated atomically with
> e.g. "nv".
> 
> Jan


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.