Re: [Xen-devel] [PATCH 16/16] x86/monitor: fix: xc_monitor _write_ctrlreg w/o previous _enable must fail

[Corneliu ZUZU <czuzu@xxxxxxxxxxxxxxx>]

On 7/9/2016 7:23 AM, Corneliu ZUZU wrote:
> Enforce presence of a monitor vm-event subscriber when the toolstack user calls
> xc_monitor_write_ctrlreg() (XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG domctl).
> Without this change, "ASSERT(monitor_domain_initialised(v->domain));" @
> hvm_set_cr0() and such would fail if the toolstack user calls
> xc_monitor_write_ctrlreg(...) w/ enable = true, without first calling
> xc_monitor_enable().
>
> Also adjust returned error code for similar check from -EINVAL to more
> descriptive -ENODEV (XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP).
>
> Signed-off-by: Corneliu ZUZU <czuzu@xxxxxxxxxxxxxxx>
> ---
>   xen/arch/x86/monitor.c        | 4 ++++
>   xen/include/asm-x86/monitor.h | 2 +-
>   2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c
> index 05a2f0d..4cf018a 100644
> --- a/xen/arch/x86/monitor.c
> +++ b/xen/arch/x86/monitor.c
> @@ -324,6 +324,10 @@ int arch_monitor_domctl_event(struct domain *d,
>           unsigned int ctrlreg_bitmask;
>           bool_t old_status;
>   
> +        /* Meaningless without a monitor vm-events subscriber. */
> +        if ( unlikely(!monitor_domain_initialised(d)) )
> +            return -ENODEV;
> +
>           /* sanity check: avoid left-shift undefined behavior */
>           if ( unlikely(mop->u.mov_to_cr.index > 31) )
>               return -EINVAL;
> diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
> index 11497ef..a6022db 100644
> --- a/xen/include/asm-x86/monitor.h
> +++ b/xen/include/asm-x86/monitor.h
> @@ -47,7 +47,7 @@ int arch_monitor_domctl_op(struct domain *d, struct 
> xen_domctl_monitor_op *mop)
>           if ( likely(monitor_domain_initialised(d)) )
>               d->arch.mem_access_emulate_each_rep = !!mop->event;
>           else
> -            rc = -EINVAL;
> +            rc = -ENODEV;
>   
>           domain_unpause(d);
>           break;
I might have forgotten to think about domain pausing (for all patches), 
where it needs to be done.
I'll leave that for v2 (obviously), I just wanted to let you know in 
case you guys have feedback on the matter until then.
Zuzu.

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