[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v5 6/8] microcode: delete microcode pointer and size from microcode_info



On Mon, Jan 28, 2019 at 03:06:48PM +0800, Chao Gao wrote:
> microcode pointer and size were passed to other CPUs to parse
> microcode locally. Now, parsing microcode is done on one CPU.
> Other CPUs needn't parse the microcode blob; the pointer and
> size can be removed.
> 
> Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
> ---
>  xen/arch/x86/microcode.c | 33 +++++++++++++++++----------------
>  1 file changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
> index 936f0b8..3c2274f 100644
> --- a/xen/arch/x86/microcode.c
> +++ b/xen/arch/x86/microcode.c
> @@ -190,9 +190,7 @@ DEFINE_PER_CPU(struct ucode_cpu_info, ucode_cpu_info);
>  
>  struct microcode_info {
>      unsigned int cpu;

I think cpu can also be removed as my previous reply to patch 5, at
which point you only need to store an error? In which case you should
also remove this struct then.

> -    uint32_t buffer_size;
>      int error;
> -    char buffer[1];
>  };
>  
>  static void microcode_fini_cpu(unsigned int cpu)
> @@ -316,6 +314,7 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) 
> buf, unsigned long len)
>  {
>      int ret;
>      struct microcode_info *info;
> +    void * buffer;
>  
>      if ( len != (uint32_t)len )
>          return -E2BIG;
> @@ -323,28 +322,26 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) 
> buf, unsigned long len)
>      if ( microcode_ops == NULL )
>          return -EINVAL;
>  
> -    info = xmalloc_bytes(sizeof(*info) + len);
> -    if ( info == NULL )
> -        return -ENOMEM;
> -
> -    ret = copy_from_guest(info->buffer, buf, len);
> -    if ( ret != 0 )
> +    info = xmalloc(struct microcode_info);
> +    buffer = xmalloc_bytes(len);
> +    if ( !info || !buffer )
>      {
> -        xfree(info);
> -        return ret;
> +        ret = -ENOMEM;
> +        goto free;
>      }
>  
> +    ret = copy_from_guest(buffer, buf, len);
> +    if ( ret != 0 )
> +        goto free;

copy_from_guest doesn't return an errno value, you have to set ret to
EFAULT:

if ( copy_from_guest(buffer, buf, len) )
{
    ret = -EFAULT;
    goto free;
}

> +
>      if ( microcode_ops->start_update )
>      {
>          ret = microcode_ops->start_update();
>          if ( ret != 0 )
> -        {
> -            xfree(info);
> -            return ret;
> -        }
> +            goto free;
>      }
>  
> -    ret = parse_microcode_blob(info->buffer, len);
> +    ret = parse_microcode_blob(buffer, len);
>      if ( ret <= 0 )

Don't you need to free info and buffer here also?

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.