| Message ID | 1384557880-14107-1-git-send-email-christoffer.dall@linaro.org |
|---|---|
| State | Accepted |
| Commit | 40c2729bab48e2832b17c1fa8af9db60e776131b |
| Headers | show |
On 2013-11-15 23:24, Christoffer Dall wrote: > Using virt_to_phys on percpu mappings is horribly wrong as it may be > backed by vmalloc. Introduce kvm_kaddr_to_phys which translates both > types of valid kernel addresses to the corresponding physical > address. > > At the same time resolves a typing issue where we were storing the > physical address as a 32 bit unsigned long (on arm), truncating the > physical address for addresses above the 4GB limit. This caused > breakage on Keystone. > > Reported-by: Santosh Shilimkar <santosh.shilimkar@ti.com> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org> > --- > > This patch is loosely based on Marc's previous patch from today but > instead of introducing another Hyp mapping function, it fixes the > existing one to deal with both kinds of kernel addresses. Looks good to me! This should probably be merged quickly (after testing by Santosh), and possibly Cc-ed to stable. Thanks, M. > > arch/arm/kvm/mmu.c | 34 ++++++++++++++++++++++++++++------ > 1 file changed, 28 insertions(+), 6 deletions(-) > > diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c > index 3719583..5809069 100644 > --- a/arch/arm/kvm/mmu.c > +++ b/arch/arm/kvm/mmu.c > @@ -334,6 +334,17 @@ out: > return err; > } > > +static phys_addr_t kvm_kaddr_to_phys(void *kaddr) > +{ > + if (!is_vmalloc_addr(kaddr)) { > + BUG_ON(!virt_addr_valid(kaddr)); > + return __pa(kaddr); > + } else { > + return page_to_phys(vmalloc_to_page(kaddr)) + > + offset_in_page(kaddr); > + } > +} > + > /** > * create_hyp_mappings - duplicate a kernel virtual address range in > Hyp mode > * @from: The virtual kernel start address of the range > @@ -345,16 +356,27 @@ out: > */ > int create_hyp_mappings(void *from, void *to) > { > - unsigned long phys_addr = virt_to_phys(from); > + phys_addr_t phys_addr; > + unsigned long virt_addr; > unsigned long start = KERN_TO_HYP((unsigned long)from); > unsigned long end = KERN_TO_HYP((unsigned long)to); > > - /* Check for a valid kernel memory mapping */ > - if (!virt_addr_valid(from) || !virt_addr_valid(to - 1)) > - return -EINVAL; > + start = start & PAGE_MASK; > + end = PAGE_ALIGN(end); > > - return __create_hyp_mappings(hyp_pgd, start, end, > - __phys_to_pfn(phys_addr), PAGE_HYP); > + for (virt_addr = start; virt_addr < end; virt_addr += PAGE_SIZE) { > + int err; > + > + phys_addr = kvm_kaddr_to_phys(from + virt_addr - start); > + err = __create_hyp_mappings(hyp_pgd, virt_addr, > + virt_addr + PAGE_SIZE, > + __phys_to_pfn(phys_addr), > + PAGE_HYP); > + if (err) > + return err; > + } > + > + return 0; > } > > /**
On Sat, Nov 16, 2013 at 10:01:01AM +0000, Marc Zyngier wrote: > On 2013-11-15 23:24, Christoffer Dall wrote: > >Using virt_to_phys on percpu mappings is horribly wrong as it may be > >backed by vmalloc. Introduce kvm_kaddr_to_phys which translates both > >types of valid kernel addresses to the corresponding physical > >address. > > > >At the same time resolves a typing issue where we were storing the > >physical address as a 32 bit unsigned long (on arm), truncating the > >physical address for addresses above the 4GB limit. This caused > >breakage on Keystone. > > > >Reported-by: Santosh Shilimkar <santosh.shilimkar@ti.com> > >Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > >Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org> > >--- > > > >This patch is loosely based on Marc's previous patch from today but > >instead of introducing another Hyp mapping function, it fixes the > >existing one to deal with both kinds of kernel addresses. > > Looks good to me! This should probably be merged quickly (after > testing by Santosh), and possibly Cc-ed to stable. > Agreed, Santhos, can you give this a quick spin? -Christoffer
On Saturday 16 November 2013 12:15 PM, Christoffer Dall wrote: > On Sat, Nov 16, 2013 at 10:01:01AM +0000, Marc Zyngier wrote: >> On 2013-11-15 23:24, Christoffer Dall wrote: >>> Using virt_to_phys on percpu mappings is horribly wrong as it may be >>> backed by vmalloc. Introduce kvm_kaddr_to_phys which translates both >>> types of valid kernel addresses to the corresponding physical >>> address. >>> >>> At the same time resolves a typing issue where we were storing the >>> physical address as a 32 bit unsigned long (on arm), truncating the >>> physical address for addresses above the 4GB limit. This caused >>> breakage on Keystone. >>> >>> Reported-by: Santosh Shilimkar <santosh.shilimkar@ti.com> >>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> >>> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org> >>> --- >>> >>> This patch is loosely based on Marc's previous patch from today but >>> instead of introducing another Hyp mapping function, it fixes the >>> existing one to deal with both kinds of kernel addresses. >> >> Looks good to me! This should probably be merged quickly (after >> testing by Santosh), and possibly Cc-ed to stable. >> > Agreed, Santhos, can you give this a quick spin? > Works as expected. Regards, Santosh
On 16 November 2013 15:30, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote: > On Saturday 16 November 2013 12:15 PM, Christoffer Dall wrote: >> On Sat, Nov 16, 2013 at 10:01:01AM +0000, Marc Zyngier wrote: >>> On 2013-11-15 23:24, Christoffer Dall wrote: >>>> Using virt_to_phys on percpu mappings is horribly wrong as it may be >>>> backed by vmalloc. Introduce kvm_kaddr_to_phys which translates both >>>> types of valid kernel addresses to the corresponding physical >>>> address. >>>> >>>> At the same time resolves a typing issue where we were storing the >>>> physical address as a 32 bit unsigned long (on arm), truncating the >>>> physical address for addresses above the 4GB limit. This caused >>>> breakage on Keystone. >>>> >>>> Reported-by: Santosh Shilimkar <santosh.shilimkar@ti.com> >>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> >>>> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org> >>>> --- >>>> >>>> This patch is loosely based on Marc's previous patch from today but >>>> instead of introducing another Hyp mapping function, it fixes the >>>> existing one to deal with both kinds of kernel addresses. >>> >>> Looks good to me! This should probably be merged quickly (after >>> testing by Santosh), and possibly Cc-ed to stable. >>> >> Agreed, Santhos, can you give this a quick spin? >> > Works as expected. > great, I'll merge the patch. Thanks for testing it. -Christoffer
diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c index 3719583..5809069 100644 --- a/arch/arm/kvm/mmu.c +++ b/arch/arm/kvm/mmu.c @@ -334,6 +334,17 @@ out: return err; } +static phys_addr_t kvm_kaddr_to_phys(void *kaddr) +{ + if (!is_vmalloc_addr(kaddr)) { + BUG_ON(!virt_addr_valid(kaddr)); + return __pa(kaddr); + } else { + return page_to_phys(vmalloc_to_page(kaddr)) + + offset_in_page(kaddr); + } +} + /** * create_hyp_mappings - duplicate a kernel virtual address range in Hyp mode * @from: The virtual kernel start address of the range @@ -345,16 +356,27 @@ out: */ int create_hyp_mappings(void *from, void *to) { - unsigned long phys_addr = virt_to_phys(from); + phys_addr_t phys_addr; + unsigned long virt_addr; unsigned long start = KERN_TO_HYP((unsigned long)from); unsigned long end = KERN_TO_HYP((unsigned long)to); - /* Check for a valid kernel memory mapping */ - if (!virt_addr_valid(from) || !virt_addr_valid(to - 1)) - return -EINVAL; + start = start & PAGE_MASK; + end = PAGE_ALIGN(end); - return __create_hyp_mappings(hyp_pgd, start, end, - __phys_to_pfn(phys_addr), PAGE_HYP); + for (virt_addr = start; virt_addr < end; virt_addr += PAGE_SIZE) { + int err; + + phys_addr = kvm_kaddr_to_phys(from + virt_addr - start); + err = __create_hyp_mappings(hyp_pgd, virt_addr, + virt_addr + PAGE_SIZE, + __phys_to_pfn(phys_addr), + PAGE_HYP); + if (err) + return err; + } + + return 0; } /**