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@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; agraf@xxxxxxx; 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.chen@xxxxxxxxxxxxx] >>>> Sent: Thursday, July 18, 2013 1:52 PM >>>> To: Bhushan Bharat-R65777 >>>> Cc: kvm-ppc@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; agraf@xxxxxxx; 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-owner@xxxxxxxxxxxxxxx >>>>>> [mailto:kvm-ppc-owner@xxxxxxxxxxxxxxx] On Behalf Of "“tiejun.chen”" >>>>>> Sent: Thursday, July 18, 2013 1:01 PM >>>>>> To: Bhushan Bharat-R65777 >>>>>> Cc: kvm-ppc@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; agraf@xxxxxxx; >>>>>> 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.chen@xxxxxxxxxxxxx] >>>>>>>> Sent: Thursday, July 18, 2013 11:56 AM >>>>>>>> To: Bhushan Bharat-R65777 >>>>>>>> Cc: kvm-ppc@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; agraf@xxxxxxx; >>>>>>>> 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.bhushan@xxxxxxxxxxxxx> >>>>>>>>> --- >>>>>>>>> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
