2015-07-20 08:06:29

by Jan Beulich

[permalink] [raw]
Subject: [PATCH] x86: adjust default caching mode translation tables

Make WT really mean WT (rather than UC).

I can't see why commit 9cd25aac1f ("x86/mm/pat: Emulate PAT when it is
disabled") didn't make this match its changes to pat_init().

Signed-off-by: Jan Beulich <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Toshi Kani <[email protected]>
---
arch/x86/mm/init.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

--- 4.2-rc3/arch/x86/mm/init.c
+++ 4.2-rc3-x86-default-cache-mode/arch/x86/mm/init.c
@@ -43,18 +43,18 @@ uint16_t __cachemode2pte_tbl[_PAGE_CACHE
[_PAGE_CACHE_MODE_WC ] = 0 | _PAGE_PCD,
[_PAGE_CACHE_MODE_UC_MINUS] = 0 | _PAGE_PCD,
[_PAGE_CACHE_MODE_UC ] = _PAGE_PWT | _PAGE_PCD,
- [_PAGE_CACHE_MODE_WT ] = 0 | _PAGE_PCD,
+ [_PAGE_CACHE_MODE_WT ] = _PAGE_PWT | 0,
[_PAGE_CACHE_MODE_WP ] = 0 | _PAGE_PCD,
};
EXPORT_SYMBOL(__cachemode2pte_tbl);

uint8_t __pte2cachemode_tbl[8] = {
[__pte2cm_idx( 0 | 0 | 0 )] = _PAGE_CACHE_MODE_WB,
- [__pte2cm_idx(_PAGE_PWT | 0 | 0 )] = _PAGE_CACHE_MODE_UC_MINUS,
+ [__pte2cm_idx(_PAGE_PWT | 0 | 0 )] = _PAGE_CACHE_MODE_WT,
[__pte2cm_idx( 0 | _PAGE_PCD | 0 )] = _PAGE_CACHE_MODE_UC_MINUS,
[__pte2cm_idx(_PAGE_PWT | _PAGE_PCD | 0 )] = _PAGE_CACHE_MODE_UC,
[__pte2cm_idx( 0 | 0 | _PAGE_PAT)] = _PAGE_CACHE_MODE_WB,
- [__pte2cm_idx(_PAGE_PWT | 0 | _PAGE_PAT)] = _PAGE_CACHE_MODE_UC_MINUS,
+ [__pte2cm_idx(_PAGE_PWT | 0 | _PAGE_PAT)] = _PAGE_CACHE_MODE_WT,
[__pte2cm_idx(0 | _PAGE_PCD | _PAGE_PAT)] = _PAGE_CACHE_MODE_UC_MINUS,
[__pte2cm_idx(_PAGE_PWT | _PAGE_PCD | _PAGE_PAT)] = _PAGE_CACHE_MODE_UC,
};



Subject: [tip:x86/urgent] x86/mm/pat: Adjust default caching mode translation tables

Commit-ID: ca1fec58bc6a90be96a59b4769e951156846c6ca
Gitweb: http://git.kernel.org/tip/ca1fec58bc6a90be96a59b4769e951156846c6ca
Author: Jan Beulich <[email protected]>
AuthorDate: Mon, 20 Jul 2015 08:46:14 +0100
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 21 Jul 2015 08:23:06 +0200

x86/mm/pat: Adjust default caching mode translation tables

Make WT really mean WT (rather than UC).

I can't see why commit 9cd25aac1f ("x86/mm/pat: Emulate PAT when
it is disabled") didn't make this to match its changes to
pat_init().

Signed-off-by: Jan Beulich <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Toshi Kani <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/mm/init.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index 8533b46..7a45322 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -43,18 +43,18 @@ uint16_t __cachemode2pte_tbl[_PAGE_CACHE_MODE_NUM] = {
[_PAGE_CACHE_MODE_WC ] = 0 | _PAGE_PCD,
[_PAGE_CACHE_MODE_UC_MINUS] = 0 | _PAGE_PCD,
[_PAGE_CACHE_MODE_UC ] = _PAGE_PWT | _PAGE_PCD,
- [_PAGE_CACHE_MODE_WT ] = 0 | _PAGE_PCD,
+ [_PAGE_CACHE_MODE_WT ] = _PAGE_PWT | 0,
[_PAGE_CACHE_MODE_WP ] = 0 | _PAGE_PCD,
};
EXPORT_SYMBOL(__cachemode2pte_tbl);

uint8_t __pte2cachemode_tbl[8] = {
[__pte2cm_idx( 0 | 0 | 0 )] = _PAGE_CACHE_MODE_WB,
- [__pte2cm_idx(_PAGE_PWT | 0 | 0 )] = _PAGE_CACHE_MODE_UC_MINUS,
+ [__pte2cm_idx(_PAGE_PWT | 0 | 0 )] = _PAGE_CACHE_MODE_WT,
[__pte2cm_idx( 0 | _PAGE_PCD | 0 )] = _PAGE_CACHE_MODE_UC_MINUS,
[__pte2cm_idx(_PAGE_PWT | _PAGE_PCD | 0 )] = _PAGE_CACHE_MODE_UC,
[__pte2cm_idx( 0 | 0 | _PAGE_PAT)] = _PAGE_CACHE_MODE_WB,
- [__pte2cm_idx(_PAGE_PWT | 0 | _PAGE_PAT)] = _PAGE_CACHE_MODE_UC_MINUS,
+ [__pte2cm_idx(_PAGE_PWT | 0 | _PAGE_PAT)] = _PAGE_CACHE_MODE_WT,
[__pte2cm_idx(0 | _PAGE_PCD | _PAGE_PAT)] = _PAGE_CACHE_MODE_UC_MINUS,
[__pte2cm_idx(_PAGE_PWT | _PAGE_PCD | _PAGE_PAT)] = _PAGE_CACHE_MODE_UC,
};

2015-07-21 22:30:14

by Toshi Kani

[permalink] [raw]
Subject: Re: [PATCH] x86: adjust default caching mode translation tables

On Mon, 2015-07-20 at 08:46 +0100, Jan Beulich wrote:
> Make WT really mean WT (rather than UC).
>
> I can't see why commit 9cd25aac1f ("x86/mm/pat: Emulate PAT when it is
> disabled") didn't make this match its changes to pat_init().

No, the default values need to be set to the fallback types, i.e. minimal
supported mode. For WC and WT, UC is the fallback type.

When PAT is disabled, pat_init() does update the tables below to enable WT
per the default BIOS setup. However, when PAT is enabled, but CPU has PAT
-errata, WT falls back to UC per the default values.

Thanks,
-Toshi

>
> Signed-off-by: Jan Beulich <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: Toshi Kani <[email protected]>
> ---
> arch/x86/mm/init.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> --- 4.2-rc3/arch/x86/mm/init.c
> +++ 4.2-rc3-x86-default-cache-mode/arch/x86/mm/init.c
> @@ -43,18 +43,18 @@ uint16_t __cachemode2pte_tbl[_PAGE_CACHE
> [_PAGE_CACHE_MODE_WC ] = 0 | _PAGE_PCD,
> [_PAGE_CACHE_MODE_UC_MINUS] = 0 | _PAGE_PCD,
> [_PAGE_CACHE_MODE_UC ] = _PAGE_PWT | _PAGE_PCD,
> - [_PAGE_CACHE_MODE_WT ] = 0 | _PAGE_PCD,
> + [_PAGE_CACHE_MODE_WT ] = _PAGE_PWT | 0,
> [_PAGE_CACHE_MODE_WP ] = 0 | _PAGE_PCD,
> };
> EXPORT_SYMBOL(__cachemode2pte_tbl);
>
> uint8_t __pte2cachemode_tbl[8] = {
> [__pte2cm_idx( 0 | 0 | 0 )] =
> _PAGE_CACHE_MODE_WB,
> - [__pte2cm_idx(_PAGE_PWT | 0 | 0 )] =
> _PAGE_CACHE_MODE_UC_MINUS,
> + [__pte2cm_idx(_PAGE_PWT | 0 | 0 )] =
> _PAGE_CACHE_MODE_WT,
> [__pte2cm_idx( 0 | _PAGE_PCD | 0 )] =
> _PAGE_CACHE_MODE_UC_MINUS,
> [__pte2cm_idx(_PAGE_PWT | _PAGE_PCD | 0 )] =
> _PAGE_CACHE_MODE_UC,
> [__pte2cm_idx( 0 | 0 | _PAGE_PAT)] =
> _PAGE_CACHE_MODE_WB,
> - [__pte2cm_idx(_PAGE_PWT | 0 | _PAGE_PAT)] =
> _PAGE_CACHE_MODE_UC_MINUS,
> + [__pte2cm_idx(_PAGE_PWT | 0 | _PAGE_PAT)] =
> _PAGE_CACHE_MODE_WT,
> [__pte2cm_idx(0 | _PAGE_PCD | _PAGE_PAT)] =
> _PAGE_CACHE_MODE_UC_MINUS,
> [__pte2cm_idx(_PAGE_PWT | _PAGE_PCD | _PAGE_PAT)] =
> _PAGE_CACHE_MODE_UC,
> };
>
>
>

2015-07-22 15:10:50

by Toshi Kani

[permalink] [raw]
Subject: Re: [tip:x86/urgent] x86/mm/pat: Adjust default caching mode translation tables

Ingo,

Please drop this change as I explained below.
http://marc.info/?l=linux-kernel&m=143751782107284&w=2

Thanks,
-Toshi


On Tue, 2015-07-21 at 02:38 -0700, tip-bot for Jan Beulich wrote:
> Commit-ID: ca1fec58bc6a90be96a59b4769e951156846c6ca
> Gitweb:
> http://git.kernel.org/tip/ca1fec58bc6a90be96a59b4769e951156846c6ca
> Author: Jan Beulich <[email protected]>
> AuthorDate: Mon, 20 Jul 2015 08:46:14 +0100
> Committer: Ingo Molnar <[email protected]>
> CommitDate: Tue, 21 Jul 2015 08:23:06 +0200
>
> x86/mm/pat: Adjust default caching mode translation tables
>
> Make WT really mean WT (rather than UC).
>
> I can't see why commit 9cd25aac1f ("x86/mm/pat: Emulate PAT when
> it is disabled") didn't make this to match its changes to
> pat_init().
>
> Signed-off-by: Jan Beulich <[email protected]>
> Cc: Andy Lutomirski <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: Linus Torvalds <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Toshi Kani <[email protected]>
> Link:
> http://lkml.kernel.org/r/[email protected]
> Signed-off-by: Ingo Molnar <[email protected]>
> ---
> arch/x86/mm/init.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
> index 8533b46..7a45322 100644
> --- a/arch/x86/mm/init.c
> +++ b/arch/x86/mm/init.c
> @@ -43,18 +43,18 @@ uint16_t __cachemode2pte_tbl[_PAGE_CACHE_MODE_NUM] =
> {
> [_PAGE_CACHE_MODE_WC ] = 0 | _PAGE_PCD,
> [_PAGE_CACHE_MODE_UC_MINUS] = 0 | _PAGE_PCD,
> [_PAGE_CACHE_MODE_UC ] = _PAGE_PWT | _PAGE_PCD,
> - [_PAGE_CACHE_MODE_WT ] = 0 | _PAGE_PCD,
> + [_PAGE_CACHE_MODE_WT ] = _PAGE_PWT | 0,
> [_PAGE_CACHE_MODE_WP ] = 0 | _PAGE_PCD,
> };
> EXPORT_SYMBOL(__cachemode2pte_tbl);
>
> uint8_t __pte2cachemode_tbl[8] = {
> [__pte2cm_idx( 0 | 0 | 0 )] =
> _PAGE_CACHE_MODE_WB,
> - [__pte2cm_idx(_PAGE_PWT | 0 | 0 )] =
> _PAGE_CACHE_MODE_UC_MINUS,
> + [__pte2cm_idx(_PAGE_PWT | 0 | 0 )] =
> _PAGE_CACHE_MODE_WT,
> [__pte2cm_idx( 0 | _PAGE_PCD | 0 )] =
> _PAGE_CACHE_MODE_UC_MINUS,
> [__pte2cm_idx(_PAGE_PWT | _PAGE_PCD | 0 )] =
> _PAGE_CACHE_MODE_UC,
> [__pte2cm_idx( 0 | 0 | _PAGE_PAT)] =
> _PAGE_CACHE_MODE_WB,
> - [__pte2cm_idx(_PAGE_PWT | 0 | _PAGE_PAT)] =
> _PAGE_CACHE_MODE_UC_MINUS,
> + [__pte2cm_idx(_PAGE_PWT | 0 | _PAGE_PAT)] =
> _PAGE_CACHE_MODE_WT,
> [__pte2cm_idx(0 | _PAGE_PCD | _PAGE_PAT)] =
> _PAGE_CACHE_MODE_UC_MINUS,
> [__pte2cm_idx(_PAGE_PWT | _PAGE_PCD | _PAGE_PAT)] =
> _PAGE_CACHE_MODE_UC,
> };

2015-07-22 15:37:35

by Jan Beulich

[permalink] [raw]
Subject: Re: [PATCH] x86: adjust default caching mode translation tables

>>> On 22.07.15 at 00:29, <[email protected]> wrote:
> On Mon, 2015-07-20 at 08:46 +0100, Jan Beulich wrote:
>> Make WT really mean WT (rather than UC).
>>
>> I can't see why commit 9cd25aac1f ("x86/mm/pat: Emulate PAT when it is
>> disabled") didn't make this match its changes to pat_init().
>
> No, the default values need to be set to the fallback types, i.e. minimal
> supported mode. For WC and WT, UC is the fallback type.

But why would that be?

> When PAT is disabled, pat_init() does update the tables below to enable WT
> per the default BIOS setup. However, when PAT is enabled, but CPU has PAT
> -errata, WT falls back to UC per the default values.

PAT related errata I'm aware of are related to either page size or
the number of bits used to index into the PAT MSR, but never to
a particular memory type. Are you saying there are errata which
make use of WT or WC impossible altogether? Otherwise I would
have thought (even more so in the absence of any comment
saying otherwise - "minimal supported modes" doesn't really say
on what basis the set is the minimal one) that the mode systems
come up in (compatible with pre-PAT) ought to be what the tables
express.

Jan

2015-07-22 15:24:45

by Toshi Kani

[permalink] [raw]
Subject: Re: [PATCH] x86: adjust default caching mode translation tables

On Wed, 2015-07-22 at 09:17 -0600, Jan Beulich wrote:
> >
> > > > On 22.07.15 at 00:29, <[email protected]> wrote:
> > On Mon, 2015-07-20 at 08:46 +0100, Jan Beulich wrote:
> > > Make WT really mean WT (rather than UC).
> > >
> > > I can't see why commit 9cd25aac1f ("x86/mm/pat: Emulate PAT when it
> > > is
> > > disabled") didn't make this match its changes to pat_init().
> >
> > No, the default values need to be set to the fallback types, i.e.
> > minimal
> > supported mode. For WC and WT, UC is the fallback type.
>
> But why would that be?
>
> > When PAT is disabled, pat_init() does update the tables below to enable
> > WT
> > per the default BIOS setup. However, when PAT is enabled, but CPU has
> > PAT
> > -errata, WT falls back to UC per the default values.
>
> PAT related errata I'm aware of are related to either page size or
> the number of bits used to index into the PAT MSR, but never to
> a particular memory type. Are you saying there are errata which
> make use of WT or WC impossible altogether? Otherwise I would
> have thought (even more so in the absence of any comment
> saying otherwise - "minimal supported modes" doesn't really say
> on what basis the set is the minimal one) that the mode systems
> come up in (compatible with pre-PAT) ought to be what the tables
> express.

Please take a look at the comments in pat_init(). WT uses slot 7 (not slot
1) in the regular case. We need to use the PAT bit in order to support
both WC and WT. The PAT errata causes the PAT bit ineffective.

Thanks,
-Toshi

2015-07-22 15:41:35

by Jan Beulich

[permalink] [raw]
Subject: Re: [PATCH] x86: adjust default caching mode translation tables

>>> On 22.07.15 at 17:23, <[email protected]> wrote:
> On Wed, 2015-07-22 at 09:17 -0600, Jan Beulich wrote:
>> >
>> > > > On 22.07.15 at 00:29, <[email protected]> wrote:
>> > On Mon, 2015-07-20 at 08:46 +0100, Jan Beulich wrote:
>> > > Make WT really mean WT (rather than UC).
>> > >
>> > > I can't see why commit 9cd25aac1f ("x86/mm/pat: Emulate PAT when it
>> > > is
>> > > disabled") didn't make this match its changes to pat_init().
>> >
>> > No, the default values need to be set to the fallback types, i.e.
>> > minimal
>> > supported mode. For WC and WT, UC is the fallback type.
>>
>> But why would that be?
>>
>> > When PAT is disabled, pat_init() does update the tables below to enable
>> > WT
>> > per the default BIOS setup. However, when PAT is enabled, but CPU has
>> > PAT
>> > -errata, WT falls back to UC per the default values.
>>
>> PAT related errata I'm aware of are related to either page size or
>> the number of bits used to index into the PAT MSR, but never to
>> a particular memory type. Are you saying there are errata which
>> make use of WT or WC impossible altogether? Otherwise I would
>> have thought (even more so in the absence of any comment
>> saying otherwise - "minimal supported modes" doesn't really say
>> on what basis the set is the minimal one) that the mode systems
>> come up in (compatible with pre-PAT) ought to be what the tables
>> express.
>
> Please take a look at the comments in pat_init(). WT uses slot 7 (not slot
> 1) in the regular case.

But that is an adjustment Linux makes to the default the system
comes up in. And again - in my opinion the pre-initialized table
values should reflect the mode the system comes up in (i.e.
correct prior to execution reaching pat_init()), and be updated
(which as it seems happens in all three possible cases) once the
MSR gets fiddled with.

Jan

2015-07-22 16:08:11

by Toshi Kani

[permalink] [raw]
Subject: Re: [PATCH] x86: adjust default caching mode translation tables

On Wed, 2015-07-22 at 09:41 -0600, Jan Beulich wrote:
> >
> > > > On 22.07.15 at 17:23, <[email protected]> wrote:
> > On Wed, 2015-07-22 at 09:17 -0600, Jan Beulich wrote:
> > > >
> > > > > > On 22.07.15 at 00:29, <[email protected]> wrote:
> > > > On Mon, 2015-07-20 at 08:46 +0100, Jan Beulich wrote:
> > > > > Make WT really mean WT (rather than UC).
> > > > >
> > > > > I can't see why commit 9cd25aac1f ("x86/mm/pat: Emulate PAT when
> > > > > it
> > > > > is
> > > > > disabled") didn't make this match its changes to pat_init().
> > > >
> > > > No, the default values need to be set to the fallback types, i.e.
> > > > minimal
> > > > supported mode. For WC and WT, UC is the fallback type.
> > >
> > > But why would that be?
> > >
> > > > When PAT is disabled, pat_init() does update the tables below to
> > > > enable
> > > > WT
> > > > per the default BIOS setup. However, when PAT is enabled, but CPU
> > > > has
> > > > PAT
> > > > -errata, WT falls back to UC per the default values.
> > >
> > > PAT related errata I'm aware of are related to either page size or
> > > the number of bits used to index into the PAT MSR, but never to
> > > a particular memory type. Are you saying there are errata which
> > > make use of WT or WC impossible altogether? Otherwise I would
> > > have thought (even more so in the absence of any comment
> > > saying otherwise - "minimal supported modes" doesn't really say
> > > on what basis the set is the minimal one) that the mode systems
> > > come up in (compatible with pre-PAT) ought to be what the tables
> > > express.
> >
> > Please take a look at the comments in pat_init(). WT uses slot 7 (not
> > slot
> > 1) in the regular case.
>
> But that is an adjustment Linux makes to the default the system
> comes up in. And again - in my opinion the pre-initialized table
> values should reflect the mode the system comes up in (i.e.
> correct prior to execution reaching pat_init()), and be updated
> (which as it seems happens in all three possible cases) once the
> MSR gets fiddled with.

'enum page_cache_mode' thru __cachemode2pte_tbl[] is a high-level
abstraction that can be used after pat_init(). For early boot-time,
__early_ioremap() takes pgprot_t directly.

Do you have a need to use __cachemode2pte_tbl[] before pat_init()?

Thanks,
-Toshi

2015-07-22 16:36:39

by Jan Beulich

[permalink] [raw]
Subject: Re: [PATCH] x86: adjust default caching mode translation tables

>>> On 22.07.15 at 18:06, <[email protected]> wrote:
> On Wed, 2015-07-22 at 09:41 -0600, Jan Beulich wrote:
>> >
>> > > > On 22.07.15 at 17:23, <[email protected]> wrote:
>> > On Wed, 2015-07-22 at 09:17 -0600, Jan Beulich wrote:
>> > > >
>> > > > > > On 22.07.15 at 00:29, <[email protected]> wrote:
>> > > > On Mon, 2015-07-20 at 08:46 +0100, Jan Beulich wrote:
>> > > > > Make WT really mean WT (rather than UC).
>> > > > >
>> > > > > I can't see why commit 9cd25aac1f ("x86/mm/pat: Emulate PAT when
>> > > > > it
>> > > > > is
>> > > > > disabled") didn't make this match its changes to pat_init().
>> > > >
>> > > > No, the default values need to be set to the fallback types, i.e.
>> > > > minimal
>> > > > supported mode. For WC and WT, UC is the fallback type.
>> > >
>> > > But why would that be?
>> > >
>> > > > When PAT is disabled, pat_init() does update the tables below to
>> > > > enable
>> > > > WT
>> > > > per the default BIOS setup. However, when PAT is enabled, but CPU
>> > > > has
>> > > > PAT
>> > > > -errata, WT falls back to UC per the default values.
>> > >
>> > > PAT related errata I'm aware of are related to either page size or
>> > > the number of bits used to index into the PAT MSR, but never to
>> > > a particular memory type. Are you saying there are errata which
>> > > make use of WT or WC impossible altogether? Otherwise I would
>> > > have thought (even more so in the absence of any comment
>> > > saying otherwise - "minimal supported modes" doesn't really say
>> > > on what basis the set is the minimal one) that the mode systems
>> > > come up in (compatible with pre-PAT) ought to be what the tables
>> > > express.
>> >
>> > Please take a look at the comments in pat_init(). WT uses slot 7 (not
>> > slot
>> > 1) in the regular case.
>>
>> But that is an adjustment Linux makes to the default the system
>> comes up in. And again - in my opinion the pre-initialized table
>> values should reflect the mode the system comes up in (i.e.
>> correct prior to execution reaching pat_init()), and be updated
>> (which as it seems happens in all three possible cases) once the
>> MSR gets fiddled with.
>
> 'enum page_cache_mode' thru __cachemode2pte_tbl[] is a high-level
> abstraction that can be used after pat_init(). For early boot-time,
> __early_ioremap() takes pgprot_t directly.
>
> Do you have a need to use __cachemode2pte_tbl[] before pat_init()?

No. I just noticed the (apparent?) inconsistency.

Jan

2015-07-22 16:49:14

by Toshi Kani

[permalink] [raw]
Subject: Re: [PATCH] x86: adjust default caching mode translation tables

On Wed, 2015-07-22 at 10:36 -0600, Jan Beulich wrote:
> >
> > > > On 22.07.15 at 18:06, <[email protected]> wrote:
> > On Wed, 2015-07-22 at 09:41 -0600, Jan Beulich wrote:
> > > >
> > > > > > On 22.07.15 at 17:23, <[email protected]> wrote:
> > > > On Wed, 2015-07-22 at 09:17 -0600, Jan Beulich wrote:
> > > > > >
> > > > > > > > On 22.07.15 at 00:29, <[email protected]> wrote:
> > > > > > On Mon, 2015-07-20 at 08:46 +0100, Jan Beulich wrote:
> > > > > > > Make WT really mean WT (rather than UC).
> > > > > > >
> > > > > > > I can't see why commit 9cd25aac1f ("x86/mm/pat: Emulate PAT
> > > > > > > when
> > > > > > > it
> > > > > > > is
> > > > > > > disabled") didn't make this match its changes to pat_init().
> > > > > >
> > > > > > No, the default values need to be set to the fallback types,
> > > > > > i.e.
> > > > > > minimal
> > > > > > supported mode. For WC and WT, UC is the fallback type.
> > > > >
> > > > > But why would that be?
> > > > >
> > > > > > When PAT is disabled, pat_init() does update the tables below
> > > > > > to
> > > > > > enable
> > > > > > WT
> > > > > > per the default BIOS setup. However, when PAT is enabled, but
> > > > > > CPU
> > > > > > has
> > > > > > PAT
> > > > > > -errata, WT falls back to UC per the default values.
> > > > >
> > > > > PAT related errata I'm aware of are related to either page size
> > > > > or
> > > > > the number of bits used to index into the PAT MSR, but never to
> > > > > a particular memory type. Are you saying there are errata which
> > > > > make use of WT or WC impossible altogether? Otherwise I would
> > > > > have thought (even more so in the absence of any comment
> > > > > saying otherwise - "minimal supported modes" doesn't really say
> > > > > on what basis the set is the minimal one) that the mode systems
> > > > > come up in (compatible with pre-PAT) ought to be what the tables
> > > > > express.
> > > >
> > > > Please take a look at the comments in pat_init(). WT uses slot 7
> > > > (not
> > > > slot
> > > > 1) in the regular case.
> > >
> > > But that is an adjustment Linux makes to the default the system
> > > comes up in. And again - in my opinion the pre-initialized table
> > > values should reflect the mode the system comes up in (i.e.
> > > correct prior to execution reaching pat_init()), and be updated
> > > (which as it seems happens in all three possible cases) once the
> > > MSR gets fiddled with.
> >
> > 'enum page_cache_mode' thru __cachemode2pte_tbl[] is a high-level
> > abstraction that can be used after pat_init(). For early boot-time,
> > __early_ioremap() takes pgprot_t directly.
> >
> > Do you have a need to use __cachemode2pte_tbl[] before pat_init()?
>
> No. I just noticed the (apparent?) inconsistency.

I will update the comment of __cachemode2pte_tbl[] to avoid such confusion.

Thanks,
-Toshi

Subject: [tip:x86/urgent] x86/mm/pat: Revert ' Adjust default caching mode translation tables'

Commit-ID: 1a4e8795711f474b31ff6eac37f3efd304ed8a93
Gitweb: http://git.kernel.org/tip/1a4e8795711f474b31ff6eac37f3efd304ed8a93
Author: Thomas Gleixner <[email protected]>
AuthorDate: Sun, 26 Jul 2015 10:27:37 +0200
Committer: Thomas Gleixner <[email protected]>
CommitDate: Sun, 26 Jul 2015 10:27:37 +0200

x86/mm/pat: Revert 'Adjust default caching mode translation tables'

Toshi explains:

"No, the default values need to be set to the fallback types,
i.e. minimal supported mode. For WC and WT, UC is the fallback type.

When PAT is disabled, pat_init() does update the tables below to
enable WT per the default BIOS setup. However, when PAT is enabled,
but CPU has PAT -errata, WT falls back to UC per the default values."

Revert: ca1fec58bc6a 'x86/mm/pat: Adjust default caching mode translation tables'
Requested-by: Toshi Kani <[email protected]>
Cc: Jan Beulich <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Thomas Gleixner <[email protected]>
---
arch/x86/mm/init.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index 7a45322..8533b46 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -43,18 +43,18 @@ uint16_t __cachemode2pte_tbl[_PAGE_CACHE_MODE_NUM] = {
[_PAGE_CACHE_MODE_WC ] = 0 | _PAGE_PCD,
[_PAGE_CACHE_MODE_UC_MINUS] = 0 | _PAGE_PCD,
[_PAGE_CACHE_MODE_UC ] = _PAGE_PWT | _PAGE_PCD,
- [_PAGE_CACHE_MODE_WT ] = _PAGE_PWT | 0,
+ [_PAGE_CACHE_MODE_WT ] = 0 | _PAGE_PCD,
[_PAGE_CACHE_MODE_WP ] = 0 | _PAGE_PCD,
};
EXPORT_SYMBOL(__cachemode2pte_tbl);

uint8_t __pte2cachemode_tbl[8] = {
[__pte2cm_idx( 0 | 0 | 0 )] = _PAGE_CACHE_MODE_WB,
- [__pte2cm_idx(_PAGE_PWT | 0 | 0 )] = _PAGE_CACHE_MODE_WT,
+ [__pte2cm_idx(_PAGE_PWT | 0 | 0 )] = _PAGE_CACHE_MODE_UC_MINUS,
[__pte2cm_idx( 0 | _PAGE_PCD | 0 )] = _PAGE_CACHE_MODE_UC_MINUS,
[__pte2cm_idx(_PAGE_PWT | _PAGE_PCD | 0 )] = _PAGE_CACHE_MODE_UC,
[__pte2cm_idx( 0 | 0 | _PAGE_PAT)] = _PAGE_CACHE_MODE_WB,
- [__pte2cm_idx(_PAGE_PWT | 0 | _PAGE_PAT)] = _PAGE_CACHE_MODE_WT,
+ [__pte2cm_idx(_PAGE_PWT | 0 | _PAGE_PAT)] = _PAGE_CACHE_MODE_UC_MINUS,
[__pte2cm_idx(0 | _PAGE_PCD | _PAGE_PAT)] = _PAGE_CACHE_MODE_UC_MINUS,
[__pte2cm_idx(_PAGE_PWT | _PAGE_PCD | _PAGE_PAT)] = _PAGE_CACHE_MODE_UC,
};