Re: [PATCH 1/3] x86: fix guest CPUID handling

[Andrew Cooper <andrew.cooper3@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2831 bytes --]

On 30/04/14 15:22, Jan Beulich wrote:
> The way XEN_DOMCTL_set_cpuid got handled so far allowed for surprises
> to the caller. With this set of operations
> - set leaf A (using array index 0)
> - set leaf B (using array index 1)
> - clear leaf A (clearing array index 0)
> - set leaf B (using array index 0)
> - clear leaf B (clearing array index 0)
> the entry for leaf B at array index 1 would still be in place, while
> the caller would expect it to be cleared.
>
> While looking at the use sites of d->arch.cpuid[] I also noticed that
> the allocation of the array needlessly uses the zeroing form - the
> relevant fields of the array elements get set in a loop immediately
> following the allocation.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

>
> --- 2014-04-16.orig/xen/arch/x86/domain.c	2014-02-05 15:21:00.000000000 +0100
> +++ 2014-04-16/xen/arch/x86/domain.c	2014-04-30 13:53:18.000000000 +0200
> @@ -549,7 +549,7 @@ int arch_domain_create(struct domain *d,
>  
>      if ( !is_idle_domain(d) )
>      {
> -        d->arch.cpuids = xzalloc_array(cpuid_input_t, MAX_CPUID_INPUT);
> +        d->arch.cpuids = xmalloc_array(cpuid_input_t, MAX_CPUID_INPUT);
>          rc = -ENOMEM;
>          if ( d->arch.cpuids == NULL )
>              goto fail;
> --- 2014-04-16.orig/xen/arch/x86/domctl.c	2014-02-05 15:21:01.000000000 +0100
> +++ 2014-04-16/xen/arch/x86/domctl.c	2014-04-30 13:43:30.000000000 +0200
> @@ -1006,14 +1006,18 @@ long arch_do_domctl(
>      case XEN_DOMCTL_set_cpuid:
>      {
>          xen_domctl_cpuid_t *ctl = &domctl->u.cpuid;
> -        cpuid_input_t *cpuid = NULL; 
> +        cpuid_input_t *cpuid, *unused = NULL;
>  
>          for ( i = 0; i < MAX_CPUID_INPUT; i++ )
>          {
>              cpuid = &d->arch.cpuids[i];
>  
>              if ( cpuid->input[0] == XEN_CPUID_INPUT_UNUSED )
> -                break;
> +            {
> +                if ( !unused )
> +                    unused = cpuid;
> +                continue;
> +            }
>  
>              if ( (cpuid->input[0] == ctl->input[0]) &&
>                   ((cpuid->input[1] == XEN_CPUID_INPUT_UNUSED) ||
> @@ -1021,15 +1025,12 @@ long arch_do_domctl(
>                  break;
>          }
>          
> -        if ( i == MAX_CPUID_INPUT )
> -        {
> -            ret = -ENOENT;
> -        }
> +        if ( i < MAX_CPUID_INPUT )
> +            *cpuid = *ctl;
> +        else if ( unused )
> +            *unused = *ctl;
>          else
> -        {
> -            memcpy(cpuid, ctl, sizeof(cpuid_input_t));
> -            ret = 0;
> -        }
> +            ret = -ENOENT;
>      }
>      break;
>  
>
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel


[-- Attachment #1.2: Type: text/html, Size: 3664 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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