Message ID | 1450214671-13446-1-git-send-email-kamal@canonical.com |
---|---|
State | New |
Headers | show |
On Tue, Dec 15, 2015 at 01:24:31PM -0800, Kamal Mostafa wrote: > This is a note to let you know that I have just added a patch titled > > ARM/arm64: KVM: test properly for a PTE's uncachedness > > to the linux-3.19.y-queue branch of the 3.19.y-ckt extended stable tree > which can be found at: > > http://kernel.ubuntu.com/git/ubuntu/linux.git/log/?h=linux-3.19.y-queue > > This patch is scheduled to be released in version 3.19.8-ckt12. > > If you, or anyone else, feels it should not be added to this tree, please > reply to this email. This fix was incorrect and was subsequently updated by the following commit in mainline: 0de58f8 (ARM/arm64: KVM: correct PTE uncachedness check, 2015-12-03) That update should be applied along with this one. Thanks, -Christoffer > ------ > > From 215d740cfc3c554856542c00581e9b7169eb18e7 Mon Sep 17 00:00:00 2001 > From: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Date: Tue, 10 Nov 2015 15:11:20 +0100 > Subject: ARM/arm64: KVM: test properly for a PTE's uncachedness > > commit e6fab54423450d699a09ec2b899473a541f61971 upstream. > > The open coded tests for checking whether a PTE maps a page as > uncached use a flawed '(pte_val(xxx) & CONST) != CONST' pattern, > which is not guaranteed to work since the type of a mapping is > not a set of mutually exclusive bits > > For HYP mappings, the type is an index into the MAIR table (i.e, the > index itself does not contain any information whatsoever about the > type of the mapping), and for stage-2 mappings it is a bit field where > normal memory and device types are defined as follows: > > #define MT_S2_NORMAL 0xf > #define MT_S2_DEVICE_nGnRE 0x1 > > I.e., masking *and* comparing with the latter matches on the former, > and we have been getting lucky merely because the S2 device mappings > also have the PTE_UXN bit set, or we would misidentify memory mappings > as device mappings. > > Since the unmap_range() code path (which contains one instance of the > flawed test) is used both for HYP mappings and stage-2 mappings, and > considering the difference between the two, it is non-trivial to fix > this by rewriting the tests in place, as it would involve passing > down the type of mapping through all the functions. > > However, since HYP mappings and stage-2 mappings both deal with host > physical addresses, we can simply check whether the mapping is backed > by memory that is managed by the host kernel, and only perform the > D-cache maintenance if this is the case. > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Tested-by: Pavel Fedin <p.fedin@samsung.com> > Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org> > Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org> > Signed-off-by: Kamal Mostafa <kamal@canonical.com> > --- > arch/arm/kvm/mmu.c | 15 +++++++-------- > 1 file changed, 7 insertions(+), 8 deletions(-) > > diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c > index 0512ed4..a2c1ebf 100644 > --- a/arch/arm/kvm/mmu.c > +++ b/arch/arm/kvm/mmu.c > @@ -78,6 +78,11 @@ static void kvm_flush_dcache_pud(pud_t pud) > __kvm_flush_dcache_pud(pud); > } > > +static bool kvm_is_device_pfn(unsigned long pfn) > +{ > + return !pfn_valid(pfn); > +} > + > static int mmu_topup_memory_cache(struct kvm_mmu_memory_cache *cache, > int min, int max) > { > @@ -174,7 +179,7 @@ static void unmap_ptes(struct kvm *kvm, pmd_t *pmd, > kvm_tlb_flush_vmid_ipa(kvm, addr); > > /* No need to invalidate the cache for device mappings */ > - if ((pte_val(old_pte) & PAGE_S2_DEVICE) != PAGE_S2_DEVICE) > + if (!kvm_is_device_pfn(__phys_to_pfn(addr))) > kvm_flush_dcache_pte(old_pte); > > put_page(virt_to_page(pte)); > @@ -266,8 +271,7 @@ static void stage2_flush_ptes(struct kvm *kvm, pmd_t *pmd, > > pte = pte_offset_kernel(pmd, addr); > do { > - if (!pte_none(*pte) && > - (pte_val(*pte) & PAGE_S2_DEVICE) != PAGE_S2_DEVICE) > + if (!pte_none(*pte) && !kvm_is_device_pfn(__phys_to_pfn(addr))) > kvm_flush_dcache_pte(*pte); > } while (pte++, addr += PAGE_SIZE, addr != end); > } > @@ -983,11 +987,6 @@ static bool kvm_is_write_fault(struct kvm_vcpu *vcpu) > return kvm_vcpu_dabt_iswrite(vcpu); > } > > -static bool kvm_is_device_pfn(unsigned long pfn) > -{ > - return !pfn_valid(pfn); > -} > - > static void coherent_cache_guest_page(struct kvm_vcpu *vcpu, pfn_t pfn, > unsigned long size, bool uncached) > { > -- > 1.9.1 >
On Tue, 2015-12-15 at 22:36 +0100, Christoffer Dall wrote: > On Tue, Dec 15, 2015 at 01:24:31PM -0800, Kamal Mostafa wrote: > > This is a note to let you know that I have just added a patch titled > > > > ARM/arm64: KVM: test properly for a PTE's uncachedness > > > > to the linux-3.19.y-queue branch of the 3.19.y-ckt extended stable tree > > which can be found at: > > > > http://kernel.ubuntu.com/git/ubuntu/linux.git/log/?h=linux-3.19.y-queue > > > > This patch is scheduled to be released in version 3.19.8-ckt12. > > > > If you, or anyone else, feels it should not be added to this tree, please > > reply to this email. > > This fix was incorrect and was subsequently updated by the following > commit in mainline: > > 0de58f8 (ARM/arm64: KVM: correct PTE uncachedness check, 2015-12-03) > > That update should be applied along with this one. > > Thanks, > -Christoffer Thanks very much Christoffer, I'll make sure the follow-up fix gets applied in the same 3.19-stable release. -Kamal > > ------ > > > > From 215d740cfc3c554856542c00581e9b7169eb18e7 Mon Sep 17 00:00:00 2001 > > From: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > Date: Tue, 10 Nov 2015 15:11:20 +0100 > > Subject: ARM/arm64: KVM: test properly for a PTE's uncachedness > > > > commit e6fab54423450d699a09ec2b899473a541f61971 upstream. > > > > The open coded tests for checking whether a PTE maps a page as > > uncached use a flawed '(pte_val(xxx) & CONST) != CONST' pattern, > > which is not guaranteed to work since the type of a mapping is > > not a set of mutually exclusive bits > > > > For HYP mappings, the type is an index into the MAIR table (i.e, the > > index itself does not contain any information whatsoever about the > > type of the mapping), and for stage-2 mappings it is a bit field where > > normal memory and device types are defined as follows: > > > > #define MT_S2_NORMAL 0xf > > #define MT_S2_DEVICE_nGnRE 0x1 > > > > I.e., masking *and* comparing with the latter matches on the former, > > and we have been getting lucky merely because the S2 device mappings > > also have the PTE_UXN bit set, or we would misidentify memory mappings > > as device mappings. > > > > Since the unmap_range() code path (which contains one instance of the > > flawed test) is used both for HYP mappings and stage-2 mappings, and > > considering the difference between the two, it is non-trivial to fix > > this by rewriting the tests in place, as it would involve passing > > down the type of mapping through all the functions. > > > > However, since HYP mappings and stage-2 mappings both deal with host > > physical addresses, we can simply check whether the mapping is backed > > by memory that is managed by the host kernel, and only perform the > > D-cache maintenance if this is the case. > > > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > Tested-by: Pavel Fedin <p.fedin@samsung.com> > > Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org> > > Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org> > > Signed-off-by: Kamal Mostafa <kamal@canonical.com> > > --- > > arch/arm/kvm/mmu.c | 15 +++++++-------- > > 1 file changed, 7 insertions(+), 8 deletions(-) > > > > diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c > > index 0512ed4..a2c1ebf 100644 > > --- a/arch/arm/kvm/mmu.c > > +++ b/arch/arm/kvm/mmu.c > > @@ -78,6 +78,11 @@ static void kvm_flush_dcache_pud(pud_t pud) > > __kvm_flush_dcache_pud(pud); > > } > > > > +static bool kvm_is_device_pfn(unsigned long pfn) > > +{ > > + return !pfn_valid(pfn); > > +} > > + > > static int mmu_topup_memory_cache(struct kvm_mmu_memory_cache *cache, > > int min, int max) > > { > > @@ -174,7 +179,7 @@ static void unmap_ptes(struct kvm *kvm, pmd_t *pmd, > > kvm_tlb_flush_vmid_ipa(kvm, addr); > > > > /* No need to invalidate the cache for device mappings */ > > - if ((pte_val(old_pte) & PAGE_S2_DEVICE) != PAGE_S2_DEVICE) > > + if (!kvm_is_device_pfn(__phys_to_pfn(addr))) > > kvm_flush_dcache_pte(old_pte); > > > > put_page(virt_to_page(pte)); > > @@ -266,8 +271,7 @@ static void stage2_flush_ptes(struct kvm *kvm, pmd_t *pmd, > > > > pte = pte_offset_kernel(pmd, addr); > > do { > > - if (!pte_none(*pte) && > > - (pte_val(*pte) & PAGE_S2_DEVICE) != PAGE_S2_DEVICE) > > + if (!pte_none(*pte) && !kvm_is_device_pfn(__phys_to_pfn(addr))) > > kvm_flush_dcache_pte(*pte); > > } while (pte++, addr += PAGE_SIZE, addr != end); > > } > > @@ -983,11 +987,6 @@ static bool kvm_is_write_fault(struct kvm_vcpu *vcpu) > > return kvm_vcpu_dabt_iswrite(vcpu); > > } > > > > -static bool kvm_is_device_pfn(unsigned long pfn) > > -{ > > - return !pfn_valid(pfn); > > -} > > - > > static void coherent_cache_guest_page(struct kvm_vcpu *vcpu, pfn_t pfn, > > unsigned long size, bool uncached) > > { > > -- > > 1.9.1 > > >
diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c index 0512ed4..a2c1ebf 100644 --- a/arch/arm/kvm/mmu.c +++ b/arch/arm/kvm/mmu.c @@ -78,6 +78,11 @@ static void kvm_flush_dcache_pud(pud_t pud) __kvm_flush_dcache_pud(pud); } +static bool kvm_is_device_pfn(unsigned long pfn) +{ + return !pfn_valid(pfn); +} + static int mmu_topup_memory_cache(struct kvm_mmu_memory_cache *cache, int min, int max) { @@ -174,7 +179,7 @@ static void unmap_ptes(struct kvm *kvm, pmd_t *pmd, kvm_tlb_flush_vmid_ipa(kvm, addr); /* No need to invalidate the cache for device mappings */ - if ((pte_val(old_pte) & PAGE_S2_DEVICE) != PAGE_S2_DEVICE) + if (!kvm_is_device_pfn(__phys_to_pfn(addr))) kvm_flush_dcache_pte(old_pte); put_page(virt_to_page(pte)); @@ -266,8 +271,7 @@ static void stage2_flush_ptes(struct kvm *kvm, pmd_t *pmd, pte = pte_offset_kernel(pmd, addr); do { - if (!pte_none(*pte) && - (pte_val(*pte) & PAGE_S2_DEVICE) != PAGE_S2_DEVICE) + if (!pte_none(*pte) && !kvm_is_device_pfn(__phys_to_pfn(addr))) kvm_flush_dcache_pte(*pte); } while (pte++, addr += PAGE_SIZE, addr != end); } @@ -983,11 +987,6 @@ static bool kvm_is_write_fault(struct kvm_vcpu *vcpu) return kvm_vcpu_dabt_iswrite(vcpu); } -static bool kvm_is_device_pfn(unsigned long pfn) -{ - return !pfn_valid(pfn); -} - static void coherent_cache_guest_page(struct kvm_vcpu *vcpu, pfn_t pfn, unsigned long size, bool uncached) {