Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/25/2013 03:50:42 AM, Gleb Natapov wrote: Why ppc uses page_is_ram() for mmap? How should I know? But looking at the function it does it only as a fallback if ppc_md.phys_mem_access_prot() is not provided. Making access to MMIO noncached as a safe fallback makes sense. There's only one current implementation of ppc_md.phys_mem_access_prot(), which is pci_phys_mem_access_prot(), which also uses page_is_ram(). If page_is_ram() returns false then it checks for write-combining PCI. But yes, we would want to call ppc_md.phys_mem_access_prot() if present. Copying from the host PTE would be ideal if doesn't come with a noticeable performance impact compared to other methods, but one way or another we want to be sure we match. It is also make sense to allow noncached access to reserved ram sometimes. Perhaps, but that's not KVM's decision to make. You should get the same result as if you mmaped it -- because QEMU already did and we need to be consistent. Not to mention the large page kernel mapping that will have been done on e500... -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] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/25/2013 03:50:42 AM, Gleb Natapov wrote: Why ppc uses page_is_ram() for mmap? How should I know? But looking at the function it does it only as a fallback if ppc_md.phys_mem_access_prot() is not provided. Making access to MMIO noncached as a safe fallback makes sense. There's only one current implementation of ppc_md.phys_mem_access_prot(), which is pci_phys_mem_access_prot(), which also uses page_is_ram(). If page_is_ram() returns false then it checks for write-combining PCI. But yes, we would want to call ppc_md.phys_mem_access_prot() if present. Copying from the host PTE would be ideal if doesn't come with a noticeable performance impact compared to other methods, but one way or another we want to be sure we match. It is also make sense to allow noncached access to reserved ram sometimes. Perhaps, but that's not KVM's decision to make. You should get the same result as if you mmaped it -- because QEMU already did and we need to be consistent. Not to mention the large page kernel mapping that will have been done on e500... -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] kvm: powerpc: set cache coherency only for kernel managed pages
On Wed, Jul 24, 2013 at 03:32:49PM -0500, Scott Wood wrote: On 07/24/2013 04:39:59 AM, Alexander Graf wrote: On 24.07.2013, at 11:35, Gleb Natapov wrote: On Wed, Jul 24, 2013 at 11:21:11AM +0200, Alexander Graf wrote: Are not we going to use page_is_ram() from e500_shadow_mas2_attrib() as Scott commented? rWhy aren't we using page_is_ram() in kvm_is_mmio_pfn()? Because it is much slower and, IIRC, actually used to build pfn map that allow us to check quickly for valid pfn. Then why should we use page_is_ram()? :) I really don't want the e500 code to diverge too much from what the rest of the kvm code is doing. I don't understand actually used to build pfn map What code is this? I don't see any calls to page_is_ram() in the KVM code, or in generic mm code. Is this a statement about what x86 does? It may be not page_is_ram() directly, but the same into page_is_ram() is using. On power both page_is_ram() and do_init_bootmem() walks some kind of memblock_region data structure. What important is that pfn_valid() does not mean that there is a memory behind page structure. See Andrea's reply. On PPC page_is_ram() is only called (AFAICT) for determining what attributes to set on mmaps. We want to be sure that KVM always makes the same decision. While pfn_valid() seems like it should be equivalent, it's not obvious from the PPC code that it is. Again pfn_valid() is not enough. If pfn_valid() is better, why is that not used for mmap? Why are there two different names for the same thing? They are not the same thing. page_is_ram() tells you if phys address is ram backed. pfn_valid() tells you if there is struct page behind the pfn. PageReserved() tells if you a pfn is marked as reserved. All non ram pfns should be reserved, but ram pfns can be reserved too. Again, see Andrea's reply. Why ppc uses page_is_ram() for mmap? How should I know? But looking at the function it does it only as a fallback if ppc_md.phys_mem_access_prot() is not provided. Making access to MMIO noncached as a safe fallback makes sense. It is also make sense to allow noncached access to reserved ram sometimes. -- Gleb. -- 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] kvm: powerpc: set cache coherency only for kernel managed pages
On 25.07.2013, at 10:50, Gleb Natapov wrote: On Wed, Jul 24, 2013 at 03:32:49PM -0500, Scott Wood wrote: On 07/24/2013 04:39:59 AM, Alexander Graf wrote: On 24.07.2013, at 11:35, Gleb Natapov wrote: On Wed, Jul 24, 2013 at 11:21:11AM +0200, Alexander Graf wrote: Are not we going to use page_is_ram() from e500_shadow_mas2_attrib() as Scott commented? rWhy aren't we using page_is_ram() in kvm_is_mmio_pfn()? Because it is much slower and, IIRC, actually used to build pfn map that allow us to check quickly for valid pfn. Then why should we use page_is_ram()? :) I really don't want the e500 code to diverge too much from what the rest of the kvm code is doing. I don't understand actually used to build pfn map What code is this? I don't see any calls to page_is_ram() in the KVM code, or in generic mm code. Is this a statement about what x86 does? It may be not page_is_ram() directly, but the same into page_is_ram() is using. On power both page_is_ram() and do_init_bootmem() walks some kind of memblock_region data structure. What important is that pfn_valid() does not mean that there is a memory behind page structure. See Andrea's reply. On PPC page_is_ram() is only called (AFAICT) for determining what attributes to set on mmaps. We want to be sure that KVM always makes the same decision. While pfn_valid() seems like it should be equivalent, it's not obvious from the PPC code that it is. Again pfn_valid() is not enough. If pfn_valid() is better, why is that not used for mmap? Why are there two different names for the same thing? They are not the same thing. page_is_ram() tells you if phys address is ram backed. pfn_valid() tells you if there is struct page behind the pfn. PageReserved() tells if you a pfn is marked as reserved. All non ram pfns should be reserved, but ram pfns can be reserved too. Again, see Andrea's reply. Why ppc uses page_is_ram() for mmap? How should I know? But looking at That one's easy. Let's just ask Ben. Ben, is there any particular reason PPC uses page_is_ram() rather than what KVM does here to figure out whether a pfn is RAM or not? It would be really useful to be able to run the exact same logic that figures out whether we're cacheable or not in both TLB writers (KVM and linux-mm). Alex the function it does it only as a fallback if ppc_md.phys_mem_access_prot() is not provided. Making access to MMIO noncached as a safe fallback makes sense. It is also make sense to allow noncached access to reserved ram sometimes. -- Gleb. -- 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] kvm: powerpc: set cache coherency only for kernel managed pages
On Thu, Jul 25, 2013 at 06:07:55PM +0200, Alexander Graf wrote: On 25.07.2013, at 10:50, Gleb Natapov wrote: On Wed, Jul 24, 2013 at 03:32:49PM -0500, Scott Wood wrote: On 07/24/2013 04:39:59 AM, Alexander Graf wrote: On 24.07.2013, at 11:35, Gleb Natapov wrote: On Wed, Jul 24, 2013 at 11:21:11AM +0200, Alexander Graf wrote: Are not we going to use page_is_ram() from e500_shadow_mas2_attrib() as Scott commented? rWhy aren't we using page_is_ram() in kvm_is_mmio_pfn()? Because it is much slower and, IIRC, actually used to build pfn map that allow us to check quickly for valid pfn. Then why should we use page_is_ram()? :) I really don't want the e500 code to diverge too much from what the rest of the kvm code is doing. I don't understand actually used to build pfn map What code is this? I don't see any calls to page_is_ram() in the KVM code, or in generic mm code. Is this a statement about what x86 does? It may be not page_is_ram() directly, but the same into page_is_ram() is using. On power both page_is_ram() and do_init_bootmem() walks some kind of memblock_region data structure. What important is that pfn_valid() does not mean that there is a memory behind page structure. See Andrea's reply. On PPC page_is_ram() is only called (AFAICT) for determining what attributes to set on mmaps. We want to be sure that KVM always makes the same decision. While pfn_valid() seems like it should be equivalent, it's not obvious from the PPC code that it is. Again pfn_valid() is not enough. If pfn_valid() is better, why is that not used for mmap? Why are there two different names for the same thing? They are not the same thing. page_is_ram() tells you if phys address is ram backed. pfn_valid() tells you if there is struct page behind the pfn. PageReserved() tells if you a pfn is marked as reserved. All non ram pfns should be reserved, but ram pfns can be reserved too. Again, see Andrea's reply. Why ppc uses page_is_ram() for mmap? How should I know? But looking at That one's easy. Let's just ask Ben. Ben, is there any particular reason PPC uses page_is_ram() rather than what KVM does here to figure out whether a pfn is RAM or not? It would be really useful to be able to run the exact same logic that figures out whether we're cacheable or not in both TLB writers (KVM and linux-mm). KVM does not only try to figure out what is RAM or not! Look at how KVM uses the function. KVM tries to figure out if refcounting needed to be used on this page among other things. -- Gleb. -- 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] kvm: powerpc: set cache coherency only for kernel managed pages
On 25.07.2013, at 10:50, Gleb Natapov wrote: On Wed, Jul 24, 2013 at 03:32:49PM -0500, Scott Wood wrote: On 07/24/2013 04:39:59 AM, Alexander Graf wrote: On 24.07.2013, at 11:35, Gleb Natapov wrote: On Wed, Jul 24, 2013 at 11:21:11AM +0200, Alexander Graf wrote: Are not we going to use page_is_ram() from e500_shadow_mas2_attrib() as Scott commented? rWhy aren't we using page_is_ram() in kvm_is_mmio_pfn()? Because it is much slower and, IIRC, actually used to build pfn map that allow us to check quickly for valid pfn. Then why should we use page_is_ram()? :) I really don't want the e500 code to diverge too much from what the rest of the kvm code is doing. I don't understand actually used to build pfn map What code is this? I don't see any calls to page_is_ram() in the KVM code, or in generic mm code. Is this a statement about what x86 does? It may be not page_is_ram() directly, but the same into page_is_ram() is using. On power both page_is_ram() and do_init_bootmem() walks some kind of memblock_region data structure. What important is that pfn_valid() does not mean that there is a memory behind page structure. See Andrea's reply. On PPC page_is_ram() is only called (AFAICT) for determining what attributes to set on mmaps. We want to be sure that KVM always makes the same decision. While pfn_valid() seems like it should be equivalent, it's not obvious from the PPC code that it is. Again pfn_valid() is not enough. If pfn_valid() is better, why is that not used for mmap? Why are there two different names for the same thing? They are not the same thing. page_is_ram() tells you if phys address is ram backed. pfn_valid() tells you if there is struct page behind the pfn. PageReserved() tells if you a pfn is marked as reserved. All non ram pfns should be reserved, but ram pfns can be reserved too. Again, see Andrea's reply. Why ppc uses page_is_ram() for mmap? How should I know? But looking at That one's easy. Let's just ask Ben. Ben, is there any particular reason PPC uses page_is_ram() rather than what KVM does here to figure out whether a pfn is RAM or not? It would be really useful to be able to run the exact same logic that figures out whether we're cacheable or not in both TLB writers (KVM and linux-mm). Alex the function it does it only as a fallback if ppc_md.phys_mem_access_prot() is not provided. Making access to MMIO noncached as a safe fallback makes sense. It is also make sense to allow noncached access to reserved ram sometimes. -- Gleb. -- 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] kvm: powerpc: set cache coherency only for kernel managed pages
On Thu, Jul 25, 2013 at 06:07:55PM +0200, Alexander Graf wrote: On 25.07.2013, at 10:50, Gleb Natapov wrote: On Wed, Jul 24, 2013 at 03:32:49PM -0500, Scott Wood wrote: On 07/24/2013 04:39:59 AM, Alexander Graf wrote: On 24.07.2013, at 11:35, Gleb Natapov wrote: On Wed, Jul 24, 2013 at 11:21:11AM +0200, Alexander Graf wrote: Are not we going to use page_is_ram() from e500_shadow_mas2_attrib() as Scott commented? rWhy aren't we using page_is_ram() in kvm_is_mmio_pfn()? Because it is much slower and, IIRC, actually used to build pfn map that allow us to check quickly for valid pfn. Then why should we use page_is_ram()? :) I really don't want the e500 code to diverge too much from what the rest of the kvm code is doing. I don't understand actually used to build pfn map What code is this? I don't see any calls to page_is_ram() in the KVM code, or in generic mm code. Is this a statement about what x86 does? It may be not page_is_ram() directly, but the same into page_is_ram() is using. On power both page_is_ram() and do_init_bootmem() walks some kind of memblock_region data structure. What important is that pfn_valid() does not mean that there is a memory behind page structure. See Andrea's reply. On PPC page_is_ram() is only called (AFAICT) for determining what attributes to set on mmaps. We want to be sure that KVM always makes the same decision. While pfn_valid() seems like it should be equivalent, it's not obvious from the PPC code that it is. Again pfn_valid() is not enough. If pfn_valid() is better, why is that not used for mmap? Why are there two different names for the same thing? They are not the same thing. page_is_ram() tells you if phys address is ram backed. pfn_valid() tells you if there is struct page behind the pfn. PageReserved() tells if you a pfn is marked as reserved. All non ram pfns should be reserved, but ram pfns can be reserved too. Again, see Andrea's reply. Why ppc uses page_is_ram() for mmap? How should I know? But looking at That one's easy. Let's just ask Ben. Ben, is there any particular reason PPC uses page_is_ram() rather than what KVM does here to figure out whether a pfn is RAM or not? It would be really useful to be able to run the exact same logic that figures out whether we're cacheable or not in both TLB writers (KVM and linux-mm). KVM does not only try to figure out what is RAM or not! Look at how KVM uses the function. KVM tries to figure out if refcounting needed to be used on this page among other things. -- Gleb. -- 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] kvm: powerpc: set cache coherency only for kernel managed pages
On 24.07.2013, at 04:26, “tiejun.chen” wrote:
On 07/18/2013 06:27 PM, Alexander Graf wrote:
On 18.07.2013, at 12:19, “tiejun.chen” wrote:
On 07/18/2013 06:12 PM, Alexander Graf wrote:
On 18.07.2013, at 12:08, “tiejun.chen” wrote:
On 07/18/2013 05:48 PM, Alexander Graf wrote:
On 18.07.2013, at 10:25, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Bhushan Bharat-R65777
Sent: Thursday, July 18, 2013 1:53 PM
To: '�tiejun.chen�'
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel
managed pages
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of �tiejun.chen�
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is
that it
returns false when the page is managed by kernel and is not
marked as RESERVED (for some reason). For us it does not matter
whether the page is reserved or not, if it is kernel visible page
then it
is DDR.
I think you are setting I|G by addressing all mmio pages, right? If
so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio
pages rather than
normal RAM. In some cases it is not enough to identify these
mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as
well.
Do you know what are those some cases and how checking
PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this should
be chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
The reason is , kvm_is_mmio_pfn() function looks pretty heavy and I do
not want to call this for all tlbwe operation unless it is necessary.
It certainly does more than we need and potentially slows down the fast
path (RAM mapping). The only thing it does on top of if (pfn_valid())
is to check for pages that are declared reserved on the host. This
happens in 2 cases:
1) Non cache coherent DMA
2) Memory hot remove
The non coherent DMA case would be interesting, as with the mechanism as
it is in place in Linux today, we could potentially break normal guest
operation if we don't take it into account. However, it's Kconfig
guarded by:
depends on 4xx || 8xx || E200 || PPC_MPC512x || GAMECUBE_COMMON
default n if PPC_47x
default y
so we never hit it with any core we care about ;).
Memory hot remove does not exist on e500 FWIW, so we don't have to worry
about that one either.
Thanks for this good information :)
So why not limit those codes with CONFIG_MEMORY_HOTPLUG inside
kvm_is_mmio_pfn() to make sure that check is only valid when that is
really needed
RE: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages
-Original Message-
From: Alexander Graf [mailto:ag...@suse.de]
Sent: Wednesday, July 24, 2013 1:55 PM
To: “tiejun.chen”
Cc: Bhushan Bharat-R65777; kvm-...@vger.kernel.org; kvm@vger.kernel.org list;
Wood Scott-B07421; Gleb Natapov; Paolo Bonzini
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
On 24.07.2013, at 04:26, “tiejun.chen” wrote:
On 07/18/2013 06:27 PM, Alexander Graf wrote:
On 18.07.2013, at 12:19, “tiejun.chen” wrote:
On 07/18/2013 06:12 PM, Alexander Graf wrote:
On 18.07.2013, at 12:08, “tiejun.chen” wrote:
On 07/18/2013 05:48 PM, Alexander Graf wrote:
On 18.07.2013, at 10:25, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Bhushan Bharat-R65777
Sent: Thursday, July 18, 2013 1:53 PM
To: ' tiejun.chen '
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org;
ag...@suse.de; Wood Scott-
B07421
Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
-Original Message-
From: tiejun.chen [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org;
ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency
only for kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of tiejun.chen
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org;
ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency
only for kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: tiejun.chen [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org;
ag...@suse.de; Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency
only for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then
it's normal DDR and the mapping sets M bit (coherent,
cacheable) else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct
me) is that it
returns false when the page is managed by kernel and is
not marked as RESERVED (for some reason). For us it does not
matter whether the page is reserved or not, if it is kernel
visible page then it
is DDR.
I think you are setting I|G by addressing all mmio pages,
right? If so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by
mmio pages rather than
normal RAM. In some cases it is not enough to identify
these mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as
well.
Do you know what are those some cases and how checking
PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this
should be chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
The reason is , kvm_is_mmio_pfn() function looks pretty heavy and I do
not want to call this for all tlbwe operation unless it is necessary.
It certainly does more than we need and potentially slows down the fast
path (RAM mapping). The only thing it does on top of if (pfn_valid()) is to
check for pages that are declared reserved on the host. This happens in 2
cases:
1) Non cache coherent DMA
2) Memory hot remove
The non coherent DMA case would be interesting, as with the mechanism
as
it is in place in Linux today, we could potentially break normal guest
operation
if we don't take it into account. However, it's Kconfig guarded
Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages
On 24.07.2013, at 11:11, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Alexander Graf [mailto:ag...@suse.de]
Sent: Wednesday, July 24, 2013 1:55 PM
To: “tiejun.chen”
Cc: Bhushan Bharat-R65777; kvm-...@vger.kernel.org; kvm@vger.kernel.org list;
Wood Scott-B07421; Gleb Natapov; Paolo Bonzini
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
On 24.07.2013, at 04:26, “tiejun.chen” wrote:
On 07/18/2013 06:27 PM, Alexander Graf wrote:
On 18.07.2013, at 12:19, “tiejun.chen” wrote:
On 07/18/2013 06:12 PM, Alexander Graf wrote:
On 18.07.2013, at 12:08, “tiejun.chen” wrote:
On 07/18/2013 05:48 PM, Alexander Graf wrote:
On 18.07.2013, at 10:25, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Bhushan Bharat-R65777
Sent: Thursday, July 18, 2013 1:53 PM
To: ' tiejun.chen '
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org;
ag...@suse.de; Wood Scott-
B07421
Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
-Original Message-
From: tiejun.chen [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org;
ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency
only for kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of tiejun.chen
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org;
ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency
only for kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: tiejun.chen [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org;
ag...@suse.de; Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency
only for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then
it's normal DDR and the mapping sets M bit (coherent,
cacheable) else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct
me) is that it
returns false when the page is managed by kernel and is
not marked as RESERVED (for some reason). For us it does not
matter whether the page is reserved or not, if it is kernel
visible page then it
is DDR.
I think you are setting I|G by addressing all mmio pages,
right? If so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by
mmio pages rather than
normal RAM. In some cases it is not enough to identify
these mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as
well.
Do you know what are those some cases and how checking
PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this
should be chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
The reason is , kvm_is_mmio_pfn() function looks pretty heavy and I do
not want to call this for all tlbwe operation unless it is necessary.
It certainly does more than we need and potentially slows down the fast
path (RAM mapping). The only thing it does on top of if (pfn_valid()) is to
check for pages that are declared reserved on the host. This happens in 2
cases:
1) Non cache coherent DMA
2) Memory hot remove
The non coherent DMA case would be interesting, as with the mechanism
as
it is in place in Linux today, we could potentially break normal guest
operation
if we don't take it into account. However, it's Kconfig guarded by:
depends on 4xx || 8xx || E200 || PPC_MPC512x || GAMECUBE_COMMON
Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages
On Wed, Jul 24, 2013 at 11:21:11AM +0200, Alexander Graf wrote: Are not we going to use page_is_ram() from e500_shadow_mas2_attrib() as Scott commented? rWhy aren't we using page_is_ram() in kvm_is_mmio_pfn()? Because it is much slower and, IIRC, actually used to build pfn map that allow us to check quickly for valid pfn. -- Gleb. -- 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] kvm: powerpc: set cache coherency only for kernel managed pages
On 24.07.2013, at 11:35, Gleb Natapov wrote: On Wed, Jul 24, 2013 at 11:21:11AM +0200, Alexander Graf wrote: Are not we going to use page_is_ram() from e500_shadow_mas2_attrib() as Scott commented? rWhy aren't we using page_is_ram() in kvm_is_mmio_pfn()? Because it is much slower and, IIRC, actually used to build pfn map that allow us to check quickly for valid pfn. Then why should we use page_is_ram()? :) I really don't want the e500 code to diverge too much from what the rest of the kvm code is doing. 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] kvm: powerpc: set cache coherency only for kernel managed pages
Copying Andrea for him to verify that I am not talking nonsense :)
On Wed, Jul 24, 2013 at 10:25:20AM +0200, Alexander Graf wrote:
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 1580dd4..5e8635b 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -102,6 +102,10 @@ static bool largepages_enabled = true;
bool kvm_is_mmio_pfn(pfn_t pfn)
{
+#ifdef CONFIG_MEMORY_HOTPLUG
I'd feel safer if we narrow this down to e500.
+ /*
+* Currently only in memory hot remove case we may still need this.
+*/
if (pfn_valid(pfn)) {
We still have to check for pfn_valid, no? So the #ifdef should be down here.
int reserved;
struct page *tail = pfn_to_page(pfn);
@@ -124,6 +128,7 @@ bool kvm_is_mmio_pfn(pfn_t pfn)
}
return PageReserved(tail);
}
+#endif
return true;
}
Before apply this change:
real(1m19.954s + 1m20.918s + 1m22.740s + 1m21.146s + 1m22.120s)/5=
1m21.376s
user(0m23.181s + 0m23.550s + 0m23.506s + 0m23.410s + 0m23.520s)/5=
0m23.433s
sys (0m49.087s + 0m49.563s + 0m51.758s + 0m50.290s + 0m51.047s)/5= 0m50.349s
After apply this change:
real(1m19.507s + 1m20.919s + 1m21.436s + 1m21.179s + 1m20.293s)/5=
1m20.667s
user(0m22.595s + 0m22.719s + 0m22.484s + 0m22.811s + 0m22.467s)/5=
0m22.615s
sys (0m48.841s + 0m49.929s + 0m50.310s + 0m49.813s + 0m48.587s)/5= 0m49.496s
So,
real(1m20.667s - 1m21.376s)/1m21.376s x 100% = -0.6%
user(0m22.615s - 0m23.433s)/0m23.433s x 100% = -3.5%
sys (0m49.496s - 0m50.349s)/0m50.349s x 100% = -1.7%
Very nice, so there is a real world performance benefit to doing this. Then
yes, I think it would make sense to change the global helper function to be
fast on e500 and use that one from e500_shadow_mas2_attrib() instead.
Gleb, Paolo, any hard feelings?
I do not see how can we break the function in such a way and get
away with it. Not all valid pfns point to memory. Physical address can
be sparse (due to PCI hole, framebuffer or just because).
--
Gleb.
--
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] kvm: powerpc: set cache coherency only for kernel managed pages
On 24.07.2013, at 12:01, Gleb Natapov wrote:
Copying Andrea for him to verify that I am not talking nonsense :)
On Wed, Jul 24, 2013 at 10:25:20AM +0200, Alexander Graf wrote:
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 1580dd4..5e8635b 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -102,6 +102,10 @@ static bool largepages_enabled = true;
bool kvm_is_mmio_pfn(pfn_t pfn)
{
+#ifdef CONFIG_MEMORY_HOTPLUG
I'd feel safer if we narrow this down to e500.
+ /*
+* Currently only in memory hot remove case we may still need this.
+*/
if (pfn_valid(pfn)) {
We still have to check for pfn_valid, no? So the #ifdef should be down here.
int reserved;
struct page *tail = pfn_to_page(pfn);
@@ -124,6 +128,7 @@ bool kvm_is_mmio_pfn(pfn_t pfn)
}
return PageReserved(tail);
}
+#endif
return true;
}
Before apply this change:
real(1m19.954s + 1m20.918s + 1m22.740s + 1m21.146s + 1m22.120s)/5=
1m21.376s
user(0m23.181s + 0m23.550s + 0m23.506s + 0m23.410s + 0m23.520s)/5=
0m23.433s
sys (0m49.087s + 0m49.563s + 0m51.758s + 0m50.290s + 0m51.047s)/5= 0m50.349s
After apply this change:
real(1m19.507s + 1m20.919s + 1m21.436s + 1m21.179s + 1m20.293s)/5=
1m20.667s
user(0m22.595s + 0m22.719s + 0m22.484s + 0m22.811s + 0m22.467s)/5=
0m22.615s
sys (0m48.841s + 0m49.929s + 0m50.310s + 0m49.813s + 0m48.587s)/5= 0m49.496s
So,
real(1m20.667s - 1m21.376s)/1m21.376s x 100% = -0.6%
user(0m22.615s - 0m23.433s)/0m23.433s x 100% = -3.5%
sys (0m49.496s - 0m50.349s)/0m50.349s x 100% = -1.7%
Very nice, so there is a real world performance benefit to doing this. Then
yes, I think it would make sense to change the global helper function to be
fast on e500 and use that one from e500_shadow_mas2_attrib() instead.
Gleb, Paolo, any hard feelings?
I do not see how can we break the function in such a way and get
away with it. Not all valid pfns point to memory. Physical address can
be sparse (due to PCI hole, framebuffer or just because).
But we don't check for sparseness today in here either. We merely check for
incomplete huge pages.
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] kvm: powerpc: set cache coherency only for kernel managed pages
On Wed, Jul 24, 2013 at 12:09:42PM +0200, Alexander Graf wrote:
On 24.07.2013, at 12:01, Gleb Natapov wrote:
Copying Andrea for him to verify that I am not talking nonsense :)
On Wed, Jul 24, 2013 at 10:25:20AM +0200, Alexander Graf wrote:
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 1580dd4..5e8635b 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -102,6 +102,10 @@ static bool largepages_enabled = true;
bool kvm_is_mmio_pfn(pfn_t pfn)
{
+#ifdef CONFIG_MEMORY_HOTPLUG
I'd feel safer if we narrow this down to e500.
+ /*
+* Currently only in memory hot remove case we may still need
this.
+*/
if (pfn_valid(pfn)) {
We still have to check for pfn_valid, no? So the #ifdef should be down
here.
int reserved;
struct page *tail = pfn_to_page(pfn);
@@ -124,6 +128,7 @@ bool kvm_is_mmio_pfn(pfn_t pfn)
}
return PageReserved(tail);
}
+#endif
return true;
}
Before apply this change:
real(1m19.954s + 1m20.918s + 1m22.740s + 1m21.146s + 1m22.120s)/5=
1m21.376s
user(0m23.181s + 0m23.550s + 0m23.506s + 0m23.410s + 0m23.520s)/5=
0m23.433s
sys (0m49.087s + 0m49.563s + 0m51.758s + 0m50.290s + 0m51.047s)/5=
0m50.349s
After apply this change:
real(1m19.507s + 1m20.919s + 1m21.436s + 1m21.179s + 1m20.293s)/5=
1m20.667s
user(0m22.595s + 0m22.719s + 0m22.484s + 0m22.811s + 0m22.467s)/5=
0m22.615s
sys (0m48.841s + 0m49.929s + 0m50.310s + 0m49.813s + 0m48.587s)/5=
0m49.496s
So,
real(1m20.667s - 1m21.376s)/1m21.376s x 100% = -0.6%
user(0m22.615s - 0m23.433s)/0m23.433s x 100% = -3.5%
sys (0m49.496s - 0m50.349s)/0m50.349s x 100% = -1.7%
Very nice, so there is a real world performance benefit to doing this.
Then yes, I think it would make sense to change the global helper function
to be fast on e500 and use that one from e500_shadow_mas2_attrib() instead.
Gleb, Paolo, any hard feelings?
I do not see how can we break the function in such a way and get
away with it. Not all valid pfns point to memory. Physical address can
be sparse (due to PCI hole, framebuffer or just because).
But we don't check for sparseness today in here either. We merely check for
incomplete huge pages.
That's not how I read the code. The code checks for reserved flag set.
It should be set on pfns that point to memory holes. As far as I
understand huge page tricks they are there to guaranty that THP does not
change flags under us, Andrea?
--
Gleb.
--
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] kvm: powerpc: set cache coherency only for kernel managed pages
On 24.07.2013, at 12:19, Gleb Natapov wrote:
On Wed, Jul 24, 2013 at 12:09:42PM +0200, Alexander Graf wrote:
On 24.07.2013, at 12:01, Gleb Natapov wrote:
Copying Andrea for him to verify that I am not talking nonsense :)
On Wed, Jul 24, 2013 at 10:25:20AM +0200, Alexander Graf wrote:
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 1580dd4..5e8635b 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -102,6 +102,10 @@ static bool largepages_enabled = true;
bool kvm_is_mmio_pfn(pfn_t pfn)
{
+#ifdef CONFIG_MEMORY_HOTPLUG
I'd feel safer if we narrow this down to e500.
+ /*
+* Currently only in memory hot remove case we may still need
this.
+*/
if (pfn_valid(pfn)) {
We still have to check for pfn_valid, no? So the #ifdef should be down
here.
int reserved;
struct page *tail = pfn_to_page(pfn);
@@ -124,6 +128,7 @@ bool kvm_is_mmio_pfn(pfn_t pfn)
}
return PageReserved(tail);
}
+#endif
return true;
}
Before apply this change:
real(1m19.954s + 1m20.918s + 1m22.740s + 1m21.146s + 1m22.120s)/5=
1m21.376s
user(0m23.181s + 0m23.550s + 0m23.506s + 0m23.410s + 0m23.520s)/5=
0m23.433s
sys (0m49.087s + 0m49.563s + 0m51.758s + 0m50.290s + 0m51.047s)/5=
0m50.349s
After apply this change:
real(1m19.507s + 1m20.919s + 1m21.436s + 1m21.179s + 1m20.293s)/5=
1m20.667s
user(0m22.595s + 0m22.719s + 0m22.484s + 0m22.811s + 0m22.467s)/5=
0m22.615s
sys (0m48.841s + 0m49.929s + 0m50.310s + 0m49.813s + 0m48.587s)/5=
0m49.496s
So,
real(1m20.667s - 1m21.376s)/1m21.376s x 100% = -0.6%
user(0m22.615s - 0m23.433s)/0m23.433s x 100% = -3.5%
sys (0m49.496s - 0m50.349s)/0m50.349s x 100% = -1.7%
Very nice, so there is a real world performance benefit to doing this.
Then yes, I think it would make sense to change the global helper function
to be fast on e500 and use that one from e500_shadow_mas2_attrib() instead.
Gleb, Paolo, any hard feelings?
I do not see how can we break the function in such a way and get
away with it. Not all valid pfns point to memory. Physical address can
be sparse (due to PCI hole, framebuffer or just because).
But we don't check for sparseness today in here either. We merely check for
incomplete huge pages.
That's not how I read the code. The code checks for reserved flag set.
It should be set on pfns that point to memory holes. As far as I
I couldn't find any traces of code that sets the reserved bits on e500 chips
though. I've only seen it getting set for memory hotplug and memory incoherent
DMA code which doesn't get used on e500.
But I'd be more than happy to get proven wrong :).
Alex
understand huge page tricks they are there to guaranty that THP does not
change flags under us, Andrea?
--
Gleb.
--
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] kvm: powerpc: set cache coherency only for kernel managed pages
On Wed, Jul 24, 2013 at 12:25:18PM +0200, Alexander Graf wrote:
On 24.07.2013, at 12:19, Gleb Natapov wrote:
On Wed, Jul 24, 2013 at 12:09:42PM +0200, Alexander Graf wrote:
On 24.07.2013, at 12:01, Gleb Natapov wrote:
Copying Andrea for him to verify that I am not talking nonsense :)
On Wed, Jul 24, 2013 at 10:25:20AM +0200, Alexander Graf wrote:
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 1580dd4..5e8635b 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -102,6 +102,10 @@ static bool largepages_enabled = true;
bool kvm_is_mmio_pfn(pfn_t pfn)
{
+#ifdef CONFIG_MEMORY_HOTPLUG
I'd feel safer if we narrow this down to e500.
+ /*
+* Currently only in memory hot remove case we may still need
this.
+*/
if (pfn_valid(pfn)) {
We still have to check for pfn_valid, no? So the #ifdef should be down
here.
int reserved;
struct page *tail = pfn_to_page(pfn);
@@ -124,6 +128,7 @@ bool kvm_is_mmio_pfn(pfn_t pfn)
}
return PageReserved(tail);
}
+#endif
return true;
}
Before apply this change:
real(1m19.954s + 1m20.918s + 1m22.740s + 1m21.146s + 1m22.120s)/5=
1m21.376s
user(0m23.181s + 0m23.550s + 0m23.506s + 0m23.410s + 0m23.520s)/5=
0m23.433s
sys (0m49.087s + 0m49.563s + 0m51.758s + 0m50.290s + 0m51.047s)/5=
0m50.349s
After apply this change:
real(1m19.507s + 1m20.919s + 1m21.436s + 1m21.179s + 1m20.293s)/5=
1m20.667s
user(0m22.595s + 0m22.719s + 0m22.484s + 0m22.811s + 0m22.467s)/5=
0m22.615s
sys (0m48.841s + 0m49.929s + 0m50.310s + 0m49.813s + 0m48.587s)/5=
0m49.496s
So,
real(1m20.667s - 1m21.376s)/1m21.376s x 100% = -0.6%
user(0m22.615s - 0m23.433s)/0m23.433s x 100% = -3.5%
sys (0m49.496s - 0m50.349s)/0m50.349s x 100% = -1.7%
Very nice, so there is a real world performance benefit to doing this.
Then yes, I think it would make sense to change the global helper
function to be fast on e500 and use that one from
e500_shadow_mas2_attrib() instead.
Gleb, Paolo, any hard feelings?
I do not see how can we break the function in such a way and get
away with it. Not all valid pfns point to memory. Physical address can
be sparse (due to PCI hole, framebuffer or just because).
But we don't check for sparseness today in here either. We merely check
for incomplete huge pages.
That's not how I read the code. The code checks for reserved flag set.
It should be set on pfns that point to memory holes. As far as I
I couldn't find any traces of code that sets the reserved bits on e500 chips
though. I've only seen it getting set for memory hotplug and memory
incoherent DMA code which doesn't get used on e500.
But I'd be more than happy to get proven wrong :).
Can you write a module that scans all page structures? AFAIK all pages
are marked as reserved and then those that become regular memory are
marked as unreserved. Hope Andrea will chime in here :)
--
Gleb.
--
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] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/24/2013 04:39:59 AM, Alexander Graf wrote: On 24.07.2013, at 11:35, Gleb Natapov wrote: On Wed, Jul 24, 2013 at 11:21:11AM +0200, Alexander Graf wrote: Are not we going to use page_is_ram() from e500_shadow_mas2_attrib() as Scott commented? rWhy aren't we using page_is_ram() in kvm_is_mmio_pfn()? Because it is much slower and, IIRC, actually used to build pfn map that allow us to check quickly for valid pfn. Then why should we use page_is_ram()? :) I really don't want the e500 code to diverge too much from what the rest of the kvm code is doing. I don't understand actually used to build pfn map What code is this? I don't see any calls to page_is_ram() in the KVM code, or in generic mm code. Is this a statement about what x86 does? On PPC page_is_ram() is only called (AFAICT) for determining what attributes to set on mmaps. We want to be sure that KVM always makes the same decision. While pfn_valid() seems like it should be equivalent, it's not obvious from the PPC code that it is. If pfn_valid() is better, why is that not used for mmap? Why are there two different names for the same thing? -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] kvm: powerpc: set cache coherency only for kernel managed pages
Hi!
On Wed, Jul 24, 2013 at 01:30:12PM +0300, Gleb Natapov wrote:
On Wed, Jul 24, 2013 at 12:25:18PM +0200, Alexander Graf wrote:
On 24.07.2013, at 12:19, Gleb Natapov wrote:
On Wed, Jul 24, 2013 at 12:09:42PM +0200, Alexander Graf wrote:
On 24.07.2013, at 12:01, Gleb Natapov wrote:
Copying Andrea for him to verify that I am not talking nonsense :)
On Wed, Jul 24, 2013 at 10:25:20AM +0200, Alexander Graf wrote:
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 1580dd4..5e8635b 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -102,6 +102,10 @@ static bool largepages_enabled = true;
bool kvm_is_mmio_pfn(pfn_t pfn)
{
+#ifdef CONFIG_MEMORY_HOTPLUG
I'd feel safer if we narrow this down to e500.
+ /*
+* Currently only in memory hot remove case we may still need
this.
+*/
if (pfn_valid(pfn)) {
We still have to check for pfn_valid, no? So the #ifdef should be down
here.
int reserved;
struct page *tail = pfn_to_page(pfn);
@@ -124,6 +128,7 @@ bool kvm_is_mmio_pfn(pfn_t pfn)
}
return PageReserved(tail);
}
+#endif
return true;
}
Before apply this change:
real(1m19.954s + 1m20.918s + 1m22.740s + 1m21.146s +
1m22.120s)/5= 1m21.376s
user(0m23.181s + 0m23.550s + 0m23.506s + 0m23.410s +
0m23.520s)/5= 0m23.433s
sys (0m49.087s + 0m49.563s + 0m51.758s + 0m50.290s + 0m51.047s)/5=
0m50.349s
After apply this change:
real(1m19.507s + 1m20.919s + 1m21.436s + 1m21.179s +
1m20.293s)/5= 1m20.667s
user(0m22.595s + 0m22.719s + 0m22.484s + 0m22.811s +
0m22.467s)/5= 0m22.615s
sys (0m48.841s + 0m49.929s + 0m50.310s + 0m49.813s + 0m48.587s)/5=
0m49.496s
So,
real(1m20.667s - 1m21.376s)/1m21.376s x 100% = -0.6%
user(0m22.615s - 0m23.433s)/0m23.433s x 100% = -3.5%
sys (0m49.496s - 0m50.349s)/0m50.349s x 100% = -1.7%
Very nice, so there is a real world performance benefit to doing this.
Then yes, I think it would make sense to change the global helper
function to be fast on e500 and use that one from
e500_shadow_mas2_attrib() instead.
Gleb, Paolo, any hard feelings?
I do not see how can we break the function in such a way and get
away with it. Not all valid pfns point to memory. Physical address can
be sparse (due to PCI hole, framebuffer or just because).
But we don't check for sparseness today in here either. We merely check
for incomplete huge pages.
That's not how I read the code. The code checks for reserved flag set.
It should be set on pfns that point to memory holes. As far as I
I couldn't find any traces of code that sets the reserved bits on e500
chips though. I've only seen it getting set for memory hotplug and memory
incoherent DMA code which doesn't get used on e500.
But I'd be more than happy to get proven wrong :).
Can you write a module that scans all page structures? AFAIK all pages
are marked as reserved and then those that become regular memory are
marked as unreserved. Hope Andrea will chime in here :)
So the situation with regard to non-RAM and PageReserved/pfn_valid is
quite simple.
struct page exists for non-RAM too as struct page must exist up to
at least 2^MAX_ORDER pfn alignment or things breaks, like the first
pfn must be 2^MXA_ORDER aligned or again things break in the buddy. We
don't make an effort to save a few struct page to keep it simpler.
But those non-RAM pages (or tiny non-RAM page holes if any) are marked
PageReserved.
If struct page doesn't exist pfn_valid returns false.
So you cannot get away skipping pfn_valid and at least one
PageReserved.
However it gets more complex than just ram vs non-RAM, because there
are pages that are real RAM (not left marked PageReserved at boot
after checking e820 or equivalent bios data for non-x86 archs) but
that are taken over by drivers, than then could use it as mmio regions
snooping the writes and mapping them in userland too as hugepages
maybe. That is the motivation for the THP related code in
kvm_is_mmio_pfn.
Those vmas have VM_PFNMAP set so vm_normal_page is zero and the
refcounting is skipped like if it's non-RAM and they're mapped with
remap_pfn_range (different mechanism for VM_MIXEDMAP that does the
refcounting and doesn't require in turn the driver to mark the page
PageReserved).
The above explains why KVM needs to skip the refcounting on
PageReserved == true pfn_valid() == true, and it must skip the
refcounting for pfn_valid == false without trying to call pfn_to_page
(or it'll crash).
Now the code doing the THP check with smp_rmb is very safe, possibly
too safe. Looking at it now, it looks a minor overengineering
oversight.
The slight oversight is that split_huge_page cannot transfer the
PG_reserved bit from
RE: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages
-Original Message-
From: Alexander Graf [mailto:ag...@suse.de]
Sent: Wednesday, July 24, 2013 1:55 PM
To: “tiejun.chen”
Cc: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org; k...@vger.kernel.org list;
Wood Scott-B07421; Gleb Natapov; Paolo Bonzini
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
On 24.07.2013, at 04:26, “tiejun.chen” wrote:
On 07/18/2013 06:27 PM, Alexander Graf wrote:
On 18.07.2013, at 12:19, “tiejun.chen” wrote:
On 07/18/2013 06:12 PM, Alexander Graf wrote:
On 18.07.2013, at 12:08, “tiejun.chen” wrote:
On 07/18/2013 05:48 PM, Alexander Graf wrote:
On 18.07.2013, at 10:25, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Bhushan Bharat-R65777
Sent: Thursday, July 18, 2013 1:53 PM
To: ' tiejun.chen '
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org;
ag...@suse.de; Wood Scott-
B07421
Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
-Original Message-
From: tiejun.chen [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org;
ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency
only for kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of tiejun.chen
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org;
ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency
only for kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: tiejun.chen [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org;
ag...@suse.de; Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency
only for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then
it's normal DDR and the mapping sets M bit (coherent,
cacheable) else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct
me) is that it
returns false when the page is managed by kernel and is
not marked as RESERVED (for some reason). For us it does not
matter whether the page is reserved or not, if it is kernel
visible page then it
is DDR.
I think you are setting I|G by addressing all mmio pages,
right? If so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by
mmio pages rather than
normal RAM. In some cases it is not enough to identify
these mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as
well.
Do you know what are those some cases and how checking
PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this
should be chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
The reason is , kvm_is_mmio_pfn() function looks pretty heavy and I do
not want to call this for all tlbwe operation unless it is necessary.
It certainly does more than we need and potentially slows down the fast
path (RAM mapping). The only thing it does on top of if (pfn_valid()) is to
check for pages that are declared reserved on the host. This happens in 2
cases:
1) Non cache coherent DMA
2) Memory hot remove
The non coherent DMA case would be interesting, as with the mechanism
as
it is in place in Linux today, we could potentially break normal guest
operation
if we don't take it into account. However, it's Kconfig guarded
Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages
On 24.07.2013, at 11:11, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Alexander Graf [mailto:ag...@suse.de]
Sent: Wednesday, July 24, 2013 1:55 PM
To: “tiejun.chen”
Cc: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org; k...@vger.kernel.org
list;
Wood Scott-B07421; Gleb Natapov; Paolo Bonzini
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
On 24.07.2013, at 04:26, “tiejun.chen” wrote:
On 07/18/2013 06:27 PM, Alexander Graf wrote:
On 18.07.2013, at 12:19, “tiejun.chen” wrote:
On 07/18/2013 06:12 PM, Alexander Graf wrote:
On 18.07.2013, at 12:08, “tiejun.chen” wrote:
On 07/18/2013 05:48 PM, Alexander Graf wrote:
On 18.07.2013, at 10:25, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Bhushan Bharat-R65777
Sent: Thursday, July 18, 2013 1:53 PM
To: ' tiejun.chen '
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org;
ag...@suse.de; Wood Scott-
B07421
Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
-Original Message-
From: tiejun.chen [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org;
ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency
only for kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of tiejun.chen
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org;
ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency
only for kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: tiejun.chen [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org;
ag...@suse.de; Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency
only for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then
it's normal DDR and the mapping sets M bit (coherent,
cacheable) else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct
me) is that it
returns false when the page is managed by kernel and is
not marked as RESERVED (for some reason). For us it does not
matter whether the page is reserved or not, if it is kernel
visible page then it
is DDR.
I think you are setting I|G by addressing all mmio pages,
right? If so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by
mmio pages rather than
normal RAM. In some cases it is not enough to identify
these mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as
well.
Do you know what are those some cases and how checking
PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this
should be chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
The reason is , kvm_is_mmio_pfn() function looks pretty heavy and I do
not want to call this for all tlbwe operation unless it is necessary.
It certainly does more than we need and potentially slows down the fast
path (RAM mapping). The only thing it does on top of if (pfn_valid()) is to
check for pages that are declared reserved on the host. This happens in 2
cases:
1) Non cache coherent DMA
2) Memory hot remove
The non coherent DMA case would be interesting, as with the mechanism
as
it is in place in Linux today, we could potentially break normal guest
operation
if we don't take it into account. However, it's Kconfig guarded by:
depends on 4xx || 8xx || E200 || PPC_MPC512x || GAMECUBE_COMMON
Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages
On 24.07.2013, at 11:35, Gleb Natapov wrote: On Wed, Jul 24, 2013 at 11:21:11AM +0200, Alexander Graf wrote: Are not we going to use page_is_ram() from e500_shadow_mas2_attrib() as Scott commented? rWhy aren't we using page_is_ram() in kvm_is_mmio_pfn()? Because it is much slower and, IIRC, actually used to build pfn map that allow us to check quickly for valid pfn. Then why should we use page_is_ram()? :) I really don't want the e500 code to diverge too much from what the rest of the kvm code is doing. 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] kvm: powerpc: set cache coherency only for kernel managed pages
Copying Andrea for him to verify that I am not talking nonsense :)
On Wed, Jul 24, 2013 at 10:25:20AM +0200, Alexander Graf wrote:
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 1580dd4..5e8635b 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -102,6 +102,10 @@ static bool largepages_enabled = true;
bool kvm_is_mmio_pfn(pfn_t pfn)
{
+#ifdef CONFIG_MEMORY_HOTPLUG
I'd feel safer if we narrow this down to e500.
+ /*
+* Currently only in memory hot remove case we may still need this.
+*/
if (pfn_valid(pfn)) {
We still have to check for pfn_valid, no? So the #ifdef should be down here.
int reserved;
struct page *tail = pfn_to_page(pfn);
@@ -124,6 +128,7 @@ bool kvm_is_mmio_pfn(pfn_t pfn)
}
return PageReserved(tail);
}
+#endif
return true;
}
Before apply this change:
real(1m19.954s + 1m20.918s + 1m22.740s + 1m21.146s + 1m22.120s)/5=
1m21.376s
user(0m23.181s + 0m23.550s + 0m23.506s + 0m23.410s + 0m23.520s)/5=
0m23.433s
sys (0m49.087s + 0m49.563s + 0m51.758s + 0m50.290s + 0m51.047s)/5= 0m50.349s
After apply this change:
real(1m19.507s + 1m20.919s + 1m21.436s + 1m21.179s + 1m20.293s)/5=
1m20.667s
user(0m22.595s + 0m22.719s + 0m22.484s + 0m22.811s + 0m22.467s)/5=
0m22.615s
sys (0m48.841s + 0m49.929s + 0m50.310s + 0m49.813s + 0m48.587s)/5= 0m49.496s
So,
real(1m20.667s - 1m21.376s)/1m21.376s x 100% = -0.6%
user(0m22.615s - 0m23.433s)/0m23.433s x 100% = -3.5%
sys (0m49.496s - 0m50.349s)/0m50.349s x 100% = -1.7%
Very nice, so there is a real world performance benefit to doing this. Then
yes, I think it would make sense to change the global helper function to be
fast on e500 and use that one from e500_shadow_mas2_attrib() instead.
Gleb, Paolo, any hard feelings?
I do not see how can we break the function in such a way and get
away with it. Not all valid pfns point to memory. Physical address can
be sparse (due to PCI hole, framebuffer or just because).
--
Gleb.
--
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] kvm: powerpc: set cache coherency only for kernel managed pages
On 24.07.2013, at 12:01, Gleb Natapov wrote:
Copying Andrea for him to verify that I am not talking nonsense :)
On Wed, Jul 24, 2013 at 10:25:20AM +0200, Alexander Graf wrote:
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 1580dd4..5e8635b 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -102,6 +102,10 @@ static bool largepages_enabled = true;
bool kvm_is_mmio_pfn(pfn_t pfn)
{
+#ifdef CONFIG_MEMORY_HOTPLUG
I'd feel safer if we narrow this down to e500.
+ /*
+* Currently only in memory hot remove case we may still need this.
+*/
if (pfn_valid(pfn)) {
We still have to check for pfn_valid, no? So the #ifdef should be down here.
int reserved;
struct page *tail = pfn_to_page(pfn);
@@ -124,6 +128,7 @@ bool kvm_is_mmio_pfn(pfn_t pfn)
}
return PageReserved(tail);
}
+#endif
return true;
}
Before apply this change:
real(1m19.954s + 1m20.918s + 1m22.740s + 1m21.146s + 1m22.120s)/5=
1m21.376s
user(0m23.181s + 0m23.550s + 0m23.506s + 0m23.410s + 0m23.520s)/5=
0m23.433s
sys (0m49.087s + 0m49.563s + 0m51.758s + 0m50.290s + 0m51.047s)/5= 0m50.349s
After apply this change:
real(1m19.507s + 1m20.919s + 1m21.436s + 1m21.179s + 1m20.293s)/5=
1m20.667s
user(0m22.595s + 0m22.719s + 0m22.484s + 0m22.811s + 0m22.467s)/5=
0m22.615s
sys (0m48.841s + 0m49.929s + 0m50.310s + 0m49.813s + 0m48.587s)/5= 0m49.496s
So,
real(1m20.667s - 1m21.376s)/1m21.376s x 100% = -0.6%
user(0m22.615s - 0m23.433s)/0m23.433s x 100% = -3.5%
sys (0m49.496s - 0m50.349s)/0m50.349s x 100% = -1.7%
Very nice, so there is a real world performance benefit to doing this. Then
yes, I think it would make sense to change the global helper function to be
fast on e500 and use that one from e500_shadow_mas2_attrib() instead.
Gleb, Paolo, any hard feelings?
I do not see how can we break the function in such a way and get
away with it. Not all valid pfns point to memory. Physical address can
be sparse (due to PCI hole, framebuffer or just because).
But we don't check for sparseness today in here either. We merely check for
incomplete huge pages.
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] kvm: powerpc: set cache coherency only for kernel managed pages
On Wed, Jul 24, 2013 at 12:09:42PM +0200, Alexander Graf wrote:
On 24.07.2013, at 12:01, Gleb Natapov wrote:
Copying Andrea for him to verify that I am not talking nonsense :)
On Wed, Jul 24, 2013 at 10:25:20AM +0200, Alexander Graf wrote:
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 1580dd4..5e8635b 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -102,6 +102,10 @@ static bool largepages_enabled = true;
bool kvm_is_mmio_pfn(pfn_t pfn)
{
+#ifdef CONFIG_MEMORY_HOTPLUG
I'd feel safer if we narrow this down to e500.
+ /*
+* Currently only in memory hot remove case we may still need
this.
+*/
if (pfn_valid(pfn)) {
We still have to check for pfn_valid, no? So the #ifdef should be down
here.
int reserved;
struct page *tail = pfn_to_page(pfn);
@@ -124,6 +128,7 @@ bool kvm_is_mmio_pfn(pfn_t pfn)
}
return PageReserved(tail);
}
+#endif
return true;
}
Before apply this change:
real(1m19.954s + 1m20.918s + 1m22.740s + 1m21.146s + 1m22.120s)/5=
1m21.376s
user(0m23.181s + 0m23.550s + 0m23.506s + 0m23.410s + 0m23.520s)/5=
0m23.433s
sys (0m49.087s + 0m49.563s + 0m51.758s + 0m50.290s + 0m51.047s)/5=
0m50.349s
After apply this change:
real(1m19.507s + 1m20.919s + 1m21.436s + 1m21.179s + 1m20.293s)/5=
1m20.667s
user(0m22.595s + 0m22.719s + 0m22.484s + 0m22.811s + 0m22.467s)/5=
0m22.615s
sys (0m48.841s + 0m49.929s + 0m50.310s + 0m49.813s + 0m48.587s)/5=
0m49.496s
So,
real(1m20.667s - 1m21.376s)/1m21.376s x 100% = -0.6%
user(0m22.615s - 0m23.433s)/0m23.433s x 100% = -3.5%
sys (0m49.496s - 0m50.349s)/0m50.349s x 100% = -1.7%
Very nice, so there is a real world performance benefit to doing this.
Then yes, I think it would make sense to change the global helper function
to be fast on e500 and use that one from e500_shadow_mas2_attrib() instead.
Gleb, Paolo, any hard feelings?
I do not see how can we break the function in such a way and get
away with it. Not all valid pfns point to memory. Physical address can
be sparse (due to PCI hole, framebuffer or just because).
But we don't check for sparseness today in here either. We merely check for
incomplete huge pages.
That's not how I read the code. The code checks for reserved flag set.
It should be set on pfns that point to memory holes. As far as I
understand huge page tricks they are there to guaranty that THP does not
change flags under us, Andrea?
--
Gleb.
--
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] kvm: powerpc: set cache coherency only for kernel managed pages
On 24.07.2013, at 12:19, Gleb Natapov wrote:
On Wed, Jul 24, 2013 at 12:09:42PM +0200, Alexander Graf wrote:
On 24.07.2013, at 12:01, Gleb Natapov wrote:
Copying Andrea for him to verify that I am not talking nonsense :)
On Wed, Jul 24, 2013 at 10:25:20AM +0200, Alexander Graf wrote:
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 1580dd4..5e8635b 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -102,6 +102,10 @@ static bool largepages_enabled = true;
bool kvm_is_mmio_pfn(pfn_t pfn)
{
+#ifdef CONFIG_MEMORY_HOTPLUG
I'd feel safer if we narrow this down to e500.
+ /*
+* Currently only in memory hot remove case we may still need
this.
+*/
if (pfn_valid(pfn)) {
We still have to check for pfn_valid, no? So the #ifdef should be down
here.
int reserved;
struct page *tail = pfn_to_page(pfn);
@@ -124,6 +128,7 @@ bool kvm_is_mmio_pfn(pfn_t pfn)
}
return PageReserved(tail);
}
+#endif
return true;
}
Before apply this change:
real(1m19.954s + 1m20.918s + 1m22.740s + 1m21.146s + 1m22.120s)/5=
1m21.376s
user(0m23.181s + 0m23.550s + 0m23.506s + 0m23.410s + 0m23.520s)/5=
0m23.433s
sys (0m49.087s + 0m49.563s + 0m51.758s + 0m50.290s + 0m51.047s)/5=
0m50.349s
After apply this change:
real(1m19.507s + 1m20.919s + 1m21.436s + 1m21.179s + 1m20.293s)/5=
1m20.667s
user(0m22.595s + 0m22.719s + 0m22.484s + 0m22.811s + 0m22.467s)/5=
0m22.615s
sys (0m48.841s + 0m49.929s + 0m50.310s + 0m49.813s + 0m48.587s)/5=
0m49.496s
So,
real(1m20.667s - 1m21.376s)/1m21.376s x 100% = -0.6%
user(0m22.615s - 0m23.433s)/0m23.433s x 100% = -3.5%
sys (0m49.496s - 0m50.349s)/0m50.349s x 100% = -1.7%
Very nice, so there is a real world performance benefit to doing this.
Then yes, I think it would make sense to change the global helper function
to be fast on e500 and use that one from e500_shadow_mas2_attrib() instead.
Gleb, Paolo, any hard feelings?
I do not see how can we break the function in such a way and get
away with it. Not all valid pfns point to memory. Physical address can
be sparse (due to PCI hole, framebuffer or just because).
But we don't check for sparseness today in here either. We merely check for
incomplete huge pages.
That's not how I read the code. The code checks for reserved flag set.
It should be set on pfns that point to memory holes. As far as I
I couldn't find any traces of code that sets the reserved bits on e500 chips
though. I've only seen it getting set for memory hotplug and memory incoherent
DMA code which doesn't get used on e500.
But I'd be more than happy to get proven wrong :).
Alex
understand huge page tricks they are there to guaranty that THP does not
change flags under us, Andrea?
--
Gleb.
--
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] kvm: powerpc: set cache coherency only for kernel managed pages
On Wed, Jul 24, 2013 at 12:25:18PM +0200, Alexander Graf wrote:
On 24.07.2013, at 12:19, Gleb Natapov wrote:
On Wed, Jul 24, 2013 at 12:09:42PM +0200, Alexander Graf wrote:
On 24.07.2013, at 12:01, Gleb Natapov wrote:
Copying Andrea for him to verify that I am not talking nonsense :)
On Wed, Jul 24, 2013 at 10:25:20AM +0200, Alexander Graf wrote:
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 1580dd4..5e8635b 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -102,6 +102,10 @@ static bool largepages_enabled = true;
bool kvm_is_mmio_pfn(pfn_t pfn)
{
+#ifdef CONFIG_MEMORY_HOTPLUG
I'd feel safer if we narrow this down to e500.
+ /*
+* Currently only in memory hot remove case we may still need
this.
+*/
if (pfn_valid(pfn)) {
We still have to check for pfn_valid, no? So the #ifdef should be down
here.
int reserved;
struct page *tail = pfn_to_page(pfn);
@@ -124,6 +128,7 @@ bool kvm_is_mmio_pfn(pfn_t pfn)
}
return PageReserved(tail);
}
+#endif
return true;
}
Before apply this change:
real(1m19.954s + 1m20.918s + 1m22.740s + 1m21.146s + 1m22.120s)/5=
1m21.376s
user(0m23.181s + 0m23.550s + 0m23.506s + 0m23.410s + 0m23.520s)/5=
0m23.433s
sys (0m49.087s + 0m49.563s + 0m51.758s + 0m50.290s + 0m51.047s)/5=
0m50.349s
After apply this change:
real(1m19.507s + 1m20.919s + 1m21.436s + 1m21.179s + 1m20.293s)/5=
1m20.667s
user(0m22.595s + 0m22.719s + 0m22.484s + 0m22.811s + 0m22.467s)/5=
0m22.615s
sys (0m48.841s + 0m49.929s + 0m50.310s + 0m49.813s + 0m48.587s)/5=
0m49.496s
So,
real(1m20.667s - 1m21.376s)/1m21.376s x 100% = -0.6%
user(0m22.615s - 0m23.433s)/0m23.433s x 100% = -3.5%
sys (0m49.496s - 0m50.349s)/0m50.349s x 100% = -1.7%
Very nice, so there is a real world performance benefit to doing this.
Then yes, I think it would make sense to change the global helper
function to be fast on e500 and use that one from
e500_shadow_mas2_attrib() instead.
Gleb, Paolo, any hard feelings?
I do not see how can we break the function in such a way and get
away with it. Not all valid pfns point to memory. Physical address can
be sparse (due to PCI hole, framebuffer or just because).
But we don't check for sparseness today in here either. We merely check
for incomplete huge pages.
That's not how I read the code. The code checks for reserved flag set.
It should be set on pfns that point to memory holes. As far as I
I couldn't find any traces of code that sets the reserved bits on e500 chips
though. I've only seen it getting set for memory hotplug and memory
incoherent DMA code which doesn't get used on e500.
But I'd be more than happy to get proven wrong :).
Can you write a module that scans all page structures? AFAIK all pages
are marked as reserved and then those that become regular memory are
marked as unreserved. Hope Andrea will chime in here :)
--
Gleb.
--
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] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/24/2013 04:39:59 AM, Alexander Graf wrote: On 24.07.2013, at 11:35, Gleb Natapov wrote: On Wed, Jul 24, 2013 at 11:21:11AM +0200, Alexander Graf wrote: Are not we going to use page_is_ram() from e500_shadow_mas2_attrib() as Scott commented? rWhy aren't we using page_is_ram() in kvm_is_mmio_pfn()? Because it is much slower and, IIRC, actually used to build pfn map that allow us to check quickly for valid pfn. Then why should we use page_is_ram()? :) I really don't want the e500 code to diverge too much from what the rest of the kvm code is doing. I don't understand actually used to build pfn map What code is this? I don't see any calls to page_is_ram() in the KVM code, or in generic mm code. Is this a statement about what x86 does? On PPC page_is_ram() is only called (AFAICT) for determining what attributes to set on mmaps. We want to be sure that KVM always makes the same decision. While pfn_valid() seems like it should be equivalent, it's not obvious from the PPC code that it is. If pfn_valid() is better, why is that not used for mmap? Why are there two different names for the same thing? -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] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/18/2013 06:27 PM, Alexander Graf wrote:
On 18.07.2013, at 12:19, “tiejun.chen” wrote:
On 07/18/2013 06:12 PM, Alexander Graf wrote:
On 18.07.2013, at 12:08, “tiejun.chen” wrote:
On 07/18/2013 05:48 PM, Alexander Graf wrote:
On 18.07.2013, at 10:25, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Bhushan Bharat-R65777
Sent: Thursday, July 18, 2013 1:53 PM
To: '�tiejun.chen�'
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood Scott-
B07421
Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of �tiejun.chen�
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is
that it
returns false when the page is managed by kernel and is not
marked as RESERVED (for some reason). For us it does not matter
whether the page is reserved or not, if it is kernel visible page then it
is DDR.
I think you are setting I|G by addressing all mmio pages, right? If
so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio
pages rather than
normal RAM. In some cases it is not enough to identify these
mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as well.
Do you know what are those some cases and how checking
PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this should
be chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
The reason is , kvm_is_mmio_pfn() function looks pretty heavy and I do not want
to call this for all tlbwe operation unless it is necessary.
It certainly does more than we need and potentially slows down the fast path (RAM
mapping). The only thing it does on top of if (pfn_valid()) is to check for
pages that are declared reserved on the host. This happens in 2 cases:
1) Non cache coherent DMA
2) Memory hot remove
The non coherent DMA case would be interesting, as with the mechanism as it is
in place in Linux today, we could potentially break normal guest operation if
we don't take it into account. However, it's Kconfig guarded by:
depends on 4xx || 8xx || E200 || PPC_MPC512x || GAMECUBE_COMMON
default n if PPC_47x
default y
so we never hit it with any core we care about ;).
Memory hot remove does not exist on e500 FWIW, so we don't have to worry about
that one either.
Thanks for this good information :)
So why not limit those codes with CONFIG_MEMORY_HOTPLUG inside
kvm_is_mmio_pfn() to make sure that check is only valid when that is really
needed? This can decrease those unnecessary performance loss.
If I'm wrong please correct me :)
You're perfectly right, but this is generic KVM code. So it gets run across all
Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/18/2013 06:27 PM, Alexander Graf wrote:
On 18.07.2013, at 12:19, “tiejun.chen” wrote:
On 07/18/2013 06:12 PM, Alexander Graf wrote:
On 18.07.2013, at 12:08, “tiejun.chen” wrote:
On 07/18/2013 05:48 PM, Alexander Graf wrote:
On 18.07.2013, at 10:25, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Bhushan Bharat-R65777
Sent: Thursday, July 18, 2013 1:53 PM
To: '�tiejun.chen�'
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood Scott-
B07421
Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of �tiejun.chen�
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de;
Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de;
Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is
that it
returns false when the page is managed by kernel and is not
marked as RESERVED (for some reason). For us it does not matter
whether the page is reserved or not, if it is kernel visible page then it
is DDR.
I think you are setting I|G by addressing all mmio pages, right? If
so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio
pages rather than
normal RAM. In some cases it is not enough to identify these
mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as well.
Do you know what are those some cases and how checking
PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this should
be chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
The reason is , kvm_is_mmio_pfn() function looks pretty heavy and I do not want
to call this for all tlbwe operation unless it is necessary.
It certainly does more than we need and potentially slows down the fast path (RAM
mapping). The only thing it does on top of if (pfn_valid()) is to check for
pages that are declared reserved on the host. This happens in 2 cases:
1) Non cache coherent DMA
2) Memory hot remove
The non coherent DMA case would be interesting, as with the mechanism as it is
in place in Linux today, we could potentially break normal guest operation if
we don't take it into account. However, it's Kconfig guarded by:
depends on 4xx || 8xx || E200 || PPC_MPC512x || GAMECUBE_COMMON
default n if PPC_47x
default y
so we never hit it with any core we care about ;).
Memory hot remove does not exist on e500 FWIW, so we don't have to worry about
that one either.
Thanks for this good information :)
So why not limit those codes with CONFIG_MEMORY_HOTPLUG inside
kvm_is_mmio_pfn() to make sure that check is only valid when that is really
needed? This can decrease those unnecessary performance loss.
If I'm wrong please correct me :)
You're perfectly right, but this is generic KVM code. So it gets run across all
[PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
+ mas2_attr |= MAS2_I | MAS2_G;
+ } else {
#ifdef CONFIG_SMP
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ mas2_attr |= MAS2_M;
#endif
+ }
+ return mas2_attr;
}
/*
@@ -313,7 +320,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] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
Tiejun
+ mas2_attr |= MAS2_I | MAS2_G;
+ } else {
#ifdef CONFIG_SMP
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ mas2_attr |= MAS2_M;
#endif
+ }
+ return mas2_attr;
}
/*
@@ -313,7 +320,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);
--
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] kvm: powerpc: set cache coherency only for kernel managed pages
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood Scott-
B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's normal
DDR and the mapping sets M bit (coherent, cacheable) else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is that it
returns false when the page is managed by kernel and is not marked as
RESERVED (for some reason). For us it does not matter whether the page is
reserved or not, if it is kernel visible page then it is DDR.
-Bharat
Tiejun
+ mas2_attr |= MAS2_I | MAS2_G;
+ } else {
#ifdef CONFIG_SMP
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ mas2_attr |= MAS2_M;
#endif
+ }
+ return mas2_attr;
}
/*
@@ -313,7 +320,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);
N�r��yb�X��ǧv�^�){.n�+h����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z� �w���?��)ߢf
Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood Scott-
B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's normal
DDR and the mapping sets M bit (coherent, cacheable) else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is that it returns
false when the page is managed by kernel and is not marked as RESERVED (for
some reason). For us it does not matter whether the page is reserved or not, if it is
kernel visible page then it is DDR.
I think you are setting I|G by addressing all mmio pages, right? If so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio pages rather than
normal RAM. In some cases it is not enough to identify these mmio pages
by pfn_valid(). This patch adds checking the PageReserved as well.
Tiejun
-Bharat
Tiejun
+ mas2_attr |= MAS2_I | MAS2_G;
+ } else {
#ifdef CONFIG_SMP
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ mas2_attr |= MAS2_M;
#endif
+ }
+ return mas2_attr;
}
/*
@@ -313,7 +320,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);
--
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] 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 “tiejun.chen”
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's normal
DDR and the mapping sets M bit (coherent, cacheable) else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is that it
returns false when the page is managed by kernel and is not marked as
RESERVED
(for some reason). For us it does not matter whether the page is reserved or
not, if it is kernel visible page then it is DDR.
I think you are setting I|G by addressing all mmio pages, right? If so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio pages rather
than
normal RAM. In some cases it is not enough to identify these mmio pages
by pfn_valid(). This patch adds checking the PageReserved as well.
Do you know what are those some cases and how checking PageReserved helps in
those cases?
-Bharat
Tiejun
-Bharat
Tiejun
+ mas2_attr |= MAS2_I | MAS2_G;
+ } else {
#ifdef CONFIG_SMP
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ mas2_attr |= MAS2_M;
#endif
+ }
+ return mas2_attr;
}
/*
@@ -313,7 +320,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);
--
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
N�r��yb�X��ǧv�^�){.n�+h����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z� �w���?��)ߢf
Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-ow...@vger.kernel.org] On
Behalf Of “tiejun.chen”
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's normal
DDR and the mapping sets M bit (coherent, cacheable) else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is that it
returns false when the page is managed by kernel and is not marked as RESERVED
(for some reason). For us it does not matter whether the page is reserved or
not, if it is kernel visible page then it is DDR.
I think you are setting I|G by addressing all mmio pages, right? If so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio pages rather
than
normal RAM. In some cases it is not enough to identify these mmio pages
by pfn_valid(). This patch adds checking the PageReserved as well.
Do you know what are those some cases and how checking PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this should be
chronically persistent as I understand ;-)
Tiejun
-Bharat
Tiejun
-Bharat
Tiejun
+ mas2_attr |= MAS2_I | MAS2_G;
+ } else {
#ifdef CONFIG_SMP
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ mas2_attr |= MAS2_M;
#endif
+ }
+ return mas2_attr;
}
/*
@@ -313,7 +320,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);
--
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] kvm: powerpc: set cache coherency only for kernel managed pages
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of “tiejun.chen”
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable) else
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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is
that it
returns false when the page is managed by kernel and is not marked
as RESERVED (for some reason). For us it does not matter whether the
page is reserved or not, if it is kernel visible page then it is DDR.
I think you are setting I|G by addressing all mmio pages, right? If
so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio
pages rather than
normal RAM. In some cases it is not enough to identify these mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as well.
Do you know what are those some cases and how checking PageReserved helps
in
those cases?
No, myself didn't see these actual cases in qemu,too. But this should be
chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
-Bharat
Tiejun
-Bharat
Tiejun
-Bharat
Tiejun
+ mas2_attr |= MAS2_I | MAS2_G;
+ } else {
#ifdef CONFIG_SMP
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ mas2_attr |= MAS2_M;
#endif
+ }
+ return mas2_attr;
}
/*
@@ -313,7 +320,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);
--
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] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
+ mas2_attr |= MAS2_I | MAS2_G;
+ } else {
#ifdef CONFIG_SMP
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ mas2_attr |= MAS2_M;
#endif
+ }
Additionally, in UP case this little chunk of code is equivalent to
if (1) {
mas2_attr |= MAS2_I | MAS2_G;
} else {
}
So you'd better wrapper MAS2_m in advance like,
#ifdef CONFIG_SMP
#define M_IF_SMPMAS2_M
#else
#define M_IF_SMP0
#endif
Then
if (1)
mas2_attr |= MAS2_I | MAS2_G;
else
mas2_attr |= M_IF_SMP;
Tiejun
+ return mas2_attr;
}
/*
@@ -313,7 +320,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);
--
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] kvm: powerpc: set cache coherency only for kernel managed pages
-Original Message-
From: Bhushan Bharat-R65777
Sent: Thursday, July 18, 2013 1:53 PM
To: '“tiejun.chen”'
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood Scott-
B07421
Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of “tiejun.chen”
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is
that it
returns false when the page is managed by kernel and is not
marked as RESERVED (for some reason). For us it does not matter
whether the page is reserved or not, if it is kernel visible page then it
is DDR.
I think you are setting I|G by addressing all mmio pages, right? If
so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio
pages rather than
normal RAM. In some cases it is not enough to identify these
mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as well.
Do you know what are those some cases and how checking
PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this should
be chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
The reason is , kvm_is_mmio_pfn() function looks pretty heavy and I do not want
to call this for all tlbwe operation unless it is necessary.
-Bharat
+ mas2_attr |= MAS2_I | MAS2_G;
+ } else {
#ifdef CONFIG_SMP
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ mas2_attr |= MAS2_M;
#endif
+ }
+ return mas2_attr;
}
/*
@@ -313,7 +320,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);
--
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
N�r��yb�X��ǧv�^�){.n�+h����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z� �w���?��)ߢf
Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/18/2013 04:25 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Bhushan Bharat-R65777
Sent: Thursday, July 18, 2013 1:53 PM
To: '“tiejun.chen”'
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood Scott-
B07421
Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of “tiejun.chen”
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is
that it
returns false when the page is managed by kernel and is not
marked as RESERVED (for some reason). For us it does not matter
whether the page is reserved or not, if it is kernel visible page then it
is DDR.
I think you are setting I|G by addressing all mmio pages, right? If
so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio
pages rather than
normal RAM. In some cases it is not enough to identify these
mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as well.
Do you know what are those some cases and how checking
PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this should
be chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
The reason is , kvm_is_mmio_pfn() function looks pretty heavy and I do not want
to call this for all tlbwe operation unless it is necessary.
Furthermore, how to distinguish we're creating TLB entry for the device assigned
directly to the GS?
I think its unnecessary to always check if that is mmio's pfn since we have more
non direct assigned devices.
So maybe we can introduce another helper to fixup that TLB entry in instead of
this path.
Tiejun
-Bharat
+ mas2_attr |= MAS2_I | MAS2_G;
+ } else {
#ifdef CONFIG_SMP
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ mas2_attr |= MAS2_M;
#endif
+ }
+ return mas2_attr;
}
/*
@@ -313,7 +320,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);
--
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
Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages
On 18.07.2013, at 10:55, “tiejun.chen” wrote:
On 07/18/2013 04:25 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Bhushan Bharat-R65777
Sent: Thursday, July 18, 2013 1:53 PM
To: '“tiejun.chen”'
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood Scott-
B07421
Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of “tiejun.chen”
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is
that it
returns false when the page is managed by kernel and is not
marked as RESERVED (for some reason). For us it does not matter
whether the page is reserved or not, if it is kernel visible page then it
is DDR.
I think you are setting I|G by addressing all mmio pages, right? If
so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio
pages rather than
normal RAM. In some cases it is not enough to identify these
mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as well.
Do you know what are those some cases and how checking
PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this should
be chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
The reason is , kvm_is_mmio_pfn() function looks pretty heavy and I do not
want to call this for all tlbwe operation unless it is necessary.
Furthermore, how to distinguish we're creating TLB entry for the device
assigned directly to the GS?
Because other devices wouldn't be available to the guest through memory slots.
I think its unnecessary to always check if that is mmio's pfn since we have
more non direct assigned devices.
I'm not sure I understand. The shadow TLB code only knows here is a host
virtual address. It needs to figure out whether the host physical address
behind that is RAM (can access with cache enabled) or not (has to disable cache)
So maybe we can introduce another helper to fixup that TLB entry in instead
of this path.
This path does fix up the shadow (host) TLB entry :).
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] kvm: powerpc: set cache coherency only for kernel managed pages
On 18.07.2013, at 10:25, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Bhushan Bharat-R65777
Sent: Thursday, July 18, 2013 1:53 PM
To: '“tiejun.chen”'
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood Scott-
B07421
Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of “tiejun.chen”
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is
that it
returns false when the page is managed by kernel and is not
marked as RESERVED (for some reason). For us it does not matter
whether the page is reserved or not, if it is kernel visible page then it
is DDR.
I think you are setting I|G by addressing all mmio pages, right? If
so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio
pages rather than
normal RAM. In some cases it is not enough to identify these
mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as well.
Do you know what are those some cases and how checking
PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this should
be chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
The reason is , kvm_is_mmio_pfn() function looks pretty heavy and I do not
want to call this for all tlbwe operation unless it is necessary.
It certainly does more than we need and potentially slows down the fast path
(RAM mapping). The only thing it does on top of if (pfn_valid()) is to check
for pages that are declared reserved on the host. This happens in 2 cases:
1) Non cache coherent DMA
2) Memory hot remove
The non coherent DMA case would be interesting, as with the mechanism as it is
in place in Linux today, we could potentially break normal guest operation if
we don't take it into account. However, it's Kconfig guarded by:
depends on 4xx || 8xx || E200 || PPC_MPC512x || GAMECUBE_COMMON
default n if PPC_47x
default y
so we never hit it with any core we care about ;).
Memory hot remove does not exist on e500 FWIW, so we don't have to worry about
that one either.
Which means I think it's fine to slim this whole thing down to only check for
pfn_valid(), as the code does in this patch. It would however be very useful to
have a comment there that explains why it's safe to do so.
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] 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 3:19 PM
To: Bhushan Bharat-R65777
Cc: “tiejun.chen”; kvm-...@vger.kernel.org; kvm@vger.kernel.org; Wood Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
On 18.07.2013, at 10:25, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Bhushan Bharat-R65777
Sent: Thursday, July 18, 2013 1:53 PM
To: '“tiejun.chen”'
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of “tiejun.chen”
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is
that it
returns false when the page is managed by kernel and is not
marked as RESERVED (for some reason). For us it does not matter
whether the page is reserved or not, if it is kernel visible page
then it
is DDR.
I think you are setting I|G by addressing all mmio pages, right?
If so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio
pages rather than
normal RAM. In some cases it is not enough to identify these
mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as well.
Do you know what are those some cases and how checking
PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this
should be chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
The reason is , kvm_is_mmio_pfn() function looks pretty heavy and I do not
want to call this for all tlbwe operation unless it is necessary.
It certainly does more than we need and potentially slows down the fast path
(RAM mapping). The only thing it does on top of if (pfn_valid()) is to check
for pages that are declared reserved on the host. This happens in 2 cases:
1) Non cache coherent DMA
2) Memory hot remove
The non coherent DMA case would be interesting, as with the mechanism as it is
in place in Linux today, we could potentially break normal guest operation if
we
don't take it into account. However, it's Kconfig guarded by:
depends on 4xx || 8xx || E200 || PPC_MPC512x || GAMECUBE_COMMON
default n if PPC_47x
default y
so we never hit it with any core we care about ;).
Memory hot remove does not exist on e500 FWIW, so we don't have to worry about
that one either.
Which means I think it's fine to slim
Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/18/2013 05:44 PM, Alexander Graf wrote:
On 18.07.2013, at 10:55, �tiejun.chen� wrote:
On 07/18/2013 04:25 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Bhushan Bharat-R65777
Sent: Thursday, July 18, 2013 1:53 PM
To: '�tiejun.chen�'
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood Scott-
B07421
Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of �tiejun.chen�
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is
that it
returns false when the page is managed by kernel and is not
marked as RESERVED (for some reason). For us it does not matter
whether the page is reserved or not, if it is kernel visible page then it
is DDR.
I think you are setting I|G by addressing all mmio pages, right? If
so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio
pages rather than
normal RAM. In some cases it is not enough to identify these
mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as well.
Do you know what are those some cases and how checking
PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this should
be chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
The reason is , kvm_is_mmio_pfn() function looks pretty heavy and I do not want
to call this for all tlbwe operation unless it is necessary.
Furthermore, how to distinguish we're creating TLB entry for the device
assigned directly to the GS?
Because other devices wouldn't be available to the guest through memory slots.
Yes.
I think its unnecessary to always check if that is mmio's pfn since we have
more non direct assigned devices.
I'm not sure I understand. The shadow TLB code only knows here is a host virtual
address. It needs to figure out whether the host physical address behind that is
RAM (can access with cache enabled) or not (has to disable cache)
Sorry, looks I'm misleading you :-P
So maybe we can introduce another helper to fixup that TLB entry in instead of
this path.
This path does fix up the shadow (host) TLB entry :).
I just mean whether we can have a separate path dedicated to those direct
assigned devices, not go this common path :)
Tiejun
--
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] kvm: powerpc: set cache coherency only for kernel managed pages
On 18.07.2013, at 11:56, “tiejun.chen” wrote:
On 07/18/2013 05:44 PM, Alexander Graf wrote:
On 18.07.2013, at 10:55, �tiejun.chen� wrote:
On 07/18/2013 04:25 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Bhushan Bharat-R65777
Sent: Thursday, July 18, 2013 1:53 PM
To: '�tiejun.chen�'
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of �tiejun.chen�
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is
that it
returns false when the page is managed by kernel and is not
marked as RESERVED (for some reason). For us it does not matter
whether the page is reserved or not, if it is kernel visible page then
it
is DDR.
I think you are setting I|G by addressing all mmio pages, right? If
so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio
pages rather than
normal RAM. In some cases it is not enough to identify these
mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as
well.
Do you know what are those some cases and how checking
PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this should
be chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
The reason is , kvm_is_mmio_pfn() function looks pretty heavy and I do not
want to call this for all tlbwe operation unless it is necessary.
Furthermore, how to distinguish we're creating TLB entry for the device
assigned directly to the GS?
Because other devices wouldn't be available to the guest through memory
slots.
Yes.
I think its unnecessary to always check if that is mmio's pfn since we have
more non direct assigned devices.
I'm not sure I understand. The shadow TLB code only knows here is a host
virtual address. It needs to figure out whether the host physical address
behind that is RAM (can access with cache enabled) or not (has to disable
cache)
Sorry, looks I'm misleading you :-P
So maybe we can introduce another helper to fixup that TLB entry in instead
of this path.
This path does fix up the shadow (host) TLB entry :).
I just mean whether we can have a separate path dedicated to those direct
assigned devices, not go this common path :)
I don't think it's possible to have a separate path without a certain level of
trust. In the current flow we don't trust anyone. We just check every
translated page whether we should enable caching or not.
We could take that information from 2 other side though:
1) Memory Slot
Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/18/2013 05:48 PM, Alexander Graf wrote:
On 18.07.2013, at 10:25, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Bhushan Bharat-R65777
Sent: Thursday, July 18, 2013 1:53 PM
To: '�tiejun.chen�'
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood Scott-
B07421
Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of �tiejun.chen�
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is
that it
returns false when the page is managed by kernel and is not
marked as RESERVED (for some reason). For us it does not matter
whether the page is reserved or not, if it is kernel visible page then it
is DDR.
I think you are setting I|G by addressing all mmio pages, right? If
so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio
pages rather than
normal RAM. In some cases it is not enough to identify these
mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as well.
Do you know what are those some cases and how checking
PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this should
be chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
The reason is , kvm_is_mmio_pfn() function looks pretty heavy and I do not want
to call this for all tlbwe operation unless it is necessary.
It certainly does more than we need and potentially slows down the fast path (RAM
mapping). The only thing it does on top of if (pfn_valid()) is to check for
pages that are declared reserved on the host. This happens in 2 cases:
1) Non cache coherent DMA
2) Memory hot remove
The non coherent DMA case would be interesting, as with the mechanism as it is
in place in Linux today, we could potentially break normal guest operation if
we don't take it into account. However, it's Kconfig guarded by:
depends on 4xx || 8xx || E200 || PPC_MPC512x || GAMECUBE_COMMON
default n if PPC_47x
default y
so we never hit it with any core we care about ;).
Memory hot remove does not exist on e500 FWIW, so we don't have to worry about
that one either.
Thanks for this good information :)
So why not limit those codes with CONFIG_MEMORY_HOTPLUG inside kvm_is_mmio_pfn()
to make sure that check is only valid when that is really needed? This can
decrease those unnecessary performance loss.
If I'm wrong please correct me :)
Tiejun
--
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] kvm: powerpc: set cache coherency only for kernel managed pages
On 18.07.2013, at 12:08, “tiejun.chen” wrote:
On 07/18/2013 05:48 PM, Alexander Graf wrote:
On 18.07.2013, at 10:25, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Bhushan Bharat-R65777
Sent: Thursday, July 18, 2013 1:53 PM
To: '�tiejun.chen�'
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of �tiejun.chen�
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is
that it
returns false when the page is managed by kernel and is not
marked as RESERVED (for some reason). For us it does not matter
whether the page is reserved or not, if it is kernel visible page then
it
is DDR.
I think you are setting I|G by addressing all mmio pages, right? If
so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio
pages rather than
normal RAM. In some cases it is not enough to identify these
mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as well.
Do you know what are those some cases and how checking
PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this should
be chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
The reason is , kvm_is_mmio_pfn() function looks pretty heavy and I do not
want to call this for all tlbwe operation unless it is necessary.
It certainly does more than we need and potentially slows down the fast path
(RAM mapping). The only thing it does on top of if (pfn_valid()) is to
check for pages that are declared reserved on the host. This happens in 2
cases:
1) Non cache coherent DMA
2) Memory hot remove
The non coherent DMA case would be interesting, as with the mechanism as it
is in place in Linux today, we could potentially break normal guest
operation if we don't take it into account. However, it's Kconfig guarded by:
depends on 4xx || 8xx || E200 || PPC_MPC512x || GAMECUBE_COMMON
default n if PPC_47x
default y
so we never hit it with any core we care about ;).
Memory hot remove does not exist on e500 FWIW, so we don't have to worry
about that one either.
Thanks for this good information :)
So why not limit those codes with CONFIG_MEMORY_HOTPLUG inside
kvm_is_mmio_pfn() to make sure that check is only valid when that is really
needed? This can decrease those unnecessary performance loss.
If I'm wrong please correct me :)
You're perfectly right, but this is generic KVM code. So it gets run across all
architectures. What if someone has the great idea
Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/18/2013 06:00 PM, Alexander Graf wrote:
On 18.07.2013, at 11:56, “tiejun.chen” wrote:
On 07/18/2013 05:44 PM, Alexander Graf wrote:
On 18.07.2013, at 10:55, �tiejun.chen� wrote:
On 07/18/2013 04:25 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Bhushan Bharat-R65777
Sent: Thursday, July 18, 2013 1:53 PM
To: '�tiejun.chen�'
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood Scott-
B07421
Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of �tiejun.chen�
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is
that it
returns false when the page is managed by kernel and is not
marked as RESERVED (for some reason). For us it does not matter
whether the page is reserved or not, if it is kernel visible page then it
is DDR.
I think you are setting I|G by addressing all mmio pages, right? If
so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio
pages rather than
normal RAM. In some cases it is not enough to identify these
mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as well.
Do you know what are those some cases and how checking
PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this should
be chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
The reason is , kvm_is_mmio_pfn() function looks pretty heavy and I do not want
to call this for all tlbwe operation unless it is necessary.
Furthermore, how to distinguish we're creating TLB entry for the device
assigned directly to the GS?
Because other devices wouldn't be available to the guest through memory slots.
Yes.
I think its unnecessary to always check if that is mmio's pfn since we have
more non direct assigned devices.
I'm not sure I understand. The shadow TLB code only knows here is a host virtual
address. It needs to figure out whether the host physical address behind that is
RAM (can access with cache enabled) or not (has to disable cache)
Sorry, looks I'm misleading you :-P
So maybe we can introduce another helper to fixup that TLB entry in instead of
this path.
This path does fix up the shadow (host) TLB entry :).
I just mean whether we can have a separate path dedicated to those direct
assigned devices, not go this common path :)
I don't think it's possible to have a separate path without a certain level of
trust. In the current flow we don't trust anyone. We just check every
translated page whether we should enable caching or not.
We could take that information from 2 other side though:
1) Memory Slot
2) Guest TLB Flags
If we take it from the memory slot we
Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/18/2013 06:12 PM, Alexander Graf wrote:
On 18.07.2013, at 12:08, “tiejun.chen” wrote:
On 07/18/2013 05:48 PM, Alexander Graf wrote:
On 18.07.2013, at 10:25, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Bhushan Bharat-R65777
Sent: Thursday, July 18, 2013 1:53 PM
To: '�tiejun.chen�'
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood Scott-
B07421
Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of �tiejun.chen�
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is
that it
returns false when the page is managed by kernel and is not
marked as RESERVED (for some reason). For us it does not matter
whether the page is reserved or not, if it is kernel visible page then it
is DDR.
I think you are setting I|G by addressing all mmio pages, right? If
so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio
pages rather than
normal RAM. In some cases it is not enough to identify these
mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as well.
Do you know what are those some cases and how checking
PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this should
be chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
The reason is , kvm_is_mmio_pfn() function looks pretty heavy and I do not want
to call this for all tlbwe operation unless it is necessary.
It certainly does more than we need and potentially slows down the fast path (RAM
mapping). The only thing it does on top of if (pfn_valid()) is to check for
pages that are declared reserved on the host. This happens in 2 cases:
1) Non cache coherent DMA
2) Memory hot remove
The non coherent DMA case would be interesting, as with the mechanism as it is
in place in Linux today, we could potentially break normal guest operation if
we don't take it into account. However, it's Kconfig guarded by:
depends on 4xx || 8xx || E200 || PPC_MPC512x || GAMECUBE_COMMON
default n if PPC_47x
default y
so we never hit it with any core we care about ;).
Memory hot remove does not exist on e500 FWIW, so we don't have to worry about
that one either.
Thanks for this good information :)
So why not limit those codes with CONFIG_MEMORY_HOTPLUG inside
kvm_is_mmio_pfn() to make sure that check is only valid when that is really
needed? This can decrease those unnecessary performance loss.
If I'm wrong please correct me :)
You're perfectly right, but this is generic KVM code. So it gets run across all
architectures. What if someone has the great idea to add a new case here for
x86, but doesn't tell us
Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages
On 18.07.2013, at 12:19, “tiejun.chen” wrote:
On 07/18/2013 06:12 PM, Alexander Graf wrote:
On 18.07.2013, at 12:08, “tiejun.chen” wrote:
On 07/18/2013 05:48 PM, Alexander Graf wrote:
On 18.07.2013, at 10:25, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Bhushan Bharat-R65777
Sent: Thursday, July 18, 2013 1:53 PM
To: '�tiejun.chen�'
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel
managed pages
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of �tiejun.chen�
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; ag...@suse.de;
Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is
that it
returns false when the page is managed by kernel and is not
marked as RESERVED (for some reason). For us it does not matter
whether the page is reserved or not, if it is kernel visible page
then it
is DDR.
I think you are setting I|G by addressing all mmio pages, right? If
so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio
pages rather than
normal RAM. In some cases it is not enough to identify these
mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as
well.
Do you know what are those some cases and how checking
PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this should
be chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
The reason is , kvm_is_mmio_pfn() function looks pretty heavy and I do
not want to call this for all tlbwe operation unless it is necessary.
It certainly does more than we need and potentially slows down the fast
path (RAM mapping). The only thing it does on top of if (pfn_valid()) is
to check for pages that are declared reserved on the host. This happens in
2 cases:
1) Non cache coherent DMA
2) Memory hot remove
The non coherent DMA case would be interesting, as with the mechanism as
it is in place in Linux today, we could potentially break normal guest
operation if we don't take it into account. However, it's Kconfig guarded
by:
depends on 4xx || 8xx || E200 || PPC_MPC512x || GAMECUBE_COMMON
default n if PPC_47x
default y
so we never hit it with any core we care about ;).
Memory hot remove does not exist on e500 FWIW, so we don't have to worry
about that one either.
Thanks for this good information :)
So why not limit those codes with CONFIG_MEMORY_HOTPLUG inside
kvm_is_mmio_pfn() to make sure that check is only valid when that is really
needed? This can decrease those unnecessary performance loss.
If I'm wrong please correct me :)
You're perfectly right
Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/18/2013 05:00:42 AM, Alexander Graf wrote: Now why is setting invalid flags a problem? If I understand Scott correctly, it can break the host if you access certain host devices with caching enabled. But to be sure I'd say we ask him directly :). 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. -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] kvm: powerpc: set cache coherency only for kernel managed pages
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
+ mas2_attr |= MAS2_I | MAS2_G;
+ } else {
#ifdef CONFIG_SMP
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ mas2_attr |= MAS2_M;
#endif
+ }
+ return mas2_attr;
}
/*
@@ -313,7 +320,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] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
Tiejun
+ mas2_attr |= MAS2_I | MAS2_G;
+ } else {
#ifdef CONFIG_SMP
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ mas2_attr |= MAS2_M;
#endif
+ }
+ return mas2_attr;
}
/*
@@ -313,7 +320,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);
--
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] kvm: powerpc: set cache coherency only for kernel managed pages
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood Scott-
B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's normal
DDR and the mapping sets M bit (coherent, cacheable) else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is that it
returns false when the page is managed by kernel and is not marked as
RESERVED (for some reason). For us it does not matter whether the page is
reserved or not, if it is kernel visible page then it is DDR.
-Bharat
Tiejun
+ mas2_attr |= MAS2_I | MAS2_G;
+ } else {
#ifdef CONFIG_SMP
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ mas2_attr |= MAS2_M;
#endif
+ }
+ return mas2_attr;
}
/*
@@ -313,7 +320,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);
N�r��yb�X��ǧv�^�){.n�+jir)w* jg��� �ݢj/���z�ޖ��2�ޙ�)ߡ�a���� �G���h��j:+v���w��٥
Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood Scott-
B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's normal
DDR and the mapping sets M bit (coherent, cacheable) else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is that it returns
false when the page is managed by kernel and is not marked as RESERVED (for
some reason). For us it does not matter whether the page is reserved or not, if it is
kernel visible page then it is DDR.
I think you are setting I|G by addressing all mmio pages, right? If so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio pages rather than
normal RAM. In some cases it is not enough to identify these mmio pages
by pfn_valid(). This patch adds checking the PageReserved as well.
Tiejun
-Bharat
Tiejun
+ mas2_attr |= MAS2_I | MAS2_G;
+ } else {
#ifdef CONFIG_SMP
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ mas2_attr |= MAS2_M;
#endif
+ }
+ return mas2_attr;
}
/*
@@ -313,7 +320,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);
--
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] 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 “tiejun.chen”
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's normal
DDR and the mapping sets M bit (coherent, cacheable) else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is that it
returns false when the page is managed by kernel and is not marked as
RESERVED
(for some reason). For us it does not matter whether the page is reserved or
not, if it is kernel visible page then it is DDR.
I think you are setting I|G by addressing all mmio pages, right? If so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio pages rather
than
normal RAM. In some cases it is not enough to identify these mmio pages
by pfn_valid(). This patch adds checking the PageReserved as well.
Do you know what are those some cases and how checking PageReserved helps in
those cases?
-Bharat
Tiejun
-Bharat
Tiejun
+ mas2_attr |= MAS2_I | MAS2_G;
+ } else {
#ifdef CONFIG_SMP
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ mas2_attr |= MAS2_M;
#endif
+ }
+ return mas2_attr;
}
/*
@@ -313,7 +320,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);
--
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
N�r��yb�X��ǧv�^�){.n�+jir)w* jg��� �ݢj/���z�ޖ��2�ޙ�)ߡ�a���� �G���h��j:+v���w��٥
Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-ow...@vger.kernel.org] On
Behalf Of “tiejun.chen”
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's normal
DDR and the mapping sets M bit (coherent, cacheable) else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is that it
returns false when the page is managed by kernel and is not marked as RESERVED
(for some reason). For us it does not matter whether the page is reserved or
not, if it is kernel visible page then it is DDR.
I think you are setting I|G by addressing all mmio pages, right? If so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio pages rather
than
normal RAM. In some cases it is not enough to identify these mmio pages
by pfn_valid(). This patch adds checking the PageReserved as well.
Do you know what are those some cases and how checking PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this should be
chronically persistent as I understand ;-)
Tiejun
-Bharat
Tiejun
-Bharat
Tiejun
+ mas2_attr |= MAS2_I | MAS2_G;
+ } else {
#ifdef CONFIG_SMP
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ mas2_attr |= MAS2_M;
#endif
+ }
+ return mas2_attr;
}
/*
@@ -313,7 +320,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);
--
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] kvm: powerpc: set cache coherency only for kernel managed pages
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of “tiejun.chen”
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de;
Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable) else
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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is
that it
returns false when the page is managed by kernel and is not marked
as RESERVED (for some reason). For us it does not matter whether the
page is reserved or not, if it is kernel visible page then it is DDR.
I think you are setting I|G by addressing all mmio pages, right? If
so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio
pages rather than
normal RAM. In some cases it is not enough to identify these mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as well.
Do you know what are those some cases and how checking PageReserved helps
in
those cases?
No, myself didn't see these actual cases in qemu,too. But this should be
chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
-Bharat
Tiejun
-Bharat
Tiejun
-Bharat
Tiejun
+ mas2_attr |= MAS2_I | MAS2_G;
+ } else {
#ifdef CONFIG_SMP
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ mas2_attr |= MAS2_M;
#endif
+ }
+ return mas2_attr;
}
/*
@@ -313,7 +320,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);
--
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] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
+ mas2_attr |= MAS2_I | MAS2_G;
+ } else {
#ifdef CONFIG_SMP
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ mas2_attr |= MAS2_M;
#endif
+ }
Additionally, in UP case this little chunk of code is equivalent to
if (1) {
mas2_attr |= MAS2_I | MAS2_G;
} else {
}
So you'd better wrapper MAS2_m in advance like,
#ifdef CONFIG_SMP
#define M_IF_SMPMAS2_M
#else
#define M_IF_SMP0
#endif
Then
if (1)
mas2_attr |= MAS2_I | MAS2_G;
else
mas2_attr |= M_IF_SMP;
Tiejun
+ return mas2_attr;
}
/*
@@ -313,7 +320,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);
--
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] kvm: powerpc: set cache coherency only for kernel managed pages
-Original Message-
From: Bhushan Bharat-R65777
Sent: Thursday, July 18, 2013 1:53 PM
To: '“tiejun.chen”'
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood Scott-
B07421
Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of “tiejun.chen”
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de;
Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de;
Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is
that it
returns false when the page is managed by kernel and is not
marked as RESERVED (for some reason). For us it does not matter
whether the page is reserved or not, if it is kernel visible page then it
is DDR.
I think you are setting I|G by addressing all mmio pages, right? If
so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio
pages rather than
normal RAM. In some cases it is not enough to identify these
mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as well.
Do you know what are those some cases and how checking
PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this should
be chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
The reason is , kvm_is_mmio_pfn() function looks pretty heavy and I do not want
to call this for all tlbwe operation unless it is necessary.
-Bharat
+ mas2_attr |= MAS2_I | MAS2_G;
+ } else {
#ifdef CONFIG_SMP
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ mas2_attr |= MAS2_M;
#endif
+ }
+ return mas2_attr;
}
/*
@@ -313,7 +320,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);
--
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
N�r��yb�X��ǧv�^�){.n�+jir)w* jg��� �ݢj/���z�ޖ��2�ޙ�)ߡ�a���� �G���h��j:+v���w��٥
Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/18/2013 04:25 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Bhushan Bharat-R65777
Sent: Thursday, July 18, 2013 1:53 PM
To: '“tiejun.chen”'
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood Scott-
B07421
Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of “tiejun.chen”
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de;
Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de;
Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is
that it
returns false when the page is managed by kernel and is not
marked as RESERVED (for some reason). For us it does not matter
whether the page is reserved or not, if it is kernel visible page then it
is DDR.
I think you are setting I|G by addressing all mmio pages, right? If
so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio
pages rather than
normal RAM. In some cases it is not enough to identify these
mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as well.
Do you know what are those some cases and how checking
PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this should
be chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
The reason is , kvm_is_mmio_pfn() function looks pretty heavy and I do not want
to call this for all tlbwe operation unless it is necessary.
Furthermore, how to distinguish we're creating TLB entry for the device assigned
directly to the GS?
I think its unnecessary to always check if that is mmio's pfn since we have more
non direct assigned devices.
So maybe we can introduce another helper to fixup that TLB entry in instead of
this path.
Tiejun
-Bharat
+ mas2_attr |= MAS2_I | MAS2_G;
+ } else {
#ifdef CONFIG_SMP
- return (mas2 MAS2_ATTRIB_MASK) | MAS2_M;
-#else
- return mas2 MAS2_ATTRIB_MASK;
+ mas2_attr |= MAS2_M;
#endif
+ }
+ return mas2_attr;
}
/*
@@ -313,7 +320,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);
--
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
Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages
On 18.07.2013, at 10:55, “tiejun.chen” wrote:
On 07/18/2013 04:25 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Bhushan Bharat-R65777
Sent: Thursday, July 18, 2013 1:53 PM
To: '“tiejun.chen”'
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of “tiejun.chen”
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de;
Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de;
Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is
that it
returns false when the page is managed by kernel and is not
marked as RESERVED (for some reason). For us it does not matter
whether the page is reserved or not, if it is kernel visible page then it
is DDR.
I think you are setting I|G by addressing all mmio pages, right? If
so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio
pages rather than
normal RAM. In some cases it is not enough to identify these
mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as well.
Do you know what are those some cases and how checking
PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this should
be chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
The reason is , kvm_is_mmio_pfn() function looks pretty heavy and I do not
want to call this for all tlbwe operation unless it is necessary.
Furthermore, how to distinguish we're creating TLB entry for the device
assigned directly to the GS?
Because other devices wouldn't be available to the guest through memory slots.
I think its unnecessary to always check if that is mmio's pfn since we have
more non direct assigned devices.
I'm not sure I understand. The shadow TLB code only knows here is a host
virtual address. It needs to figure out whether the host physical address
behind that is RAM (can access with cache enabled) or not (has to disable cache)
So maybe we can introduce another helper to fixup that TLB entry in instead
of this path.
This path does fix up the shadow (host) TLB entry :).
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] kvm: powerpc: set cache coherency only for kernel managed pages
On 18.07.2013, at 10:25, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Bhushan Bharat-R65777
Sent: Thursday, July 18, 2013 1:53 PM
To: '“tiejun.chen”'
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood Scott-
B07421
Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of “tiejun.chen”
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de;
Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de;
Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is
that it
returns false when the page is managed by kernel and is not
marked as RESERVED (for some reason). For us it does not matter
whether the page is reserved or not, if it is kernel visible page then it
is DDR.
I think you are setting I|G by addressing all mmio pages, right? If
so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio
pages rather than
normal RAM. In some cases it is not enough to identify these
mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as well.
Do you know what are those some cases and how checking
PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this should
be chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
The reason is , kvm_is_mmio_pfn() function looks pretty heavy and I do not
want to call this for all tlbwe operation unless it is necessary.
It certainly does more than we need and potentially slows down the fast path
(RAM mapping). The only thing it does on top of if (pfn_valid()) is to check
for pages that are declared reserved on the host. This happens in 2 cases:
1) Non cache coherent DMA
2) Memory hot remove
The non coherent DMA case would be interesting, as with the mechanism as it is
in place in Linux today, we could potentially break normal guest operation if
we don't take it into account. However, it's Kconfig guarded by:
depends on 4xx || 8xx || E200 || PPC_MPC512x || GAMECUBE_COMMON
default n if PPC_47x
default y
so we never hit it with any core we care about ;).
Memory hot remove does not exist on e500 FWIW, so we don't have to worry about
that one either.
Which means I think it's fine to slim this whole thing down to only check for
pfn_valid(), as the code does in this patch. It would however be very useful to
have a comment there that explains why it's safe to do so.
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] 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 3:19 PM
To: Bhushan Bharat-R65777
Cc: “tiejun.chen”; kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
On 18.07.2013, at 10:25, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Bhushan Bharat-R65777
Sent: Thursday, July 18, 2013 1:53 PM
To: '“tiejun.chen”'
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de;
Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of “tiejun.chen”
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de;
Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: “tiejun.chen” [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de;
Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is
that it
returns false when the page is managed by kernel and is not
marked as RESERVED (for some reason). For us it does not matter
whether the page is reserved or not, if it is kernel visible page
then it
is DDR.
I think you are setting I|G by addressing all mmio pages, right?
If so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio
pages rather than
normal RAM. In some cases it is not enough to identify these
mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as well.
Do you know what are those some cases and how checking
PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this
should be chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
The reason is , kvm_is_mmio_pfn() function looks pretty heavy and I do not
want to call this for all tlbwe operation unless it is necessary.
It certainly does more than we need and potentially slows down the fast path
(RAM mapping). The only thing it does on top of if (pfn_valid()) is to check
for pages that are declared reserved on the host. This happens in 2 cases:
1) Non cache coherent DMA
2) Memory hot remove
The non coherent DMA case would be interesting, as with the mechanism as it is
in place in Linux today, we could potentially break normal guest operation if
we
don't take it into account. However, it's Kconfig guarded by:
depends on 4xx || 8xx || E200 || PPC_MPC512x || GAMECUBE_COMMON
default n if PPC_47x
default y
so we never hit it with any core we care about ;).
Memory hot remove does not exist on e500 FWIW, so we don't have to worry about
that one either.
Which means I think it's fine
Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/18/2013 05:44 PM, Alexander Graf wrote:
On 18.07.2013, at 10:55, �tiejun.chen� wrote:
On 07/18/2013 04:25 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Bhushan Bharat-R65777
Sent: Thursday, July 18, 2013 1:53 PM
To: '�tiejun.chen�'
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood Scott-
B07421
Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of �tiejun.chen�
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de;
Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de;
Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is
that it
returns false when the page is managed by kernel and is not
marked as RESERVED (for some reason). For us it does not matter
whether the page is reserved or not, if it is kernel visible page then it
is DDR.
I think you are setting I|G by addressing all mmio pages, right? If
so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio
pages rather than
normal RAM. In some cases it is not enough to identify these
mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as well.
Do you know what are those some cases and how checking
PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this should
be chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
The reason is , kvm_is_mmio_pfn() function looks pretty heavy and I do not want
to call this for all tlbwe operation unless it is necessary.
Furthermore, how to distinguish we're creating TLB entry for the device
assigned directly to the GS?
Because other devices wouldn't be available to the guest through memory slots.
Yes.
I think its unnecessary to always check if that is mmio's pfn since we have
more non direct assigned devices.
I'm not sure I understand. The shadow TLB code only knows here is a host virtual
address. It needs to figure out whether the host physical address behind that is
RAM (can access with cache enabled) or not (has to disable cache)
Sorry, looks I'm misleading you :-P
So maybe we can introduce another helper to fixup that TLB entry in instead of
this path.
This path does fix up the shadow (host) TLB entry :).
I just mean whether we can have a separate path dedicated to those direct
assigned devices, not go this common path :)
Tiejun
--
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] kvm: powerpc: set cache coherency only for kernel managed pages
On 18.07.2013, at 11:56, “tiejun.chen” wrote:
On 07/18/2013 05:44 PM, Alexander Graf wrote:
On 18.07.2013, at 10:55, �tiejun.chen� wrote:
On 07/18/2013 04:25 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Bhushan Bharat-R65777
Sent: Thursday, July 18, 2013 1:53 PM
To: '�tiejun.chen�'
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of �tiejun.chen�
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de;
Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de;
Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is
that it
returns false when the page is managed by kernel and is not
marked as RESERVED (for some reason). For us it does not matter
whether the page is reserved or not, if it is kernel visible page then
it
is DDR.
I think you are setting I|G by addressing all mmio pages, right? If
so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio
pages rather than
normal RAM. In some cases it is not enough to identify these
mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as
well.
Do you know what are those some cases and how checking
PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this should
be chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
The reason is , kvm_is_mmio_pfn() function looks pretty heavy and I do not
want to call this for all tlbwe operation unless it is necessary.
Furthermore, how to distinguish we're creating TLB entry for the device
assigned directly to the GS?
Because other devices wouldn't be available to the guest through memory
slots.
Yes.
I think its unnecessary to always check if that is mmio's pfn since we have
more non direct assigned devices.
I'm not sure I understand. The shadow TLB code only knows here is a host
virtual address. It needs to figure out whether the host physical address
behind that is RAM (can access with cache enabled) or not (has to disable
cache)
Sorry, looks I'm misleading you :-P
So maybe we can introduce another helper to fixup that TLB entry in instead
of this path.
This path does fix up the shadow (host) TLB entry :).
I just mean whether we can have a separate path dedicated to those direct
assigned devices, not go this common path :)
I don't think it's possible to have a separate path without a certain level of
trust. In the current flow we don't trust anyone. We just check every
translated page whether we should enable caching or not.
We could take that information from 2 other side though:
1) Memory
Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages
On 18.07.2013, at 12:08, “tiejun.chen” wrote:
On 07/18/2013 05:48 PM, Alexander Graf wrote:
On 18.07.2013, at 10:25, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Bhushan Bharat-R65777
Sent: Thursday, July 18, 2013 1:53 PM
To: '�tiejun.chen�'
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of �tiejun.chen�
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de;
Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de;
Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is
that it
returns false when the page is managed by kernel and is not
marked as RESERVED (for some reason). For us it does not matter
whether the page is reserved or not, if it is kernel visible page then
it
is DDR.
I think you are setting I|G by addressing all mmio pages, right? If
so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio
pages rather than
normal RAM. In some cases it is not enough to identify these
mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as well.
Do you know what are those some cases and how checking
PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this should
be chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
The reason is , kvm_is_mmio_pfn() function looks pretty heavy and I do not
want to call this for all tlbwe operation unless it is necessary.
It certainly does more than we need and potentially slows down the fast path
(RAM mapping). The only thing it does on top of if (pfn_valid()) is to
check for pages that are declared reserved on the host. This happens in 2
cases:
1) Non cache coherent DMA
2) Memory hot remove
The non coherent DMA case would be interesting, as with the mechanism as it
is in place in Linux today, we could potentially break normal guest
operation if we don't take it into account. However, it's Kconfig guarded by:
depends on 4xx || 8xx || E200 || PPC_MPC512x || GAMECUBE_COMMON
default n if PPC_47x
default y
so we never hit it with any core we care about ;).
Memory hot remove does not exist on e500 FWIW, so we don't have to worry
about that one either.
Thanks for this good information :)
So why not limit those codes with CONFIG_MEMORY_HOTPLUG inside
kvm_is_mmio_pfn() to make sure that check is only valid when that is really
needed? This can decrease those unnecessary performance loss.
If I'm wrong please correct me :)
You're perfectly right, but this is generic KVM code. So it gets run across all
architectures. What if someone has the great
Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/18/2013 06:00 PM, Alexander Graf wrote:
On 18.07.2013, at 11:56, “tiejun.chen” wrote:
On 07/18/2013 05:44 PM, Alexander Graf wrote:
On 18.07.2013, at 10:55, �tiejun.chen� wrote:
On 07/18/2013 04:25 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Bhushan Bharat-R65777
Sent: Thursday, July 18, 2013 1:53 PM
To: '�tiejun.chen�'
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood Scott-
B07421
Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of �tiejun.chen�
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de;
Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de;
Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is
that it
returns false when the page is managed by kernel and is not
marked as RESERVED (for some reason). For us it does not matter
whether the page is reserved or not, if it is kernel visible page then it
is DDR.
I think you are setting I|G by addressing all mmio pages, right? If
so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio
pages rather than
normal RAM. In some cases it is not enough to identify these
mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as well.
Do you know what are those some cases and how checking
PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this should
be chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
The reason is , kvm_is_mmio_pfn() function looks pretty heavy and I do not want
to call this for all tlbwe operation unless it is necessary.
Furthermore, how to distinguish we're creating TLB entry for the device
assigned directly to the GS?
Because other devices wouldn't be available to the guest through memory slots.
Yes.
I think its unnecessary to always check if that is mmio's pfn since we have
more non direct assigned devices.
I'm not sure I understand. The shadow TLB code only knows here is a host virtual
address. It needs to figure out whether the host physical address behind that is
RAM (can access with cache enabled) or not (has to disable cache)
Sorry, looks I'm misleading you :-P
So maybe we can introduce another helper to fixup that TLB entry in instead of
this path.
This path does fix up the shadow (host) TLB entry :).
I just mean whether we can have a separate path dedicated to those direct
assigned devices, not go this common path :)
I don't think it's possible to have a separate path without a certain level of
trust. In the current flow we don't trust anyone. We just check every
translated page whether we should enable caching or not.
We could take that information from 2 other side though:
1) Memory Slot
2) Guest TLB Flags
If we take it from the memory
Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/18/2013 06:12 PM, Alexander Graf wrote:
On 18.07.2013, at 12:08, “tiejun.chen” wrote:
On 07/18/2013 05:48 PM, Alexander Graf wrote:
On 18.07.2013, at 10:25, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Bhushan Bharat-R65777
Sent: Thursday, July 18, 2013 1:53 PM
To: '�tiejun.chen�'
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood Scott-
B07421
Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel
managed pages
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 1:52 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 04:08 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of �tiejun.chen�
Sent: Thursday, July 18, 2013 1:01 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de;
Wood
Scott-
B07421
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for
kernel managed pages
On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: �tiejun.chen� [mailto:tiejun.c...@windriver.com]
Sent: Thursday, July 18, 2013 11:56 AM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de;
Wood
Scott- B07421; Bhushan Bharat-R65777
Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only
for kernel managed pages
On 07/18/2013 02:04 PM, Bharat Bhushan wrote:
If there is a struct page for the requested mapping then it's
normal DDR and the mapping sets M bit (coherent, cacheable)
else 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
---
arch/powerpc/kvm/e500_mmu_host.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..089c227 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,20 @@ 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;
+
+ if (!pfn_valid(pfn)) {
Why not directly use kvm_is_mmio_pfn()?
What I understand from this function (someone can correct me) is
that it
returns false when the page is managed by kernel and is not
marked as RESERVED (for some reason). For us it does not matter
whether the page is reserved or not, if it is kernel visible page then it
is DDR.
I think you are setting I|G by addressing all mmio pages, right? If
so,
KVM: direct mmio pfn check
Userspace may specify memory slots that are backed by mmio
pages rather than
normal RAM. In some cases it is not enough to identify these
mmio
pages
by pfn_valid(). This patch adds checking the PageReserved as well.
Do you know what are those some cases and how checking
PageReserved helps in
those cases?
No, myself didn't see these actual cases in qemu,too. But this should
be chronically persistent as I understand ;-)
Then I will wait till someone educate me :)
The reason is , kvm_is_mmio_pfn() function looks pretty heavy and I do not want
to call this for all tlbwe operation unless it is necessary.
It certainly does more than we need and potentially slows down the fast path (RAM
mapping). The only thing it does on top of if (pfn_valid()) is to check for
pages that are declared reserved on the host. This happens in 2 cases:
1) Non cache coherent DMA
2) Memory hot remove
The non coherent DMA case would be interesting, as with the mechanism as it is
in place in Linux today, we could potentially break normal guest operation if
we don't take it into account. However, it's Kconfig guarded by:
depends on 4xx || 8xx || E200 || PPC_MPC512x || GAMECUBE_COMMON
default n if PPC_47x
default y
so we never hit it with any core we care about ;).
Memory hot remove does not exist on e500 FWIW, so we don't have to worry about
that one either.
Thanks for this good information :)
So why not limit those codes with CONFIG_MEMORY_HOTPLUG inside
kvm_is_mmio_pfn() to make sure that check is only valid when that is really
needed? This can decrease those unnecessary performance loss.
If I'm wrong please correct me :)
You're perfectly right, but this is generic KVM code. So it gets run across all
architectures. What if someone has the great idea to add a new case here for
x86, but doesn't tell us
Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages
On 07/18/2013 05:00:42 AM, Alexander Graf wrote: Now why is setting invalid flags a problem? If I understand Scott correctly, it can break the host if you access certain host devices with caching enabled. But to be sure I'd say we ask him directly :). 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. -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
