| Message ID | 20220224051439.640768-2-kaleshsingh@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: Hypervisor stack enhancements | expand |
Hi Kalesh, On Thu, Feb 24, 2022 at 5:16 AM Kalesh Singh <kaleshsingh@google.com> wrote: > > hyp_alloc_private_va_range() can be used to reserve private VA ranges > in the nVHE hypervisor. Also update __create_hyp_private_mapping() > to allow specifying an alignment for the private VA mapping. > > These will be used to implement stack guard pages for KVM nVHE hypervisor > (nVHE Hyp mode / not pKVM), in a subsequent patch in the series. > > Signed-off-by: Kalesh Singh <kaleshsingh@google.com> > --- > > Changes in v3: > - Handle null ptr in IS_ERR_OR_NULL checks, per Mark > > arch/arm64/include/asm/kvm_mmu.h | 4 +++ > arch/arm64/kvm/mmu.c | 62 ++++++++++++++++++++------------ > 2 files changed, 43 insertions(+), 23 deletions(-) > > diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h > index 81839e9a8a24..0b0c71302b92 100644 > --- a/arch/arm64/include/asm/kvm_mmu.h > +++ b/arch/arm64/include/asm/kvm_mmu.h > @@ -153,6 +153,10 @@ static __always_inline unsigned long __kern_hyp_va(unsigned long v) > int kvm_share_hyp(void *from, void *to); > void kvm_unshare_hyp(void *from, void *to); > int create_hyp_mappings(void *from, void *to, enum kvm_pgtable_prot prot); > +unsigned long hyp_alloc_private_va_range(size_t size, size_t align); > +int __create_hyp_private_mapping(phys_addr_t phys_addr, size_t size, > + size_t align, unsigned long *haddr, > + enum kvm_pgtable_prot prot); > int create_hyp_io_mappings(phys_addr_t phys_addr, size_t size, > void __iomem **kaddr, > void __iomem **haddr); > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c > index bc2aba953299..fc09536c8197 100644 > --- a/arch/arm64/kvm/mmu.c > +++ b/arch/arm64/kvm/mmu.c > @@ -457,22 +457,16 @@ int create_hyp_mappings(void *from, void *to, enum kvm_pgtable_prot prot) > return 0; > } > > -static int __create_hyp_private_mapping(phys_addr_t phys_addr, size_t size, > - unsigned long *haddr, > - enum kvm_pgtable_prot prot) > + > +/* > + * Allocates a private VA range below io_map_base. > + * > + * @size: The size of the VA range to reserve. > + * @align: The required alignment for the allocation. > + */ Many of the functions in this file use the kernel-doc format, and your added comments are close, but not quite conforment. If you want to use the kernel-doc for these you can refer to: https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html > +unsigned long hyp_alloc_private_va_range(size_t size, size_t align) > { > unsigned long base; > - int ret = 0; > - > - if (!kvm_host_owns_hyp_mappings()) { > - base = kvm_call_hyp_nvhe(__pkvm_create_private_mapping, > - phys_addr, size, prot); > - if (IS_ERR_OR_NULL((void *)base)) > - return PTR_ERR((void *)base); > - *haddr = base; > - > - return 0; > - } > > mutex_lock(&kvm_hyp_pgd_mutex); > > @@ -484,8 +478,8 @@ static int __create_hyp_private_mapping(phys_addr_t phys_addr, size_t size, > * > * The allocated size is always a multiple of PAGE_SIZE. > */ > - size = PAGE_ALIGN(size + offset_in_page(phys_addr)); > - base = io_map_base - size; > + base = io_map_base - PAGE_ALIGN(size); > + base = ALIGN_DOWN(base, align); > > /* > * Verify that BIT(VA_BITS - 1) hasn't been flipped by > @@ -493,20 +487,42 @@ static int __create_hyp_private_mapping(phys_addr_t phys_addr, size_t size, > * overflowed the idmap/IO address range. > */ > if ((base ^ io_map_base) & BIT(VA_BITS - 1)) > - ret = -ENOMEM; > + base = (unsigned long)ERR_PTR(-ENOMEM); > else > io_map_base = base; > > mutex_unlock(&kvm_hyp_pgd_mutex); > > - if (ret) > - goto out; > + return base; > +} > + > +int __create_hyp_private_mapping(phys_addr_t phys_addr, size_t size, > + size_t align, unsigned long *haddr, > + enum kvm_pgtable_prot prot) > +{ > + unsigned long addr; > + int ret = 0; > + > + if (!kvm_host_owns_hyp_mappings()) { > + addr = kvm_call_hyp_nvhe(__pkvm_create_private_mapping, > + phys_addr, size, prot); > + if (IS_ERR_OR_NULL((void *)addr)) > + return addr ? PTR_ERR((void *)addr) : -ENOMEM; > + *haddr = addr; > + > + return 0; > + } > + > + size += offset_in_page(phys_addr); You're not page-aligning the size, which was the behavior before this patch. However, looking at where it's being used it seems to be fine because the users of size would align it if necessary. Thanks, /fuad > + addr = hyp_alloc_private_va_range(size, align); > + if (IS_ERR_OR_NULL((void *)addr)) > + return addr ? PTR_ERR((void *)addr) : -ENOMEM; > > - ret = __create_hyp_mappings(base, size, phys_addr, prot); > + ret = __create_hyp_mappings(addr, size, phys_addr, prot); > if (ret) > goto out; > > - *haddr = base + offset_in_page(phys_addr); > + *haddr = addr + offset_in_page(phys_addr); > out: > return ret; > } > @@ -537,7 +553,7 @@ int create_hyp_io_mappings(phys_addr_t phys_addr, size_t size, > return 0; > } > > - ret = __create_hyp_private_mapping(phys_addr, size, > + ret = __create_hyp_private_mapping(phys_addr, size, PAGE_SIZE, > &addr, PAGE_HYP_DEVICE); > if (ret) { > iounmap(*kaddr); > @@ -564,7 +580,7 @@ int create_hyp_exec_mappings(phys_addr_t phys_addr, size_t size, > > BUG_ON(is_kernel_in_hyp_mode()); > > - ret = __create_hyp_private_mapping(phys_addr, size, > + ret = __create_hyp_private_mapping(phys_addr, size, PAGE_SIZE, > &addr, PAGE_HYP_EXEC); > if (ret) { > *haddr = NULL; > -- > 2.35.1.473.g83b2b277ed-goog >
On Thu, Feb 24, 2022 at 4:25 AM Fuad Tabba <tabba@google.com> wrote: > > Hi Kalesh, > > On Thu, Feb 24, 2022 at 5:16 AM Kalesh Singh <kaleshsingh@google.com> wrote: > > > > hyp_alloc_private_va_range() can be used to reserve private VA ranges > > in the nVHE hypervisor. Also update __create_hyp_private_mapping() > > to allow specifying an alignment for the private VA mapping. > > > > These will be used to implement stack guard pages for KVM nVHE hypervisor > > (nVHE Hyp mode / not pKVM), in a subsequent patch in the series. > > > > Signed-off-by: Kalesh Singh <kaleshsingh@google.com> > > --- > > > > Changes in v3: > > - Handle null ptr in IS_ERR_OR_NULL checks, per Mark > > > > arch/arm64/include/asm/kvm_mmu.h | 4 +++ > > arch/arm64/kvm/mmu.c | 62 ++++++++++++++++++++------------ > > 2 files changed, 43 insertions(+), 23 deletions(-) > > > > diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h > > index 81839e9a8a24..0b0c71302b92 100644 > > --- a/arch/arm64/include/asm/kvm_mmu.h > > +++ b/arch/arm64/include/asm/kvm_mmu.h > > @@ -153,6 +153,10 @@ static __always_inline unsigned long __kern_hyp_va(unsigned long v) > > int kvm_share_hyp(void *from, void *to); > > void kvm_unshare_hyp(void *from, void *to); > > int create_hyp_mappings(void *from, void *to, enum kvm_pgtable_prot prot); > > +unsigned long hyp_alloc_private_va_range(size_t size, size_t align); > > +int __create_hyp_private_mapping(phys_addr_t phys_addr, size_t size, > > + size_t align, unsigned long *haddr, > > + enum kvm_pgtable_prot prot); > > int create_hyp_io_mappings(phys_addr_t phys_addr, size_t size, > > void __iomem **kaddr, > > void __iomem **haddr); > > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c > > index bc2aba953299..fc09536c8197 100644 > > --- a/arch/arm64/kvm/mmu.c > > +++ b/arch/arm64/kvm/mmu.c > > @@ -457,22 +457,16 @@ int create_hyp_mappings(void *from, void *to, enum kvm_pgtable_prot prot) > > return 0; > > } > > > > -static int __create_hyp_private_mapping(phys_addr_t phys_addr, size_t size, > > - unsigned long *haddr, > > - enum kvm_pgtable_prot prot) > > + > > +/* > > + * Allocates a private VA range below io_map_base. > > + * > > + * @size: The size of the VA range to reserve. > > + * @align: The required alignment for the allocation. > > + */ > > Many of the functions in this file use the kernel-doc format, and your > added comments are close, but not quite conforment. If you want to use > the kernel-doc for these you can refer to: > https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html Hi Fuad, Thanks for the pointer. I will update the function comments to match when I send the next version. > > > +unsigned long hyp_alloc_private_va_range(size_t size, size_t align) > > { > > unsigned long base; > > - int ret = 0; > > - > > - if (!kvm_host_owns_hyp_mappings()) { > > - base = kvm_call_hyp_nvhe(__pkvm_create_private_mapping, > > - phys_addr, size, prot); > > - if (IS_ERR_OR_NULL((void *)base)) > > - return PTR_ERR((void *)base); > > - *haddr = base; > > - > > - return 0; > > - } > > > > mutex_lock(&kvm_hyp_pgd_mutex); > > > > @@ -484,8 +478,8 @@ static int __create_hyp_private_mapping(phys_addr_t phys_addr, size_t size, > > * > > * The allocated size is always a multiple of PAGE_SIZE. > > */ > > - size = PAGE_ALIGN(size + offset_in_page(phys_addr)); > > - base = io_map_base - size; > > + base = io_map_base - PAGE_ALIGN(size); > > + base = ALIGN_DOWN(base, align); > > > > /* > > * Verify that BIT(VA_BITS - 1) hasn't been flipped by > > @@ -493,20 +487,42 @@ static int __create_hyp_private_mapping(phys_addr_t phys_addr, size_t size, > > * overflowed the idmap/IO address range. > > */ > > if ((base ^ io_map_base) & BIT(VA_BITS - 1)) > > - ret = -ENOMEM; > > + base = (unsigned long)ERR_PTR(-ENOMEM); > > else > > io_map_base = base; > > > > mutex_unlock(&kvm_hyp_pgd_mutex); > > > > - if (ret) > > - goto out; > > + return base; > > +} > > + > > +int __create_hyp_private_mapping(phys_addr_t phys_addr, size_t size, > > + size_t align, unsigned long *haddr, > > + enum kvm_pgtable_prot prot) > > +{ > > + unsigned long addr; > > + int ret = 0; > > + > > + if (!kvm_host_owns_hyp_mappings()) { > > + addr = kvm_call_hyp_nvhe(__pkvm_create_private_mapping, > > + phys_addr, size, prot); > > + if (IS_ERR_OR_NULL((void *)addr)) > > + return addr ? PTR_ERR((void *)addr) : -ENOMEM; > > + *haddr = addr; > > + > > + return 0; > > + } > > + > > + size += offset_in_page(phys_addr); > > You're not page-aligning the size, which was the behavior before this > patch. However, looking at where it's being used it seems to be fine > because the users of size would align it if necessary. This is now done by hyp_alloc_private_va_range() when calculating the new base: ... * The allocated size is always a multiple of PAGE_SIZE. */ base = io_map_base - PAGE_ALIGN(size); ... Thanks, Kalesh > > Thanks, > /fuad > > > > > + addr = hyp_alloc_private_va_range(size, align); > > + if (IS_ERR_OR_NULL((void *)addr)) > > + return addr ? PTR_ERR((void *)addr) : -ENOMEM; > > > > - ret = __create_hyp_mappings(base, size, phys_addr, prot); > > + ret = __create_hyp_mappings(addr, size, phys_addr, prot); > > if (ret) > > goto out; > > > > - *haddr = base + offset_in_page(phys_addr); > > + *haddr = addr + offset_in_page(phys_addr); > > out: > > return ret; > > } > > @@ -537,7 +553,7 @@ int create_hyp_io_mappings(phys_addr_t phys_addr, size_t size, > > return 0; > > } > > > > - ret = __create_hyp_private_mapping(phys_addr, size, > > + ret = __create_hyp_private_mapping(phys_addr, size, PAGE_SIZE, > > &addr, PAGE_HYP_DEVICE); > > if (ret) { > > iounmap(*kaddr); > > @@ -564,7 +580,7 @@ int create_hyp_exec_mappings(phys_addr_t phys_addr, size_t size, > > > > BUG_ON(is_kernel_in_hyp_mode()); > > > > - ret = __create_hyp_private_mapping(phys_addr, size, > > + ret = __create_hyp_private_mapping(phys_addr, size, PAGE_SIZE, > > &addr, PAGE_HYP_EXEC); > > if (ret) { > > *haddr = NULL; > > -- > > 2.35.1.473.g83b2b277ed-goog > >
diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h index 81839e9a8a24..0b0c71302b92 100644 --- a/arch/arm64/include/asm/kvm_mmu.h +++ b/arch/arm64/include/asm/kvm_mmu.h @@ -153,6 +153,10 @@ static __always_inline unsigned long __kern_hyp_va(unsigned long v) int kvm_share_hyp(void *from, void *to); void kvm_unshare_hyp(void *from, void *to); int create_hyp_mappings(void *from, void *to, enum kvm_pgtable_prot prot); +unsigned long hyp_alloc_private_va_range(size_t size, size_t align); +int __create_hyp_private_mapping(phys_addr_t phys_addr, size_t size, + size_t align, unsigned long *haddr, + enum kvm_pgtable_prot prot); int create_hyp_io_mappings(phys_addr_t phys_addr, size_t size, void __iomem **kaddr, void __iomem **haddr); diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c index bc2aba953299..fc09536c8197 100644 --- a/arch/arm64/kvm/mmu.c +++ b/arch/arm64/kvm/mmu.c @@ -457,22 +457,16 @@ int create_hyp_mappings(void *from, void *to, enum kvm_pgtable_prot prot) return 0; } -static int __create_hyp_private_mapping(phys_addr_t phys_addr, size_t size, - unsigned long *haddr, - enum kvm_pgtable_prot prot) + +/* + * Allocates a private VA range below io_map_base. + * + * @size: The size of the VA range to reserve. + * @align: The required alignment for the allocation. + */ +unsigned long hyp_alloc_private_va_range(size_t size, size_t align) { unsigned long base; - int ret = 0; - - if (!kvm_host_owns_hyp_mappings()) { - base = kvm_call_hyp_nvhe(__pkvm_create_private_mapping, - phys_addr, size, prot); - if (IS_ERR_OR_NULL((void *)base)) - return PTR_ERR((void *)base); - *haddr = base; - - return 0; - } mutex_lock(&kvm_hyp_pgd_mutex); @@ -484,8 +478,8 @@ static int __create_hyp_private_mapping(phys_addr_t phys_addr, size_t size, * * The allocated size is always a multiple of PAGE_SIZE. */ - size = PAGE_ALIGN(size + offset_in_page(phys_addr)); - base = io_map_base - size; + base = io_map_base - PAGE_ALIGN(size); + base = ALIGN_DOWN(base, align); /* * Verify that BIT(VA_BITS - 1) hasn't been flipped by @@ -493,20 +487,42 @@ static int __create_hyp_private_mapping(phys_addr_t phys_addr, size_t size, * overflowed the idmap/IO address range. */ if ((base ^ io_map_base) & BIT(VA_BITS - 1)) - ret = -ENOMEM; + base = (unsigned long)ERR_PTR(-ENOMEM); else io_map_base = base; mutex_unlock(&kvm_hyp_pgd_mutex); - if (ret) - goto out; + return base; +} + +int __create_hyp_private_mapping(phys_addr_t phys_addr, size_t size, + size_t align, unsigned long *haddr, + enum kvm_pgtable_prot prot) +{ + unsigned long addr; + int ret = 0; + + if (!kvm_host_owns_hyp_mappings()) { + addr = kvm_call_hyp_nvhe(__pkvm_create_private_mapping, + phys_addr, size, prot); + if (IS_ERR_OR_NULL((void *)addr)) + return addr ? PTR_ERR((void *)addr) : -ENOMEM; + *haddr = addr; + + return 0; + } + + size += offset_in_page(phys_addr); + addr = hyp_alloc_private_va_range(size, align); + if (IS_ERR_OR_NULL((void *)addr)) + return addr ? PTR_ERR((void *)addr) : -ENOMEM; - ret = __create_hyp_mappings(base, size, phys_addr, prot); + ret = __create_hyp_mappings(addr, size, phys_addr, prot); if (ret) goto out; - *haddr = base + offset_in_page(phys_addr); + *haddr = addr + offset_in_page(phys_addr); out: return ret; } @@ -537,7 +553,7 @@ int create_hyp_io_mappings(phys_addr_t phys_addr, size_t size, return 0; } - ret = __create_hyp_private_mapping(phys_addr, size, + ret = __create_hyp_private_mapping(phys_addr, size, PAGE_SIZE, &addr, PAGE_HYP_DEVICE); if (ret) { iounmap(*kaddr); @@ -564,7 +580,7 @@ int create_hyp_exec_mappings(phys_addr_t phys_addr, size_t size, BUG_ON(is_kernel_in_hyp_mode()); - ret = __create_hyp_private_mapping(phys_addr, size, + ret = __create_hyp_private_mapping(phys_addr, size, PAGE_SIZE, &addr, PAGE_HYP_EXEC); if (ret) { *haddr = NULL;
hyp_alloc_private_va_range() can be used to reserve private VA ranges in the nVHE hypervisor. Also update __create_hyp_private_mapping() to allow specifying an alignment for the private VA mapping. These will be used to implement stack guard pages for KVM nVHE hypervisor (nVHE Hyp mode / not pKVM), in a subsequent patch in the series. Signed-off-by: Kalesh Singh <kaleshsingh@google.com> --- Changes in v3: - Handle null ptr in IS_ERR_OR_NULL checks, per Mark arch/arm64/include/asm/kvm_mmu.h | 4 +++ arch/arm64/kvm/mmu.c | 62 ++++++++++++++++++++------------ 2 files changed, 43 insertions(+), 23 deletions(-)