Hi,
At 22:07 +0000 on 04 Jan (1294178845), Joe Epstein wrote:
> diff -r 3500724e8052 -r c1866aff7a5f xen/arch/x86/hvm/hvm.c
> --- a/xen/arch/x86/hvm/hvm.c Tue Jan 04 12:34:06 2011 -0800
> +++ b/xen/arch/x86/hvm/hvm.c Tue Jan 04 12:35:49 2011 -0800
> @@ -309,6 +309,15 @@
> return; /* bail */
> }
> }
> +
> + /* Inject pending hw/sw trap */
> + if (v->arch.hvm_vcpu.inject_trap != -1)
> + {
> + hvm_inject_exception(v->arch.hvm_vcpu.inject_trap,
> + v->arch.hvm_vcpu.inject_error_code,
> + v->arch.hvm_vcpu.inject_cr2);
> + v->arch.hvm_vcpu.inject_trap = -1;
> + }
What if the guest already has a trap pending for some reason? This
could turn it into a double-fault, which is probably not what the caller
wanted.
> }
>
> static void hvm_init_ioreq_page(
> @@ -949,6 +958,8 @@
> spin_lock_init(&v->arch.hvm_vcpu.tm_lock);
> INIT_LIST_HEAD(&v->arch.hvm_vcpu.tm_list);
>
> + v->arch.hvm_vcpu.inject_trap = -1;
> +
> #ifdef CONFIG_COMPAT
> rc = setup_compat_arg_xlat(v);
> if ( rc != 0 )
> @@ -3216,10 +3227,34 @@
> case HVM_PARAM_ACPI_IOPORTS_LOCATION:
> rc = pmtimer_change_ioport(d, a.value);
> break;
> + case HVM_PARAM_MEMORY_EVENT_INT3:
> + if ( a.value & HVMPME_onchangeonly )
> + rc = -EINVAL;
> + break;
> }
>
> - if ( rc == 0 )
> + if ( rc == 0 )
> + {
> d->arch.hvm_domain.params[a.index] = a.value;
> +
> + switch( a.index )
> + {
> + case HVM_PARAM_MEMORY_EVENT_INT3:
> + {
> + domain_pause(d);
You need to make sure a VM can't set this param on itself, or else this
domain_pause() can deadlock Xen. Also, you probably don't want the
daomin turning these intercepts off anyway. :)
Now that I think of it, it's worth checking your other hypercalls to
check that a VM can't change its own p2m mapping types.
[...]
> @@ -1589,13 +1601,25 @@
> switch ( cr )
> {
> case 0:
> - return !hvm_set_cr0(value);
> + old = v->arch.hvm_vcpu.guest_cr[0];
> + rc = !hvm_set_cr0(value);
> + if (rc)
> + hvm_memory_event_cr0(value, old);
You defined the hvm_memory_event_foo() functions to return error codes
but you don't handle them in the caller. Please either handle them or
arrange for the functions to return void.
Cheers,
Tim.
--
Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd. (Company #02937203, SL9 0BG)
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home