Index | Thread | Search

From:
Mark Kettenis <mark.kettenis@xs4all.nl>
Subject:
Re: km_alloc(9), UAREA and kv_pageable
To:
Martin Pieuchot <mpi@grenadille.net>
Cc:
tech@openbsd.org
Date:
Sun, 20 Oct 2024 14:36:42 +0200

Download raw body.

Thread
> Date: Sun, 20 Oct 2024 11:37:57 +0200
> From: Martin Pieuchot <mpi@grenadille.net>

Sorry, I didn't have time yet to investigate.  But I'd rather not
change the km_alloc() API if it is not necessary, especially when the
it introduces a somewhat vague concept like "managed" that doesn't
even make sense in the context where it gets used.

> On 10/10/24(Thu) 10:40, Martin Pieuchot wrote:
> > On 09/10/24(Wed) 13:39, Mark Kettenis wrote:
> > > > Date: Wed, 9 Oct 2024 13:16:25 +0200
> > > > From: Martin Pieuchot <mpi@grenadille.net>
> > > > 
> > > > On 02/10/24(Wed) 17:28, Martin Pieuchot wrote:
> > > > > It is currently impossible to use km_alloc(9) to get a managed mapping
> > > > > backed with physical pages allocated up front.  I guess this is why
> > > > > uvm_km_kmemalloc_pla() is still used to allocate the UAREA.
> > > > > 
> > > > > To fix that, I suggest the diff below which turns the "kp_pageable" flag
> > > > > of km_alloc(9) into a "kp_managed".  This new flag no longer implies
> > > > > "kp_nomem" so I updated the description and fixed the remaining XXX to
> > > > > preserve existing behavior of the `kp_pageable' global.
> > > > > 
> > > > > With this change I believe we could also get rid of uvm_km_zalloc() used
> > > > > in the i386 pmap.
> > > > > 
> > > > > Comments?  Oks?
> > > > 
> > > > Anyone?
> > > 
> > > I don't really understand why this needs to be "managed" memory.  We
> > > stopped paging out the uarea ages ago, and I wonder if all this
> > > complexity is just from the time when we did.  Can't this just be
> > > kp_zero memory?
> > 
> > On amd64 using kp_zero results in the kernel faulting in loop from which
> > it cannot recover.  Could you please look at it and tell me if there is a
> > bug somewhere else?

Since the memory is no longer "managed", setguardpage() needs to use
pmap_kremove() instead of pmap_remove().

The following diff works for me on both amd64 and arm64.


Index: arch/amd64/amd64/vm_machdep.c
===================================================================
RCS file: /cvs/src/sys/arch/amd64/amd64/vm_machdep.c,v
diff -u -p -r1.47 vm_machdep.c
--- arch/amd64/amd64/vm_machdep.c	11 Apr 2023 00:45:07 -0000	1.47
+++ arch/amd64/amd64/vm_machdep.c	20 Oct 2024 12:34:07 -0000
@@ -135,8 +135,7 @@ cpu_exit(struct proc *p)
 void
 setguardpage(struct proc *p)
 {
-	pmap_remove(pmap_kernel(), (vaddr_t)p->p_addr + PAGE_SIZE,
-	    (vaddr_t)p->p_addr + 2 * PAGE_SIZE);
+	pmap_kremove((vaddr_t)p->p_addr + PAGE_SIZE, PAGE_SIZE);
 	pmap_update(pmap_kernel());
 }
 
Index: uvm/uvm_glue.c
===================================================================
RCS file: /cvs/src/sys/uvm/uvm_glue.c,v
diff -u -p -r1.85 uvm_glue.c
--- uvm/uvm_glue.c	8 Oct 2024 02:29:10 -0000	1.85
+++ uvm/uvm_glue.c	20 Oct 2024 12:34:07 -0000
@@ -257,20 +257,18 @@ uvm_vsunlock_device(struct proc *p, void
 	uvm_km_free(kernel_map, kva, sz);
 }
 
+const struct kmem_va_mode kv_uarea = {
+	.kv_map = &kernel_map,
+	.kv_align = USPACE_ALIGN
+};
+
 /*
  * uvm_uarea_alloc: allocate the u-area for a new thread
  */
 vaddr_t
 uvm_uarea_alloc(void)
 {
-	vaddr_t uaddr;
-
-	uaddr = uvm_km_kmemalloc_pla(kernel_map, uvm.kernel_object, USPACE,
-	    USPACE_ALIGN, UVM_KMF_ZERO,
-	    no_constraint.ucr_low, no_constraint.ucr_high,
-	    0, 0, USPACE/PAGE_SIZE);
-
-	return (uaddr);
+	return (vaddr_t)km_alloc(USPACE, &kv_uarea, &kp_zero, &kd_waitok);
 }
 
 /*
@@ -282,7 +280,7 @@ uvm_uarea_alloc(void)
 void
 uvm_uarea_free(struct proc *p)
 {
-	uvm_km_free(kernel_map, (vaddr_t)p->p_addr, USPACE);
+	km_free(p->p_addr, USPACE, &kv_uarea, &kp_zero);
 	p->p_addr = NULL;
 }