Re: [PATCH 1/3] xen/monitor: Control register values

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.05.2020 18:53, Tamas K Lengyel wrote:
> Extend the monitor_op domctl to include option that enables
> controlling what values certain registers are permitted to hold
> by a monitor subscriber.

This needs a bit more explanation, especially for those of us
who aren't that introspection savvy. For example, from the text
here I didn't expect a simple bool control, but something where
actual (register) values get passed back and forth.

> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -2263,9 +2263,10 @@ int hvm_set_cr0(unsigned long value, bool may_defer)
>      {
>          ASSERT(v->arch.vm_event);
>  
> -        if ( hvm_monitor_crX(CR0, value, old_value) )
> +        if ( hvm_monitor_crX(CR0, value, old_value) &&
> +             v->domain->arch.monitor.control_register_values )
>          {
> -            /* The actual write will occur in hvm_do_resume(), if permitted. 
> */
> +            /* The actual write will occur in hvm_do_resume, if permitted. */

Please can you leave alone this and the similar comments below.
And for consistency _add_ parentheses to the one new instance
you add?

> --- a/xen/arch/x86/monitor.c
> +++ b/xen/arch/x86/monitor.c
> @@ -144,7 +144,15 @@ int arch_monitor_domctl_event(struct domain *d,
>                                struct xen_domctl_monitor_op *mop)
>  {
>      struct arch_domain *ad = &d->arch;
> -    bool requested_status = (XEN_DOMCTL_MONITOR_OP_ENABLE == mop->op);
> +    bool requested_status;
> +
> +    if ( XEN_DOMCTL_MONITOR_OP_CONTROL_REGISTERS == mop->op )
> +    {
> +        ad->monitor.control_register_values = true;

And there's no way to clear this flag again?

Jan