Add comments to the cachemode translation tables to clarify that
the default values are set as minimal supported mode, which are
necessary to handle WC and WT fallback to UC- when they are not
enabled.
Signed-off-by: Toshi Kani <[email protected]>
Cc: Jan Beulich <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
---
Ingo, please replace the patch below with this patch.
https://www.mail-archive.com/[email protected]/msg937806.html
---
arch/x86/mm/init.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index 8533b46..1d8a83d 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -30,8 +30,11 @@
/*
* Tables translating between page_cache_type_t and pte encoding.
*
- * Minimal supported modes are defined statically, they are modified
- * during bootup if more supported cache modes are available.
+ * The default values are defined statically as minimal supported mode;
+ * WC and WT fall back to UC-. pat_init() updates these values to support
+ * more cache modes, WC and WT, when it is safe to do so. See pat_init()
+ * for the details. Note, __early_ioremap() used during early boot-time
+ * takes pgprot_t (pte encoding) and does not use these tables.
*
* Index into __cachemode2pte_tbl[] is the cachemode.
*
>>> On 22.07.15 at 20:06, <[email protected]> wrote:
> Add comments to the cachemode translation tables to clarify that
> the default values are set as minimal supported mode, which are
> necessary to handle WC and WT fallback to UC- when they are not
> enabled.
Wait - shouldn't WT fall back to UC (so to not be affected by WC
MTRR settings)?
Jan
On Thu, 2015-07-23 at 00:42 -0600, Jan Beulich wrote:
> >
> > > > On 22.07.15 at 20:06, <[email protected]> wrote:
> > Add comments to the cachemode translation tables to clarify that
> > the default values are set as minimal supported mode, which are
> > necessary to handle WC and WT fallback to UC- when they are not
> > enabled.
>
> Wait - shouldn't WT fall back to UC (so to not be affected by WC
> MTRR settings)?
Well, when MTRR is set to WC, WT will become UC. So, it is not safe to
start with.
I know Luis is driving to support UC. When UC can be used for both regular
memory and IO memory, yes, it can be changed to fall back to UC. At this
point, however, UC cannot be set to regular memory. See
reserve_ram_pages_type().
Thanks,
-Toshi
>>> On 23.07.15 at 16:27, <[email protected]> wrote:
> On Thu, 2015-07-23 at 00:42 -0600, Jan Beulich wrote:
>> >
>> > > > On 22.07.15 at 20:06, <[email protected]> wrote:
>> > Add comments to the cachemode translation tables to clarify that
>> > the default values are set as minimal supported mode, which are
>> > necessary to handle WC and WT fallback to UC- when they are not
>> > enabled.
>>
>> Wait - shouldn't WT fall back to UC (so to not be affected by WC
>> MTRR settings)?
>
> Well, when MTRR is set to WC, WT will become UC. So, it is not safe to
> start with.
Hmm, considering how my question was meant, your reply looks
contradictory to me. What I was talking about is the table as it is
in RC3, i.e. WT mapping to PAGE_PCD (matching WC and
UC_MINUS). I.e. if a page is mapped WT (i.e. UC-) and the
MTRRs for it say WC, then the result will be WC. Whereas if
WT translated to PAGE_PWT|PAGE_PCD (i.e. matching UC), an
MTRR setting of WC would have no effect - the resulting type
would still be UC.
> I know Luis is driving to support UC. When UC can be used for both regular
> memory and IO memory, yes, it can be changed to fall back to UC. At this
> point, however, UC cannot be set to regular memory. See
> reserve_ram_pages_type().
With the above in mind, that's a problem then I would say.
Yet no matter whether UC can't be expressed as an
attribute on a page, what gets stored in the page table
entries should, together with how PAT is set, result in
something that doesn't weaken the requested memory
type. I.e. if WT was requested, the end result shouldn't
be WC. Yet any representable type may still be mapped to
UC (as being the strictest one possible), and hence if WT
can't be expressed it should be made UC.
Jan
On Thu, 2015-07-23 at 08:50 -0600, Jan Beulich wrote:
> >
> > > > On 23.07.15 at 16:27, <[email protected]> wrote:
> > On Thu, 2015-07-23 at 00:42 -0600, Jan Beulich wrote:
> > > >
> > > > > > On 22.07.15 at 20:06, <[email protected]> wrote:
> > > > Add comments to the cachemode translation tables to clarify that
> > > > the default values are set as minimal supported mode, which are
> > > > necessary to handle WC and WT fallback to UC- when they are not
> > > > enabled.
> > >
> > > Wait - shouldn't WT fall back to UC (so to not be affected by WC
> > > MTRR settings)?
> >
> > Well, when MTRR is set to WC, WT will become UC. So, it is not safe to
> > start with.
>
> Hmm, considering how my question was meant, your reply looks
> contradictory to me. What I was talking about is the table as it is
> in RC3, i.e. WT mapping to PAGE_PCD (matching WC and
> UC_MINUS). I.e. if a page is mapped WT (i.e. UC-) and the
> MTRRs for it say WC, then the result will be WC. Whereas if
> WT translated to PAGE_PWT|PAGE_PCD (i.e. matching UC), an
> MTRR setting of WC would have no effect - the resulting type
> would still be UC.
Yes, I know what you meant. I agree that WT should fall back to UC when
considering MTRR.
In my reply, I was referring the regular case that WT is supported (i.e. no
fallback). WT request to a range covered by MTRR with WC will become UC.
I think such code is not written properly (I should not have said "not
safe") since it won't get WT in the regular case, either. A range with
MTRR covered by WC is a special memory, ex. frame buffer, and the code
should know what type of memory it is dealing with.
> > I know Luis is driving to support UC. When UC can be used for both
> > regular
> > memory and IO memory, yes, it can be changed to fall back to UC. At
> > this
> > point, however, UC cannot be set to regular memory. See
> > reserve_ram_pages_type().
>
> With the above in mind, that's a problem then I would say.
> Yet no matter whether UC can't be expressed as an
> attribute on a page, what gets stored in the page table
> entries should, together with how PAT is set, result in
> something that doesn't weaken the requested memory
> type. I.e. if WT was requested, the end result shouldn't
> be WC. Yet any representable type may still be mapped to
> UC (as being the strictest one possible), and hence if WT
> can't be expressed it should be made UC.
Yes, I agree with you. But such risk is very low -- 1) the regular case
(no fallback) is used most of the cases, 2) the code using WT knows what
type of memory it is dealing with. For example, pmem may map NVDIMM with
WT, and any sane BIOS sets MTRR to WB for NVDIMM.
Thanks,
-Toshi
>>> On 23.07.15 at 17:25, <[email protected]> wrote:
> Yes, I agree with you. But such risk is very low -- 1) the regular case
> (no fallback) is used most of the cases, 2) the code using WT knows what
> type of memory it is dealing with. For example, pmem may map NVDIMM with
> WT, and any sane BIOS sets MTRR to WB for NVDIMM.
Do the words "sane" and "BIOS" really fit together in your opinion?
Jan
On Thu, 2015-07-23 at 09:36 -0600, Jan Beulich wrote:
> >
> > > > On 23.07.15 at 17:25, <[email protected]> wrote:
> > Yes, I agree with you. But such risk is very low -- 1) the regular
> > case
> > (no fallback) is used most of the cases, 2) the code using WT knows
> > what
> > type of memory it is dealing with. For example, pmem may map NVDIMM
> > with
> > WT, and any sane BIOS sets MTRR to WB for NVDIMM.
>
> Do the words "sane" and "BIOS" really fit together in your opinion?
:-)
Anyway, I am not disagreeing with you... When UC is ready for both regular
memory and IO memory, it should be changed to fall back to UC.
Thanks,
-Toshi
On Thu, 23 Jul 2015, Toshi Kani wrote:
> On Thu, 2015-07-23 at 09:36 -0600, Jan Beulich wrote:
> > >
> > > > > On 23.07.15 at 17:25, <[email protected]> wrote:
> > > Yes, I agree with you. But such risk is very low -- 1) the regular
> > > case
> > > (no fallback) is used most of the cases, 2) the code using WT knows
> > > what
> > > type of memory it is dealing with. For example, pmem may map NVDIMM
> > > with
> > > WT, and any sane BIOS sets MTRR to WB for NVDIMM.
> >
> > Do the words "sane" and "BIOS" really fit together in your opinion?
>
> :-)
>
> Anyway, I am not disagreeing with you... When UC is ready for both regular
> memory and IO memory, it should be changed to fall back to UC.
What's the resolution of this discussion? Is that patch correct as is
or do we get an updated version?
Thanks,
tglx
On Sun, 2015-08-02 at 12:07 +0200, Thomas Gleixner wrote:
> On Thu, 23 Jul 2015, Toshi Kani wrote:
> > On Thu, 2015-07-23 at 09:36 -0600, Jan Beulich wrote:
> > > >
> > > > > > On 23.07.15 at 17:25, <[email protected]> wrote:
> > > > Yes, I agree with you. But such risk is very low -- 1) the regular
> > > > case
> > > > (no fallback) is used most of the cases, 2) the code using WT knows
> > > > what
> > > > type of memory it is dealing with. For example, pmem may map NVDIMM
> > > >
> > > > with
> > > > WT, and any sane BIOS sets MTRR to WB for NVDIMM.
> > >
> > > Do the words "sane" and "BIOS" really fit together in your opinion?
> >
> > :-)
> >
> > Anyway, I am not disagreeing with you... When UC is ready for both
> > regular
> > memory and IO memory, it should be changed to fall back to UC.
>
> What's the resolution of this discussion? Is that patch correct as is
> or do we get an updated version?
Yes, this patch is correct and we are in agreement.
What Jan mentioned about falling back to UC, instead of UC-, is a separate
item, and the code does not support it yet.
Thanks,
-Toshi
Commit-ID: d5dc861bd601b546ae6b36af54485142cca36a5e
Gitweb: http://git.kernel.org/tip/d5dc861bd601b546ae6b36af54485142cca36a5e
Author: Toshi Kani <[email protected]>
AuthorDate: Wed, 22 Jul 2015 12:06:11 -0600
Committer: Thomas Gleixner <[email protected]>
CommitDate: Thu, 20 Aug 2015 21:26:12 +0200
x86/mm/pat: Add comments to cachemode translation tables
Add comments to the cachemode translation tables to clarify that
the default values are set as minimal supported mode, which are
necessary to handle WC and WT fallback to UC- when they are not
enabled.
Signed-off-by: Toshi Kani <[email protected]>
Cc: Jan Beulich <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Thomas Gleixner <[email protected]>
---
arch/x86/mm/init.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index 8533b46..1d8a83d 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -30,8 +30,11 @@
/*
* Tables translating between page_cache_type_t and pte encoding.
*
- * Minimal supported modes are defined statically, they are modified
- * during bootup if more supported cache modes are available.
+ * The default values are defined statically as minimal supported mode;
+ * WC and WT fall back to UC-. pat_init() updates these values to support
+ * more cache modes, WC and WT, when it is safe to do so. See pat_init()
+ * for the details. Note, __early_ioremap() used during early boot-time
+ * takes pgprot_t (pte encoding) and does not use these tables.
*
* Index into __cachemode2pte_tbl[] is the cachemode.
*