Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages

5 messages, 3 authors, 2013-07-26 · open the first message on its own page

Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages

From: Scott Wood <hidden>
Date: 2013-07-24 20:33:22

On 07/24/2013 04:39:59 AM, Alexander Graf wrote:
=20
On 24.07.2013, at 11:35, Gleb Natapov wrote:
=20
quoted
On Wed, Jul 24, 2013 at 11:21:11AM +0200, Alexander Graf wrote:
quoted
quoted
Are not we going to use page_is_ram() from  =20
e500_shadow_mas2_attrib() as Scott commented?
quoted
quoted
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
quoted
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=

Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages

From: Gleb Natapov <hidden>
Date: 2013-07-25 08:50:47

On Wed, Jul 24, 2013 at 03:32:49PM -0500, Scott Wood wrote:
On 07/24/2013 04:39:59 AM, Alexander Graf wrote:
quoted
On 24.07.2013, at 11:35, Gleb Natapov wrote:
quoted
On Wed, Jul 24, 2013 at 11:21:11AM +0200, Alexander Graf wrote:
quoted
quoted
Are not we going to use page_is_ram() from
e500_shadow_mas2_attrib() as Scott commented?
quoted
quoted
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
quoted
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.

Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages

From: Alexander Graf <hidden>
Date: 2013-07-25 16:08:00

On 25.07.2013, at 10:50, Gleb Natapov wrote:
On Wed, Jul 24, 2013 at 03:32:49PM -0500, Scott Wood wrote:
quoted
On 07/24/2013 04:39:59 AM, Alexander Graf wrote:
quoted
=20
On 24.07.2013, at 11:35, Gleb Natapov wrote:
=20
quoted
On Wed, Jul 24, 2013 at 11:21:11AM +0200, Alexander Graf wrote:
quoted
quoted
Are not we going to use page_is_ram() from
e500_shadow_mas2_attrib() as Scott commented?
quoted
quoted
=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
quoted
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
quoted
=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
quoted
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.

Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages

From: Gleb Natapov <hidden>
Date: 2013-07-25 16:14:56

On Thu, Jul 25, 2013 at 06:07:55PM +0200, Alexander Graf wrote:
On 25.07.2013, at 10:50, Gleb Natapov wrote:
quoted
On Wed, Jul 24, 2013 at 03:32:49PM -0500, Scott Wood wrote:
quoted
On 07/24/2013 04:39:59 AM, Alexander Graf wrote:
quoted
On 24.07.2013, at 11:35, Gleb Natapov wrote:
quoted
On Wed, Jul 24, 2013 at 11:21:11AM +0200, Alexander Graf wrote:
quoted
quoted
Are not we going to use page_is_ram() from
e500_shadow_mas2_attrib() as Scott commented?
quoted
quoted
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
quoted
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.
quoted
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.
quoted
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.

Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages

From: Scott Wood <hidden>
Date: 2013-07-26 22:27:11

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=
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help