> -----Original Message----- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: Friday, July 10, 2015 9:08 PM > To: Wu, Feng > Cc: andrew.coop...@citrix.com; george.dun...@eu.citrix.com; Tian, Kevin; > Zhang, Yang Z; xen-devel@lists.xen.org; k...@xen.org > Subject: Re: [v3 06/15] vmx: Extend struct pi_desc to support VT-d > Posted-Interrupts > > >>> On 24.06.15 at 07:18, <feng...@intel.com> 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. > > 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". >
When the vCPU is to be blocked, we need to atomically update the "nv" and "ndst", then the wakeup notification event can be delivered to the right destination. Thanks, Feng > Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
