RE: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages
-Original Message-
From: Wood Scott-B07421
Sent: Thursday, July 18, 2013 10:48 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Bhushan
Bharat-
R65777
Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel
managed pages
On 07/18/2013 08:19:03 AM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's normal
RAM and the mapping is set to M bit (coherent, cacheable) otherwise
this is treated as I/O and we set I + G (cache inhibited, guarded)
This helps setting proper TLB mapping for direct assigned device
Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
---
v2: some cleanup and added comment
-
arch/powerpc/kvm/e500_mmu_host.c | 23 ++-
1 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..02eb973 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,26 @@ static inline u32 e500_shadow_mas3_attrib(u32
mas3, int usermode)
return mas3;
}
-static inline u32 e500_shadow_mas2_attrib(u32 mas2, int usermode)
+static inline u32 e500_shadow_mas2_attrib(u32 mas2, pfn_t pfn)
{
+ u32 mas2_attr;
+
+ mas2_attr = mas2 MAS2_ATTRIB_MASK;
+
+ /*
+* RAM is always mappable on e500 systems, so this is identical
+* to kvm_is_mmio_pfn(), just without its overhead.
+*/
+ if (!pfn_valid(pfn)) {
Please use page_is_ram(), which is what gets used when setting the WIMG for
the
host userspace mapping. We want to make sure the two are consistent.
+ /* Pages not managed by Kernel are treated as I/O, set
I + G */
+ mas2_attr |= MAS2_I | MAS2_G;
#ifdef CONFIG_SMP
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ } else {
+ /* Kernel managed pages are actually RAM so set M */
+ mas2_attr |= MAS2_M;
#endif
Likewise, we want to make sure this matches the host entry.
Unfortunately, this is a bit of a mess already. 64-bit booke appears to
always
set MAS2_M for TLB0 mappings.
Scott, can you please point to the code where MAS2_M is always set for TLB0?
-Bharat
The initial KERNELBASE mapping on boot uses
M_IF_SMP, and the settlbcam() that (IIRC) replaces it uses _PAGE_COHERENT.
32-
bit always uses _PAGE_COHERENT, except that initial KERNELBASE mapping.
_PAGE_COHERENT appears to be set based on CONFIG_SMP || CONFIG_PPC_STD_MMU
(the
latter config clears _PAGE_COHERENT in the non-CPU_FTR_NEED_COHERENT case).
As for what we actually want to happen, there are cases when we want M to be
set
for non-SMP. One such case is AMP, where CPUs may be sharing memory even if
the
Linux instance only runs on one CPU (this is not hypothetical, BTW). It's
also
possible that we encounter a hardware bug that requires MAS2_M, similar to
what
some of our non-booke chips require.
-Scott
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/22/2013 10:39:16 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: Wood Scott-B07421 Sent: Tuesday, July 23, 2013 12:18 AM To: Bhushan Bharat-R65777 Cc: Wood Scott-B07421; Alexander Graf; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages On 07/21/2013 11:39:45 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: Wood Scott-B07421 Sent: Thursday, July 18, 2013 11:09 PM To: Alexander Graf Cc: Bhushan Bharat-R65777; kvm-...@vger.kernel.org; kvm@vger.kernel.org; Bhushan Bharat-R65777 Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages On 07/18/2013 12:32:18 PM, Alexander Graf wrote: On 18.07.2013, at 19:17, Scott Wood wrote: On 07/18/2013 08:19:03 AM, Bharat Bhushan wrote: Likewise, we want to make sure this matches the host entry. Unfortunately, this is a bit of a mess already. 64-bit booke appears to always set MAS2_M for TLB0 mappings. The initial KERNELBASE mapping on boot uses M_IF_SMP, and the settlbcam() that (IIRC) replaces it uses _PAGE_COHERENT. 32-bit always uses _PAGE_COHERENT, except that initial KERNELBASE mapping. _PAGE_COHERENT appears to be set based on CONFIG_SMP || CONFIG_PPC_STD_MMU (the latter config clears _PAGE_COHERENT in the non-CPU_FTR_NEED_COHERENT case). As for what we actually want to happen, there are cases when we want M to be set for non-SMP. One such case is AMP, where CPUs may be sharing memory even if the Linux instance only runs on one CPU (this is not hypothetical, BTW). It's also possible that we encounter a hardware bug that requires MAS2_M, similar to what some of our non-booke chips require. How about we always set M then for RAM? M is like I in that bad things happen if you mix them. I am trying to list the invalid mixing of WIMG: 1) I M 2) W I 3) W M (Scott mentioned that he observed issues when mixing these two) 4) is there any other? That's not what I was talking about (and I don't think I mentioned W at all, though it is also potentially problematic). Here is cut paste of your one response: The architecture makes it illegal to mix cacheable and cache-inhibited mappings to the same physical page. Mixing W or M bits is generally bad as well. I've seen it cause machine checks, error interrupts, etc. -- not just corrupting the page in question. So I added not mixing W M. But at that time I missed to understood why mixing M I for same physical address can be issue :). W or M, not W and M. I meant that each one, separately, is in a similar situation as the I bit. None of this is about invalid combinations of attributes on a single TLB entry (though there are architectural restrictions there as well). -Scott -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages
-Original Message- From: Wood Scott-B07421 Sent: Tuesday, July 23, 2013 10:15 PM To: Bhushan Bharat-R65777 Cc: Wood Scott-B07421; Alexander Graf; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages On 07/22/2013 10:39:16 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: Wood Scott-B07421 Sent: Tuesday, July 23, 2013 12:18 AM To: Bhushan Bharat-R65777 Cc: Wood Scott-B07421; Alexander Graf; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages On 07/21/2013 11:39:45 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: Wood Scott-B07421 Sent: Thursday, July 18, 2013 11:09 PM To: Alexander Graf Cc: Bhushan Bharat-R65777; kvm-...@vger.kernel.org; kvm@vger.kernel.org; Bhushan Bharat-R65777 Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages On 07/18/2013 12:32:18 PM, Alexander Graf wrote: On 18.07.2013, at 19:17, Scott Wood wrote: On 07/18/2013 08:19:03 AM, Bharat Bhushan wrote: Likewise, we want to make sure this matches the host entry. Unfortunately, this is a bit of a mess already. 64-bit booke appears to always set MAS2_M for TLB0 mappings. The initial KERNELBASE mapping on boot uses M_IF_SMP, and the settlbcam() that (IIRC) replaces it uses _PAGE_COHERENT. 32-bit always uses _PAGE_COHERENT, except that initial KERNELBASE mapping. _PAGE_COHERENT appears to be set based on CONFIG_SMP || CONFIG_PPC_STD_MMU (the latter config clears _PAGE_COHERENT in the non-CPU_FTR_NEED_COHERENT case). As for what we actually want to happen, there are cases when we want M to be set for non-SMP. One such case is AMP, where CPUs may be sharing memory even if the Linux instance only runs on one CPU (this is not hypothetical, BTW). It's also possible that we encounter a hardware bug that requires MAS2_M, similar to what some of our non-booke chips require. How about we always set M then for RAM? M is like I in that bad things happen if you mix them. I am trying to list the invalid mixing of WIMG: 1) I M 2) W I 3) W M (Scott mentioned that he observed issues when mixing these two) 4) is there any other? That's not what I was talking about (and I don't think I mentioned W at all, though it is also potentially problematic). Here is cut paste of your one response: The architecture makes it illegal to mix cacheable and cache-inhibited mappings to the same physical page. Mixing W or M bits is generally bad as well. I've seen it cause machine checks, error interrupts, etc. -- not just corrupting the page in question. So I added not mixing W M. But at that time I missed to understood why mixing M I for same physical address can be issue :). W or M, not W and M. I meant that each one, separately, is in a similar situation as the I bit. None of this is about invalid combinations of attributes on a single TLB entry (though there are architectural restrictions there as well). Ok, I misread again :(. The second part of comment was (looks like you missed so copy pasted below) When we say all RAM (page_is_ram() is true) will be having M bit, then same RAM physical address will not have M mixed with any other, right? Similarly, For IO (which is not RAM), we will set I+G, so I will not be mixed with M. Is not that? -Bharat -Scott -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/23/2013 11:50:35 AM, Bhushan Bharat-R65777 wrote: -Original Message- From: Wood Scott-B07421 Sent: Tuesday, July 23, 2013 10:15 PM To: Bhushan Bharat-R65777 Cc: Wood Scott-B07421; Alexander Graf; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages On 07/22/2013 10:39:16 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: Wood Scott-B07421 Sent: Tuesday, July 23, 2013 12:18 AM To: Bhushan Bharat-R65777 Cc: Wood Scott-B07421; Alexander Graf; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages On 07/21/2013 11:39:45 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: Wood Scott-B07421 Sent: Thursday, July 18, 2013 11:09 PM To: Alexander Graf Cc: Bhushan Bharat-R65777; kvm-...@vger.kernel.org; kvm@vger.kernel.org; Bhushan Bharat-R65777 Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages On 07/18/2013 12:32:18 PM, Alexander Graf wrote: On 18.07.2013, at 19:17, Scott Wood wrote: On 07/18/2013 08:19:03 AM, Bharat Bhushan wrote: Likewise, we want to make sure this matches the host entry. Unfortunately, this is a bit of a mess already. 64-bit booke appears to always set MAS2_M for TLB0 mappings. The initial KERNELBASE mapping on boot uses M_IF_SMP, and the settlbcam() that (IIRC) replaces it uses _PAGE_COHERENT. 32-bit always uses _PAGE_COHERENT, except that initial KERNELBASE mapping. _PAGE_COHERENT appears to be set based on CONFIG_SMP || CONFIG_PPC_STD_MMU (the latter config clears _PAGE_COHERENT in the non-CPU_FTR_NEED_COHERENT case). As for what we actually want to happen, there are cases when we want M to be set for non-SMP. One such case is AMP, where CPUs may be sharing memory even if the Linux instance only runs on one CPU (this is not hypothetical, BTW). It's also possible that we encounter a hardware bug that requires MAS2_M, similar to what some of our non-booke chips require. How about we always set M then for RAM? M is like I in that bad things happen if you mix them. I am trying to list the invalid mixing of WIMG: 1) I M 2) W I 3) W M (Scott mentioned that he observed issues when mixing these two) 4) is there any other? That's not what I was talking about (and I don't think I mentioned W at all, though it is also potentially problematic). Here is cut paste of your one response: The architecture makes it illegal to mix cacheable and cache-inhibited mappings to the same physical page. Mixing W or M bits is generally bad as well. I've seen it cause machine checks, error interrupts, etc. -- not just corrupting the page in question. So I added not mixing W M. But at that time I missed to understood why mixing M I for same physical address can be issue :). W or M, not W and M. I meant that each one, separately, is in a similar situation as the I bit. None of this is about invalid combinations of attributes on a single TLB entry (though there are architectural restrictions there as well). Ok, I misread again :(. The second part of comment was (looks like you missed so copy pasted below) When we say all RAM (page_is_ram() is true) will be having M bit, then same RAM physical address will not have M mixed with any other, right? Similarly, For IO (which is not RAM), we will set I+G, so I will not be mixed with M. Is not that? I didn't miss it; it just seemed moot given the earlier confusion. But yes, for now we will set all RAM to M, and all I/O to I+G. Eventually that will change if/when we do vfio for QMan portals or other devices that require cacheable I/O. -Scott -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/23/2013 06:13:58 AM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Wood Scott-B07421
Sent: Thursday, July 18, 2013 10:48 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Bhushan Bharat-
R65777
Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only
for kernel
managed pages
On 07/18/2013 08:19:03 AM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal
RAM and the mapping is set to M bit (coherent, cacheable)
otherwise
this is treated as I/O and we set I + G (cache inhibited,
guarded)
This helps setting proper TLB mapping for direct assigned device
Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
---
v2: some cleanup and added comment
-
arch/powerpc/kvm/e500_mmu_host.c | 23 ++-
1 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..02eb973 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,26 @@ static inline u32 e500_shadow_mas3_attrib(u32
mas3, int usermode)
return mas3;
}
-static inline u32 e500_shadow_mas2_attrib(u32 mas2, int usermode)
+static inline u32 e500_shadow_mas2_attrib(u32 mas2, pfn_t pfn)
{
+ u32 mas2_attr;
+
+ mas2_attr = mas2 MAS2_ATTRIB_MASK;
+
+ /*
+ * RAM is always mappable on e500 systems, so this is identical
+ * to kvm_is_mmio_pfn(), just without its overhead.
+ */
+ if (!pfn_valid(pfn)) {
Please use page_is_ram(), which is what gets used when setting the
WIMG for the
host userspace mapping. We want to make sure the two are
consistent.
+ /* Pages not managed by Kernel are treated as I/O, set
I + G */
+ mas2_attr |= MAS2_I | MAS2_G;
#ifdef CONFIG_SMP
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ } else {
+ /* Kernel managed pages are actually RAM so set M */
+ mas2_attr |= MAS2_M;
#endif
Likewise, we want to make sure this matches the host entry.
Unfortunately, this is a bit of a mess already. 64-bit booke
appears to always
set MAS2_M for TLB0 mappings.
Scott, can you please point to the code where MAS2_M is always set
for TLB0?
__early_init_mmu() sets MAS4[MD], without regard to CONFIG_SMP.
However, the TLB miss handler replaces WIMGE with the flags from the
PTE, so I guess MAS4 is only relevant for things like indirect PTEs and
(for non-FSL chips) linear faults.
So never mind about 64-bit. There's still the AMP case, though.
-Scott
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages
-Original Message- From: Wood Scott-B07421 Sent: Tuesday, July 23, 2013 11:50 PM To: Bhushan Bharat-R65777 Cc: Wood Scott-B07421; Alexander Graf; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages On 07/23/2013 11:50:35 AM, Bhushan Bharat-R65777 wrote: -Original Message- From: Wood Scott-B07421 Sent: Tuesday, July 23, 2013 10:15 PM To: Bhushan Bharat-R65777 Cc: Wood Scott-B07421; Alexander Graf; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages On 07/22/2013 10:39:16 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: Wood Scott-B07421 Sent: Tuesday, July 23, 2013 12:18 AM To: Bhushan Bharat-R65777 Cc: Wood Scott-B07421; Alexander Graf; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages On 07/21/2013 11:39:45 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: Wood Scott-B07421 Sent: Thursday, July 18, 2013 11:09 PM To: Alexander Graf Cc: Bhushan Bharat-R65777; kvm-...@vger.kernel.org; kvm@vger.kernel.org; Bhushan Bharat-R65777 Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages On 07/18/2013 12:32:18 PM, Alexander Graf wrote: On 18.07.2013, at 19:17, Scott Wood wrote: On 07/18/2013 08:19:03 AM, Bharat Bhushan wrote: Likewise, we want to make sure this matches the host entry. Unfortunately, this is a bit of a mess already. 64-bit booke appears to always set MAS2_M for TLB0 mappings. The initial KERNELBASE mapping on boot uses M_IF_SMP, and the settlbcam() that (IIRC) replaces it uses _PAGE_COHERENT. 32-bit always uses _PAGE_COHERENT, except that initial KERNELBASE mapping. _PAGE_COHERENT appears to be set based on CONFIG_SMP || CONFIG_PPC_STD_MMU (the latter config clears _PAGE_COHERENT in the non-CPU_FTR_NEED_COHERENT case). As for what we actually want to happen, there are cases when we want M to be set for non-SMP. One such case is AMP, where CPUs may be sharing memory even if the Linux instance only runs on one CPU (this is not hypothetical, BTW). It's also possible that we encounter a hardware bug that requires MAS2_M, similar to what some of our non-booke chips require. How about we always set M then for RAM? M is like I in that bad things happen if you mix them. I am trying to list the invalid mixing of WIMG: 1) I M 2) W I 3) W M (Scott mentioned that he observed issues when mixing these two) 4) is there any other? That's not what I was talking about (and I don't think I mentioned W at all, though it is also potentially problematic). Here is cut paste of your one response: The architecture makes it illegal to mix cacheable and cache-inhibited mappings to the same physical page. Mixing W or M bits is generally bad as well. I've seen it cause machine checks, error interrupts, etc. -- not just corrupting the page in question. So I added not mixing W M. But at that time I missed to understood why mixing M I for same physical address can be issue :). W or M, not W and M. I meant that each one, separately, is in a similar situation as the I bit. None of this is about invalid combinations of attributes on a single TLB entry (though there are architectural restrictions there as well). Ok, I misread again :(. The second part of comment was (looks like you missed so copy pasted below) When we say all RAM (page_is_ram() is true) will be having M bit, then same RAM physical address will not have M mixed with any other, right? Similarly, For IO (which is not RAM), we will set I+G, so I will not be mixed with M. Is not that? I didn't miss it; it just seemed moot given the earlier confusion. But yes, for now we will set all RAM to M, and all I/O to I+G. Eventually that will change if/when we do vfio for QMan portals or other devices that require cacheable I/O. Agree :) -Bharat -Scott -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages
-Original Message-
From: Wood Scott-B07421
Sent: Thursday, July 18, 2013 10:48 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Bhushan
Bharat-
R65777
Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel
managed pages
On 07/18/2013 08:19:03 AM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's normal
RAM and the mapping is set to M bit (coherent, cacheable) otherwise
this is treated as I/O and we set I + G (cache inhibited, guarded)
This helps setting proper TLB mapping for direct assigned device
Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
---
v2: some cleanup and added comment
-
arch/powerpc/kvm/e500_mmu_host.c | 23 ++-
1 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..02eb973 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,26 @@ static inline u32 e500_shadow_mas3_attrib(u32
mas3, int usermode)
return mas3;
}
-static inline u32 e500_shadow_mas2_attrib(u32 mas2, int usermode)
+static inline u32 e500_shadow_mas2_attrib(u32 mas2, pfn_t pfn)
{
+ u32 mas2_attr;
+
+ mas2_attr = mas2 MAS2_ATTRIB_MASK;
+
+ /*
+* RAM is always mappable on e500 systems, so this is identical
+* to kvm_is_mmio_pfn(), just without its overhead.
+*/
+ if (!pfn_valid(pfn)) {
Please use page_is_ram(), which is what gets used when setting the WIMG for
the
host userspace mapping. We want to make sure the two are consistent.
+ /* Pages not managed by Kernel are treated as I/O, set
I + G */
+ mas2_attr |= MAS2_I | MAS2_G;
#ifdef CONFIG_SMP
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ } else {
+ /* Kernel managed pages are actually RAM so set M */
+ mas2_attr |= MAS2_M;
#endif
Likewise, we want to make sure this matches the host entry.
Unfortunately, this is a bit of a mess already. 64-bit booke appears to
always
set MAS2_M for TLB0 mappings.
Scott, can you please point to the code where MAS2_M is always set for TLB0?
-Bharat
The initial KERNELBASE mapping on boot uses
M_IF_SMP, and the settlbcam() that (IIRC) replaces it uses _PAGE_COHERENT.
32-
bit always uses _PAGE_COHERENT, except that initial KERNELBASE mapping.
_PAGE_COHERENT appears to be set based on CONFIG_SMP || CONFIG_PPC_STD_MMU
(the
latter config clears _PAGE_COHERENT in the non-CPU_FTR_NEED_COHERENT case).
As for what we actually want to happen, there are cases when we want M to be
set
for non-SMP. One such case is AMP, where CPUs may be sharing memory even if
the
Linux instance only runs on one CPU (this is not hypothetical, BTW). It's
also
possible that we encounter a hardware bug that requires MAS2_M, similar to
what
some of our non-booke chips require.
-Scott
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/22/2013 10:39:16 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: Wood Scott-B07421 Sent: Tuesday, July 23, 2013 12:18 AM To: Bhushan Bharat-R65777 Cc: Wood Scott-B07421; Alexander Graf; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages On 07/21/2013 11:39:45 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: Wood Scott-B07421 Sent: Thursday, July 18, 2013 11:09 PM To: Alexander Graf Cc: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Bhushan Bharat-R65777 Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages On 07/18/2013 12:32:18 PM, Alexander Graf wrote: On 18.07.2013, at 19:17, Scott Wood wrote: On 07/18/2013 08:19:03 AM, Bharat Bhushan wrote: Likewise, we want to make sure this matches the host entry. Unfortunately, this is a bit of a mess already. 64-bit booke appears to always set MAS2_M for TLB0 mappings. The initial KERNELBASE mapping on boot uses M_IF_SMP, and the settlbcam() that (IIRC) replaces it uses _PAGE_COHERENT. 32-bit always uses _PAGE_COHERENT, except that initial KERNELBASE mapping. _PAGE_COHERENT appears to be set based on CONFIG_SMP || CONFIG_PPC_STD_MMU (the latter config clears _PAGE_COHERENT in the non-CPU_FTR_NEED_COHERENT case). As for what we actually want to happen, there are cases when we want M to be set for non-SMP. One such case is AMP, where CPUs may be sharing memory even if the Linux instance only runs on one CPU (this is not hypothetical, BTW). It's also possible that we encounter a hardware bug that requires MAS2_M, similar to what some of our non-booke chips require. How about we always set M then for RAM? M is like I in that bad things happen if you mix them. I am trying to list the invalid mixing of WIMG: 1) I M 2) W I 3) W M (Scott mentioned that he observed issues when mixing these two) 4) is there any other? That's not what I was talking about (and I don't think I mentioned W at all, though it is also potentially problematic). Here is cut paste of your one response: The architecture makes it illegal to mix cacheable and cache-inhibited mappings to the same physical page. Mixing W or M bits is generally bad as well. I've seen it cause machine checks, error interrupts, etc. -- not just corrupting the page in question. So I added not mixing W M. But at that time I missed to understood why mixing M I for same physical address can be issue :). W or M, not W and M. I meant that each one, separately, is in a similar situation as the I bit. None of this is about invalid combinations of attributes on a single TLB entry (though there are architectural restrictions there as well). -Scott -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages
-Original Message- From: Wood Scott-B07421 Sent: Tuesday, July 23, 2013 10:15 PM To: Bhushan Bharat-R65777 Cc: Wood Scott-B07421; Alexander Graf; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages On 07/22/2013 10:39:16 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: Wood Scott-B07421 Sent: Tuesday, July 23, 2013 12:18 AM To: Bhushan Bharat-R65777 Cc: Wood Scott-B07421; Alexander Graf; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages On 07/21/2013 11:39:45 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: Wood Scott-B07421 Sent: Thursday, July 18, 2013 11:09 PM To: Alexander Graf Cc: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Bhushan Bharat-R65777 Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages On 07/18/2013 12:32:18 PM, Alexander Graf wrote: On 18.07.2013, at 19:17, Scott Wood wrote: On 07/18/2013 08:19:03 AM, Bharat Bhushan wrote: Likewise, we want to make sure this matches the host entry. Unfortunately, this is a bit of a mess already. 64-bit booke appears to always set MAS2_M for TLB0 mappings. The initial KERNELBASE mapping on boot uses M_IF_SMP, and the settlbcam() that (IIRC) replaces it uses _PAGE_COHERENT. 32-bit always uses _PAGE_COHERENT, except that initial KERNELBASE mapping. _PAGE_COHERENT appears to be set based on CONFIG_SMP || CONFIG_PPC_STD_MMU (the latter config clears _PAGE_COHERENT in the non-CPU_FTR_NEED_COHERENT case). As for what we actually want to happen, there are cases when we want M to be set for non-SMP. One such case is AMP, where CPUs may be sharing memory even if the Linux instance only runs on one CPU (this is not hypothetical, BTW). It's also possible that we encounter a hardware bug that requires MAS2_M, similar to what some of our non-booke chips require. How about we always set M then for RAM? M is like I in that bad things happen if you mix them. I am trying to list the invalid mixing of WIMG: 1) I M 2) W I 3) W M (Scott mentioned that he observed issues when mixing these two) 4) is there any other? That's not what I was talking about (and I don't think I mentioned W at all, though it is also potentially problematic). Here is cut paste of your one response: The architecture makes it illegal to mix cacheable and cache-inhibited mappings to the same physical page. Mixing W or M bits is generally bad as well. I've seen it cause machine checks, error interrupts, etc. -- not just corrupting the page in question. So I added not mixing W M. But at that time I missed to understood why mixing M I for same physical address can be issue :). W or M, not W and M. I meant that each one, separately, is in a similar situation as the I bit. None of this is about invalid combinations of attributes on a single TLB entry (though there are architectural restrictions there as well). Ok, I misread again :(. The second part of comment was (looks like you missed so copy pasted below) When we say all RAM (page_is_ram() is true) will be having M bit, then same RAM physical address will not have M mixed with any other, right? Similarly, For IO (which is not RAM), we will set I+G, so I will not be mixed with M. Is not that? -Bharat -Scott -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/23/2013 11:50:35 AM, Bhushan Bharat-R65777 wrote: -Original Message- From: Wood Scott-B07421 Sent: Tuesday, July 23, 2013 10:15 PM To: Bhushan Bharat-R65777 Cc: Wood Scott-B07421; Alexander Graf; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages On 07/22/2013 10:39:16 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: Wood Scott-B07421 Sent: Tuesday, July 23, 2013 12:18 AM To: Bhushan Bharat-R65777 Cc: Wood Scott-B07421; Alexander Graf; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages On 07/21/2013 11:39:45 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: Wood Scott-B07421 Sent: Thursday, July 18, 2013 11:09 PM To: Alexander Graf Cc: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Bhushan Bharat-R65777 Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages On 07/18/2013 12:32:18 PM, Alexander Graf wrote: On 18.07.2013, at 19:17, Scott Wood wrote: On 07/18/2013 08:19:03 AM, Bharat Bhushan wrote: Likewise, we want to make sure this matches the host entry. Unfortunately, this is a bit of a mess already. 64-bit booke appears to always set MAS2_M for TLB0 mappings. The initial KERNELBASE mapping on boot uses M_IF_SMP, and the settlbcam() that (IIRC) replaces it uses _PAGE_COHERENT. 32-bit always uses _PAGE_COHERENT, except that initial KERNELBASE mapping. _PAGE_COHERENT appears to be set based on CONFIG_SMP || CONFIG_PPC_STD_MMU (the latter config clears _PAGE_COHERENT in the non-CPU_FTR_NEED_COHERENT case). As for what we actually want to happen, there are cases when we want M to be set for non-SMP. One such case is AMP, where CPUs may be sharing memory even if the Linux instance only runs on one CPU (this is not hypothetical, BTW). It's also possible that we encounter a hardware bug that requires MAS2_M, similar to what some of our non-booke chips require. How about we always set M then for RAM? M is like I in that bad things happen if you mix them. I am trying to list the invalid mixing of WIMG: 1) I M 2) W I 3) W M (Scott mentioned that he observed issues when mixing these two) 4) is there any other? That's not what I was talking about (and I don't think I mentioned W at all, though it is also potentially problematic). Here is cut paste of your one response: The architecture makes it illegal to mix cacheable and cache-inhibited mappings to the same physical page. Mixing W or M bits is generally bad as well. I've seen it cause machine checks, error interrupts, etc. -- not just corrupting the page in question. So I added not mixing W M. But at that time I missed to understood why mixing M I for same physical address can be issue :). W or M, not W and M. I meant that each one, separately, is in a similar situation as the I bit. None of this is about invalid combinations of attributes on a single TLB entry (though there are architectural restrictions there as well). Ok, I misread again :(. The second part of comment was (looks like you missed so copy pasted below) When we say all RAM (page_is_ram() is true) will be having M bit, then same RAM physical address will not have M mixed with any other, right? Similarly, For IO (which is not RAM), we will set I+G, so I will not be mixed with M. Is not that? I didn't miss it; it just seemed moot given the earlier confusion. But yes, for now we will set all RAM to M, and all I/O to I+G. Eventually that will change if/when we do vfio for QMan portals or other devices that require cacheable I/O. -Scott -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/21/2013 11:39:45 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: Wood Scott-B07421 Sent: Thursday, July 18, 2013 11:09 PM To: Alexander Graf Cc: Bhushan Bharat-R65777; kvm-...@vger.kernel.org; kvm@vger.kernel.org; Bhushan Bharat-R65777 Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages On 07/18/2013 12:32:18 PM, Alexander Graf wrote: On 18.07.2013, at 19:17, Scott Wood wrote: On 07/18/2013 08:19:03 AM, Bharat Bhushan wrote: Likewise, we want to make sure this matches the host entry. Unfortunately, this is a bit of a mess already. 64-bit booke appears to always set MAS2_M for TLB0 mappings. The initial KERNELBASE mapping on boot uses M_IF_SMP, and the settlbcam() that (IIRC) replaces it uses _PAGE_COHERENT. 32-bit always uses _PAGE_COHERENT, except that initial KERNELBASE mapping. _PAGE_COHERENT appears to be set based on CONFIG_SMP || CONFIG_PPC_STD_MMU (the latter config clears _PAGE_COHERENT in the non-CPU_FTR_NEED_COHERENT case). As for what we actually want to happen, there are cases when we want M to be set for non-SMP. One such case is AMP, where CPUs may be sharing memory even if the Linux instance only runs on one CPU (this is not hypothetical, BTW). It's also possible that we encounter a hardware bug that requires MAS2_M, similar to what some of our non-booke chips require. How about we always set M then for RAM? M is like I in that bad things happen if you mix them. I am trying to list the invalid mixing of WIMG: 1) I M 2) W I 3) W M (Scott mentioned that he observed issues when mixing these two) 4) is there any other? That's not what I was talking about (and I don't think I mentioned W at all, though it is also potentially problematic). I'm talking about mixing I with not-I (on two different virtual addresses pointing to the same physical), M with not-M, etc. So we really want to match exactly what the rest of the kernel is doing. How the rest of kernel is doing is a bit complex. IIUC, if we forget about the boot state then this is how kernel set WIMG bits: 1) For Memory always set M if CONFIG_SMP set. - So KVM can do same. M will not be mixed with W and I. G and E are guest control. I don't think this is accurate for 64-bit. And what about the AMP case? -Scott -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages
-Original Message- From: Wood Scott-B07421 Sent: Tuesday, July 23, 2013 12:18 AM To: Bhushan Bharat-R65777 Cc: Wood Scott-B07421; Alexander Graf; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages On 07/21/2013 11:39:45 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: Wood Scott-B07421 Sent: Thursday, July 18, 2013 11:09 PM To: Alexander Graf Cc: Bhushan Bharat-R65777; kvm-...@vger.kernel.org; kvm@vger.kernel.org; Bhushan Bharat-R65777 Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages On 07/18/2013 12:32:18 PM, Alexander Graf wrote: On 18.07.2013, at 19:17, Scott Wood wrote: On 07/18/2013 08:19:03 AM, Bharat Bhushan wrote: Likewise, we want to make sure this matches the host entry. Unfortunately, this is a bit of a mess already. 64-bit booke appears to always set MAS2_M for TLB0 mappings. The initial KERNELBASE mapping on boot uses M_IF_SMP, and the settlbcam() that (IIRC) replaces it uses _PAGE_COHERENT. 32-bit always uses _PAGE_COHERENT, except that initial KERNELBASE mapping. _PAGE_COHERENT appears to be set based on CONFIG_SMP || CONFIG_PPC_STD_MMU (the latter config clears _PAGE_COHERENT in the non-CPU_FTR_NEED_COHERENT case). As for what we actually want to happen, there are cases when we want M to be set for non-SMP. One such case is AMP, where CPUs may be sharing memory even if the Linux instance only runs on one CPU (this is not hypothetical, BTW). It's also possible that we encounter a hardware bug that requires MAS2_M, similar to what some of our non-booke chips require. How about we always set M then for RAM? M is like I in that bad things happen if you mix them. I am trying to list the invalid mixing of WIMG: 1) I M 2) W I 3) W M (Scott mentioned that he observed issues when mixing these two) 4) is there any other? That's not what I was talking about (and I don't think I mentioned W at all, though it is also potentially problematic). Here is cut paste of your one response: The architecture makes it illegal to mix cacheable and cache-inhibited mappings to the same physical page. Mixing W or M bits is generally bad as well. I've seen it cause machine checks, error interrupts, etc. -- not just corrupting the page in question. So I added not mixing W M. But at that time I missed to understood why mixing M I for same physical address can be issue :). I'm talking about mixing I with not-I (on two different virtual addresses pointing to the same physical), M with not-M, etc. When we say all RAM (page_is_ram() is true) will be having M bit, then RAM physical address will not have M mixed with any other, right? Similarly, For IO (which is not RAM), we will set I+G, so I will not be mixed with M. Is not that? -Bharat So we really want to match exactly what the rest of the kernel is doing. How the rest of kernel is doing is a bit complex. IIUC, if we forget about the boot state then this is how kernel set WIMG bits: 1) For Memory always set M if CONFIG_SMP set. - So KVM can do same. M will not be mixed with W and I. G and E are guest control. I don't think this is accurate for 64-bit. And what about the AMP case? -Scott -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/21/2013 11:39:45 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: Wood Scott-B07421 Sent: Thursday, July 18, 2013 11:09 PM To: Alexander Graf Cc: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Bhushan Bharat-R65777 Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages On 07/18/2013 12:32:18 PM, Alexander Graf wrote: On 18.07.2013, at 19:17, Scott Wood wrote: On 07/18/2013 08:19:03 AM, Bharat Bhushan wrote: Likewise, we want to make sure this matches the host entry. Unfortunately, this is a bit of a mess already. 64-bit booke appears to always set MAS2_M for TLB0 mappings. The initial KERNELBASE mapping on boot uses M_IF_SMP, and the settlbcam() that (IIRC) replaces it uses _PAGE_COHERENT. 32-bit always uses _PAGE_COHERENT, except that initial KERNELBASE mapping. _PAGE_COHERENT appears to be set based on CONFIG_SMP || CONFIG_PPC_STD_MMU (the latter config clears _PAGE_COHERENT in the non-CPU_FTR_NEED_COHERENT case). As for what we actually want to happen, there are cases when we want M to be set for non-SMP. One such case is AMP, where CPUs may be sharing memory even if the Linux instance only runs on one CPU (this is not hypothetical, BTW). It's also possible that we encounter a hardware bug that requires MAS2_M, similar to what some of our non-booke chips require. How about we always set M then for RAM? M is like I in that bad things happen if you mix them. I am trying to list the invalid mixing of WIMG: 1) I M 2) W I 3) W M (Scott mentioned that he observed issues when mixing these two) 4) is there any other? That's not what I was talking about (and I don't think I mentioned W at all, though it is also potentially problematic). I'm talking about mixing I with not-I (on two different virtual addresses pointing to the same physical), M with not-M, etc. So we really want to match exactly what the rest of the kernel is doing. How the rest of kernel is doing is a bit complex. IIUC, if we forget about the boot state then this is how kernel set WIMG bits: 1) For Memory always set M if CONFIG_SMP set. - So KVM can do same. M will not be mixed with W and I. G and E are guest control. I don't think this is accurate for 64-bit. And what about the AMP case? -Scott -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages
-Original Message- From: Wood Scott-B07421 Sent: Tuesday, July 23, 2013 12:18 AM To: Bhushan Bharat-R65777 Cc: Wood Scott-B07421; Alexander Graf; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages On 07/21/2013 11:39:45 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: Wood Scott-B07421 Sent: Thursday, July 18, 2013 11:09 PM To: Alexander Graf Cc: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Bhushan Bharat-R65777 Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages On 07/18/2013 12:32:18 PM, Alexander Graf wrote: On 18.07.2013, at 19:17, Scott Wood wrote: On 07/18/2013 08:19:03 AM, Bharat Bhushan wrote: Likewise, we want to make sure this matches the host entry. Unfortunately, this is a bit of a mess already. 64-bit booke appears to always set MAS2_M for TLB0 mappings. The initial KERNELBASE mapping on boot uses M_IF_SMP, and the settlbcam() that (IIRC) replaces it uses _PAGE_COHERENT. 32-bit always uses _PAGE_COHERENT, except that initial KERNELBASE mapping. _PAGE_COHERENT appears to be set based on CONFIG_SMP || CONFIG_PPC_STD_MMU (the latter config clears _PAGE_COHERENT in the non-CPU_FTR_NEED_COHERENT case). As for what we actually want to happen, there are cases when we want M to be set for non-SMP. One such case is AMP, where CPUs may be sharing memory even if the Linux instance only runs on one CPU (this is not hypothetical, BTW). It's also possible that we encounter a hardware bug that requires MAS2_M, similar to what some of our non-booke chips require. How about we always set M then for RAM? M is like I in that bad things happen if you mix them. I am trying to list the invalid mixing of WIMG: 1) I M 2) W I 3) W M (Scott mentioned that he observed issues when mixing these two) 4) is there any other? That's not what I was talking about (and I don't think I mentioned W at all, though it is also potentially problematic). Here is cut paste of your one response: The architecture makes it illegal to mix cacheable and cache-inhibited mappings to the same physical page. Mixing W or M bits is generally bad as well. I've seen it cause machine checks, error interrupts, etc. -- not just corrupting the page in question. So I added not mixing W M. But at that time I missed to understood why mixing M I for same physical address can be issue :). I'm talking about mixing I with not-I (on two different virtual addresses pointing to the same physical), M with not-M, etc. When we say all RAM (page_is_ram() is true) will be having M bit, then RAM physical address will not have M mixed with any other, right? Similarly, For IO (which is not RAM), we will set I+G, so I will not be mixed with M. Is not that? -Bharat So we really want to match exactly what the rest of the kernel is doing. How the rest of kernel is doing is a bit complex. IIUC, if we forget about the boot state then this is how kernel set WIMG bits: 1) For Memory always set M if CONFIG_SMP set. - So KVM can do same. M will not be mixed with W and I. G and E are guest control. I don't think this is accurate for 64-bit. And what about the AMP case? -Scott -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages
-Original Message- From: Wood Scott-B07421 Sent: Thursday, July 18, 2013 11:09 PM To: Alexander Graf Cc: Bhushan Bharat-R65777; kvm-...@vger.kernel.org; kvm@vger.kernel.org; Bhushan Bharat-R65777 Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages On 07/18/2013 12:32:18 PM, Alexander Graf wrote: On 18.07.2013, at 19:17, Scott Wood wrote: On 07/18/2013 08:19:03 AM, Bharat Bhushan wrote: Likewise, we want to make sure this matches the host entry. Unfortunately, this is a bit of a mess already. 64-bit booke appears to always set MAS2_M for TLB0 mappings. The initial KERNELBASE mapping on boot uses M_IF_SMP, and the settlbcam() that (IIRC) replaces it uses _PAGE_COHERENT. 32-bit always uses _PAGE_COHERENT, except that initial KERNELBASE mapping. _PAGE_COHERENT appears to be set based on CONFIG_SMP || CONFIG_PPC_STD_MMU (the latter config clears _PAGE_COHERENT in the non-CPU_FTR_NEED_COHERENT case). As for what we actually want to happen, there are cases when we want M to be set for non-SMP. One such case is AMP, where CPUs may be sharing memory even if the Linux instance only runs on one CPU (this is not hypothetical, BTW). It's also possible that we encounter a hardware bug that requires MAS2_M, similar to what some of our non-booke chips require. How about we always set M then for RAM? M is like I in that bad things happen if you mix them. I am trying to list the invalid mixing of WIMG: 1) I M 2) W I 3) W M (Scott mentioned that he observed issues when mixing these two) 4) is there any other? So it mean it is safe to let guest control G and E. So we really want to match exactly what the rest of the kernel is doing. How the rest of kernel is doing is a bit complex. IIUC, if we forget about the boot state then this is how kernel set WIMG bits: 1) For Memory always set M if CONFIG_SMP set. - So KVM can do same. M will not be mixed with W and I. G and E are guest control. 2) For I/O , drivers can pass flags to set M or I + G. - For KVM; if not memory then it is I/O. For now we can always set I + G. - Later we can design some mechanism in VFIO interface to let KVM somehow know whether to set M or I+G. -Bharat Plus, the performance penalty on some single-core chips can be pretty bad. -Scott -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages
-Original Message- From: Wood Scott-B07421 Sent: Thursday, July 18, 2013 11:09 PM To: Alexander Graf Cc: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Bhushan Bharat-R65777 Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages On 07/18/2013 12:32:18 PM, Alexander Graf wrote: On 18.07.2013, at 19:17, Scott Wood wrote: On 07/18/2013 08:19:03 AM, Bharat Bhushan wrote: Likewise, we want to make sure this matches the host entry. Unfortunately, this is a bit of a mess already. 64-bit booke appears to always set MAS2_M for TLB0 mappings. The initial KERNELBASE mapping on boot uses M_IF_SMP, and the settlbcam() that (IIRC) replaces it uses _PAGE_COHERENT. 32-bit always uses _PAGE_COHERENT, except that initial KERNELBASE mapping. _PAGE_COHERENT appears to be set based on CONFIG_SMP || CONFIG_PPC_STD_MMU (the latter config clears _PAGE_COHERENT in the non-CPU_FTR_NEED_COHERENT case). As for what we actually want to happen, there are cases when we want M to be set for non-SMP. One such case is AMP, where CPUs may be sharing memory even if the Linux instance only runs on one CPU (this is not hypothetical, BTW). It's also possible that we encounter a hardware bug that requires MAS2_M, similar to what some of our non-booke chips require. How about we always set M then for RAM? M is like I in that bad things happen if you mix them. I am trying to list the invalid mixing of WIMG: 1) I M 2) W I 3) W M (Scott mentioned that he observed issues when mixing these two) 4) is there any other? So it mean it is safe to let guest control G and E. So we really want to match exactly what the rest of the kernel is doing. How the rest of kernel is doing is a bit complex. IIUC, if we forget about the boot state then this is how kernel set WIMG bits: 1) For Memory always set M if CONFIG_SMP set. - So KVM can do same. M will not be mixed with W and I. G and E are guest control. 2) For I/O , drivers can pass flags to set M or I + G. - For KVM; if not memory then it is I/O. For now we can always set I + G. - Later we can design some mechanism in VFIO interface to let KVM somehow know whether to set M or I+G. -Bharat Plus, the performance penalty on some single-core chips can be pretty bad. -Scott -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages
If there is a struct page for the requested mapping then it's
normal RAM and the mapping is set to M bit (coherent, cacheable)
otherwise this is treated as I/O and we set I + G (cache inhibited, guarded)
This helps setting proper TLB mapping for direct assigned device
Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
---
v2: some cleanup and added comment
-
arch/powerpc/kvm/e500_mmu_host.c | 23 ++-
1 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..02eb973 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,26 @@ static inline u32 e500_shadow_mas3_attrib(u32 mas3, int
usermode)
return mas3;
}
-static inline u32 e500_shadow_mas2_attrib(u32 mas2, int usermode)
+static inline u32 e500_shadow_mas2_attrib(u32 mas2, pfn_t pfn)
{
+ u32 mas2_attr;
+
+ mas2_attr = mas2 MAS2_ATTRIB_MASK;
+
+ /*
+* RAM is always mappable on e500 systems, so this is identical
+* to kvm_is_mmio_pfn(), just without its overhead.
+*/
+ if (!pfn_valid(pfn)) {
+ /* Pages not managed by Kernel are treated as I/O, set I + G */
+ mas2_attr |= MAS2_I | MAS2_G;
#ifdef CONFIG_SMP
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ } else {
+ /* Kernel managed pages are actually RAM so set M */
+ mas2_attr |= MAS2_M;
#endif
+ }
+ return mas2_attr;
}
/*
@@ -313,7 +326,7 @@ static void kvmppc_e500_setup_stlbe(
/* Force IPROT=0 for all guest mappings. */
stlbe-mas1 = MAS1_TSIZE(tsize) | get_tlb_sts(gtlbe) | MAS1_VALID;
stlbe-mas2 = (gvaddr MAS2_EPN) |
- e500_shadow_mas2_attrib(gtlbe-mas2, pr);
+ e500_shadow_mas2_attrib(gtlbe-mas2, pfn);
stlbe-mas7_3 = ((u64)pfn PAGE_SHIFT) |
e500_shadow_mas3_attrib(gtlbe-mas7_3, pr);
--
1.7.0.4
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages
On 18.07.2013, at 15:19, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal RAM and the mapping is set to M bit (coherent, cacheable)
otherwise this is treated as I/O and we set I + G (cache inhibited,
guarded)
This helps setting proper TLB mapping for direct assigned device
Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
---
v2: some cleanup and added comment
-
arch/powerpc/kvm/e500_mmu_host.c | 23 ++-
1 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..02eb973 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,26 @@ static inline u32 e500_shadow_mas3_attrib(u32 mas3, int
usermode)
return mas3;
}
-static inline u32 e500_shadow_mas2_attrib(u32 mas2, int usermode)
+static inline u32 e500_shadow_mas2_attrib(u32 mas2, pfn_t pfn)
{
+ u32 mas2_attr;
+
+ mas2_attr = mas2 MAS2_ATTRIB_MASK;
+
+ /*
+ * RAM is always mappable on e500 systems, so this is identical
+ * to kvm_is_mmio_pfn(), just without its overhead.
+ */
+ if (!pfn_valid(pfn)) {
+ /* Pages not managed by Kernel are treated as I/O, set I + G */
Please also document the intermediate thought that I/O should be mapped
non-cached.
+ mas2_attr |= MAS2_I | MAS2_G;
#ifdef CONFIG_SMP
Please separate the SMP case out of the branch.
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ } else {
+ /* Kernel managed pages are actually RAM so set M */
This comment doesn't tell me why M can be set ;).
Alex
+ mas2_attr |= MAS2_M;
#endif
+ }
+ return mas2_attr;
}
/*
@@ -313,7 +326,7 @@ static void kvmppc_e500_setup_stlbe(
/* Force IPROT=0 for all guest mappings. */
stlbe-mas1 = MAS1_TSIZE(tsize) | get_tlb_sts(gtlbe) | MAS1_VALID;
stlbe-mas2 = (gvaddr MAS2_EPN) |
- e500_shadow_mas2_attrib(gtlbe-mas2, pr);
+ e500_shadow_mas2_attrib(gtlbe-mas2, pfn);
stlbe-mas7_3 = ((u64)pfn PAGE_SHIFT) |
e500_shadow_mas3_attrib(gtlbe-mas7_3, pr);
--
1.7.0.4
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-ow...@vger.kernel.org] On
Behalf Of Alexander Graf
Sent: Thursday, July 18, 2013 8:23 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; Wood Scott-B07421; Bhushan
Bharat-R65777
Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel
managed pages
On 18.07.2013, at 15:19, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's normal
RAM and the mapping is set to M bit (coherent, cacheable) otherwise
this is treated as I/O and we set I + G (cache inhibited, guarded)
This helps setting proper TLB mapping for direct assigned device
Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
---
v2: some cleanup and added comment
-
arch/powerpc/kvm/e500_mmu_host.c | 23 ++-
1 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..02eb973 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,26 @@ static inline u32 e500_shadow_mas3_attrib(u32 mas3, int
usermode)
return mas3;
}
-static inline u32 e500_shadow_mas2_attrib(u32 mas2, int usermode)
+static inline u32 e500_shadow_mas2_attrib(u32 mas2, pfn_t pfn)
{
+ u32 mas2_attr;
+
+ mas2_attr = mas2 MAS2_ATTRIB_MASK;
+
+ /*
+* RAM is always mappable on e500 systems, so this is identical
+* to kvm_is_mmio_pfn(), just without its overhead.
+*/
+ if (!pfn_valid(pfn)) {
+ /* Pages not managed by Kernel are treated as I/O, set I + G */
Please also document the intermediate thought that I/O should be mapped non-
cached.
I did not get what you mean to document?
+ mas2_attr |= MAS2_I | MAS2_G;
#ifdef CONFIG_SMP
Please separate the SMP case out of the branch.
Really :) this was looking simple to me.
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ } else {
+ /* Kernel managed pages are actually RAM so set M */
This comment doesn't tell me why M can be set ;).
RAM in SMP, so setting coherent, is not that obvious?
-Bharat
Alex
+ mas2_attr |= MAS2_M;
#endif
+ }
+ return mas2_attr;
}
/*
@@ -313,7 +326,7 @@ static void kvmppc_e500_setup_stlbe(
/* Force IPROT=0 for all guest mappings. */
stlbe-mas1 = MAS1_TSIZE(tsize) | get_tlb_sts(gtlbe) | MAS1_VALID;
stlbe-mas2 = (gvaddr MAS2_EPN) |
- e500_shadow_mas2_attrib(gtlbe-mas2, pr);
+ e500_shadow_mas2_attrib(gtlbe-mas2, pfn);
stlbe-mas7_3 = ((u64)pfn PAGE_SHIFT) |
e500_shadow_mas3_attrib(gtlbe-mas7_3, pr);
--
1.7.0.4
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org More majordomo info
at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body
of a message to majord...@vger.kernel.org More majordomo info at
http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages
On 18.07.2013, at 17:15, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-ow...@vger.kernel.org] On
Behalf Of Alexander Graf
Sent: Thursday, July 18, 2013 8:23 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; Wood Scott-B07421; Bhushan
Bharat-R65777
Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel
managed pages
On 18.07.2013, at 15:19, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's normal
RAM and the mapping is set to M bit (coherent, cacheable) otherwise
this is treated as I/O and we set I + G (cache inhibited, guarded)
This helps setting proper TLB mapping for direct assigned device
Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
---
v2: some cleanup and added comment
-
arch/powerpc/kvm/e500_mmu_host.c | 23 ++-
1 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..02eb973 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,26 @@ static inline u32 e500_shadow_mas3_attrib(u32 mas3, int
usermode)
return mas3;
}
-static inline u32 e500_shadow_mas2_attrib(u32 mas2, int usermode)
+static inline u32 e500_shadow_mas2_attrib(u32 mas2, pfn_t pfn)
{
+ u32 mas2_attr;
+
+ mas2_attr = mas2 MAS2_ATTRIB_MASK;
+
+ /*
+* RAM is always mappable on e500 systems, so this is identical
+* to kvm_is_mmio_pfn(), just without its overhead.
+*/
+ if (!pfn_valid(pfn)) {
+ /* Pages not managed by Kernel are treated as I/O, set I + G */
Please also document the intermediate thought that I/O should be mapped non-
cached.
I did not get what you mean to document?
Page snot managed by the kernel are treated as I/O. Map it
with disabled cache.
+ mas2_attr |= MAS2_I | MAS2_G;
#ifdef CONFIG_SMP
Please separate the SMP case out of the branch.
Really :) this was looking simple to me.
Two branches intertwined never look simple :).
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ } else {
+ /* Kernel managed pages are actually RAM so set M */
This comment doesn't tell me why M can be set ;).
RAM in SMP, so setting coherent, is not that obvious?
No.
Alex
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/18/2013 08:19:03 AM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal RAM and the mapping is set to M bit (coherent, cacheable)
otherwise this is treated as I/O and we set I + G (cache
inhibited, guarded)
This helps setting proper TLB mapping for direct assigned device
Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
---
v2: some cleanup and added comment
-
arch/powerpc/kvm/e500_mmu_host.c | 23 ++-
1 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..02eb973 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,26 @@ static inline u32 e500_shadow_mas3_attrib(u32
mas3, int usermode)
return mas3;
}
-static inline u32 e500_shadow_mas2_attrib(u32 mas2, int usermode)
+static inline u32 e500_shadow_mas2_attrib(u32 mas2, pfn_t pfn)
{
+ u32 mas2_attr;
+
+ mas2_attr = mas2 MAS2_ATTRIB_MASK;
+
+ /*
+* RAM is always mappable on e500 systems, so this is identical
+* to kvm_is_mmio_pfn(), just without its overhead.
+*/
+ if (!pfn_valid(pfn)) {
Please use page_is_ram(), which is what gets used when setting the WIMG
for the host userspace mapping. We want to make sure the two are
consistent.
+ /* Pages not managed by Kernel are treated as I/O, set
I + G */
+ mas2_attr |= MAS2_I | MAS2_G;
#ifdef CONFIG_SMP
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ } else {
+ /* Kernel managed pages are actually RAM so set M */
+ mas2_attr |= MAS2_M;
#endif
Likewise, we want to make sure this matches the host entry.
Unfortunately, this is a bit of a mess already. 64-bit booke appears
to always set MAS2_M for TLB0 mappings. The initial KERNELBASE mapping
on boot uses M_IF_SMP, and the settlbcam() that (IIRC) replaces it uses
_PAGE_COHERENT. 32-bit always uses _PAGE_COHERENT, except that initial
KERNELBASE mapping. _PAGE_COHERENT appears to be set based on
CONFIG_SMP || CONFIG_PPC_STD_MMU (the latter config clears
_PAGE_COHERENT in the non-CPU_FTR_NEED_COHERENT case).
As for what we actually want to happen, there are cases when we want M
to be set for non-SMP. One such case is AMP, where CPUs may be sharing
memory even if the Linux instance only runs on one CPU (this is not
hypothetical, BTW). It's also possible that we encounter a hardware
bug that requires MAS2_M, similar to what some of our non-booke chips
require.
-Scott
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages
On 18.07.2013, at 19:17, Scott Wood wrote:
On 07/18/2013 08:19:03 AM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal RAM and the mapping is set to M bit (coherent, cacheable)
otherwise this is treated as I/O and we set I + G (cache inhibited,
guarded)
This helps setting proper TLB mapping for direct assigned device
Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
---
v2: some cleanup and added comment
-
arch/powerpc/kvm/e500_mmu_host.c | 23 ++-
1 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..02eb973 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,26 @@ static inline u32 e500_shadow_mas3_attrib(u32 mas3, int
usermode)
return mas3;
}
-static inline u32 e500_shadow_mas2_attrib(u32 mas2, int usermode)
+static inline u32 e500_shadow_mas2_attrib(u32 mas2, pfn_t pfn)
{
+u32 mas2_attr;
+
+mas2_attr = mas2 MAS2_ATTRIB_MASK;
+
+/*
+ * RAM is always mappable on e500 systems, so this is identical
+ * to kvm_is_mmio_pfn(), just without its overhead.
+ */
+if (!pfn_valid(pfn)) {
Please use page_is_ram(), which is what gets used when setting the WIMG for
the host userspace mapping. We want to make sure the two are consistent.
+/* Pages not managed by Kernel are treated as I/O, set I + G */
+mas2_attr |= MAS2_I | MAS2_G;
#ifdef CONFIG_SMP
-return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
-return mas2 MAS2_ATTRIB_MASK;
+} else {
+/* Kernel managed pages are actually RAM so set M */
+mas2_attr |= MAS2_M;
#endif
Likewise, we want to make sure this matches the host entry. Unfortunately,
this is a bit of a mess already. 64-bit booke appears to always set MAS2_M
for TLB0 mappings. The initial KERNELBASE mapping on boot uses M_IF_SMP, and
the settlbcam() that (IIRC) replaces it uses _PAGE_COHERENT. 32-bit always
uses _PAGE_COHERENT, except that initial KERNELBASE mapping. _PAGE_COHERENT
appears to be set based on CONFIG_SMP || CONFIG_PPC_STD_MMU (the latter
config clears _PAGE_COHERENT in the non-CPU_FTR_NEED_COHERENT case).
As for what we actually want to happen, there are cases when we want M to be
set for non-SMP. One such case is AMP, where CPUs may be sharing memory even
if the Linux instance only runs on one CPU (this is not hypothetical, BTW).
It's also possible that we encounter a hardware bug that requires MAS2_M,
similar to what some of our non-booke chips require.
How about we always set M then for RAM?
Alex
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/18/2013 12:32:18 PM, Alexander Graf wrote: On 18.07.2013, at 19:17, Scott Wood wrote: On 07/18/2013 08:19:03 AM, Bharat Bhushan wrote: Likewise, we want to make sure this matches the host entry. Unfortunately, this is a bit of a mess already. 64-bit booke appears to always set MAS2_M for TLB0 mappings. The initial KERNELBASE mapping on boot uses M_IF_SMP, and the settlbcam() that (IIRC) replaces it uses _PAGE_COHERENT. 32-bit always uses _PAGE_COHERENT, except that initial KERNELBASE mapping. _PAGE_COHERENT appears to be set based on CONFIG_SMP || CONFIG_PPC_STD_MMU (the latter config clears _PAGE_COHERENT in the non-CPU_FTR_NEED_COHERENT case). As for what we actually want to happen, there are cases when we want M to be set for non-SMP. One such case is AMP, where CPUs may be sharing memory even if the Linux instance only runs on one CPU (this is not hypothetical, BTW). It's also possible that we encounter a hardware bug that requires MAS2_M, similar to what some of our non-booke chips require. How about we always set M then for RAM? M is like I in that bad things happen if you mix them. So we really want to match exactly what the rest of the kernel is doing. Plus, the performance penalty on some single-core chips can be pretty bad. -Scott -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages
If there is a struct page for the requested mapping then it's
normal RAM and the mapping is set to M bit (coherent, cacheable)
otherwise this is treated as I/O and we set I + G (cache inhibited, guarded)
This helps setting proper TLB mapping for direct assigned device
Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
---
v2: some cleanup and added comment
-
arch/powerpc/kvm/e500_mmu_host.c | 23 ++-
1 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..02eb973 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,26 @@ static inline u32 e500_shadow_mas3_attrib(u32 mas3, int
usermode)
return mas3;
}
-static inline u32 e500_shadow_mas2_attrib(u32 mas2, int usermode)
+static inline u32 e500_shadow_mas2_attrib(u32 mas2, pfn_t pfn)
{
+ u32 mas2_attr;
+
+ mas2_attr = mas2 MAS2_ATTRIB_MASK;
+
+ /*
+* RAM is always mappable on e500 systems, so this is identical
+* to kvm_is_mmio_pfn(), just without its overhead.
+*/
+ if (!pfn_valid(pfn)) {
+ /* Pages not managed by Kernel are treated as I/O, set I + G */
+ mas2_attr |= MAS2_I | MAS2_G;
#ifdef CONFIG_SMP
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ } else {
+ /* Kernel managed pages are actually RAM so set M */
+ mas2_attr |= MAS2_M;
#endif
+ }
+ return mas2_attr;
}
/*
@@ -313,7 +326,7 @@ static void kvmppc_e500_setup_stlbe(
/* Force IPROT=0 for all guest mappings. */
stlbe-mas1 = MAS1_TSIZE(tsize) | get_tlb_sts(gtlbe) | MAS1_VALID;
stlbe-mas2 = (gvaddr MAS2_EPN) |
- e500_shadow_mas2_attrib(gtlbe-mas2, pr);
+ e500_shadow_mas2_attrib(gtlbe-mas2, pfn);
stlbe-mas7_3 = ((u64)pfn PAGE_SHIFT) |
e500_shadow_mas3_attrib(gtlbe-mas7_3, pr);
--
1.7.0.4
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages
On 18.07.2013, at 15:19, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal RAM and the mapping is set to M bit (coherent, cacheable)
otherwise this is treated as I/O and we set I + G (cache inhibited,
guarded)
This helps setting proper TLB mapping for direct assigned device
Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
---
v2: some cleanup and added comment
-
arch/powerpc/kvm/e500_mmu_host.c | 23 ++-
1 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..02eb973 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,26 @@ static inline u32 e500_shadow_mas3_attrib(u32 mas3, int
usermode)
return mas3;
}
-static inline u32 e500_shadow_mas2_attrib(u32 mas2, int usermode)
+static inline u32 e500_shadow_mas2_attrib(u32 mas2, pfn_t pfn)
{
+ u32 mas2_attr;
+
+ mas2_attr = mas2 MAS2_ATTRIB_MASK;
+
+ /*
+ * RAM is always mappable on e500 systems, so this is identical
+ * to kvm_is_mmio_pfn(), just without its overhead.
+ */
+ if (!pfn_valid(pfn)) {
+ /* Pages not managed by Kernel are treated as I/O, set I + G */
Please also document the intermediate thought that I/O should be mapped
non-cached.
+ mas2_attr |= MAS2_I | MAS2_G;
#ifdef CONFIG_SMP
Please separate the SMP case out of the branch.
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ } else {
+ /* Kernel managed pages are actually RAM so set M */
This comment doesn't tell me why M can be set ;).
Alex
+ mas2_attr |= MAS2_M;
#endif
+ }
+ return mas2_attr;
}
/*
@@ -313,7 +326,7 @@ static void kvmppc_e500_setup_stlbe(
/* Force IPROT=0 for all guest mappings. */
stlbe-mas1 = MAS1_TSIZE(tsize) | get_tlb_sts(gtlbe) | MAS1_VALID;
stlbe-mas2 = (gvaddr MAS2_EPN) |
- e500_shadow_mas2_attrib(gtlbe-mas2, pr);
+ e500_shadow_mas2_attrib(gtlbe-mas2, pfn);
stlbe-mas7_3 = ((u64)pfn PAGE_SHIFT) |
e500_shadow_mas3_attrib(gtlbe-mas7_3, pr);
--
1.7.0.4
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages
On 18.07.2013, at 17:15, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-ow...@vger.kernel.org] On
Behalf Of Alexander Graf
Sent: Thursday, July 18, 2013 8:23 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Bhushan
Bharat-R65777
Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel
managed pages
On 18.07.2013, at 15:19, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's normal
RAM and the mapping is set to M bit (coherent, cacheable) otherwise
this is treated as I/O and we set I + G (cache inhibited, guarded)
This helps setting proper TLB mapping for direct assigned device
Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
---
v2: some cleanup and added comment
-
arch/powerpc/kvm/e500_mmu_host.c | 23 ++-
1 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..02eb973 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,26 @@ static inline u32 e500_shadow_mas3_attrib(u32 mas3, int
usermode)
return mas3;
}
-static inline u32 e500_shadow_mas2_attrib(u32 mas2, int usermode)
+static inline u32 e500_shadow_mas2_attrib(u32 mas2, pfn_t pfn)
{
+ u32 mas2_attr;
+
+ mas2_attr = mas2 MAS2_ATTRIB_MASK;
+
+ /*
+* RAM is always mappable on e500 systems, so this is identical
+* to kvm_is_mmio_pfn(), just without its overhead.
+*/
+ if (!pfn_valid(pfn)) {
+ /* Pages not managed by Kernel are treated as I/O, set I + G */
Please also document the intermediate thought that I/O should be mapped non-
cached.
I did not get what you mean to document?
Page snot managed by the kernel are treated as I/O. Map it
with disabled cache.
+ mas2_attr |= MAS2_I | MAS2_G;
#ifdef CONFIG_SMP
Please separate the SMP case out of the branch.
Really :) this was looking simple to me.
Two branches intertwined never look simple :).
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ } else {
+ /* Kernel managed pages are actually RAM so set M */
This comment doesn't tell me why M can be set ;).
RAM in SMP, so setting coherent, is not that obvious?
No.
Alex
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages
On 18.07.2013, at 19:17, Scott Wood wrote:
On 07/18/2013 08:19:03 AM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal RAM and the mapping is set to M bit (coherent, cacheable)
otherwise this is treated as I/O and we set I + G (cache inhibited,
guarded)
This helps setting proper TLB mapping for direct assigned device
Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
---
v2: some cleanup and added comment
-
arch/powerpc/kvm/e500_mmu_host.c | 23 ++-
1 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..02eb973 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,26 @@ static inline u32 e500_shadow_mas3_attrib(u32 mas3, int
usermode)
return mas3;
}
-static inline u32 e500_shadow_mas2_attrib(u32 mas2, int usermode)
+static inline u32 e500_shadow_mas2_attrib(u32 mas2, pfn_t pfn)
{
+u32 mas2_attr;
+
+mas2_attr = mas2 MAS2_ATTRIB_MASK;
+
+/*
+ * RAM is always mappable on e500 systems, so this is identical
+ * to kvm_is_mmio_pfn(), just without its overhead.
+ */
+if (!pfn_valid(pfn)) {
Please use page_is_ram(), which is what gets used when setting the WIMG for
the host userspace mapping. We want to make sure the two are consistent.
+/* Pages not managed by Kernel are treated as I/O, set I + G */
+mas2_attr |= MAS2_I | MAS2_G;
#ifdef CONFIG_SMP
-return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
-return mas2 MAS2_ATTRIB_MASK;
+} else {
+/* Kernel managed pages are actually RAM so set M */
+mas2_attr |= MAS2_M;
#endif
Likewise, we want to make sure this matches the host entry. Unfortunately,
this is a bit of a mess already. 64-bit booke appears to always set MAS2_M
for TLB0 mappings. The initial KERNELBASE mapping on boot uses M_IF_SMP, and
the settlbcam() that (IIRC) replaces it uses _PAGE_COHERENT. 32-bit always
uses _PAGE_COHERENT, except that initial KERNELBASE mapping. _PAGE_COHERENT
appears to be set based on CONFIG_SMP || CONFIG_PPC_STD_MMU (the latter
config clears _PAGE_COHERENT in the non-CPU_FTR_NEED_COHERENT case).
As for what we actually want to happen, there are cases when we want M to be
set for non-SMP. One such case is AMP, where CPUs may be sharing memory even
if the Linux instance only runs on one CPU (this is not hypothetical, BTW).
It's also possible that we encounter a hardware bug that requires MAS2_M,
similar to what some of our non-booke chips require.
How about we always set M then for RAM?
Alex
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
