Re: [PATCH v2 2/2] xen: add update indicator to vcpu_runstate_info

["Jan Beulich" <JBeulich@suse.com>]

>>> On 15.06.16 at 13:34, <JGross@suse.com> wrote:
> In order to support reading another vcpu's mapped vcpu_runstate_info
> an indicator for an occurring update of the runstate information is
> needed.
> 
> Add the possibility to activate setting this indicator in the highest
> bit of state_entry_time via a vm_assist hypercall. When activated the
> update indicator will be set before the runstate information is
> modified in guest memory and it will be reset after modification is
> done. As state_entry_time is guaranteed to be different after each
> update the guest can detect any update (either in progress or while
> reading the runstate data) by comparing state_entry_time before and
> after reading runstate data: in case the values differ or the update
> indicator was set the data might be inconsistent and should be reread.

Neither here nor in the cover letter you mention why this is useful
to have (and to be honest I did forget what you need this for since
the time the original discussion happened), yet that's quite relevant
since for many years we lived happily without it.

> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1926,12 +1926,35 @@ bool_t update_runstate_area(struct vcpu *v)
>  {
>      bool_t rc;
>      smap_check_policy_t smap_policy;
> +    void __user *guest_handle = NULL;
> +    unsigned off = 0;
>  
>      if ( guest_handle_is_null(runstate_guest(v)) )
>          return 1;
>  
>      smap_policy = smap_policy_change(v, SMAP_CHECK_ENABLED);
>  
> +    if ( VM_ASSIST(v->domain, runstate_update_flag) )
> +    {
> +        off = offsetof(struct vcpu_runstate_info, state_entry_time) +
> +              sizeof(v->runstate.state_entry_time) - 1;
> +        if ( has_32bit_shinfo(v->domain) )
> +        {
> +            guest_handle = v->runstate_guest.compat.p;
> +            guest_handle +=
> +                offsetof(struct compat_vcpu_runstate_info, state_entry_time) +
> +                sizeof(v->runstate.state_entry_time) - 1;

The sizes of the native and compat fields happen to be the same,
but it would be nice if the right field/type could be used here.

> --- a/xen/include/public/vcpu.h
> +++ b/xen/include/public/vcpu.h
> @@ -84,6 +84,12 @@ struct vcpu_runstate_info {
>      /* When was current state entered (system time, ns)? */
>      uint64_t state_entry_time;
>      /*
> +     * Update indicator set in state_entry_time:
> +     * When activated via VMASST_TYPE_runstate_update_flag, set during
> +     * updates in guest memory mapped copy of vcpu_runstate_info.
> +     */
> +#define XEN_RUNSTATE_UPDATE          (1ULL << 63)

I continue to be not particularly happy with the C99ism here, but
I see we have a few other instances of such already in the public
headers.

Jan


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