Re: [Xen-devel] [PATCH v2 07/11] xen/hvmlite: Initialize context for secondary VCPUs

From	Doug Goldstein <cardoe@cardoe.com>
Newsgroups	linux.kernel
Subject	Re: [Xen-devel] [PATCH v2 07/11] xen/hvmlite: Initialize context for secondary VCPUs
Date	2016-02-04 14:00 +0100
Message-ID	<qYrFU-jS-3@gated-at.bofh.it> (permalink)
References	<qXoTM-4WL-7@gated-at.bofh.it> <qXoTM-4WL-5@gated-at.bofh.it> <qXM01-5FW-5@gated-at.bofh.it> <qXMt5-5Rl-7@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

[Multipart message — attachments visible in raw view] - view raw

On 2/2/16 10:58 AM, Boris Ostrovsky wrote:
> On 02/02/2016 11:21 AM, David Vrabel wrote:
>> This needs some more description in the commit message.
>>
>>> --- a/arch/x86/xen/smp.c
>>> +++ b/arch/x86/xen/smp.c
>> [...]
>>> +        hctxt->cpu_regs.x86_32.cs_base = 0;
>>> +        hctxt->cpu_regs.x86_32.cs_limit = ~0u;
>>> +        hctxt->cpu_regs.x86_32.cs_ar = 0xc9b;
>>> +        hctxt->cpu_regs.x86_32.ds_base = 0;
>>> +        hctxt->cpu_regs.x86_32.ds_limit = ~0u;
>>> +        hctxt->cpu_regs.x86_32.ds_ar = 0xc93;
>>> +        hctxt->cpu_regs.x86_32.es_base = 0;
>>> +        hctxt->cpu_regs.x86_32.es_limit = ~0u;
>>> +        hctxt->cpu_regs.x86_32.es_ar = 0xc93;
>>> +        hctxt->cpu_regs.x86_32.ss_base = 0;
>>> +        hctxt->cpu_regs.x86_32.ss_limit = ~0u;
>>> +        hctxt->cpu_regs.x86_32.ss_ar = 0xc93;
>>> +        hctxt->cpu_regs.x86_32.tr_base = 0;
>>> +        hctxt->cpu_regs.x86_32.tr_limit = 0xff;
>>> +        hctxt->cpu_regs.x86_32.tr_ar = 0x8b;
>> Lots of hard-coded values here.  Should this be #defined somewhere?
> 
> We also don't need to set bases to zero since hctxt is kzalloc'd. I'll
> remove that and add a comment.
> 
> As for macros --- I couldn't find the bits defined symbolically anywhere
> and since this is the only place this is used the macros would be local
> here.
> 
> -boris
> 

It could be useful to have them defined locally if only to give them
some more meaning by having a name rather than 0x8b. Just a thought.

-- 
Doug Goldstein

Thread

[PATCH v2 07/11] xen/hvmlite: Initialize context for secondary VCPUs Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-02-01 16:50 +0100
  Re: [Xen-devel] [PATCH v2 07/11] xen/hvmlite: Initialize context for  secondary VCPUs David Vrabel <david.vrabel@citrix.com> - 2016-02-02 17:30 +0100
    Re: [Xen-devel] [PATCH v2 07/11] xen/hvmlite: Initialize context for  secondary VCPUs Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-02-02 18:00 +0100
      Re: [Xen-devel] [PATCH v2 07/11] xen/hvmlite: Initialize context for  secondary VCPUs David Vrabel <david.vrabel@citrix.com> - 2016-02-04 11:10 +0100
      Re: [Xen-devel] [PATCH v2 07/11] xen/hvmlite: Initialize context for  secondary VCPUs Doug Goldstein <cardoe@cardoe.com> - 2016-02-04 14:00 +0100
        Re: [Xen-devel] [PATCH v2 07/11] xen/hvmlite: Initialize context for  secondary VCPUs Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-02-04 20:10 +0100

csiph-web