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

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Wed, May 20, 2020 at 7:36 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> 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?

I changed to because now it doesn't fit into the 80-line limit below,
and then changed it everywhere _for_ consistency.

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

There is. Disable the monitor vm_event interface and reinitialize.

Tamas