Re: [PATCH] x86/speculation/l1tf: fix off-by-one error when warning that system has too much RAM

2018-08-28 Thread Christopher Snowhill


On 08/23/2018 07:22 PM, Andre Tomt wrote:
> On 23. aug. 2018 17:44, Andi Kleen wrote:
>> On Thu, Aug 23, 2018 at 03:44:18PM +0200, Vlastimil Babka wrote:
>>> Two users have reported [1] that they have an "extremely unlikely"
>>> system
>>> with more than MAX_PA/2 memory and L1TF mitigation is not effective.
>>> In fact
>>> it's a CPU with 36bits phys limit (64GB) and 32GB memory, but due to
>>> holes
>>> in the e820 map, the main region is almost 500MB over the 32GB limit:
>>
>> Ah I see it's a client part with very large DIMMs and someone being
>> very brave and using that much memory without ECC.
>
> FYI; It is also happening on Xeon E3v2 (Ivy Bridge generation) w/ 32GB
> of ECC RAM here, a low-end server part that officially supports up to
> 32GB.
>

Indeed, I must be "very brave" to not have chucked this CPU and
motherboard and RAM in the bin, and bought a new board, Xeon CPU, and
ECC RAM. Maybe I'll consider that in the future, when I again have
$1000+ to buy new kit. Which will probably be never, at this rate.

>> [    0.00] microcode: microcode updated early to revision 0x20,
>> date = 2018-04-10
>> [    0.029728] L1TF: System has more than MAX_PA/2 memory. L1TF
>> mitigation not effective.
>> [    1.063155] microcode: sig=0x306a9, pf=0x2, revision=0x20
>
>
>> processor    : 7
>> vendor_id    : GenuineIntel
>> cpu family    : 6
>> model    : 58
>> model name    : Intel(R) Xeon(R) CPU E3-1230 V2 @ 3.30GHz
>> stepping    : 9
>> microcode    : 0x20
>> cpu MHz    : 3500.044
>> cache size    : 8192 KB
>> physical id    : 0
>> siblings    : 8
>> core id    : 3
>> cpu cores    : 4
>> apicid    : 7
>> initial apicid    : 7
>> fpu    : yes
>> fpu_exception    : yes
>> cpuid level    : 13
>> wp    : yes
>> flags    : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge
>> mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe
>> syscall nx rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl
>> xtopology nonstop_tsc cpuid aperfmperf pni pclmulqdq dtes64 monitor
>> ds_cpl vmx smx est tm2 ssse3 cx16 xtpr pdcm pcid sse4_1 sse4_2 x2apic
>> popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm
>> cpuid_fault epb pti ssbd ibrs ibpb stibp tpr_shadow vnmi flexpriority
>> ept vpid fsgsbase smep erms xsaveopt dtherm ida arat pln pts flush_l1d
>> bugs    : cpu_meltdown spectre_v1 spectre_v2 spec_store_bypass l1tf
>> bogomips    : 6602.15
>> clflush size    : 64
>> cache_alignment    : 64
>> address sizes    : 36 bits physical, 48 bits virtual
>> power management:
>
>



pEpkey.asc
Description: application/pgp-keys


Re: [PATCH] x86/speculation/l1tf: fix off-by-one error when warning that system has too much RAM

2018-08-28 Thread Christopher Snowhill


On 08/23/2018 07:22 PM, Andre Tomt wrote:
> On 23. aug. 2018 17:44, Andi Kleen wrote:
>> On Thu, Aug 23, 2018 at 03:44:18PM +0200, Vlastimil Babka wrote:
>>> Two users have reported [1] that they have an "extremely unlikely"
>>> system
>>> with more than MAX_PA/2 memory and L1TF mitigation is not effective.
>>> In fact
>>> it's a CPU with 36bits phys limit (64GB) and 32GB memory, but due to
>>> holes
>>> in the e820 map, the main region is almost 500MB over the 32GB limit:
>>
>> Ah I see it's a client part with very large DIMMs and someone being
>> very brave and using that much memory without ECC.
>
> FYI; It is also happening on Xeon E3v2 (Ivy Bridge generation) w/ 32GB
> of ECC RAM here, a low-end server part that officially supports up to
> 32GB.
>

Indeed, I must be "very brave" to not have chucked this CPU and
motherboard and RAM in the bin, and bought a new board, Xeon CPU, and
ECC RAM. Maybe I'll consider that in the future, when I again have
$1000+ to buy new kit. Which will probably be never, at this rate.

>> [    0.00] microcode: microcode updated early to revision 0x20,
>> date = 2018-04-10
>> [    0.029728] L1TF: System has more than MAX_PA/2 memory. L1TF
>> mitigation not effective.
>> [    1.063155] microcode: sig=0x306a9, pf=0x2, revision=0x20
>
>
>> processor    : 7
>> vendor_id    : GenuineIntel
>> cpu family    : 6
>> model    : 58
>> model name    : Intel(R) Xeon(R) CPU E3-1230 V2 @ 3.30GHz
>> stepping    : 9
>> microcode    : 0x20
>> cpu MHz    : 3500.044
>> cache size    : 8192 KB
>> physical id    : 0
>> siblings    : 8
>> core id    : 3
>> cpu cores    : 4
>> apicid    : 7
>> initial apicid    : 7
>> fpu    : yes
>> fpu_exception    : yes
>> cpuid level    : 13
>> wp    : yes
>> flags    : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge
>> mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe
>> syscall nx rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl
>> xtopology nonstop_tsc cpuid aperfmperf pni pclmulqdq dtes64 monitor
>> ds_cpl vmx smx est tm2 ssse3 cx16 xtpr pdcm pcid sse4_1 sse4_2 x2apic
>> popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm
>> cpuid_fault epb pti ssbd ibrs ibpb stibp tpr_shadow vnmi flexpriority
>> ept vpid fsgsbase smep erms xsaveopt dtherm ida arat pln pts flush_l1d
>> bugs    : cpu_meltdown spectre_v1 spectre_v2 spec_store_bypass l1tf
>> bogomips    : 6602.15
>> clflush size    : 64
>> cache_alignment    : 64
>> address sizes    : 36 bits physical, 48 bits virtual
>> power management:
>
>



pEpkey.asc
Description: application/pgp-keys


Re: [PATCH] x86/speculation/l1tf: fix off-by-one error when warning that system has too much RAM

2018-08-24 Thread George Anchev
On Thu, 23 Aug 2018 08:44:37 -0700 Andi Kleen wrote:

> Ah I see it's a client part with very large DIMMs
> and someone being very brave and using that much
> memory without ECC.

It is not about being "brave" but about being
informed. As of 2018 you can probably call "brave"
everyone who uses any modern computer. However this
machine was purchased in 2012. Consider what was known
then and what is known now. The motherboard ASUS
P8Z77-V does not support ECC memory. Still we needed
and paid for 32GB of RAM, not for 31.5.

--
George


Re: [PATCH] x86/speculation/l1tf: fix off-by-one error when warning that system has too much RAM

2018-08-24 Thread George Anchev
On Thu, 23 Aug 2018 08:44:37 -0700 Andi Kleen wrote:

> Ah I see it's a client part with very large DIMMs
> and someone being very brave and using that much
> memory without ECC.

It is not about being "brave" but about being
informed. As of 2018 you can probably call "brave"
everyone who uses any modern computer. However this
machine was purchased in 2012. Consider what was known
then and what is known now. The motherboard ASUS
P8Z77-V does not support ECC memory. Still we needed
and paid for 32GB of RAM, not for 31.5.

--
George


Re: [PATCH] x86/speculation/l1tf: fix off-by-one error when warning that system has too much RAM

2018-08-23 Thread Andi Kleen
On Fri, Aug 24, 2018 at 04:22:57AM +0200, Andre Tomt wrote:
> On 23. aug. 2018 17:44, Andi Kleen wrote:
> > On Thu, Aug 23, 2018 at 03:44:18PM +0200, Vlastimil Babka wrote:
> > > Two users have reported [1] that they have an "extremely unlikely" system
> > > with more than MAX_PA/2 memory and L1TF mitigation is not effective. In 
> > > fact
> > > it's a CPU with 36bits phys limit (64GB) and 32GB memory, but due to holes
> > > in the e820 map, the main region is almost 500MB over the 32GB limit:
> > 
> > Ah I see it's a client part with very large DIMMs and someone being
> > very brave and using that much memory without ECC.
> 
> FYI; It is also happening on Xeon E3v2 (Ivy Bridge generation) w/ 32GB of
> ECC RAM here, a low-end server part that officially supports up to 32GB.

Good point.

-andi


Re: [PATCH] x86/speculation/l1tf: fix off-by-one error when warning that system has too much RAM

2018-08-23 Thread Andi Kleen
On Fri, Aug 24, 2018 at 04:22:57AM +0200, Andre Tomt wrote:
> On 23. aug. 2018 17:44, Andi Kleen wrote:
> > On Thu, Aug 23, 2018 at 03:44:18PM +0200, Vlastimil Babka wrote:
> > > Two users have reported [1] that they have an "extremely unlikely" system
> > > with more than MAX_PA/2 memory and L1TF mitigation is not effective. In 
> > > fact
> > > it's a CPU with 36bits phys limit (64GB) and 32GB memory, but due to holes
> > > in the e820 map, the main region is almost 500MB over the 32GB limit:
> > 
> > Ah I see it's a client part with very large DIMMs and someone being
> > very brave and using that much memory without ECC.
> 
> FYI; It is also happening on Xeon E3v2 (Ivy Bridge generation) w/ 32GB of
> ECC RAM here, a low-end server part that officially supports up to 32GB.

Good point.

-andi


Re: [PATCH] x86/speculation/l1tf: fix off-by-one error when warning that system has too much RAM

2018-08-23 Thread Andre Tomt

On 23. aug. 2018 17:44, Andi Kleen wrote:

On Thu, Aug 23, 2018 at 03:44:18PM +0200, Vlastimil Babka wrote:

Two users have reported [1] that they have an "extremely unlikely" system
with more than MAX_PA/2 memory and L1TF mitigation is not effective. In fact
it's a CPU with 36bits phys limit (64GB) and 32GB memory, but due to holes
in the e820 map, the main region is almost 500MB over the 32GB limit:


Ah I see it's a client part with very large DIMMs and someone being
very brave and using that much memory without ECC.


FYI; It is also happening on Xeon E3v2 (Ivy Bridge generation) w/ 32GB 
of ECC RAM here, a low-end server part that officially supports up to 32GB.



[0.00] microcode: microcode updated early to revision 0x20, date = 
2018-04-10
[0.029728] L1TF: System has more than MAX_PA/2 memory. L1TF mitigation not 
effective.
[1.063155] microcode: sig=0x306a9, pf=0x2, revision=0x20




processor   : 7
vendor_id   : GenuineIntel
cpu family  : 6
model   : 58
model name  : Intel(R) Xeon(R) CPU E3-1230 V2 @ 3.30GHz
stepping: 9
microcode   : 0x20
cpu MHz : 3500.044
cache size  : 8192 KB
physical id : 0
siblings: 8
core id : 3
cpu cores   : 4
apicid  : 7
initial apicid  : 7
fpu : yes
fpu_exception   : yes
cpuid level : 13
wp  : yes
flags   : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov 
pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx rdtscp lm 
constant_tsc arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc cpuid 
aperfmperf pni pclmulqdq dtes64 monitor ds_cpl vmx smx est tm2 ssse3 cx16 xtpr 
pdcm pcid sse4_1 sse4_2 x2apic popcnt tsc_deadline_timer aes xsave avx f16c 
rdrand lahf_lm cpuid_fault epb pti ssbd ibrs ibpb stibp tpr_shadow vnmi 
flexpriority ept vpid fsgsbase smep erms xsaveopt dtherm ida arat pln pts 
flush_l1d
bugs: cpu_meltdown spectre_v1 spectre_v2 spec_store_bypass l1tf
bogomips: 6602.15
clflush size: 64
cache_alignment : 64
address sizes   : 36 bits physical, 48 bits virtual
power management:





Re: [PATCH] x86/speculation/l1tf: fix off-by-one error when warning that system has too much RAM

2018-08-23 Thread Andre Tomt

On 23. aug. 2018 17:44, Andi Kleen wrote:

On Thu, Aug 23, 2018 at 03:44:18PM +0200, Vlastimil Babka wrote:

Two users have reported [1] that they have an "extremely unlikely" system
with more than MAX_PA/2 memory and L1TF mitigation is not effective. In fact
it's a CPU with 36bits phys limit (64GB) and 32GB memory, but due to holes
in the e820 map, the main region is almost 500MB over the 32GB limit:


Ah I see it's a client part with very large DIMMs and someone being
very brave and using that much memory without ECC.


FYI; It is also happening on Xeon E3v2 (Ivy Bridge generation) w/ 32GB 
of ECC RAM here, a low-end server part that officially supports up to 32GB.



[0.00] microcode: microcode updated early to revision 0x20, date = 
2018-04-10
[0.029728] L1TF: System has more than MAX_PA/2 memory. L1TF mitigation not 
effective.
[1.063155] microcode: sig=0x306a9, pf=0x2, revision=0x20




processor   : 7
vendor_id   : GenuineIntel
cpu family  : 6
model   : 58
model name  : Intel(R) Xeon(R) CPU E3-1230 V2 @ 3.30GHz
stepping: 9
microcode   : 0x20
cpu MHz : 3500.044
cache size  : 8192 KB
physical id : 0
siblings: 8
core id : 3
cpu cores   : 4
apicid  : 7
initial apicid  : 7
fpu : yes
fpu_exception   : yes
cpuid level : 13
wp  : yes
flags   : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov 
pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx rdtscp lm 
constant_tsc arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc cpuid 
aperfmperf pni pclmulqdq dtes64 monitor ds_cpl vmx smx est tm2 ssse3 cx16 xtpr 
pdcm pcid sse4_1 sse4_2 x2apic popcnt tsc_deadline_timer aes xsave avx f16c 
rdrand lahf_lm cpuid_fault epb pti ssbd ibrs ibpb stibp tpr_shadow vnmi 
flexpriority ept vpid fsgsbase smep erms xsaveopt dtherm ida arat pln pts 
flush_l1d
bugs: cpu_meltdown spectre_v1 spectre_v2 spec_store_bypass l1tf
bogomips: 6602.15
clflush size: 64
cache_alignment : 64
address sizes   : 36 bits physical, 48 bits virtual
power management:





Re: [PATCH] x86/speculation/l1tf: fix off-by-one error when warning that system has too much RAM

2018-08-23 Thread Vlastimil Babka
On 8/23/18 5:44 PM, Andi Kleen wrote:
> On Thu, Aug 23, 2018 at 03:44:18PM +0200, Vlastimil Babka wrote:
>> Two users have reported [1] that they have an "extremely unlikely" system
>> with more than MAX_PA/2 memory and L1TF mitigation is not effective. In fact
>> it's a CPU with 36bits phys limit (64GB) and 32GB memory, but due to holes
>> in the e820 map, the main region is almost 500MB over the 32GB limit:
> 
> Ah I see it's a client part with very large DIMMs and someone being
> very brave and using that much memory without ECC.

Are you trying to mock the users and diminsh their report because of that?

>>
>> [0.00] BIOS-e820: [mem 0x0001-0x00081eff] usable
>>
>> Suggestions to use 'mem=32G' to prefer L1TF mitigation while losing the 500MB
>> revealed, that there's an off-by-one error in the check in
>> l1tf_select_mitigation(). l1tf_pfn_limit() returns the last usable pfn
>> (inclusive), but it's more common and hopefully less error-prone to return 
>> the
>> first pfn that's over limit, so this patch changes that and updates the other
>> callers.
> 
> I can see the off by one, but does it really cause the user's problem?
> 
> They will be still over the limit in any case, with or without off-by-one.
> 
> So the description has nothing to do with the fix. Or do I miss something?

The off-by-one happens when 'mem=32G' is used to limit the amount
memory. It's easier to do "mem=32G" with fixed code than "mem=33554428k"
to workaround the error.

> 
> -Andi
> 



Re: [PATCH] x86/speculation/l1tf: fix off-by-one error when warning that system has too much RAM

2018-08-23 Thread Vlastimil Babka
On 8/23/18 5:44 PM, Andi Kleen wrote:
> On Thu, Aug 23, 2018 at 03:44:18PM +0200, Vlastimil Babka wrote:
>> Two users have reported [1] that they have an "extremely unlikely" system
>> with more than MAX_PA/2 memory and L1TF mitigation is not effective. In fact
>> it's a CPU with 36bits phys limit (64GB) and 32GB memory, but due to holes
>> in the e820 map, the main region is almost 500MB over the 32GB limit:
> 
> Ah I see it's a client part with very large DIMMs and someone being
> very brave and using that much memory without ECC.

Are you trying to mock the users and diminsh their report because of that?

>>
>> [0.00] BIOS-e820: [mem 0x0001-0x00081eff] usable
>>
>> Suggestions to use 'mem=32G' to prefer L1TF mitigation while losing the 500MB
>> revealed, that there's an off-by-one error in the check in
>> l1tf_select_mitigation(). l1tf_pfn_limit() returns the last usable pfn
>> (inclusive), but it's more common and hopefully less error-prone to return 
>> the
>> first pfn that's over limit, so this patch changes that and updates the other
>> callers.
> 
> I can see the off by one, but does it really cause the user's problem?
> 
> They will be still over the limit in any case, with or without off-by-one.
> 
> So the description has nothing to do with the fix. Or do I miss something?

The off-by-one happens when 'mem=32G' is used to limit the amount
memory. It's easier to do "mem=32G" with fixed code than "mem=33554428k"
to workaround the error.

> 
> -Andi
> 



Re: [PATCH] x86/speculation/l1tf: fix off-by-one error when warning that system has too much RAM

2018-08-23 Thread Andi Kleen
On Thu, Aug 23, 2018 at 03:44:18PM +0200, Vlastimil Babka wrote:
> Two users have reported [1] that they have an "extremely unlikely" system
> with more than MAX_PA/2 memory and L1TF mitigation is not effective. In fact
> it's a CPU with 36bits phys limit (64GB) and 32GB memory, but due to holes
> in the e820 map, the main region is almost 500MB over the 32GB limit:

Ah I see it's a client part with very large DIMMs and someone being
very brave and using that much memory without ECC.

> 
> [0.00] BIOS-e820: [mem 0x0001-0x00081eff] usable
> 
> Suggestions to use 'mem=32G' to prefer L1TF mitigation while losing the 500MB
> revealed, that there's an off-by-one error in the check in
> l1tf_select_mitigation(). l1tf_pfn_limit() returns the last usable pfn
> (inclusive), but it's more common and hopefully less error-prone to return the
> first pfn that's over limit, so this patch changes that and updates the other
> callers.

I can see the off by one, but does it really cause the user's problem?

They will be still over the limit in any case, with or without off-by-one.

So the description has nothing to do with the fix. Or do I miss something?

-Andi



Re: [PATCH] x86/speculation/l1tf: fix off-by-one error when warning that system has too much RAM

2018-08-23 Thread Andi Kleen
On Thu, Aug 23, 2018 at 03:44:18PM +0200, Vlastimil Babka wrote:
> Two users have reported [1] that they have an "extremely unlikely" system
> with more than MAX_PA/2 memory and L1TF mitigation is not effective. In fact
> it's a CPU with 36bits phys limit (64GB) and 32GB memory, but due to holes
> in the e820 map, the main region is almost 500MB over the 32GB limit:

Ah I see it's a client part with very large DIMMs and someone being
very brave and using that much memory without ECC.

> 
> [0.00] BIOS-e820: [mem 0x0001-0x00081eff] usable
> 
> Suggestions to use 'mem=32G' to prefer L1TF mitigation while losing the 500MB
> revealed, that there's an off-by-one error in the check in
> l1tf_select_mitigation(). l1tf_pfn_limit() returns the last usable pfn
> (inclusive), but it's more common and hopefully less error-prone to return the
> first pfn that's over limit, so this patch changes that and updates the other
> callers.

I can see the off by one, but does it really cause the user's problem?

They will be still over the limit in any case, with or without off-by-one.

So the description has nothing to do with the fix. Or do I miss something?

-Andi



Re: [PATCH] x86/speculation/l1tf: fix off-by-one error when warning that system has too much RAM

2018-08-23 Thread Michal Hocko
On Thu 23-08-18 15:44:18, Vlastimil Babka wrote:
> Two users have reported [1] that they have an "extremely unlikely" system
> with more than MAX_PA/2 memory and L1TF mitigation is not effective. In fact
> it's a CPU with 36bits phys limit (64GB) and 32GB memory, but due to holes
> in the e820 map, the main region is almost 500MB over the 32GB limit:
> 
> [0.00] BIOS-e820: [mem 0x0001-0x00081eff] usable
> 
> Suggestions to use 'mem=32G' to prefer L1TF mitigation while losing the 500MB
> revealed, that there's an off-by-one error in the check in
> l1tf_select_mitigation(). l1tf_pfn_limit() returns the last usable pfn
> (inclusive), but it's more common and hopefully less error-prone to return the
> first pfn that's over limit, so this patch changes that and updates the other
> callers.
> 
> [1] https://bugzilla.suse.com/show_bug.cgi?id=1105536
> 
> Reported-by: George Anchev 
> Reported-by: Christopher Snowhill 
> Fixes: 17dbca119312 ("x86/speculation/l1tf: Add sysfs reporting for l1tf")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Vlastimil Babka 

Yes this should be less error prone.
Acked-by: Michal Hocko 

> ---
>  arch/x86/include/asm/processor.h | 2 +-
>  arch/x86/mm/init.c   | 2 +-
>  arch/x86/mm/mmap.c   | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/include/asm/processor.h 
> b/arch/x86/include/asm/processor.h
> index a0a52274cb4a..c24297268ebc 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -183,7 +183,7 @@ extern void cpu_detect(struct cpuinfo_x86 *c);
>  
>  static inline unsigned long long l1tf_pfn_limit(void)
>  {
> - return BIT_ULL(boot_cpu_data.x86_phys_bits - 1 - PAGE_SHIFT) - 1;
> + return BIT_ULL(boot_cpu_data.x86_phys_bits - 1 - PAGE_SHIFT);
>  }
>  
>  extern void early_cpu_init(void);
> diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
> index 02de3d6065c4..63a6f9fcaf20 100644
> --- a/arch/x86/mm/init.c
> +++ b/arch/x86/mm/init.c
> @@ -923,7 +923,7 @@ unsigned long max_swapfile_size(void)
>  
>   if (boot_cpu_has_bug(X86_BUG_L1TF)) {
>   /* Limit the swap file size to MAX_PA/2 for L1TF workaround */
> - unsigned long long l1tf_limit = l1tf_pfn_limit() + 1;
> + unsigned long long l1tf_limit = l1tf_pfn_limit();
>   /*
>* We encode swap offsets also with 3 bits below those for pfn
>* which makes the usable limit higher.
> diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
> index f40ab8185d94..1e95d57760cf 100644
> --- a/arch/x86/mm/mmap.c
> +++ b/arch/x86/mm/mmap.c
> @@ -257,7 +257,7 @@ bool pfn_modify_allowed(unsigned long pfn, pgprot_t prot)
>   /* If it's real memory always allow */
>   if (pfn_valid(pfn))
>   return true;
> - if (pfn > l1tf_pfn_limit() && !capable(CAP_SYS_ADMIN))
> + if (pfn >= l1tf_pfn_limit() && !capable(CAP_SYS_ADMIN))
>   return false;
>   return true;
>  }
> -- 
> 2.18.0

-- 
Michal Hocko
SUSE Labs


Re: [PATCH] x86/speculation/l1tf: fix off-by-one error when warning that system has too much RAM

2018-08-23 Thread Michal Hocko
On Thu 23-08-18 15:44:18, Vlastimil Babka wrote:
> Two users have reported [1] that they have an "extremely unlikely" system
> with more than MAX_PA/2 memory and L1TF mitigation is not effective. In fact
> it's a CPU with 36bits phys limit (64GB) and 32GB memory, but due to holes
> in the e820 map, the main region is almost 500MB over the 32GB limit:
> 
> [0.00] BIOS-e820: [mem 0x0001-0x00081eff] usable
> 
> Suggestions to use 'mem=32G' to prefer L1TF mitigation while losing the 500MB
> revealed, that there's an off-by-one error in the check in
> l1tf_select_mitigation(). l1tf_pfn_limit() returns the last usable pfn
> (inclusive), but it's more common and hopefully less error-prone to return the
> first pfn that's over limit, so this patch changes that and updates the other
> callers.
> 
> [1] https://bugzilla.suse.com/show_bug.cgi?id=1105536
> 
> Reported-by: George Anchev 
> Reported-by: Christopher Snowhill 
> Fixes: 17dbca119312 ("x86/speculation/l1tf: Add sysfs reporting for l1tf")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Vlastimil Babka 

Yes this should be less error prone.
Acked-by: Michal Hocko 

> ---
>  arch/x86/include/asm/processor.h | 2 +-
>  arch/x86/mm/init.c   | 2 +-
>  arch/x86/mm/mmap.c   | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/include/asm/processor.h 
> b/arch/x86/include/asm/processor.h
> index a0a52274cb4a..c24297268ebc 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -183,7 +183,7 @@ extern void cpu_detect(struct cpuinfo_x86 *c);
>  
>  static inline unsigned long long l1tf_pfn_limit(void)
>  {
> - return BIT_ULL(boot_cpu_data.x86_phys_bits - 1 - PAGE_SHIFT) - 1;
> + return BIT_ULL(boot_cpu_data.x86_phys_bits - 1 - PAGE_SHIFT);
>  }
>  
>  extern void early_cpu_init(void);
> diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
> index 02de3d6065c4..63a6f9fcaf20 100644
> --- a/arch/x86/mm/init.c
> +++ b/arch/x86/mm/init.c
> @@ -923,7 +923,7 @@ unsigned long max_swapfile_size(void)
>  
>   if (boot_cpu_has_bug(X86_BUG_L1TF)) {
>   /* Limit the swap file size to MAX_PA/2 for L1TF workaround */
> - unsigned long long l1tf_limit = l1tf_pfn_limit() + 1;
> + unsigned long long l1tf_limit = l1tf_pfn_limit();
>   /*
>* We encode swap offsets also with 3 bits below those for pfn
>* which makes the usable limit higher.
> diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
> index f40ab8185d94..1e95d57760cf 100644
> --- a/arch/x86/mm/mmap.c
> +++ b/arch/x86/mm/mmap.c
> @@ -257,7 +257,7 @@ bool pfn_modify_allowed(unsigned long pfn, pgprot_t prot)
>   /* If it's real memory always allow */
>   if (pfn_valid(pfn))
>   return true;
> - if (pfn > l1tf_pfn_limit() && !capable(CAP_SYS_ADMIN))
> + if (pfn >= l1tf_pfn_limit() && !capable(CAP_SYS_ADMIN))
>   return false;
>   return true;
>  }
> -- 
> 2.18.0

-- 
Michal Hocko
SUSE Labs


[PATCH] x86/speculation/l1tf: fix off-by-one error when warning that system has too much RAM

2018-08-23 Thread Vlastimil Babka
Two users have reported [1] that they have an "extremely unlikely" system
with more than MAX_PA/2 memory and L1TF mitigation is not effective. In fact
it's a CPU with 36bits phys limit (64GB) and 32GB memory, but due to holes
in the e820 map, the main region is almost 500MB over the 32GB limit:

[0.00] BIOS-e820: [mem 0x0001-0x00081eff] usable

Suggestions to use 'mem=32G' to prefer L1TF mitigation while losing the 500MB
revealed, that there's an off-by-one error in the check in
l1tf_select_mitigation(). l1tf_pfn_limit() returns the last usable pfn
(inclusive), but it's more common and hopefully less error-prone to return the
first pfn that's over limit, so this patch changes that and updates the other
callers.

[1] https://bugzilla.suse.com/show_bug.cgi?id=1105536

Reported-by: George Anchev 
Reported-by: Christopher Snowhill 
Fixes: 17dbca119312 ("x86/speculation/l1tf: Add sysfs reporting for l1tf")
Cc: sta...@vger.kernel.org
Signed-off-by: Vlastimil Babka 
---
 arch/x86/include/asm/processor.h | 2 +-
 arch/x86/mm/init.c   | 2 +-
 arch/x86/mm/mmap.c   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index a0a52274cb4a..c24297268ebc 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -183,7 +183,7 @@ extern void cpu_detect(struct cpuinfo_x86 *c);
 
 static inline unsigned long long l1tf_pfn_limit(void)
 {
-   return BIT_ULL(boot_cpu_data.x86_phys_bits - 1 - PAGE_SHIFT) - 1;
+   return BIT_ULL(boot_cpu_data.x86_phys_bits - 1 - PAGE_SHIFT);
 }
 
 extern void early_cpu_init(void);
diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index 02de3d6065c4..63a6f9fcaf20 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -923,7 +923,7 @@ unsigned long max_swapfile_size(void)
 
if (boot_cpu_has_bug(X86_BUG_L1TF)) {
/* Limit the swap file size to MAX_PA/2 for L1TF workaround */
-   unsigned long long l1tf_limit = l1tf_pfn_limit() + 1;
+   unsigned long long l1tf_limit = l1tf_pfn_limit();
/*
 * We encode swap offsets also with 3 bits below those for pfn
 * which makes the usable limit higher.
diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
index f40ab8185d94..1e95d57760cf 100644
--- a/arch/x86/mm/mmap.c
+++ b/arch/x86/mm/mmap.c
@@ -257,7 +257,7 @@ bool pfn_modify_allowed(unsigned long pfn, pgprot_t prot)
/* If it's real memory always allow */
if (pfn_valid(pfn))
return true;
-   if (pfn > l1tf_pfn_limit() && !capable(CAP_SYS_ADMIN))
+   if (pfn >= l1tf_pfn_limit() && !capable(CAP_SYS_ADMIN))
return false;
return true;
 }
-- 
2.18.0



[PATCH] x86/speculation/l1tf: fix off-by-one error when warning that system has too much RAM

2018-08-23 Thread Vlastimil Babka
Two users have reported [1] that they have an "extremely unlikely" system
with more than MAX_PA/2 memory and L1TF mitigation is not effective. In fact
it's a CPU with 36bits phys limit (64GB) and 32GB memory, but due to holes
in the e820 map, the main region is almost 500MB over the 32GB limit:

[0.00] BIOS-e820: [mem 0x0001-0x00081eff] usable

Suggestions to use 'mem=32G' to prefer L1TF mitigation while losing the 500MB
revealed, that there's an off-by-one error in the check in
l1tf_select_mitigation(). l1tf_pfn_limit() returns the last usable pfn
(inclusive), but it's more common and hopefully less error-prone to return the
first pfn that's over limit, so this patch changes that and updates the other
callers.

[1] https://bugzilla.suse.com/show_bug.cgi?id=1105536

Reported-by: George Anchev 
Reported-by: Christopher Snowhill 
Fixes: 17dbca119312 ("x86/speculation/l1tf: Add sysfs reporting for l1tf")
Cc: sta...@vger.kernel.org
Signed-off-by: Vlastimil Babka 
---
 arch/x86/include/asm/processor.h | 2 +-
 arch/x86/mm/init.c   | 2 +-
 arch/x86/mm/mmap.c   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index a0a52274cb4a..c24297268ebc 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -183,7 +183,7 @@ extern void cpu_detect(struct cpuinfo_x86 *c);
 
 static inline unsigned long long l1tf_pfn_limit(void)
 {
-   return BIT_ULL(boot_cpu_data.x86_phys_bits - 1 - PAGE_SHIFT) - 1;
+   return BIT_ULL(boot_cpu_data.x86_phys_bits - 1 - PAGE_SHIFT);
 }
 
 extern void early_cpu_init(void);
diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index 02de3d6065c4..63a6f9fcaf20 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -923,7 +923,7 @@ unsigned long max_swapfile_size(void)
 
if (boot_cpu_has_bug(X86_BUG_L1TF)) {
/* Limit the swap file size to MAX_PA/2 for L1TF workaround */
-   unsigned long long l1tf_limit = l1tf_pfn_limit() + 1;
+   unsigned long long l1tf_limit = l1tf_pfn_limit();
/*
 * We encode swap offsets also with 3 bits below those for pfn
 * which makes the usable limit higher.
diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
index f40ab8185d94..1e95d57760cf 100644
--- a/arch/x86/mm/mmap.c
+++ b/arch/x86/mm/mmap.c
@@ -257,7 +257,7 @@ bool pfn_modify_allowed(unsigned long pfn, pgprot_t prot)
/* If it's real memory always allow */
if (pfn_valid(pfn))
return true;
-   if (pfn > l1tf_pfn_limit() && !capable(CAP_SYS_ADMIN))
+   if (pfn >= l1tf_pfn_limit() && !capable(CAP_SYS_ADMIN))
return false;
return true;
 }
-- 
2.18.0