Re: [Xen-devel] [PATCH V9] xen/vm_event: Clean up control-register-write vm_events and add XCR0 event

[Tim Deegan <tim@xxxxxxx>]

At 16:19 +0200 on 03 Jun (1433348379), Lengyel, Tamas wrote:
> On Wed, Jun 3, 2015 at 3:48 PM, Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
> wrote:
> > On 06/03/2015 04:29 PM, Lengyel, Tamas wrote:
> > >          struct {
> > >     -        uint16_t mov_to_cr0_enabled          : 1;
> > >     -        uint16_t mov_to_cr0_sync             : 1;
> > >     -        uint16_t mov_to_cr0_onchangeonly     : 1;
> > >     -        uint16_t mov_to_cr3_enabled          : 1;
> > >     -        uint16_t mov_to_cr3_sync             : 1;
> > >     -        uint16_t mov_to_cr3_onchangeonly     : 1;
> > >     -        uint16_t mov_to_cr4_enabled          : 1;
> > >     -        uint16_t mov_to_cr4_sync             : 1;
> > >     -        uint16_t mov_to_cr4_onchangeonly     : 1;
> > >     +        uint16_t write_ctrlreg_enabled       : 4;
> > >     +        uint16_t write_ctrlreg_sync          : 4;
> > >     +        uint16_t write_ctrlreg_onchangeonly  : 4;
> > >
> > >
> > > Just looking at this here again, we will now have a bitmap within a
> > > bitmap, which doesn't seem to be very efficient. IMHO it would be better
> > > to just take the ctrlreg bitmap out as a separate uint8_t within struct
> > > {} monitor.
> >
> > How is it inefficient? I don't see that at all. And I'm not sure what
> > you mean about the uint8_t: there are 3 fields there, each 4-bits wide
> > (write_ctrlreg_enabled, write_ctrlreg_sync, write_ctrlreg_onchangeonly)
> > and only (at most) two of them would fit into a uint8_t. To put them all
> > into a new struct would mean wasting an uint16_t for 12-bits.
> 
> Right now if you want to access a bit using the index on the ctrlreg
> fields, you would for example do (monitor.write_ctrlreg_enabled & index).
> This is actually going to perform two bitmask operations. First,
> monitor.write_ctrlreg_enabled
> does a masking operating to get you only the 4 bits corresponding to this
> field, then you do another mask with the index.

The compiler can optimize away the first mask operation.  E.g., gcc
folds that into a single operation at anything above -O0.

Tim.

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