On 18.07.2013, at 17:15, Bhushan Bharat-R65777 wrote: > > >> -----Original Message----- >> From: kvm-ppc-owner@xxxxxxxxxxxxxxx [mailto:kvm-ppc-owner@xxxxxxxxxxxxxxx] On >> Behalf Of Alexander Graf >> Sent: Thursday, July 18, 2013 8:23 PM >> To: Bhushan Bharat-R65777 >> Cc: kvm-ppc@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; Wood Scott-B07421; Bhushan >> Bharat-R65777 >> Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel >> managed pages >> >> >> On 18.07.2013, at 15:19, Bharat Bhushan wrote: >> >>> If there is a struct page for the requested mapping then it's normal >>> RAM and the mapping is set to "M" bit (coherent, cacheable) otherwise >>> this is treated as I/O and we set "I + G" (cache inhibited, guarded) >>> >>> This helps setting proper TLB mapping for direct assigned device >>> >>> Signed-off-by: Bharat Bhushan <bharat.bhushan@xxxxxxxxxxxxx> >>> --- >>> v2: some cleanup and added comment >>> - >>> arch/powerpc/kvm/e500_mmu_host.c | 23 ++++++++++++++++++----- >>> 1 files changed, 18 insertions(+), 5 deletions(-) >>> >>> diff --git a/arch/powerpc/kvm/e500_mmu_host.c >>> b/arch/powerpc/kvm/e500_mmu_host.c >>> index 1c6a9d7..02eb973 100644 >>> --- a/arch/powerpc/kvm/e500_mmu_host.c >>> +++ b/arch/powerpc/kvm/e500_mmu_host.c >>> @@ -64,13 +64,26 @@ static inline u32 e500_shadow_mas3_attrib(u32 mas3, int >> usermode) >>> return mas3; >>> } >>> >>> -static inline u32 e500_shadow_mas2_attrib(u32 mas2, int usermode) >>> +static inline u32 e500_shadow_mas2_attrib(u32 mas2, pfn_t pfn) >>> { >>> + u32 mas2_attr; >>> + >>> + mas2_attr = mas2 & MAS2_ATTRIB_MASK; >>> + >>> + /* >>> + * RAM is always mappable on e500 systems, so this is identical >>> + * to kvm_is_mmio_pfn(), just without its overhead. >>> + */ >>> + if (!pfn_valid(pfn)) { >>> + /* Pages not managed by Kernel are treated as I/O, set I + G */ >> >> Please also document the intermediate thought that I/O should be mapped non- >> cached. > > I did not get what you mean to document? Page snot managed by the kernel are treated as I/O. Map it with disabled cache. > >> >>> + mas2_attr |= MAS2_I | MAS2_G; >>> #ifdef CONFIG_SMP >> >> Please separate the SMP case out of the branch. > > Really :) this was looking simple to me. Two branches intertwined never look simple :). > >> >>> - return (mas2 & MAS2_ATTRIB_MASK) | MAS2_M; >>> -#else >>> - return mas2 & MAS2_ATTRIB_MASK; >>> + } else { >>> + /* Kernel managed pages are actually RAM so set "M" */ >> >> This comment doesn't tell me why M can be set ;). > > RAM in SMP, so setting coherent, is not that obvious? No. Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
