* [PATCH 0/4] ARM: KVM: random mmu related fixes for 3.10 @ 2013-04-30 14:17 ` Marc Zyngier 0 siblings, 0 replies; 24+ messages in thread From: Marc Zyngier @ 2013-04-30 14:17 UTC (permalink / raw) To: linux-arm-kernel This patch series fixes a number of of KVM/ARM issues that have either been spotted during the review of the arm64 code, or while reworking related code. Only the first patch fixes a potential (if unlikely) problem, the others are either cosmetic or performance related. Tested on TC-2. Marc Zyngier (4): ARM: KVM: be more thorough when invalidating TLBs ARM: KVM: remove dead prototype for __kvm_tlb_flush_vmid ARM: KVM: relax cache maintainance when building page pables ARM: KVM: fix use of S2_PGD_SIZE arch/arm/include/asm/kvm_asm.h | 2 -- arch/arm/include/asm/kvm_mmu.h | 1 + arch/arm/kvm/interrupts.S | 2 ++ arch/arm/kvm/mmu.c | 44 ++++++++++++++++++++++++------------------ 4 files changed, 28 insertions(+), 21 deletions(-) -- 1.8.2.1 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 0/4] ARM: KVM: random mmu related fixes for 3.10 @ 2013-04-30 14:17 ` Marc Zyngier 0 siblings, 0 replies; 24+ messages in thread From: Marc Zyngier @ 2013-04-30 14:17 UTC (permalink / raw) To: linux-arm-kernel, kvmarm, kvm This patch series fixes a number of of KVM/ARM issues that have either been spotted during the review of the arm64 code, or while reworking related code. Only the first patch fixes a potential (if unlikely) problem, the others are either cosmetic or performance related. Tested on TC-2. Marc Zyngier (4): ARM: KVM: be more thorough when invalidating TLBs ARM: KVM: remove dead prototype for __kvm_tlb_flush_vmid ARM: KVM: relax cache maintainance when building page pables ARM: KVM: fix use of S2_PGD_SIZE arch/arm/include/asm/kvm_asm.h | 2 -- arch/arm/include/asm/kvm_mmu.h | 1 + arch/arm/kvm/interrupts.S | 2 ++ arch/arm/kvm/mmu.c | 44 ++++++++++++++++++++++++------------------ 4 files changed, 28 insertions(+), 21 deletions(-) -- 1.8.2.1 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/4] ARM: KVM: be more thorough when invalidating TLBs 2013-04-30 14:17 ` Marc Zyngier @ 2013-04-30 14:17 ` Marc Zyngier -1 siblings, 0 replies; 24+ messages in thread From: Marc Zyngier @ 2013-04-30 14:17 UTC (permalink / raw) To: linux-arm-kernel The KVM/ARM MMU code doesn't take care of invalidating TLBs before freeing a {pte,pmd} table. This could cause problems if the page is reallocated and then speculated into by another CPU. Reported-by: Catalin Marinas <catalin.marinas@arm.com> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> --- arch/arm/kvm/interrupts.S | 2 ++ arch/arm/kvm/mmu.c | 36 +++++++++++++++++++++--------------- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S index f7793df..9e2d906c 100644 --- a/arch/arm/kvm/interrupts.S +++ b/arch/arm/kvm/interrupts.S @@ -37,6 +37,8 @@ __kvm_hyp_code_start: * * void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa); * + * Invalidate TLB for the given IPA, or invalidate all if ipa is zero. + * * We rely on the hardware to broadcast the TLB invalidation to all CPUs * inside the inner-shareable domain (which is the case for all v7 * implementations). If we come across a non-IS SMP implementation, we'll diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c index 9657065..71f2a57 100644 --- a/arch/arm/kvm/mmu.c +++ b/arch/arm/kvm/mmu.c @@ -43,7 +43,8 @@ static phys_addr_t hyp_idmap_vector; static void kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa) { - kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, kvm, ipa); + if (kvm) + kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, kvm, ipa); } static int mmu_topup_memory_cache(struct kvm_mmu_memory_cache *cache, @@ -78,18 +79,20 @@ static void *mmu_memory_cache_alloc(struct kvm_mmu_memory_cache *mc) return p; } -static void clear_pud_entry(pud_t *pud) +static void clear_pud_entry(struct kvm *kvm, pud_t *pud, phys_addr_t addr) { pmd_t *pmd_table = pmd_offset(pud, 0); pud_clear(pud); + kvm_tlb_flush_vmid_ipa(kvm, addr); pmd_free(NULL, pmd_table); put_page(virt_to_page(pud)); } -static void clear_pmd_entry(pmd_t *pmd) +static void clear_pmd_entry(struct kvm *kvm, pmd_t *pmd, phys_addr_t addr) { pte_t *pte_table = pte_offset_kernel(pmd, 0); pmd_clear(pmd); + kvm_tlb_flush_vmid_ipa(kvm, addr); pte_free_kernel(NULL, pte_table); put_page(virt_to_page(pmd)); } @@ -100,11 +103,12 @@ static bool pmd_empty(pmd_t *pmd) return page_count(pmd_page) == 1; } -static void clear_pte_entry(pte_t *pte) +static void clear_pte_entry(struct kvm *kvm, pte_t *pte, phys_addr_t addr) { if (pte_present(*pte)) { kvm_set_pte(pte, __pte(0)); put_page(virt_to_page(pte)); + kvm_tlb_flush_vmid_ipa(kvm, addr); } } @@ -114,7 +118,8 @@ static bool pte_empty(pte_t *pte) return page_count(pte_page) == 1; } -static void unmap_range(pgd_t *pgdp, unsigned long long start, u64 size) +static void unmap_range(struct kvm *kvm, pgd_t *pgdp, + unsigned long long start, u64 size) { pgd_t *pgd; pud_t *pud; @@ -138,15 +143,15 @@ static void unmap_range(pgd_t *pgdp, unsigned long long start, u64 size) } pte = pte_offset_kernel(pmd, addr); - clear_pte_entry(pte); + clear_pte_entry(kvm, pte, addr); range = PAGE_SIZE; /* If we emptied the pte, walk back up the ladder */ if (pte_empty(pte)) { - clear_pmd_entry(pmd); + clear_pmd_entry(kvm, pmd, addr); range = PMD_SIZE; if (pmd_empty(pmd)) { - clear_pud_entry(pud); + clear_pud_entry(kvm, pud, addr); range = PUD_SIZE; } } @@ -165,14 +170,14 @@ void free_boot_hyp_pgd(void) mutex_lock(&kvm_hyp_pgd_mutex); if (boot_hyp_pgd) { - unmap_range(boot_hyp_pgd, hyp_idmap_start, PAGE_SIZE); - unmap_range(boot_hyp_pgd, TRAMPOLINE_VA, PAGE_SIZE); + unmap_range(NULL, boot_hyp_pgd, hyp_idmap_start, PAGE_SIZE); + unmap_range(NULL, boot_hyp_pgd, TRAMPOLINE_VA, PAGE_SIZE); kfree(boot_hyp_pgd); boot_hyp_pgd = NULL; } if (hyp_pgd) - unmap_range(hyp_pgd, TRAMPOLINE_VA, PAGE_SIZE); + unmap_range(NULL, hyp_pgd, TRAMPOLINE_VA, PAGE_SIZE); kfree(init_bounce_page); init_bounce_page = NULL; @@ -200,9 +205,10 @@ void free_hyp_pgds(void) if (hyp_pgd) { for (addr = PAGE_OFFSET; virt_addr_valid(addr); addr += PGDIR_SIZE) - unmap_range(hyp_pgd, KERN_TO_HYP(addr), PGDIR_SIZE); + unmap_range(NULL, hyp_pgd, KERN_TO_HYP(addr), PGDIR_SIZE); for (addr = VMALLOC_START; is_vmalloc_addr((void*)addr); addr += PGDIR_SIZE) - unmap_range(hyp_pgd, KERN_TO_HYP(addr), PGDIR_SIZE); + unmap_range(NULL, hyp_pgd, KERN_TO_HYP(addr), PGDIR_SIZE); + kfree(hyp_pgd); hyp_pgd = NULL; } @@ -393,7 +399,7 @@ int kvm_alloc_stage2_pgd(struct kvm *kvm) */ static void unmap_stage2_range(struct kvm *kvm, phys_addr_t start, u64 size) { - unmap_range(kvm->arch.pgd, start, size); + unmap_range(kvm, kvm->arch.pgd, start, size); } /** @@ -413,6 +419,7 @@ void kvm_free_stage2_pgd(struct kvm *kvm) return; unmap_stage2_range(kvm, 0, KVM_PHYS_SIZE); + kvm_tlb_flush_vmid_ipa(kvm, 0); /* Invalidate TLB ALL */ free_pages((unsigned long)kvm->arch.pgd, S2_PGD_ORDER); kvm->arch.pgd = NULL; } @@ -675,7 +682,6 @@ static void handle_hva_to_gpa(struct kvm *kvm, static void kvm_unmap_hva_handler(struct kvm *kvm, gpa_t gpa, void *data) { unmap_stage2_range(kvm, gpa, PAGE_SIZE); - kvm_tlb_flush_vmid_ipa(kvm, gpa); } int kvm_unmap_hva(struct kvm *kvm, unsigned long hva) -- 1.8.2.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 1/4] ARM: KVM: be more thorough when invalidating TLBs @ 2013-04-30 14:17 ` Marc Zyngier 0 siblings, 0 replies; 24+ messages in thread From: Marc Zyngier @ 2013-04-30 14:17 UTC (permalink / raw) To: linux-arm-kernel, kvmarm, kvm; +Cc: Catalin Marinas The KVM/ARM MMU code doesn't take care of invalidating TLBs before freeing a {pte,pmd} table. This could cause problems if the page is reallocated and then speculated into by another CPU. Reported-by: Catalin Marinas <catalin.marinas@arm.com> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> --- arch/arm/kvm/interrupts.S | 2 ++ arch/arm/kvm/mmu.c | 36 +++++++++++++++++++++--------------- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S index f7793df..9e2d906c 100644 --- a/arch/arm/kvm/interrupts.S +++ b/arch/arm/kvm/interrupts.S @@ -37,6 +37,8 @@ __kvm_hyp_code_start: * * void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa); * + * Invalidate TLB for the given IPA, or invalidate all if ipa is zero. + * * We rely on the hardware to broadcast the TLB invalidation to all CPUs * inside the inner-shareable domain (which is the case for all v7 * implementations). If we come across a non-IS SMP implementation, we'll diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c index 9657065..71f2a57 100644 --- a/arch/arm/kvm/mmu.c +++ b/arch/arm/kvm/mmu.c @@ -43,7 +43,8 @@ static phys_addr_t hyp_idmap_vector; static void kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa) { - kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, kvm, ipa); + if (kvm) + kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, kvm, ipa); } static int mmu_topup_memory_cache(struct kvm_mmu_memory_cache *cache, @@ -78,18 +79,20 @@ static void *mmu_memory_cache_alloc(struct kvm_mmu_memory_cache *mc) return p; } -static void clear_pud_entry(pud_t *pud) +static void clear_pud_entry(struct kvm *kvm, pud_t *pud, phys_addr_t addr) { pmd_t *pmd_table = pmd_offset(pud, 0); pud_clear(pud); + kvm_tlb_flush_vmid_ipa(kvm, addr); pmd_free(NULL, pmd_table); put_page(virt_to_page(pud)); } -static void clear_pmd_entry(pmd_t *pmd) +static void clear_pmd_entry(struct kvm *kvm, pmd_t *pmd, phys_addr_t addr) { pte_t *pte_table = pte_offset_kernel(pmd, 0); pmd_clear(pmd); + kvm_tlb_flush_vmid_ipa(kvm, addr); pte_free_kernel(NULL, pte_table); put_page(virt_to_page(pmd)); } @@ -100,11 +103,12 @@ static bool pmd_empty(pmd_t *pmd) return page_count(pmd_page) == 1; } -static void clear_pte_entry(pte_t *pte) +static void clear_pte_entry(struct kvm *kvm, pte_t *pte, phys_addr_t addr) { if (pte_present(*pte)) { kvm_set_pte(pte, __pte(0)); put_page(virt_to_page(pte)); + kvm_tlb_flush_vmid_ipa(kvm, addr); } } @@ -114,7 +118,8 @@ static bool pte_empty(pte_t *pte) return page_count(pte_page) == 1; } -static void unmap_range(pgd_t *pgdp, unsigned long long start, u64 size) +static void unmap_range(struct kvm *kvm, pgd_t *pgdp, + unsigned long long start, u64 size) { pgd_t *pgd; pud_t *pud; @@ -138,15 +143,15 @@ static void unmap_range(pgd_t *pgdp, unsigned long long start, u64 size) } pte = pte_offset_kernel(pmd, addr); - clear_pte_entry(pte); + clear_pte_entry(kvm, pte, addr); range = PAGE_SIZE; /* If we emptied the pte, walk back up the ladder */ if (pte_empty(pte)) { - clear_pmd_entry(pmd); + clear_pmd_entry(kvm, pmd, addr); range = PMD_SIZE; if (pmd_empty(pmd)) { - clear_pud_entry(pud); + clear_pud_entry(kvm, pud, addr); range = PUD_SIZE; } } @@ -165,14 +170,14 @@ void free_boot_hyp_pgd(void) mutex_lock(&kvm_hyp_pgd_mutex); if (boot_hyp_pgd) { - unmap_range(boot_hyp_pgd, hyp_idmap_start, PAGE_SIZE); - unmap_range(boot_hyp_pgd, TRAMPOLINE_VA, PAGE_SIZE); + unmap_range(NULL, boot_hyp_pgd, hyp_idmap_start, PAGE_SIZE); + unmap_range(NULL, boot_hyp_pgd, TRAMPOLINE_VA, PAGE_SIZE); kfree(boot_hyp_pgd); boot_hyp_pgd = NULL; } if (hyp_pgd) - unmap_range(hyp_pgd, TRAMPOLINE_VA, PAGE_SIZE); + unmap_range(NULL, hyp_pgd, TRAMPOLINE_VA, PAGE_SIZE); kfree(init_bounce_page); init_bounce_page = NULL; @@ -200,9 +205,10 @@ void free_hyp_pgds(void) if (hyp_pgd) { for (addr = PAGE_OFFSET; virt_addr_valid(addr); addr += PGDIR_SIZE) - unmap_range(hyp_pgd, KERN_TO_HYP(addr), PGDIR_SIZE); + unmap_range(NULL, hyp_pgd, KERN_TO_HYP(addr), PGDIR_SIZE); for (addr = VMALLOC_START; is_vmalloc_addr((void*)addr); addr += PGDIR_SIZE) - unmap_range(hyp_pgd, KERN_TO_HYP(addr), PGDIR_SIZE); + unmap_range(NULL, hyp_pgd, KERN_TO_HYP(addr), PGDIR_SIZE); + kfree(hyp_pgd); hyp_pgd = NULL; } @@ -393,7 +399,7 @@ int kvm_alloc_stage2_pgd(struct kvm *kvm) */ static void unmap_stage2_range(struct kvm *kvm, phys_addr_t start, u64 size) { - unmap_range(kvm->arch.pgd, start, size); + unmap_range(kvm, kvm->arch.pgd, start, size); } /** @@ -413,6 +419,7 @@ void kvm_free_stage2_pgd(struct kvm *kvm) return; unmap_stage2_range(kvm, 0, KVM_PHYS_SIZE); + kvm_tlb_flush_vmid_ipa(kvm, 0); /* Invalidate TLB ALL */ free_pages((unsigned long)kvm->arch.pgd, S2_PGD_ORDER); kvm->arch.pgd = NULL; } @@ -675,7 +682,6 @@ static void handle_hva_to_gpa(struct kvm *kvm, static void kvm_unmap_hva_handler(struct kvm *kvm, gpa_t gpa, void *data) { unmap_stage2_range(kvm, gpa, PAGE_SIZE); - kvm_tlb_flush_vmid_ipa(kvm, gpa); } int kvm_unmap_hva(struct kvm *kvm, unsigned long hva) -- 1.8.2.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 1/4] ARM: KVM: be more thorough when invalidating TLBs 2013-04-30 14:17 ` Marc Zyngier @ 2013-04-30 17:17 ` Christoffer Dall -1 siblings, 0 replies; 24+ messages in thread From: Christoffer Dall @ 2013-04-30 17:17 UTC (permalink / raw) To: linux-arm-kernel On Tue, Apr 30, 2013 at 7:17 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: > The KVM/ARM MMU code doesn't take care of invalidating TLBs before > freeing a {pte,pmd} table. This could cause problems if the page > is reallocated and then speculated into by another CPU. > > Reported-by: Catalin Marinas <catalin.marinas@arm.com> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > --- > arch/arm/kvm/interrupts.S | 2 ++ > arch/arm/kvm/mmu.c | 36 +++++++++++++++++++++--------------- > 2 files changed, 23 insertions(+), 15 deletions(-) > > diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S > index f7793df..9e2d906c 100644 > --- a/arch/arm/kvm/interrupts.S > +++ b/arch/arm/kvm/interrupts.S > @@ -37,6 +37,8 @@ __kvm_hyp_code_start: > * > * void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa); > * > + * Invalidate TLB for the given IPA, or invalidate all if ipa is zero. > + * > * We rely on the hardware to broadcast the TLB invalidation to all CPUs > * inside the inner-shareable domain (which is the case for all v7 > * implementations). If we come across a non-IS SMP implementation, we'll > diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c > index 9657065..71f2a57 100644 > --- a/arch/arm/kvm/mmu.c > +++ b/arch/arm/kvm/mmu.c > @@ -43,7 +43,8 @@ static phys_addr_t hyp_idmap_vector; > > static void kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa) > { > - kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, kvm, ipa); > + if (kvm) > + kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, kvm, ipa); this feels slightly abusive? could we add a comment that hyp page table freeing don't need tlb flushing from these functions and don't have a struct kvm? alternatively it may be more clear to have something like: if (kvm) /* Check if Hyp or Stage-2 page table */ kvm_tlb_flush_vmid_ipa(kvm, addr); in the callers, but, eh, maybe I'm over thinking this. > } > > static int mmu_topup_memory_cache(struct kvm_mmu_memory_cache *cache, > @@ -78,18 +79,20 @@ static void *mmu_memory_cache_alloc(struct kvm_mmu_memory_cache *mc) > return p; > } > > -static void clear_pud_entry(pud_t *pud) > +static void clear_pud_entry(struct kvm *kvm, pud_t *pud, phys_addr_t addr) > { > pmd_t *pmd_table = pmd_offset(pud, 0); > pud_clear(pud); > + kvm_tlb_flush_vmid_ipa(kvm, addr); > pmd_free(NULL, pmd_table); > put_page(virt_to_page(pud)); > } > > -static void clear_pmd_entry(pmd_t *pmd) > +static void clear_pmd_entry(struct kvm *kvm, pmd_t *pmd, phys_addr_t addr) > { > pte_t *pte_table = pte_offset_kernel(pmd, 0); > pmd_clear(pmd); > + kvm_tlb_flush_vmid_ipa(kvm, addr); > pte_free_kernel(NULL, pte_table); > put_page(virt_to_page(pmd)); > } > @@ -100,11 +103,12 @@ static bool pmd_empty(pmd_t *pmd) > return page_count(pmd_page) == 1; > } > > -static void clear_pte_entry(pte_t *pte) > +static void clear_pte_entry(struct kvm *kvm, pte_t *pte, phys_addr_t addr) > { > if (pte_present(*pte)) { > kvm_set_pte(pte, __pte(0)); > put_page(virt_to_page(pte)); > + kvm_tlb_flush_vmid_ipa(kvm, addr); I enjoyed the fact that it's perfectly correct to have this flush after the put_page. > } > } > > @@ -114,7 +118,8 @@ static bool pte_empty(pte_t *pte) > return page_count(pte_page) == 1; > } > > -static void unmap_range(pgd_t *pgdp, unsigned long long start, u64 size) > +static void unmap_range(struct kvm *kvm, pgd_t *pgdp, > + unsigned long long start, u64 size) > { > pgd_t *pgd; > pud_t *pud; > @@ -138,15 +143,15 @@ static void unmap_range(pgd_t *pgdp, unsigned long long start, u64 size) > } > > pte = pte_offset_kernel(pmd, addr); > - clear_pte_entry(pte); > + clear_pte_entry(kvm, pte, addr); > range = PAGE_SIZE; > > /* If we emptied the pte, walk back up the ladder */ > if (pte_empty(pte)) { > - clear_pmd_entry(pmd); > + clear_pmd_entry(kvm, pmd, addr); > range = PMD_SIZE; > if (pmd_empty(pmd)) { > - clear_pud_entry(pud); > + clear_pud_entry(kvm, pud, addr); > range = PUD_SIZE; > } > } > @@ -165,14 +170,14 @@ void free_boot_hyp_pgd(void) > mutex_lock(&kvm_hyp_pgd_mutex); > > if (boot_hyp_pgd) { > - unmap_range(boot_hyp_pgd, hyp_idmap_start, PAGE_SIZE); > - unmap_range(boot_hyp_pgd, TRAMPOLINE_VA, PAGE_SIZE); > + unmap_range(NULL, boot_hyp_pgd, hyp_idmap_start, PAGE_SIZE); > + unmap_range(NULL, boot_hyp_pgd, TRAMPOLINE_VA, PAGE_SIZE); > kfree(boot_hyp_pgd); > boot_hyp_pgd = NULL; > } > > if (hyp_pgd) > - unmap_range(hyp_pgd, TRAMPOLINE_VA, PAGE_SIZE); > + unmap_range(NULL, hyp_pgd, TRAMPOLINE_VA, PAGE_SIZE); > > kfree(init_bounce_page); > init_bounce_page = NULL; > @@ -200,9 +205,10 @@ void free_hyp_pgds(void) > > if (hyp_pgd) { > for (addr = PAGE_OFFSET; virt_addr_valid(addr); addr += PGDIR_SIZE) > - unmap_range(hyp_pgd, KERN_TO_HYP(addr), PGDIR_SIZE); > + unmap_range(NULL, hyp_pgd, KERN_TO_HYP(addr), PGDIR_SIZE); > for (addr = VMALLOC_START; is_vmalloc_addr((void*)addr); addr += PGDIR_SIZE) > - unmap_range(hyp_pgd, KERN_TO_HYP(addr), PGDIR_SIZE); > + unmap_range(NULL, hyp_pgd, KERN_TO_HYP(addr), PGDIR_SIZE); > + > kfree(hyp_pgd); > hyp_pgd = NULL; > } > @@ -393,7 +399,7 @@ int kvm_alloc_stage2_pgd(struct kvm *kvm) > */ > static void unmap_stage2_range(struct kvm *kvm, phys_addr_t start, u64 size) > { > - unmap_range(kvm->arch.pgd, start, size); > + unmap_range(kvm, kvm->arch.pgd, start, size); > } > > /** > @@ -413,6 +419,7 @@ void kvm_free_stage2_pgd(struct kvm *kvm) > return; > > unmap_stage2_range(kvm, 0, KVM_PHYS_SIZE); > + kvm_tlb_flush_vmid_ipa(kvm, 0); /* Invalidate TLB ALL */ > free_pages((unsigned long)kvm->arch.pgd, S2_PGD_ORDER); > kvm->arch.pgd = NULL; > } > @@ -675,7 +682,6 @@ static void handle_hva_to_gpa(struct kvm *kvm, > static void kvm_unmap_hva_handler(struct kvm *kvm, gpa_t gpa, void *data) > { > unmap_stage2_range(kvm, gpa, PAGE_SIZE); > - kvm_tlb_flush_vmid_ipa(kvm, gpa); > } > > int kvm_unmap_hva(struct kvm *kvm, unsigned long hva) > -- > 1.8.2.1 > > looks good to me, -Christoffer ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/4] ARM: KVM: be more thorough when invalidating TLBs @ 2013-04-30 17:17 ` Christoffer Dall 0 siblings, 0 replies; 24+ messages in thread From: Christoffer Dall @ 2013-04-30 17:17 UTC (permalink / raw) To: Marc Zyngier Cc: linux-arm-kernel, kvmarm@lists.cs.columbia.edu, KVM General, Catalin Marinas On Tue, Apr 30, 2013 at 7:17 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: > The KVM/ARM MMU code doesn't take care of invalidating TLBs before > freeing a {pte,pmd} table. This could cause problems if the page > is reallocated and then speculated into by another CPU. > > Reported-by: Catalin Marinas <catalin.marinas@arm.com> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > --- > arch/arm/kvm/interrupts.S | 2 ++ > arch/arm/kvm/mmu.c | 36 +++++++++++++++++++++--------------- > 2 files changed, 23 insertions(+), 15 deletions(-) > > diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S > index f7793df..9e2d906c 100644 > --- a/arch/arm/kvm/interrupts.S > +++ b/arch/arm/kvm/interrupts.S > @@ -37,6 +37,8 @@ __kvm_hyp_code_start: > * > * void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa); > * > + * Invalidate TLB for the given IPA, or invalidate all if ipa is zero. > + * > * We rely on the hardware to broadcast the TLB invalidation to all CPUs > * inside the inner-shareable domain (which is the case for all v7 > * implementations). If we come across a non-IS SMP implementation, we'll > diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c > index 9657065..71f2a57 100644 > --- a/arch/arm/kvm/mmu.c > +++ b/arch/arm/kvm/mmu.c > @@ -43,7 +43,8 @@ static phys_addr_t hyp_idmap_vector; > > static void kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa) > { > - kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, kvm, ipa); > + if (kvm) > + kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, kvm, ipa); this feels slightly abusive? could we add a comment that hyp page table freeing don't need tlb flushing from these functions and don't have a struct kvm? alternatively it may be more clear to have something like: if (kvm) /* Check if Hyp or Stage-2 page table */ kvm_tlb_flush_vmid_ipa(kvm, addr); in the callers, but, eh, maybe I'm over thinking this. > } > > static int mmu_topup_memory_cache(struct kvm_mmu_memory_cache *cache, > @@ -78,18 +79,20 @@ static void *mmu_memory_cache_alloc(struct kvm_mmu_memory_cache *mc) > return p; > } > > -static void clear_pud_entry(pud_t *pud) > +static void clear_pud_entry(struct kvm *kvm, pud_t *pud, phys_addr_t addr) > { > pmd_t *pmd_table = pmd_offset(pud, 0); > pud_clear(pud); > + kvm_tlb_flush_vmid_ipa(kvm, addr); > pmd_free(NULL, pmd_table); > put_page(virt_to_page(pud)); > } > > -static void clear_pmd_entry(pmd_t *pmd) > +static void clear_pmd_entry(struct kvm *kvm, pmd_t *pmd, phys_addr_t addr) > { > pte_t *pte_table = pte_offset_kernel(pmd, 0); > pmd_clear(pmd); > + kvm_tlb_flush_vmid_ipa(kvm, addr); > pte_free_kernel(NULL, pte_table); > put_page(virt_to_page(pmd)); > } > @@ -100,11 +103,12 @@ static bool pmd_empty(pmd_t *pmd) > return page_count(pmd_page) == 1; > } > > -static void clear_pte_entry(pte_t *pte) > +static void clear_pte_entry(struct kvm *kvm, pte_t *pte, phys_addr_t addr) > { > if (pte_present(*pte)) { > kvm_set_pte(pte, __pte(0)); > put_page(virt_to_page(pte)); > + kvm_tlb_flush_vmid_ipa(kvm, addr); I enjoyed the fact that it's perfectly correct to have this flush after the put_page. > } > } > > @@ -114,7 +118,8 @@ static bool pte_empty(pte_t *pte) > return page_count(pte_page) == 1; > } > > -static void unmap_range(pgd_t *pgdp, unsigned long long start, u64 size) > +static void unmap_range(struct kvm *kvm, pgd_t *pgdp, > + unsigned long long start, u64 size) > { > pgd_t *pgd; > pud_t *pud; > @@ -138,15 +143,15 @@ static void unmap_range(pgd_t *pgdp, unsigned long long start, u64 size) > } > > pte = pte_offset_kernel(pmd, addr); > - clear_pte_entry(pte); > + clear_pte_entry(kvm, pte, addr); > range = PAGE_SIZE; > > /* If we emptied the pte, walk back up the ladder */ > if (pte_empty(pte)) { > - clear_pmd_entry(pmd); > + clear_pmd_entry(kvm, pmd, addr); > range = PMD_SIZE; > if (pmd_empty(pmd)) { > - clear_pud_entry(pud); > + clear_pud_entry(kvm, pud, addr); > range = PUD_SIZE; > } > } > @@ -165,14 +170,14 @@ void free_boot_hyp_pgd(void) > mutex_lock(&kvm_hyp_pgd_mutex); > > if (boot_hyp_pgd) { > - unmap_range(boot_hyp_pgd, hyp_idmap_start, PAGE_SIZE); > - unmap_range(boot_hyp_pgd, TRAMPOLINE_VA, PAGE_SIZE); > + unmap_range(NULL, boot_hyp_pgd, hyp_idmap_start, PAGE_SIZE); > + unmap_range(NULL, boot_hyp_pgd, TRAMPOLINE_VA, PAGE_SIZE); > kfree(boot_hyp_pgd); > boot_hyp_pgd = NULL; > } > > if (hyp_pgd) > - unmap_range(hyp_pgd, TRAMPOLINE_VA, PAGE_SIZE); > + unmap_range(NULL, hyp_pgd, TRAMPOLINE_VA, PAGE_SIZE); > > kfree(init_bounce_page); > init_bounce_page = NULL; > @@ -200,9 +205,10 @@ void free_hyp_pgds(void) > > if (hyp_pgd) { > for (addr = PAGE_OFFSET; virt_addr_valid(addr); addr += PGDIR_SIZE) > - unmap_range(hyp_pgd, KERN_TO_HYP(addr), PGDIR_SIZE); > + unmap_range(NULL, hyp_pgd, KERN_TO_HYP(addr), PGDIR_SIZE); > for (addr = VMALLOC_START; is_vmalloc_addr((void*)addr); addr += PGDIR_SIZE) > - unmap_range(hyp_pgd, KERN_TO_HYP(addr), PGDIR_SIZE); > + unmap_range(NULL, hyp_pgd, KERN_TO_HYP(addr), PGDIR_SIZE); > + > kfree(hyp_pgd); > hyp_pgd = NULL; > } > @@ -393,7 +399,7 @@ int kvm_alloc_stage2_pgd(struct kvm *kvm) > */ > static void unmap_stage2_range(struct kvm *kvm, phys_addr_t start, u64 size) > { > - unmap_range(kvm->arch.pgd, start, size); > + unmap_range(kvm, kvm->arch.pgd, start, size); > } > > /** > @@ -413,6 +419,7 @@ void kvm_free_stage2_pgd(struct kvm *kvm) > return; > > unmap_stage2_range(kvm, 0, KVM_PHYS_SIZE); > + kvm_tlb_flush_vmid_ipa(kvm, 0); /* Invalidate TLB ALL */ > free_pages((unsigned long)kvm->arch.pgd, S2_PGD_ORDER); > kvm->arch.pgd = NULL; > } > @@ -675,7 +682,6 @@ static void handle_hva_to_gpa(struct kvm *kvm, > static void kvm_unmap_hva_handler(struct kvm *kvm, gpa_t gpa, void *data) > { > unmap_stage2_range(kvm, gpa, PAGE_SIZE); > - kvm_tlb_flush_vmid_ipa(kvm, gpa); > } > > int kvm_unmap_hva(struct kvm *kvm, unsigned long hva) > -- > 1.8.2.1 > > looks good to me, -Christoffer ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/4] ARM: KVM: be more thorough when invalidating TLBs 2013-04-30 17:17 ` Christoffer Dall @ 2013-04-30 18:02 ` Marc Zyngier -1 siblings, 0 replies; 24+ messages in thread From: Marc Zyngier @ 2013-04-30 18:02 UTC (permalink / raw) To: linux-arm-kernel On 30/04/13 18:17, Christoffer Dall wrote: > On Tue, Apr 30, 2013 at 7:17 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: >> The KVM/ARM MMU code doesn't take care of invalidating TLBs before >> freeing a {pte,pmd} table. This could cause problems if the page >> is reallocated and then speculated into by another CPU. >> >> Reported-by: Catalin Marinas <catalin.marinas@arm.com> >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> >> --- >> arch/arm/kvm/interrupts.S | 2 ++ >> arch/arm/kvm/mmu.c | 36 +++++++++++++++++++++--------------- >> 2 files changed, 23 insertions(+), 15 deletions(-) >> >> diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S >> index f7793df..9e2d906c 100644 >> --- a/arch/arm/kvm/interrupts.S >> +++ b/arch/arm/kvm/interrupts.S >> @@ -37,6 +37,8 @@ __kvm_hyp_code_start: >> * >> * void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa); >> * >> + * Invalidate TLB for the given IPA, or invalidate all if ipa is zero. >> + * >> * We rely on the hardware to broadcast the TLB invalidation to all CPUs >> * inside the inner-shareable domain (which is the case for all v7 >> * implementations). If we come across a non-IS SMP implementation, we'll >> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c >> index 9657065..71f2a57 100644 >> --- a/arch/arm/kvm/mmu.c >> +++ b/arch/arm/kvm/mmu.c >> @@ -43,7 +43,8 @@ static phys_addr_t hyp_idmap_vector; >> >> static void kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa) >> { >> - kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, kvm, ipa); >> + if (kvm) >> + kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, kvm, ipa); > > this feels slightly abusive? could we add a comment that hyp page > table freeing don't need tlb flushing from these functions and don't > have a struct kvm? > > alternatively it may be more clear to have something like: > > if (kvm) /* Check if Hyp or Stage-2 page table */ > kvm_tlb_flush_vmid_ipa(kvm, addr); > > in the callers, but, eh, maybe I'm over thinking this. That code would be repeated three times, which feels a bit overkill. I'll add a comment to the function so it is crystal clear. Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/4] ARM: KVM: be more thorough when invalidating TLBs @ 2013-04-30 18:02 ` Marc Zyngier 0 siblings, 0 replies; 24+ messages in thread From: Marc Zyngier @ 2013-04-30 18:02 UTC (permalink / raw) To: Christoffer Dall Cc: linux-arm-kernel, kvmarm@lists.cs.columbia.edu, KVM General, Catalin Marinas On 30/04/13 18:17, Christoffer Dall wrote: > On Tue, Apr 30, 2013 at 7:17 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: >> The KVM/ARM MMU code doesn't take care of invalidating TLBs before >> freeing a {pte,pmd} table. This could cause problems if the page >> is reallocated and then speculated into by another CPU. >> >> Reported-by: Catalin Marinas <catalin.marinas@arm.com> >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> >> --- >> arch/arm/kvm/interrupts.S | 2 ++ >> arch/arm/kvm/mmu.c | 36 +++++++++++++++++++++--------------- >> 2 files changed, 23 insertions(+), 15 deletions(-) >> >> diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S >> index f7793df..9e2d906c 100644 >> --- a/arch/arm/kvm/interrupts.S >> +++ b/arch/arm/kvm/interrupts.S >> @@ -37,6 +37,8 @@ __kvm_hyp_code_start: >> * >> * void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa); >> * >> + * Invalidate TLB for the given IPA, or invalidate all if ipa is zero. >> + * >> * We rely on the hardware to broadcast the TLB invalidation to all CPUs >> * inside the inner-shareable domain (which is the case for all v7 >> * implementations). If we come across a non-IS SMP implementation, we'll >> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c >> index 9657065..71f2a57 100644 >> --- a/arch/arm/kvm/mmu.c >> +++ b/arch/arm/kvm/mmu.c >> @@ -43,7 +43,8 @@ static phys_addr_t hyp_idmap_vector; >> >> static void kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa) >> { >> - kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, kvm, ipa); >> + if (kvm) >> + kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, kvm, ipa); > > this feels slightly abusive? could we add a comment that hyp page > table freeing don't need tlb flushing from these functions and don't > have a struct kvm? > > alternatively it may be more clear to have something like: > > if (kvm) /* Check if Hyp or Stage-2 page table */ > kvm_tlb_flush_vmid_ipa(kvm, addr); > > in the callers, but, eh, maybe I'm over thinking this. That code would be repeated three times, which feels a bit overkill. I'll add a comment to the function so it is crystal clear. Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 2/4] ARM: KVM: remove dead prototype for __kvm_tlb_flush_vmid 2013-04-30 14:17 ` Marc Zyngier @ 2013-04-30 14:17 ` Marc Zyngier -1 siblings, 0 replies; 24+ messages in thread From: Marc Zyngier @ 2013-04-30 14:17 UTC (permalink / raw) To: linux-arm-kernel __kvm_tlb_flush_vmid has been renamed to __kvm_tlb_flush_vmid_ipa, and the old prototype should have been removed when the code was modified. Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> --- arch/arm/include/asm/kvm_asm.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/arch/arm/include/asm/kvm_asm.h b/arch/arm/include/asm/kvm_asm.h index 18d5032..6ae3d2a 100644 --- a/arch/arm/include/asm/kvm_asm.h +++ b/arch/arm/include/asm/kvm_asm.h @@ -72,8 +72,6 @@ extern char __kvm_hyp_vector[]; extern char __kvm_hyp_code_start[]; extern char __kvm_hyp_code_end[]; -extern void __kvm_tlb_flush_vmid(struct kvm *kvm); - extern void __kvm_flush_vm_context(void); extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa); -- 1.8.2.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/4] ARM: KVM: remove dead prototype for __kvm_tlb_flush_vmid @ 2013-04-30 14:17 ` Marc Zyngier 0 siblings, 0 replies; 24+ messages in thread From: Marc Zyngier @ 2013-04-30 14:17 UTC (permalink / raw) To: linux-arm-kernel, kvmarm, kvm __kvm_tlb_flush_vmid has been renamed to __kvm_tlb_flush_vmid_ipa, and the old prototype should have been removed when the code was modified. Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> --- arch/arm/include/asm/kvm_asm.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/arch/arm/include/asm/kvm_asm.h b/arch/arm/include/asm/kvm_asm.h index 18d5032..6ae3d2a 100644 --- a/arch/arm/include/asm/kvm_asm.h +++ b/arch/arm/include/asm/kvm_asm.h @@ -72,8 +72,6 @@ extern char __kvm_hyp_vector[]; extern char __kvm_hyp_code_start[]; extern char __kvm_hyp_code_end[]; -extern void __kvm_tlb_flush_vmid(struct kvm *kvm); - extern void __kvm_flush_vm_context(void); extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa); -- 1.8.2.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/4] ARM: KVM: relax cache maintainance when building page pables 2013-04-30 14:17 ` Marc Zyngier @ 2013-04-30 14:17 ` Marc Zyngier -1 siblings, 0 replies; 24+ messages in thread From: Marc Zyngier @ 2013-04-30 14:17 UTC (permalink / raw) To: linux-arm-kernel Patch 5a677ce044f1 (ARM: KVM: switch to a dual-step HYP init code) introduced code that flushes page tables to the point of coherency. This is overkill (point of unification is enough), and actually not required if running on a SMP capable platform. Change this code to use clean_dcache_area(), which is updated by ae8a8b9553bd (ARM: 7691/1: mm: kill unused TLB_CAN_READ_FROM_L1_CACHE and use ALT_SMP instead) to behave as a no-op on SMP ARMv7. Reported-by: Catalin Marinas <catalin.marinas@arm.com> Cc: Will Deacon <will.deacon@arm.com> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> --- arch/arm/include/asm/kvm_mmu.h | 1 + arch/arm/kvm/mmu.c | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h index 472ac70..2459821 100644 --- a/arch/arm/include/asm/kvm_mmu.h +++ b/arch/arm/include/asm/kvm_mmu.h @@ -129,6 +129,7 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn) } #define kvm_flush_dcache_to_poc(a,l) __cpuc_flush_dcache_area((a), (l)) +#define kvm_clean_dcache_to_pou(a,l) clean_dcache_area((a), (l)) #endif /* !__ASSEMBLY__ */ diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c index 71f2a57..b978ebe 100644 --- a/arch/arm/kvm/mmu.c +++ b/arch/arm/kvm/mmu.c @@ -228,7 +228,7 @@ static void create_hyp_pte_mappings(pmd_t *pmd, unsigned long start, pte = pte_offset_kernel(pmd, addr); kvm_set_pte(pte, pfn_pte(pfn, prot)); get_page(virt_to_page(pte)); - kvm_flush_dcache_to_poc(pte, sizeof(*pte)); + kvm_clean_dcache_to_pou(pte, sizeof(*pte)); pfn++; } while (addr += PAGE_SIZE, addr != end); } @@ -255,7 +255,7 @@ static int create_hyp_pmd_mappings(pud_t *pud, unsigned long start, } pmd_populate_kernel(NULL, pmd, pte); get_page(virt_to_page(pmd)); - kvm_flush_dcache_to_poc(pmd, sizeof(*pmd)); + kvm_clean_dcache_to_pou(pmd, sizeof(*pmd)); } next = pmd_addr_end(addr, end); @@ -293,7 +293,7 @@ static int __create_hyp_mappings(pgd_t *pgdp, } pud_populate(NULL, pud, pmd); get_page(virt_to_page(pud)); - kvm_flush_dcache_to_poc(pud, sizeof(*pud)); + kvm_clean_dcache_to_pou(pud, sizeof(*pud)); } next = pgd_addr_end(addr, end); -- 1.8.2.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/4] ARM: KVM: relax cache maintainance when building page pables @ 2013-04-30 14:17 ` Marc Zyngier 0 siblings, 0 replies; 24+ messages in thread From: Marc Zyngier @ 2013-04-30 14:17 UTC (permalink / raw) To: linux-arm-kernel, kvmarm, kvm; +Cc: Will Deacon, Catalin Marinas Patch 5a677ce044f1 (ARM: KVM: switch to a dual-step HYP init code) introduced code that flushes page tables to the point of coherency. This is overkill (point of unification is enough), and actually not required if running on a SMP capable platform. Change this code to use clean_dcache_area(), which is updated by ae8a8b9553bd (ARM: 7691/1: mm: kill unused TLB_CAN_READ_FROM_L1_CACHE and use ALT_SMP instead) to behave as a no-op on SMP ARMv7. Reported-by: Catalin Marinas <catalin.marinas@arm.com> Cc: Will Deacon <will.deacon@arm.com> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> --- arch/arm/include/asm/kvm_mmu.h | 1 + arch/arm/kvm/mmu.c | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h index 472ac70..2459821 100644 --- a/arch/arm/include/asm/kvm_mmu.h +++ b/arch/arm/include/asm/kvm_mmu.h @@ -129,6 +129,7 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn) } #define kvm_flush_dcache_to_poc(a,l) __cpuc_flush_dcache_area((a), (l)) +#define kvm_clean_dcache_to_pou(a,l) clean_dcache_area((a), (l)) #endif /* !__ASSEMBLY__ */ diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c index 71f2a57..b978ebe 100644 --- a/arch/arm/kvm/mmu.c +++ b/arch/arm/kvm/mmu.c @@ -228,7 +228,7 @@ static void create_hyp_pte_mappings(pmd_t *pmd, unsigned long start, pte = pte_offset_kernel(pmd, addr); kvm_set_pte(pte, pfn_pte(pfn, prot)); get_page(virt_to_page(pte)); - kvm_flush_dcache_to_poc(pte, sizeof(*pte)); + kvm_clean_dcache_to_pou(pte, sizeof(*pte)); pfn++; } while (addr += PAGE_SIZE, addr != end); } @@ -255,7 +255,7 @@ static int create_hyp_pmd_mappings(pud_t *pud, unsigned long start, } pmd_populate_kernel(NULL, pmd, pte); get_page(virt_to_page(pmd)); - kvm_flush_dcache_to_poc(pmd, sizeof(*pmd)); + kvm_clean_dcache_to_pou(pmd, sizeof(*pmd)); } next = pmd_addr_end(addr, end); @@ -293,7 +293,7 @@ static int __create_hyp_mappings(pgd_t *pgdp, } pud_populate(NULL, pud, pmd); get_page(virt_to_page(pud)); - kvm_flush_dcache_to_poc(pud, sizeof(*pud)); + kvm_clean_dcache_to_pou(pud, sizeof(*pud)); } next = pgd_addr_end(addr, end); -- 1.8.2.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/4] ARM: KVM: relax cache maintainance when building page pables 2013-04-30 14:17 ` Marc Zyngier @ 2013-04-30 14:30 ` Will Deacon -1 siblings, 0 replies; 24+ messages in thread From: Will Deacon @ 2013-04-30 14:30 UTC (permalink / raw) To: linux-arm-kernel On Tue, Apr 30, 2013 at 03:17:25PM +0100, Marc Zyngier wrote: > Patch 5a677ce044f1 (ARM: KVM: switch to a dual-step HYP init code) > introduced code that flushes page tables to the point of coherency. > This is overkill (point of unification is enough), and actually > not required if running on a SMP capable platform. > > Change this code to use clean_dcache_area(), which is updated by > ae8a8b9553bd (ARM: 7691/1: mm: kill unused TLB_CAN_READ_FROM_L1_CACHE > and use ALT_SMP instead) to behave as a no-op on SMP ARMv7. > > Reported-by: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will.deacon@arm.com> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > --- > arch/arm/include/asm/kvm_mmu.h | 1 + > arch/arm/kvm/mmu.c | 6 +++--- > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h > index 472ac70..2459821 100644 > --- a/arch/arm/include/asm/kvm_mmu.h > +++ b/arch/arm/include/asm/kvm_mmu.h > @@ -129,6 +129,7 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn) > } > > #define kvm_flush_dcache_to_poc(a,l) __cpuc_flush_dcache_area((a), (l)) > +#define kvm_clean_dcache_to_pou(a,l) clean_dcache_area((a), (l)) I don't think the name helps, since this sounds like it could be used for things other than flushing page tables (for example, writing instructions) but apart from that: Reviewed-by: Will Deacon <will.deacon@arm.com> Will ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/4] ARM: KVM: relax cache maintainance when building page pables @ 2013-04-30 14:30 ` Will Deacon 0 siblings, 0 replies; 24+ messages in thread From: Will Deacon @ 2013-04-30 14:30 UTC (permalink / raw) To: Marc Zyngier Cc: linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, Catalin Marinas On Tue, Apr 30, 2013 at 03:17:25PM +0100, Marc Zyngier wrote: > Patch 5a677ce044f1 (ARM: KVM: switch to a dual-step HYP init code) > introduced code that flushes page tables to the point of coherency. > This is overkill (point of unification is enough), and actually > not required if running on a SMP capable platform. > > Change this code to use clean_dcache_area(), which is updated by > ae8a8b9553bd (ARM: 7691/1: mm: kill unused TLB_CAN_READ_FROM_L1_CACHE > and use ALT_SMP instead) to behave as a no-op on SMP ARMv7. > > Reported-by: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will.deacon@arm.com> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > --- > arch/arm/include/asm/kvm_mmu.h | 1 + > arch/arm/kvm/mmu.c | 6 +++--- > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h > index 472ac70..2459821 100644 > --- a/arch/arm/include/asm/kvm_mmu.h > +++ b/arch/arm/include/asm/kvm_mmu.h > @@ -129,6 +129,7 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn) > } > > #define kvm_flush_dcache_to_poc(a,l) __cpuc_flush_dcache_area((a), (l)) > +#define kvm_clean_dcache_to_pou(a,l) clean_dcache_area((a), (l)) I don't think the name helps, since this sounds like it could be used for things other than flushing page tables (for example, writing instructions) but apart from that: Reviewed-by: Will Deacon <will.deacon@arm.com> Will ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 4/4] ARM: KVM: fix use of S2_PGD_SIZE 2013-04-30 14:17 ` Marc Zyngier @ 2013-04-30 14:17 ` Marc Zyngier -1 siblings, 0 replies; 24+ messages in thread From: Marc Zyngier @ 2013-04-30 14:17 UTC (permalink / raw) To: linux-arm-kernel S2_PGD_SIZE describe the number of pages a used by a stage-2 PGD, not the size of the PGD. Fix the VM_BUG_ON() call that doesn't check much when S2_PGD_SIZE is equal to 1. Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> --- arch/arm/kvm/mmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c index b978ebe..09ece5c 100644 --- a/arch/arm/kvm/mmu.c +++ b/arch/arm/kvm/mmu.c @@ -377,7 +377,7 @@ int kvm_alloc_stage2_pgd(struct kvm *kvm) return -ENOMEM; /* stage-2 pgd must be aligned to its size */ - VM_BUG_ON((unsigned long)pgd & (S2_PGD_SIZE - 1)); + VM_BUG_ON((unsigned long)pgd & (PAGE_SIZE * S2_PGD_SIZE - 1)); memset(pgd, 0, PTRS_PER_S2_PGD * sizeof(pgd_t)); kvm_clean_pgd(pgd); -- 1.8.2.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 4/4] ARM: KVM: fix use of S2_PGD_SIZE @ 2013-04-30 14:17 ` Marc Zyngier 0 siblings, 0 replies; 24+ messages in thread From: Marc Zyngier @ 2013-04-30 14:17 UTC (permalink / raw) To: linux-arm-kernel, kvmarm, kvm S2_PGD_SIZE describe the number of pages a used by a stage-2 PGD, not the size of the PGD. Fix the VM_BUG_ON() call that doesn't check much when S2_PGD_SIZE is equal to 1. Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> --- arch/arm/kvm/mmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c index b978ebe..09ece5c 100644 --- a/arch/arm/kvm/mmu.c +++ b/arch/arm/kvm/mmu.c @@ -377,7 +377,7 @@ int kvm_alloc_stage2_pgd(struct kvm *kvm) return -ENOMEM; /* stage-2 pgd must be aligned to its size */ - VM_BUG_ON((unsigned long)pgd & (S2_PGD_SIZE - 1)); + VM_BUG_ON((unsigned long)pgd & (PAGE_SIZE * S2_PGD_SIZE - 1)); memset(pgd, 0, PTRS_PER_S2_PGD * sizeof(pgd_t)); kvm_clean_pgd(pgd); -- 1.8.2.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 4/4] ARM: KVM: fix use of S2_PGD_SIZE 2013-04-30 14:17 ` Marc Zyngier @ 2013-04-30 17:40 ` Christoffer Dall -1 siblings, 0 replies; 24+ messages in thread From: Christoffer Dall @ 2013-04-30 17:40 UTC (permalink / raw) To: linux-arm-kernel On Tue, Apr 30, 2013 at 7:17 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: > S2_PGD_SIZE describe the number of pages a used by a stage-2 PGD, > not the size of the PGD. > > Fix the VM_BUG_ON() call that doesn't check much when S2_PGD_SIZE > is equal to 1. > > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > --- > arch/arm/kvm/mmu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c > index b978ebe..09ece5c 100644 > --- a/arch/arm/kvm/mmu.c > +++ b/arch/arm/kvm/mmu.c > @@ -377,7 +377,7 @@ int kvm_alloc_stage2_pgd(struct kvm *kvm) > return -ENOMEM; > > /* stage-2 pgd must be aligned to its size */ > - VM_BUG_ON((unsigned long)pgd & (S2_PGD_SIZE - 1)); > + VM_BUG_ON((unsigned long)pgd & (PAGE_SIZE * S2_PGD_SIZE - 1)); I think the define is broken and should be fixed and/or renamed instead. > > memset(pgd, 0, PTRS_PER_S2_PGD * sizeof(pgd_t)); > kvm_clean_pgd(pgd); > -- > 1.8.2.1 > > > > _______________________________________________ > kvmarm mailing list > kvmarm at lists.cs.columbia.edu > https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 4/4] ARM: KVM: fix use of S2_PGD_SIZE @ 2013-04-30 17:40 ` Christoffer Dall 0 siblings, 0 replies; 24+ messages in thread From: Christoffer Dall @ 2013-04-30 17:40 UTC (permalink / raw) To: Marc Zyngier; +Cc: linux-arm-kernel, kvmarm@lists.cs.columbia.edu, KVM General On Tue, Apr 30, 2013 at 7:17 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: > S2_PGD_SIZE describe the number of pages a used by a stage-2 PGD, > not the size of the PGD. > > Fix the VM_BUG_ON() call that doesn't check much when S2_PGD_SIZE > is equal to 1. > > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > --- > arch/arm/kvm/mmu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c > index b978ebe..09ece5c 100644 > --- a/arch/arm/kvm/mmu.c > +++ b/arch/arm/kvm/mmu.c > @@ -377,7 +377,7 @@ int kvm_alloc_stage2_pgd(struct kvm *kvm) > return -ENOMEM; > > /* stage-2 pgd must be aligned to its size */ > - VM_BUG_ON((unsigned long)pgd & (S2_PGD_SIZE - 1)); > + VM_BUG_ON((unsigned long)pgd & (PAGE_SIZE * S2_PGD_SIZE - 1)); I think the define is broken and should be fixed and/or renamed instead. > > memset(pgd, 0, PTRS_PER_S2_PGD * sizeof(pgd_t)); > kvm_clean_pgd(pgd); > -- > 1.8.2.1 > > > > _______________________________________________ > kvmarm mailing list > kvmarm@lists.cs.columbia.edu > https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 4/4] ARM: KVM: fix use of S2_PGD_SIZE 2013-04-30 17:40 ` Christoffer Dall @ 2013-04-30 17:52 ` Marc Zyngier -1 siblings, 0 replies; 24+ messages in thread From: Marc Zyngier @ 2013-04-30 17:52 UTC (permalink / raw) To: linux-arm-kernel On 30/04/13 18:40, Christoffer Dall wrote: > On Tue, Apr 30, 2013 at 7:17 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: >> S2_PGD_SIZE describe the number of pages a used by a stage-2 PGD, >> not the size of the PGD. >> >> Fix the VM_BUG_ON() call that doesn't check much when S2_PGD_SIZE >> is equal to 1. >> >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> >> --- >> arch/arm/kvm/mmu.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c >> index b978ebe..09ece5c 100644 >> --- a/arch/arm/kvm/mmu.c >> +++ b/arch/arm/kvm/mmu.c >> @@ -377,7 +377,7 @@ int kvm_alloc_stage2_pgd(struct kvm *kvm) >> return -ENOMEM; >> >> /* stage-2 pgd must be aligned to its size */ >> - VM_BUG_ON((unsigned long)pgd & (S2_PGD_SIZE - 1)); >> + VM_BUG_ON((unsigned long)pgd & (PAGE_SIZE * S2_PGD_SIZE - 1)); > > I think the define is broken and should be fixed and/or renamed instead. To be fair, I wouldn't mind the whole check to be dropped altogether. If __get_free_pages doesn't give you an aligned allocation, the whole kernel is going the way of the dodo, and we shouldn't really care... Your call, really. M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 4/4] ARM: KVM: fix use of S2_PGD_SIZE @ 2013-04-30 17:52 ` Marc Zyngier 0 siblings, 0 replies; 24+ messages in thread From: Marc Zyngier @ 2013-04-30 17:52 UTC (permalink / raw) To: Christoffer Dall Cc: linux-arm-kernel, kvmarm@lists.cs.columbia.edu, KVM General On 30/04/13 18:40, Christoffer Dall wrote: > On Tue, Apr 30, 2013 at 7:17 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: >> S2_PGD_SIZE describe the number of pages a used by a stage-2 PGD, >> not the size of the PGD. >> >> Fix the VM_BUG_ON() call that doesn't check much when S2_PGD_SIZE >> is equal to 1. >> >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> >> --- >> arch/arm/kvm/mmu.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c >> index b978ebe..09ece5c 100644 >> --- a/arch/arm/kvm/mmu.c >> +++ b/arch/arm/kvm/mmu.c >> @@ -377,7 +377,7 @@ int kvm_alloc_stage2_pgd(struct kvm *kvm) >> return -ENOMEM; >> >> /* stage-2 pgd must be aligned to its size */ >> - VM_BUG_ON((unsigned long)pgd & (S2_PGD_SIZE - 1)); >> + VM_BUG_ON((unsigned long)pgd & (PAGE_SIZE * S2_PGD_SIZE - 1)); > > I think the define is broken and should be fixed and/or renamed instead. To be fair, I wouldn't mind the whole check to be dropped altogether. If __get_free_pages doesn't give you an aligned allocation, the whole kernel is going the way of the dodo, and we shouldn't really care... Your call, really. M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 4/4] ARM: KVM: fix use of S2_PGD_SIZE 2013-04-30 17:52 ` Marc Zyngier @ 2013-04-30 17:56 ` Christoffer Dall -1 siblings, 0 replies; 24+ messages in thread From: Christoffer Dall @ 2013-04-30 17:56 UTC (permalink / raw) To: linux-arm-kernel On Tue, Apr 30, 2013 at 06:52:24PM +0100, Marc Zyngier wrote: > On 30/04/13 18:40, Christoffer Dall wrote: > > On Tue, Apr 30, 2013 at 7:17 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: > >> S2_PGD_SIZE describe the number of pages a used by a stage-2 PGD, > >> not the size of the PGD. > >> > >> Fix the VM_BUG_ON() call that doesn't check much when S2_PGD_SIZE > >> is equal to 1. > >> > >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > >> --- > >> arch/arm/kvm/mmu.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c > >> index b978ebe..09ece5c 100644 > >> --- a/arch/arm/kvm/mmu.c > >> +++ b/arch/arm/kvm/mmu.c > >> @@ -377,7 +377,7 @@ int kvm_alloc_stage2_pgd(struct kvm *kvm) > >> return -ENOMEM; > >> > >> /* stage-2 pgd must be aligned to its size */ > >> - VM_BUG_ON((unsigned long)pgd & (S2_PGD_SIZE - 1)); > >> + VM_BUG_ON((unsigned long)pgd & (PAGE_SIZE * S2_PGD_SIZE - 1)); > > > > I think the define is broken and should be fixed and/or renamed instead. > > To be fair, I wouldn't mind the whole check to be dropped altogether. If > __get_free_pages doesn't give you an aligned allocation, the whole > kernel is going the way of the dodo, and we shouldn't really care... > > Your call, really. > Yeah, this is from way back when, where the whole thing was different and we were debugging all sorts of issues, so we can get rid of both the check and the define in stead. Want to take care of it as part of your series? Thanks, -Christoffer ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 4/4] ARM: KVM: fix use of S2_PGD_SIZE @ 2013-04-30 17:56 ` Christoffer Dall 0 siblings, 0 replies; 24+ messages in thread From: Christoffer Dall @ 2013-04-30 17:56 UTC (permalink / raw) To: Marc Zyngier; +Cc: kvmarm@lists.cs.columbia.edu, linux-arm-kernel, KVM General On Tue, Apr 30, 2013 at 06:52:24PM +0100, Marc Zyngier wrote: > On 30/04/13 18:40, Christoffer Dall wrote: > > On Tue, Apr 30, 2013 at 7:17 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: > >> S2_PGD_SIZE describe the number of pages a used by a stage-2 PGD, > >> not the size of the PGD. > >> > >> Fix the VM_BUG_ON() call that doesn't check much when S2_PGD_SIZE > >> is equal to 1. > >> > >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > >> --- > >> arch/arm/kvm/mmu.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c > >> index b978ebe..09ece5c 100644 > >> --- a/arch/arm/kvm/mmu.c > >> +++ b/arch/arm/kvm/mmu.c > >> @@ -377,7 +377,7 @@ int kvm_alloc_stage2_pgd(struct kvm *kvm) > >> return -ENOMEM; > >> > >> /* stage-2 pgd must be aligned to its size */ > >> - VM_BUG_ON((unsigned long)pgd & (S2_PGD_SIZE - 1)); > >> + VM_BUG_ON((unsigned long)pgd & (PAGE_SIZE * S2_PGD_SIZE - 1)); > > > > I think the define is broken and should be fixed and/or renamed instead. > > To be fair, I wouldn't mind the whole check to be dropped altogether. If > __get_free_pages doesn't give you an aligned allocation, the whole > kernel is going the way of the dodo, and we shouldn't really care... > > Your call, really. > Yeah, this is from way back when, where the whole thing was different and we were debugging all sorts of issues, so we can get rid of both the check and the define in stead. Want to take care of it as part of your series? Thanks, -Christoffer ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 4/4] ARM: KVM: fix use of S2_PGD_SIZE 2013-04-30 17:56 ` Christoffer Dall @ 2013-04-30 17:59 ` Marc Zyngier -1 siblings, 0 replies; 24+ messages in thread From: Marc Zyngier @ 2013-04-30 17:59 UTC (permalink / raw) To: linux-arm-kernel On 30/04/13 18:56, Christoffer Dall wrote: > On Tue, Apr 30, 2013 at 06:52:24PM +0100, Marc Zyngier wrote: >> On 30/04/13 18:40, Christoffer Dall wrote: >>> On Tue, Apr 30, 2013 at 7:17 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: >>>> S2_PGD_SIZE describe the number of pages a used by a stage-2 PGD, >>>> not the size of the PGD. >>>> >>>> Fix the VM_BUG_ON() call that doesn't check much when S2_PGD_SIZE >>>> is equal to 1. >>>> >>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> >>>> --- >>>> arch/arm/kvm/mmu.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c >>>> index b978ebe..09ece5c 100644 >>>> --- a/arch/arm/kvm/mmu.c >>>> +++ b/arch/arm/kvm/mmu.c >>>> @@ -377,7 +377,7 @@ int kvm_alloc_stage2_pgd(struct kvm *kvm) >>>> return -ENOMEM; >>>> >>>> /* stage-2 pgd must be aligned to its size */ >>>> - VM_BUG_ON((unsigned long)pgd & (S2_PGD_SIZE - 1)); >>>> + VM_BUG_ON((unsigned long)pgd & (PAGE_SIZE * S2_PGD_SIZE - 1)); >>> >>> I think the define is broken and should be fixed and/or renamed instead. >> >> To be fair, I wouldn't mind the whole check to be dropped altogether. If >> __get_free_pages doesn't give you an aligned allocation, the whole >> kernel is going the way of the dodo, and we shouldn't really care... >> >> Your call, really. >> > Yeah, this is from way back when, where the whole thing was different > and we were debugging all sorts of issues, so we can get rid of both the > check and the define in stead. Want to take care of it as part of your > series? Sure. I'll kill it when I repost the series. M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 4/4] ARM: KVM: fix use of S2_PGD_SIZE @ 2013-04-30 17:59 ` Marc Zyngier 0 siblings, 0 replies; 24+ messages in thread From: Marc Zyngier @ 2013-04-30 17:59 UTC (permalink / raw) To: Christoffer Dall Cc: kvmarm@lists.cs.columbia.edu, linux-arm-kernel, KVM General On 30/04/13 18:56, Christoffer Dall wrote: > On Tue, Apr 30, 2013 at 06:52:24PM +0100, Marc Zyngier wrote: >> On 30/04/13 18:40, Christoffer Dall wrote: >>> On Tue, Apr 30, 2013 at 7:17 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: >>>> S2_PGD_SIZE describe the number of pages a used by a stage-2 PGD, >>>> not the size of the PGD. >>>> >>>> Fix the VM_BUG_ON() call that doesn't check much when S2_PGD_SIZE >>>> is equal to 1. >>>> >>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> >>>> --- >>>> arch/arm/kvm/mmu.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c >>>> index b978ebe..09ece5c 100644 >>>> --- a/arch/arm/kvm/mmu.c >>>> +++ b/arch/arm/kvm/mmu.c >>>> @@ -377,7 +377,7 @@ int kvm_alloc_stage2_pgd(struct kvm *kvm) >>>> return -ENOMEM; >>>> >>>> /* stage-2 pgd must be aligned to its size */ >>>> - VM_BUG_ON((unsigned long)pgd & (S2_PGD_SIZE - 1)); >>>> + VM_BUG_ON((unsigned long)pgd & (PAGE_SIZE * S2_PGD_SIZE - 1)); >>> >>> I think the define is broken and should be fixed and/or renamed instead. >> >> To be fair, I wouldn't mind the whole check to be dropped altogether. If >> __get_free_pages doesn't give you an aligned allocation, the whole >> kernel is going the way of the dodo, and we shouldn't really care... >> >> Your call, really. >> > Yeah, this is from way back when, where the whole thing was different > and we were debugging all sorts of issues, so we can get rid of both the > check and the define in stead. Want to take care of it as part of your > series? Sure. I'll kill it when I repost the series. M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2013-04-30 18:02 UTC | newest] Thread overview: 24+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-04-30 14:17 [PATCH 0/4] ARM: KVM: random mmu related fixes for 3.10 Marc Zyngier 2013-04-30 14:17 ` Marc Zyngier 2013-04-30 14:17 ` [PATCH 1/4] ARM: KVM: be more thorough when invalidating TLBs Marc Zyngier 2013-04-30 14:17 ` Marc Zyngier 2013-04-30 17:17 ` Christoffer Dall 2013-04-30 17:17 ` Christoffer Dall 2013-04-30 18:02 ` Marc Zyngier 2013-04-30 18:02 ` Marc Zyngier 2013-04-30 14:17 ` [PATCH 2/4] ARM: KVM: remove dead prototype for __kvm_tlb_flush_vmid Marc Zyngier 2013-04-30 14:17 ` Marc Zyngier 2013-04-30 14:17 ` [PATCH 3/4] ARM: KVM: relax cache maintainance when building page pables Marc Zyngier 2013-04-30 14:17 ` Marc Zyngier 2013-04-30 14:30 ` Will Deacon 2013-04-30 14:30 ` Will Deacon 2013-04-30 14:17 ` [PATCH 4/4] ARM: KVM: fix use of S2_PGD_SIZE Marc Zyngier 2013-04-30 14:17 ` Marc Zyngier 2013-04-30 17:40 ` Christoffer Dall 2013-04-30 17:40 ` Christoffer Dall 2013-04-30 17:52 ` Marc Zyngier 2013-04-30 17:52 ` Marc Zyngier 2013-04-30 17:56 ` Christoffer Dall 2013-04-30 17:56 ` Christoffer Dall 2013-04-30 17:59 ` Marc Zyngier 2013-04-30 17:59 ` Marc Zyngier
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.