* Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages [not found] <03EEFDFE-4603-44FC-8449-2450607F2864@suse.de> @ 2013-07-24 20:32 ` Scott Wood 2013-07-25 8:50 ` Gleb Natapov 0 siblings, 1 reply; 5+ messages in thread From: Scott Wood @ 2013-07-24 20:32 UTC (permalink / raw) To: Alexander Graf Cc: Wood Scott-B07421, Gleb Natapov, kvm@vger.kernel.org list, kvm-ppc@vger.kernel.org, “tiejun.chen”, Bhushan Bharat-R65777, Paolo Bonzini, linuxppc-dev On 07/24/2013 04:39:59 AM, Alexander Graf wrote: >=20 > On 24.07.2013, at 11:35, Gleb Natapov wrote: >=20 > > On Wed, Jul 24, 2013 at 11:21:11AM +0200, Alexander Graf wrote: > >>> Are not we going to use page_is_ram() from =20 > 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 =20 > that allow > > us to check quickly for valid pfn. >=20 > Then why should we use page_is_ram()? :) >=20 > I really don't want the e500 code to diverge too much from what the =20 > rest of the kvm code is doing. I don't understand "actually used to build pfn map...". What code is =20 this? I don't see any calls to page_is_ram() in the KVM code, or in =20 generic mm code. Is this a statement about what x86 does? On PPC page_is_ram() is only called (AFAICT) for determining what =20 attributes to set on mmaps. We want to be sure that KVM always makes =20 the same decision. While pfn_valid() seems like it should be =20 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 =20 two different names for the same thing? -Scott= ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages 2013-07-24 20:32 ` [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages Scott Wood @ 2013-07-25 8:50 ` Gleb Natapov 2013-07-25 16:07 ` Alexander Graf 2013-07-26 22:27 ` Scott Wood 0 siblings, 2 replies; 5+ messages in thread From: Gleb Natapov @ 2013-07-25 8:50 UTC (permalink / raw) To: Scott Wood Cc: Wood Scott-B07421, kvm@vger.kernel.org list, Alexander Graf, kvm-ppc@vger.kernel.org, “tiejun.chen”, Bhushan Bharat-R65777, Paolo Bonzini, linuxppc-dev 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. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages 2013-07-25 8:50 ` Gleb Natapov @ 2013-07-25 16:07 ` Alexander Graf 2013-07-25 16:14 ` Gleb Natapov 2013-07-26 22:27 ` Scott Wood 1 sibling, 1 reply; 5+ messages in thread From: Alexander Graf @ 2013-07-25 16:07 UTC (permalink / raw) To: Gleb Natapov Cc: Wood Scott-B07421, kvm@vger.kernel.org list, kvm-ppc, "“tiejun.chen” Chen", Bhushan Bharat-R65777, Scott Wood, Paolo Bonzini, linuxppc-dev 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: >>>=20 >>> On 24.07.2013, at 11:35, Gleb Natapov wrote: >>>=20 >>>> 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? >>>>>=20 >>>>> rWhy aren't we using page_is_ram() in kvm_is_mmio_pfn()? >>>>>=20 >>>>>=20 >>>> Because it is much slower and, IIRC, actually used to build pfn >>> map that allow >>>> us to check quickly for valid pfn. >>>=20 >>> Then why should we use page_is_ram()? :) >>>=20 >>> I really don't want the e500 code to diverge too much from what >>> the rest of the kvm code is doing. >>=20 >> 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. >=20 >>=20 >> 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. >>=20 > Again pfn_valid() is not enough. >=20 >> If pfn_valid() is better, why is that not used for mmap? Why are >> there two different names for the same thing? >>=20 > 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. >=20 > 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. >=20 > -- > Gleb. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages 2013-07-25 16:07 ` Alexander Graf @ 2013-07-25 16:14 ` Gleb Natapov 0 siblings, 0 replies; 5+ messages in thread From: Gleb Natapov @ 2013-07-25 16:14 UTC (permalink / raw) To: Alexander Graf Cc: Wood Scott-B07421, kvm@vger.kernel.org list, kvm-ppc, "“tiejun.chen” Chen", Bhushan Bharat-R65777, Scott Wood, Paolo Bonzini, linuxppc-dev 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. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages 2013-07-25 8:50 ` Gleb Natapov 2013-07-25 16:07 ` Alexander Graf @ 2013-07-26 22:27 ` Scott Wood 1 sibling, 0 replies; 5+ messages in thread From: Scott Wood @ 2013-07-26 22:27 UTC (permalink / raw) To: Gleb Natapov Cc: Wood Scott-B07421, kvm@vger.kernel.org list, Alexander Graf, kvm-ppc@vger.kernel.org, “tiejun.chen”, Bhushan Bharat-R65777, Paolo Bonzini, linuxppc-dev 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 =20 ppc_md.phys_mem_access_prot(), which is pci_phys_mem_access_prot(), =20 which also uses page_is_ram(). If page_is_ram() returns false then it =20 checks for write-combining PCI. But yes, we would want to call =20 ppc_md.phys_mem_access_prot() if present. Copying from the host PTE would be ideal if doesn't come with a =20 noticeable performance impact compared to other methods, but one way or =20 another we want to be sure we match. > It is also make sense to allow noncached access to reserved ram =20 > sometimes. Perhaps, but that's not KVM's decision to make. You should get the =20 same result as if you mmaped it -- because QEMU already did and we need =20 to be consistent. Not to mention the large page kernel mapping that =20 will have been done on e500... -Scott= ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-07-26 22:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <03EEFDFE-4603-44FC-8449-2450607F2864@suse.de>
2013-07-24 20:32 ` [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages Scott Wood
2013-07-25 8:50 ` Gleb Natapov
2013-07-25 16:07 ` Alexander Graf
2013-07-25 16:14 ` Gleb Natapov
2013-07-26 22:27 ` Scott Wood
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).