Xen devel - Mar 2012 - [PATCH] xen: Add command line option to enable ASID support -- on by default

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>

diff --git a/xen/arch/x86/hvm/svm/asid.c b/xen/arch/x86/hvm/svm/asid.c
--- a/xen/arch/x86/hvm/svm/asid.c
+++ b/xen/arch/x86/hvm/svm/asid.c
@@ -24,12 +24,16 @@
 #include <asm/amd.h>
 #include <asm/hvm/nestedhvm.h>
 
+/* Xen command-line option to enable ASIDs */
+static int opt_asid_enabled = 1;
+boolean_param("asid", opt_asid_enabled);
+
 void svm_asid_init(struct cpuinfo_x86 *c)
 {
     int nasids = 0;
 
     /* Check for erratum #170, and leave ASIDs disabled if it''s
present. */
-    if ( !cpu_has_amd_erratum(c, AMD_ERRATUM_170) )
+    if ( opt_asid_enabled && !cpu_has_amd_erratum(c, AMD_ERRATUM_170) )
         nasids = cpuid_ebx(0x8000000A);
 
     hvm_asid_init(nasids);

On 07/03/2012 17:58, "George Dunlap"
<george.dunlap@eu.citrix.com> wrote:
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
Why would you want to be able to disable the feature?

 -- Keir
> diff --git a/xen/arch/x86/hvm/svm/asid.c b/xen/arch/x86/hvm/svm/asid.c
> --- a/xen/arch/x86/hvm/svm/asid.c
> +++ b/xen/arch/x86/hvm/svm/asid.c
> @@ -24,12 +24,16 @@
>  #include <asm/amd.h>
>  #include <asm/hvm/nestedhvm.h>
>  
> +/* Xen command-line option to enable ASIDs */
> +static int opt_asid_enabled = 1;
> +boolean_param("asid", opt_asid_enabled);
> +
>  void svm_asid_init(struct cpuinfo_x86 *c)
>  {
>      int nasids = 0;
>  
>      /* Check for erratum #170, and leave ASIDs disabled if it''s
present. */
> -    if ( !cpu_has_amd_erratum(c, AMD_ERRATUM_170) )
> +    if ( opt_asid_enabled && !cpu_has_amd_erratum(c,
AMD_ERRATUM_170) )
>          nasids = cpuid_ebx(0x8000000A);
>  
>      hvm_asid_init(nasids);
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

On Wed, 2012-03-07 at 18:07 +0000, Keir Fraser wrote:
> On 07/03/2012 17:58, "George Dunlap"
<george.dunlap@eu.citrix.com> wrote:
> 
> > Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> 
> Why would you want to be able to disable the feature?
Well at very least to measure the performance benefit (or overhead) of
using the feature.  There may be hardware in the future where ASIDs may
cause problems; who knows.

AFAICT this isn''t documented anywhere, so if you really don''t
want Yet
Another Option, we could probably get rid of it from the XenServer
patchqueue.  But I didn''t see any harm in it; and you never know when
the flexibility of disabling something without recompiling will come in
handy.

 -George
> 
>  -- Keir
> 
> > diff --git a/xen/arch/x86/hvm/svm/asid.c b/xen/arch/x86/hvm/svm/asid.c
> > --- a/xen/arch/x86/hvm/svm/asid.c
> > +++ b/xen/arch/x86/hvm/svm/asid.c
> > @@ -24,12 +24,16 @@
> >  #include <asm/amd.h>
> >  #include <asm/hvm/nestedhvm.h>
> >  
> > +/* Xen command-line option to enable ASIDs */
> > +static int opt_asid_enabled = 1;
> > +boolean_param("asid", opt_asid_enabled);
> > +
> >  void svm_asid_init(struct cpuinfo_x86 *c)
> >  {
> >      int nasids = 0;
> >  
> >      /* Check for erratum #170, and leave ASIDs disabled if
it''s present. */
> > -    if ( !cpu_has_amd_erratum(c, AMD_ERRATUM_170) )
> > +    if ( opt_asid_enabled && !cpu_has_amd_erratum(c,
AMD_ERRATUM_170) )
> >          nasids = cpuid_ebx(0x8000000A);
> >  
> >      hvm_asid_init(nasids);
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel
> 
>

On 07/03/2012 18:14, "George Dunlap" <george.dunlap@citrix.com>
wrote:
> On Wed, 2012-03-07 at 18:07 +0000, Keir Fraser wrote:
>> On 07/03/2012 17:58, "George Dunlap"
<george.dunlap@eu.citrix.com> wrote:
>> 
>>> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
>> 
>> Why would you want to be able to disable the feature?
> 
> Well at very least to measure the performance benefit (or overhead) of
> using the feature.  There may be hardware in the future where ASIDs may
> cause problems; who knows.
> 
> AFAICT this isn''t documented anywhere, so if you really
don''t want Yet
> Another Option, we could probably get rid of it from the XenServer
> patchqueue.  But I didn''t see any harm in it; and you never know
when
> the flexibility of disabling something without recompiling will come in
> handy.
Okay, I applied it. I also moved the implementation into hvm/asid.c by the
way -- your patch was unnecessarily AMD-specific.

xen-unstable:24986

 -- Keir
>  -George
> 
>> 
>>  -- Keir
>> 
>>> diff --git a/xen/arch/x86/hvm/svm/asid.c
b/xen/arch/x86/hvm/svm/asid.c
>>> --- a/xen/arch/x86/hvm/svm/asid.c
>>> +++ b/xen/arch/x86/hvm/svm/asid.c
>>> @@ -24,12 +24,16 @@
>>>  #include <asm/amd.h>
>>>  #include <asm/hvm/nestedhvm.h>
>>>  
>>> +/* Xen command-line option to enable ASIDs */
>>> +static int opt_asid_enabled = 1;
>>> +boolean_param("asid", opt_asid_enabled);
>>> +
>>>  void svm_asid_init(struct cpuinfo_x86 *c)
>>>  {
>>>      int nasids = 0;
>>>  
>>>      /* Check for erratum #170, and leave ASIDs disabled if
it''s present. */
>>> -    if ( !cpu_has_amd_erratum(c, AMD_ERRATUM_170) )
>>> +    if ( opt_asid_enabled && !cpu_has_amd_erratum(c,
AMD_ERRATUM_170) )
>>>          nasids = cpuid_ebx(0x8000000A);
>>>  
>>>      hvm_asid_init(nasids);
>>> 
>>> _______________________________________________
>>> Xen-devel mailing list
>>> Xen-devel@lists.xen.org
>>> http://lists.xen.org/xen-devel
>> 
>> 
> 
>

Keir Fraser writes ("Re: [Xen-devel] [PATCH] xen: Add command line option
to enable ASID support -- on by default"):
> On 07/03/2012 18:14, "George Dunlap"
<george.dunlap@citrix.com> wrote:
> > Well at very least to measure the performance benefit (or overhead) of
> > using the feature.  There may be hardware in the future where ASIDs
may
> > cause problems; who knows.
> > 
> > AFAICT this isn''t documented anywhere, so if you really
don''t want Yet
> > Another Option, we could probably get rid of it from the XenServer
> > patchqueue.  But I didn''t see any harm in it; and you never
know when
> > the flexibility of disabling something without recompiling will come
in
> > handy.
> 
> Okay, I applied it. I also moved the implementation into hvm/asid.c by the
> way -- your patch was unnecessarily AMD-specific.
> 
> xen-unstable:24986
Surely given that we now have a document listing the command line
options we shouldn''t be introducing new options with no documentation ?

If George''s original patch had come with a docs hunk it probably would
have already contained the answers to the questions you asked, too...

Ian.

On Tue, Mar 13, 2012 at 4:28 PM, Ian Jackson <Ian.Jackson@eu.citrix.com>
wrote:
> Keir Fraser writes ("Re: [Xen-devel] [PATCH] xen: Add command line
option to enable ASID support -- on by default"):
>> On 07/03/2012 18:14, "George Dunlap"
<george.dunlap@citrix.com> wrote:
>> > Well at very least to measure the performance benefit (or
overhead) of
>> > using the feature.  There may be hardware in the future where
ASIDs may
>> > cause problems; who knows.
>> >
>> > AFAICT this isn''t documented anywhere, so if you really
don''t want Yet
>> > Another Option, we could probably get rid of it from the XenServer
>> > patchqueue.  But I didn''t see any harm in it; and you
never know when
>> > the flexibility of disabling something without recompiling will
come in
>> > handy.
>>
>> Okay, I applied it. I also moved the implementation into hvm/asid.c by
the
>> way -- your patch was unnecessarily AMD-specific.
>>
>> xen-unstable:24986
>
> Surely given that we now have a document listing the command line
> options we shouldn''t be introducing new options with no
documentation ?
>
> If George''s original patch had come with a docs hunk it probably
would
> have already contained the answers to the questions you asked, too...
When I wrote the patch I wasn''t aware of the new doc file.  (IIUC it
was at most a week old at that point.)  I''ll put updating it on my
to-do list.

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

George Dunlap writes ("Re: [Xen-devel] [PATCH] xen: Add command line option
to enable ASID support -- on by default"):
> On Tue, Mar 13, 2012 at 4:28 PM, Ian Jackson
<Ian.Jackson@eu.citrix.com> wrote:
> > Keir Fraser writes ("Re: [Xen-devel] [PATCH] xen: Add command
line option to enable ASID support -- on by default"):
> > Surely given that we now have a document listing the command line
> > options we shouldn''t be introducing new options with no
documentation ?
> >
> > If George''s original patch had come with a docs hunk it
probably would
> > have already contained the answers to the questions you asked, too...
> 
> When I wrote the patch I wasn''t aware of the new doc file.  (IIUC
it
> was at most a week old at that point.)  I''ll put updating it on my
> to-do list.
Well it was only introduced last week :-).  Thanks.

Ian.