>
> diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c
> index 1fec412..1be058a 100644
> --- a/xen/arch/x86/monitor.c
> +++ b/xen/arch/x86/monitor.c
> @@ -20,6 +20,7 @@
> */
>
> #include <asm/monitor.h>
> +#include <asm/vm_event.h>
> #include <public/vm_event.h>
>
> int arch_monitor_domctl_event(struct domain *d,
> @@ -77,25 +78,25 @@ int arch_monitor_domctl_event(struct domain *d,
>
> case XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR:
> {
> - bool_t old_status = ad->monitor.mov_to_msr_enabled;
> + bool_t old_status;
> + int rc;
> + u32 msr = mop->u.mov_to_msr.msr;
>
> - if ( unlikely(old_status == requested_status) )
> - return -EEXIST;
> + domain_pause(d);
>
> - if ( requested_status && mop->u.mov_to_msr.extended_capture &&
> - !hvm_enable_msr_exit_interception(d) )
> - return -EOPNOTSUPP;
> + old_status = vm_event_is_msr_enabled(d, msr);
>
> - domain_pause(d);
> + if ( unlikely(old_status == requested_status) )
> + return -EEXIST;
>
Moving this test after the domain gets paused will leave the guest paused
on error condition. Any reason why this rearrangement is necessary?
>
> - if ( requested_status && mop->u.mov_to_msr.extended_capture )
> - ad->monitor.mov_to_msr_extended = 1;
> + if ( requested_status )
> + rc = vm_event_enable_msr(d, msr);
> else
> - ad->monitor.mov_to_msr_extended = 0;
> + rc = vm_event_disable_msr(d, msr);
>
> - ad->monitor.mov_to_msr_enabled = requested_status;
> domain_unpause(d);
> - break;
> +
> + return rc;
> }
>
> case XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP:
> diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c
> index 5635603..b851e39 100644
> --- a/xen/arch/x86/vm_event.c
> +++ b/xen/arch/x86/vm_event.c
> @@ -27,6 +27,13 @@ int vm_event_init_domain(struct domain *d)
> {
> struct vcpu *v;
>
> + d->arch.monitor_msr_bitmap = alloc_xenheap_page();
> +
> + if ( !d->arch.monitor_msr_bitmap )
> + return -ENOMEM;
> +
> + memset(d->arch.monitor_msr_bitmap, 0, PAGE_SIZE);
> +
> for_each_vcpu ( d, v )
> {
> if ( v->arch.vm_event )
> @@ -55,11 +62,66 @@ void vm_event_cleanup_domain(struct domain *d)
> v->arch.vm_event = NULL;
> }
>
> + free_xenheap_page(d->arch.monitor_msr_bitmap);
> + d->arch.monitor_msr_bitmap = NULL;
> +
> d->arch.mem_access_emulate_each_rep = 0;
> memset(&d->arch.monitor, 0, sizeof(d->arch.monitor));
> memset(&d->monitor, 0, sizeof(d->monitor));
> }
>
> +int vm_event_enable_msr(struct domain *d, u32 msr)
>
This function..
> +{
> + if ( !d->arch.monitor_msr_bitmap )
> + return -EINVAL;
> +
> + if ( msr <= 0x1fff )
> + set_bit(msr, d->arch.monitor_msr_bitmap + 0x000/BYTES_PER_LONG);
> + else if ( (msr >= 0xc0000000) && (msr <= 0xc0001fff) )
> + {
> + msr &= 0x1fff;
> + set_bit(msr, d->arch.monitor_msr_bitmap + 0x400/BYTES_PER_LONG);
> + }
> +
> + hvm_enable_msr_interception(d, msr);
> +
> + return 0;
> +}
> +
> +int vm_event_disable_msr(struct domain *d, u32 msr)
>
..and this..
> +{
> + if ( !d->arch.monitor_msr_bitmap )
> + return -EINVAL;
> +
> + if ( msr <= 0x1fff )
> + clear_bit(msr, d->arch.monitor_msr_bitmap + 0x000/BYTES_PER_LONG);
> + else if ( (msr >= 0xc0000000) && (msr <= 0xc0001fff) )
> + {
> + msr &= 0x1fff;
> + clear_bit(msr, d->arch.monitor_msr_bitmap + 0x400/BYTES_PER_LONG);
> + }
> +
> + return 0;
> +}
> +
> +bool_t vm_event_is_msr_enabled(const struct domain *d, u32 msr)
>
..and this one has nothing to do with vm_event really. These belong in the
monitor.c side.
> +{
> + bool_t rc = 0;
> +
> + if ( !d->arch.monitor_msr_bitmap )
> + return 0;
> +
> + if ( msr <= 0x1fff )
> + rc = test_bit(msr, d->arch.monitor_msr_bitmap +
> 0x000/BYTES_PER_LONG);
> + else if ( (msr >= 0xc0000000) && (msr <= 0xc0001fff) )
> + {
> + msr &= 0x1fff;
> + rc = test_bit(msr, d->arch.monitor_msr_bitmap +
> 0x400/BYTES_PER_LONG);
> + }
> +
> + return rc;
> +}
> +
> void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v)
> {
> if ( !is_hvm_domain(d) || !atomic_read(&v->vm_event_pause_count) )
>
>
Cheers,
Tamas
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
