Re: [Xen-devel] [RFC PATCH V3 10/12] xen: Introduce monitor_op domctl

[Tamas K Lengyel <tamas.lengyel@xxxxxxxxxxxx>]

>> +int monitor_domctl(struct xen_domctl_monitor_op *domctl, struct domain *d)
>> +{
>> +    /*
>> +     * At the moment only HVM domains are supported. However, event delivery
>> +     * could be extended to PV domains. See comments below.
>> +     */
>> +    if ( !is_hvm_domain(d) )
>> +        return -ENOSYS;
>> +
>> +    if ( domctl->op != XEN_DOMCTL_MONITOR_OP_ENABLE &&
>> +         domctl->op != XEN_DOMCTL_MONITOR_OP_DISABLE )
>> +        return -EFAULT;
>> +
>> +    switch ( domctl->subop )
>> +    {
>> +    case XEN_DOMCTL_MONITOR_SUBOP_MOV_TO_CR0:
>> +    {
>> +        /* Note: could be supported on PV domains. */
>> +        struct mov_to_cr0 *options = &d->arch.monitor_options.mov_to_cr0;
>> +
>> +        if ( domctl->op == XEN_DOMCTL_MONITOR_OP_ENABLE )
>> +        {
>> +            if ( options->enabled )
>> +                return -EBUSY;
>> +
>> +            options->enabled = 1;
>> +            options->sync = domctl->options.mov_to_cr0.sync;
>> +            options->onchangeonly = domctl->options.mov_to_cr0.onchangeonly;
>
> Shouldn't you set "->enabled" last, after a suitable barrier (and with
> the consuming sides using suitable barriers too)? Or are you missing
> a domain_pause() here?

Hm, there wasn't any barrier previously with HVM params so I was not
sure. Would make sense as now we have multiple fields instead of a
single one so its no longer being set atomically. I think
pause/unpause would make the most sense as it is already used in some
of the settings.

...

>
>> --- a/xen/include/public/hvm/params.h
>> +++ b/xen/include/public/hvm/params.h
>> @@ -162,21 +162,6 @@
>>   */
>>  #define HVM_PARAM_ACPI_IOPORTS_LOCATION 19
>>
>> -/* Enable blocking memory events, async or sync (pause vcpu until response)
>> - * onchangeonly indicates messages only on a change of value */
>> -#define HVM_PARAM_MEMORY_EVENT_CR0          20
>> -#define HVM_PARAM_MEMORY_EVENT_CR3          21
>> -#define HVM_PARAM_MEMORY_EVENT_CR4          22
>> -#define HVM_PARAM_MEMORY_EVENT_INT3         23
>> -#define HVM_PARAM_MEMORY_EVENT_SINGLE_STEP  25
>> -#define HVM_PARAM_MEMORY_EVENT_MSR          30
>
> I'm not sure if HVM param slots can be re-used. If they can, leaving a
> note that the deleted numbers are available for re-sue would be nice.
> If they can't, leaving a note that they shouldn't be re-used would
> seem mandatory.
>
> Jan

I'm not sure either if they can be reused, I would assume yes so I'll
make the comment accordingly (unless someone knows more and objects).

All other comments: Ack.

Thanks!
Tamas

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