From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?B?IuKAnHRpZWp1bi5jaGVu4oCdIg==?= Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only for kernel managed pages Date: Thu, 18 Jul 2013 16:55:27 +0800 Message-ID: <51E7AD7F.5080004@windriver.com> References: <1374127456-9614-1-git-send-email-Bharat.Bhushan@freescale.com> <1374127456-9614-2-git-send-email-Bharat.Bhushan@freescale.com> <51E78A88.4000904@windriver.com> <6A3DF150A5B70D4F9B66A25E3F7C888D070D6AFF@039-SN2MPN1-013.039d.mgd.msft.net> <51E799D0.1030406@windriver.com> <6A3DF150A5B70D4F9B66A25E3F7C888D070D6DAB@039-SN2MPN1-013.039d.mgd.msft.net> <51E7A5A2.5040107@windriver.com> <6A3DF150A5B70D4F9B66A25E3F7C888D070D6E79@039-SN2MPN1-013.039d.mgd.msft.net> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: "kvm-ppc@vger.kernel.org" , "kvm@vger.kernel.org" , "agraf@suse.de" , Wood Scott-B07421 To: Bhushan Bharat-R65777 Return-path: In-Reply-To: <6A3DF150A5B70D4F9B66A25E3F7C888D070D6E79@039-SN2MPN1-013.039d.mgd.msft.net> Sender: kvm-ppc-owner@vger.kernel.org List-Id: kvm.vger.kernel.org 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: '"=E2=80=9Ctiejun.chen=E2=80=9D"' >> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; agraf@suse.de; Woo= d Scott- >> B07421 >> Subject: RE: [PATCH 2/2] kvm: powerpc: set cache coherency only for = kernel >> managed pages >> >> >> >>> -----Original Message----- >>> From: "=E2=80=9Ctiejun.chen=E2=80=9D" [mailto:tiejun.chen@windriver= =2Ecom] >>> Sent: Thursday, July 18, 2013 1:52 PM >>> To: Bhushan Bharat-R65777 >>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; agraf@suse.de; Wo= od >>> 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-owner@vger.kernel.org >>>>> [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of "=E2=80=9Ctie= jun.chen=E2=80=9D" >>>>> Sent: Thursday, July 18, 2013 1:01 PM >>>>> To: Bhushan Bharat-R65777 >>>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; agraf@suse.de; >>>>> Wood >>>>> Scott- >>>>> B07421 >>>>> Subject: Re: [PATCH 2/2] kvm: powerpc: set cache coherency only f= or >>>>> kernel managed pages >>>>> >>>>> On 07/18/2013 03:12 PM, Bhushan Bharat-R65777 wrote: >>>>>> >>>>>> >>>>>>> -----Original Message----- >>>>>>> From: "=E2=80=9Ctiejun.chen=E2=80=9D" [mailto:tiejun.chen@windr= iver.com] >>>>>>> Sent: Thursday, July 18, 2013 11:56 AM >>>>>>> To: Bhushan Bharat-R65777 >>>>>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; agraf@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 devi= ce >>>>>>>> >>>>>>>> Signed-off-by: Bharat Bhushan >>>>>>>> --- >>>>>>>> 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 =3D 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 th= ese >>>>> 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 shou= ld >>> 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 d= o not want to call this for all tlbwe operation unless it is necessary. =46urthermore, how to distinguish we're creating TLB entry for the devi= ce assigned=20 directly to the GS? I think its unnecessary to always check if that is mmio's pfn since we = have more=20 non direct assigned devices. So maybe we can introduce another helper to fixup that TLB entry in ins= tead of=20 this path. Tiejun > > -Bharat > >>>>>>>> + mas2_attr |=3D MAS2_I | MAS2_G; >>>>>>>> + } else { >>>>>>>> #ifdef CONFIG_SMP >>>>>>>> - return (mas2 & MAS2_ATTRIB_MASK) | MAS2_M; >>>>>>>> -#else >>>>>>>> - return mas2 & MAS2_ATTRIB_MASK; >>>>>>>> + mas2_attr |=3D MAS2_M; >>>>>>>> #endif >>>>>>>> + } >>>>>>>> + return mas2_attr; >>>>>>>> } >>>>>>>> >>>>>>>> /* >>>>>>>> @@ -313,7 +320,7 @@ static void kvmppc_e500_setup_stlbe( >>>>>>>> /* Force IPROT=3D0 for all guest mappings. */ >>>>>>>> stlbe->mas1 =3D MAS1_TSIZE(tsize) | get_tlb_sts(gtlbe) |= MAS1_VALID; >>>>>>>> stlbe->mas2 =3D (gvaddr & MAS2_EPN) | >>>>>>>> - e500_shadow_mas2_attrib(gtlbe->mas2, pr); >>>>>>>> + e500_shadow_mas2_attrib(gtlbe->mas2, pfn); >>>>>>>> stlbe->mas7_3 =3D ((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 majordomo@vger.kernel.org More >>>>> majordomo info at http://vger.kernel.org/majordomo-info.html >>>> >>> >