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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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.

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