Re: [Xen-devel] [PATCH 2/2] xen: add update indicator to vcpu_runstate_info

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 20.05.16 at 15:22, <JGross@xxxxxxxx> wrote:
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1925,13 +1925,37 @@ static void paravirt_ctxt_switch_to(struct vcpu *v)
>  bool_t update_runstate_area(struct vcpu *v)
>  {
>      bool_t rc;
> +    bool_t update_flag;

I think this variable is superfluous (and causes more register pressure
in the compiler), since ...

>      smap_check_policy_t smap_policy;
> +    void __user *guest_handle = NULL;

... you can key off of this being non-NULL or ...

> +    unsigned off = 0;

... this being non-zero in the second if().

>      if ( guest_handle_is_null(runstate_guest(v)) )
>          return 1;
>  
> +    update_flag = VM_ASSIST(v->domain, runstate_update_flag);
> +
>      smap_policy = smap_policy_change(v, SMAP_CHECK_ENABLED);
>  
> +    if ( update_flag )
> +    {
> +        off = offsetof(struct vcpu_runstate_info, state_entry_time) + 7;

How come this is outside the following if()? Also sizeof(...) - 1 please
instead of the literal 7.

> --- 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 think this should be UINT64_C(1), as ULL is not a C89 compatible
suffix, but by requiring uint64_t I think we can imply that along with
that C99 type the platform also surfaces respective macros.

Jan


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