Re: [Xen-devel] [PATCH 1/2] x86/monitor: add masking support for write_ctrlreg events

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.05.17 at 15:41, <ppircalabu@xxxxxxxxxxxxxxx> wrote:
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -37,7 +37,7 @@
>  #include "hvm/save.h"
>  #include "memory.h"
>  
> -#define XEN_DOMCTL_INTERFACE_VERSION 0x0000000d
> +#define XEN_DOMCTL_INTERFACE_VERSION 0x0000000e
>  
>  /*
>   * NB. xen_domctl.domain is an IN/OUT parameter for this operation.
> @@ -1107,6 +1107,10 @@ struct xen_domctl_monitor_op {
>              uint8_t sync;
>              /* Send event only on a change of value */
>              uint8_t onchangeonly;
> +            /* Send event only if the changed bit in the control register
> +             * is not masked
> +             */
> +            unsigned long bitmask;

You can't use "unsigned long" in public headers. Also beware of
alignment issues between 32-bit and 64-bit callers (I didn't check
whether there actually is any, I'm merely hinting towards the
use of uint64_aligned_t).

> --- a/xen/include/public/vm_event.h
> +++ b/xen/include/public/vm_event.h
> @@ -155,6 +155,15 @@
>  #define VM_EVENT_X86_CR4    2
>  #define VM_EVENT_X86_XCR0   3
>  
> +/* vm_event_write_ctrlreg default bit mask
> + * If the changed bit in the control register is masked the event is
> + * not triggered
> + */

Comment style.

> +#define VM_EVENT_X86_CR0_DEFAULT_MASK                0x00000000
> +#define VM_EVENT_X86_CR3_DEFAULT_MASK                0x00000000
> +#define VM_EVENT_X86_CR4_DEFAULT_MASK                0x00000080
> +#define VM_EVENT_X86_XCR0_DEFAULT_MASK               0x00000000

These are unused - what are they good for? And why would
you want to special case CR4.PGE (and even without any
comment)?

Jan


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