Re: [Xen-devel] [PATCH v3 4/7] xen/x86: use invpcid for flushing the TLB

[Juergen Gross]

On 22/03/18 16:35, Jan Beulich wrote:
 On 21.03.18 at 13:51,  wrote:
>> --- a/docs/misc/xen-command-line.markdown
>> +++ b/docs/misc/xen-command-line.markdown
>> @@ -1380,6 +1380,14 @@ Because responsibility for APIC setup is shared 
>> between Xen and the
>>  domain 0 kernel this option is automatically propagated to the domain
>>  0 command line.
>>  
>> +### noinvpcid (x86)
>> +> `= `
>> +
>> +Disable using the INVPCID instruction for flushing TLB entries.
>> +This should only be used in case of known issues on the current platform
>> +with that instruction. Disabling INVPCID will normally result in a slightly
>> +degraded performance.
> 
> At the first glance this looks as if it wants to be a cpuid=
> sub-option. However, that would disable use by both Xen and
> (HVM) guests. Andrew, what are your plans here as to
> distinguishing the "Xen uses a feature" from the "disable use of
> a feature altogether"?
> 
> If we stay with a separate option, then please make this a
> normal boolean one (i.e. drop the "no" prefix), as "no-noinvpcid"
> is rather ugly.

Okay.

> 
>> @@ -457,7 +472,6 @@ static void generic_set_all(void)
>>  set_bit(count, _changes_mask);
>>  mask >>= 1;
>>  }
>> -
>>  }
>>  
>>  static void generic_set_mtrr(unsigned int reg, unsigned long base,
> 
> I don't mind this line being dropped, but in general please avoid
> stray changes which aren't assimilated into changes you do anyway.

The main reason I did drop this line was the trailing tab. I just took
the risk of someone complaining.


Juergen


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

Re: [Xen-devel] [PATCH v3 4/7] xen/x86: use invpcid for flushing the TLB

[Jan Beulich]

>>> On 21.03.18 at 13:51,  wrote:
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1380,6 +1380,14 @@ Because responsibility for APIC setup is shared 
> between Xen and the
>  domain 0 kernel this option is automatically propagated to the domain
>  0 command line.
>  
> +### noinvpcid (x86)
> +> `= `
> +
> +Disable using the INVPCID instruction for flushing TLB entries.
> +This should only be used in case of known issues on the current platform
> +with that instruction. Disabling INVPCID will normally result in a slightly
> +degraded performance.

At the first glance this looks as if it wants to be a cpuid=
sub-option. However, that would disable use by both Xen and
(HVM) guests. Andrew, what are your plans here as to
distinguishing the "Xen uses a feature" from the "disable use of
a feature altogether"?

If we stay with a separate option, then please make this a
normal boolean one (i.e. drop the "no" prefix), as "no-noinvpcid"
is rather ugly.

> @@ -457,7 +472,6 @@ static void generic_set_all(void)
>   set_bit(count, _changes_mask);
>   mask >>= 1;
>   }
> - 
>  }
>  
>  static void generic_set_mtrr(unsigned int reg, unsigned long base,

I don't mind this line being dropped, but in general please avoid
stray changes which aren't assimilated into changes you do anyway.

Jan


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