You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "pussuw (via GitHub)" <gi...@apache.org> on 2023/05/23 13:34:20 UTC

[GitHub] [nuttx] pussuw opened a new pull request, #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

pussuw opened a new pull request, #9368:
URL: https://github.com/apache/nuttx/pull/9368

   …memory
   
   ## Summary
   This adds functionality to map pages dynamically into kernel virtual memory. This allows implementing I/O remap for example, which is a useful (future) feature.
   
   Now, the first target is to support mapping user pages for the kernel.
   
   Why? There are some userspace structures that might be needed when the userspace process is not running. Semaphores are one such example. Signals and the WDT timeout both need access to the user semaphore to work properly. Even though for this only obtaining the kernel addressable page pool virtual address is needed, for completeness a procedure is provided to map several pages.
   
   NOTE: ARM target does not implement support yet, I'm not sure how to implement it.
   ## Impact
   Add new feature for CONFIG_BUILD_KERNEL, nothing else is affected
   ## Testing
   icicle:knsh
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pussuw commented on pull request #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on PR #9368:
URL: https://github.com/apache/nuttx/pull/9368#issuecomment-1561644671

   >The basic tests should be all/mostly re-usable. It really should be a big but low risk packaging change.
   Yes agree, mostly the signal/waitpid and such tests need to be refactored.
   
   > Does vfork not work? fork() or vfork() would ultimately be followed by exec() and so would still need to be exec'ed from a file.
   I was thinking task_spawn in this case is sort of just fork()'ing without exec(). vfork IMHO is superseded by posix_spawn anyway and both need a file to exec(). This should all be doable with posix_spawn and I in fact started writing a wrapper for task_create that uses posix_spawn() but then something more important came up.
   
   > This is a good discussion, but unfortunately off-topic for this PR.
   
   I do agree with Alan, we should have a test suite for these new features. However as AFAIK there are not many users of kernel mode right now / yet, and I can personally just use my application (which uses a wide POSIX feature set) to test the features I need. So again, writing the kernel mode support for ostest is unfortunately very low priority for me right now :(


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] patacongo commented on pull request #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

Posted by "patacongo (via GitHub)" <gi...@apache.org>.
patacongo commented on PR #9368:
URL: https://github.com/apache/nuttx/pull/9368#issuecomment-1561478634

   > @pussuw is there some test that could be add to ostest to verify if dynamic map pages is working? Also please consider added some basic Documentation/ it will help people discover that NuttX supports it.
   
   ostest does not currently work in KERNEL mode.  It relies things like task_create() which don't exist in KERNEL mode.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] acassis commented on pull request #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on PR #9368:
URL: https://github.com/apache/nuttx/pull/9368#issuecomment-1561454691

   @pussuw is there some test that could be add to ostest to verify if dynamic map pages is working? Also please consider added some basic Documentation/ it will help people discover that NuttX supports it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] xiaoxiang781216 merged pull request #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 merged PR #9368:
URL: https://github.com/apache/nuttx/pull/9368


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pussuw commented on a diff in pull request #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on code in PR #9368:
URL: https://github.com/apache/nuttx/pull/9368#discussion_r1204609511


##########
arch/risc-v/src/common/pgalloc.h:
##########
@@ -78,6 +80,24 @@ static inline uintptr_t riscv_pgvaddr(uintptr_t paddr)
 }
 #endif /* CONFIG_ARCH_PGPOOL_MAPPING */
 
+/****************************************************************************
+ * Name: riscv_uservaddr
+ *
+ * Description:
+ *   Return true if the virtual address, vaddr, lies in the user address
+ *   space.
+ *
+ ****************************************************************************/
+
+static inline bool riscv_uservaddr(uintptr_t vaddr)
+{
+  /* Check if this address is within the range of the virtualized .bss/.data,
+   * heap, or stack regions.
+   */
+
+  return vaddr >= ARCH_ADDRENV_VBASE;
+}

Review Comment:
   Correct.
   
   Currently the upper bound is not known or even decided, but we could limit userland size to e.g. 1GB (the page directory currently only has room for 1GB anyway..)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pussuw commented on a diff in pull request #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on code in PR #9368:
URL: https://github.com/apache/nuttx/pull/9368#discussion_r1204611641


##########
arch/risc-v/src/common/riscv_mmu.h:
##########
@@ -308,24 +308,40 @@ static inline uintptr_t mmu_pte_to_paddr(uintptr_t pte)
 }
 
 /****************************************************************************
- * Name: mmu_get_satp_pgbase
+ * Name: mmu_satp_to_paddr
  *
  * Description:
- *   Utility function to read the base page table physical address
+ *   Extract physical address from SATP
  *
  * Returned Value:
- *   Physical address of the current base page table
+ *   Physical address from SATP value
  *
  ****************************************************************************/
 
-static inline uintptr_t mmu_get_satp_pgbase(void)
+static inline uintptr_t mmu_satp_to_paddr(uintptr_t satp)
 {
   uintptr_t ppn;
-  ppn = mmu_read_satp();
+  ppn = satp;

Review Comment:
   Yes making a copy is not -strictly- necessary here. I presume the compiler sees this and does not allocate separate storage for ppn anyway.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pussuw commented on a diff in pull request #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on code in PR #9368:
URL: https://github.com/apache/nuttx/pull/9368#discussion_r1204610718


##########
arch/risc-v/src/common/addrenv.h:
##########
@@ -44,6 +44,14 @@
 #  define ARCH_KERNEL_STACKSIZE STACK_ALIGN_UP(CONFIG_ARCH_KERNEL_STACKSIZE)
 #endif
 
+/* Base address for address environment */
+
+#if CONFIG_ARCH_TEXT_VBASE != 0
+#  define ARCH_ADDRENV_VBASE    (CONFIG_ARCH_TEXT_VBASE)
+#else
+#  define ARCH_ADDRENV_VBASE    (CONFIG_ARCH_DATA_VBASE)
+#endif
+

Review Comment:
   Sure, we just need to define a maximum userland size, currently it is not defined.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pussuw commented on pull request #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on PR #9368:
URL: https://github.com/apache/nuttx/pull/9368#issuecomment-1561506449

   @acassis like @patacongo said currently ostest does not work at all for kernel mode. I tried doing an intermediate step https://github.com/apache/nuttx-apps/pull/1500 by forwarding task_create to pthread_create but that is not the same thing. The correct implementation would be to go through the tests case-by-case and:
   - When task_create is used to make workers, they can be replaced with pthreads
   - When task_create is used to make other "processes", they must be replaced with posix_spawn() or similar (as we don't have fork()). Making the tests compatible to be spawned requires a bit of refactoring.
   
   As a summary; making all ostest steps work in kernel mode is doable but not an insignificant amount of work which I simply do not have time to do now.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] patacongo commented on pull request #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

Posted by "patacongo (via GitHub)" <gi...@apache.org>.
patacongo commented on PR #9368:
URL: https://github.com/apache/nuttx/pull/9368#issuecomment-1561598047

   > I think this is unavoidable, some split is expected.
   
   The basic tests should be all/mostly re-usable.  It really should be a big but low risk packaging change.
   
   > I think kernel mode needs this anyway, due to not having fork() i.e. only spawning from file works.
   
   Does vfork not work?  fork() or vfork() would ultimately be followed by exec() and so would still need to be exec'ed from a file.
   
   This is a good discussion, but unfortunately off-topic for this PR.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] patacongo commented on pull request #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

Posted by "patacongo (via GitHub)" <gi...@apache.org>.
patacongo commented on PR #9368:
URL: https://github.com/apache/nuttx/pull/9368#issuecomment-1561677770

   > I was thinking task_spawn in this case is sort of just fork()'ing without exec().
   
   task_spawn() is not available in KERNEL mode either:
   
       138 #ifndef CONFIG_BUILD_KERNEL
       139 /* Non-standard task_spawn interface.  This function uses the same
       140  * semantics to execute a file in memory at 'entry', giving it the name
       141  * 'name'.
       142  */
       143
       144 int task_spawn(FAR const char *name, main_t entry,
       145       FAR const posix_spawn_file_actions_t *file_actions,
       146       FAR const posix_spawnattr_t *attr,
       147       FAR char * const argv[], FAR char * const envp[]);
       148 #endif
   
   Nothing that includes the address of main() can be used to start tasks in that mode since the address of main() is not available in that address environment.  You would have to use posix_spawn() which does take a file path instead of an address.  posix_spawn() is essentially a fork+exec.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #9368:
URL: https://github.com/apache/nuttx/pull/9368#discussion_r1213150169


##########
mm/kmap/kmm_map.c:
##########
@@ -0,0 +1,466 @@
+/****************************************************************************
+ * mm/kmap/kmm_map.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdint.h>
+#include <stddef.h>
+
+#include <nuttx/addrenv.h>
+#include <nuttx/arch.h>
+#include <nuttx/compiler.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/gran.h>
+#include <nuttx/mm/map.h>
+#include <nuttx/pgalloc.h>
+#include <nuttx/sched.h>
+
+#include <sys/mman.h>
+
+#if defined(CONFIG_BUILD_KERNEL)
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static GRAN_HANDLE     g_kmm_map_vpages;
+static struct mm_map_s g_kmm_map;
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: get_user_pages
+ *
+ * Description:
+ *   Get the physical pages mapped to a user virtual memory region.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer where the page addresses are recorded.
+ *   npages - Amount of pages.
+ *   vaddr  - Start address of the user virtual memory region.
+ *
+ * Returned Value:
+ *   Zero (OK) is returned on success; a negated errno value is returned
+ *   on failure.
+ *
+ ****************************************************************************/
+
+static int get_user_pages(FAR void **pages, size_t npages, uintptr_t vaddr)
+{
+  FAR struct tcb_s *tcb = nxsched_self();
+  uintptr_t         page;
+  int               i;
+
+  /* Find the pages associated with the user virtual address space */
+
+  for (i = 0; i < npages; i++, vaddr += MM_PGSIZE)
+    {
+      page = up_addrenv_find_page(&tcb->addrenv_own->addrenv, vaddr);
+      if (!page)
+        {
+          /* Something went wrong, get out */
+
+          return -EINVAL;
+        }
+
+      pages[i] = (FAR void *)page;
+    }
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: map_pages
+ *
+ * Description:
+ *   Map pages into kernel virtual memory.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer that contains the physical page addresses.
+ *   npages - Amount of pages.
+ *   prot   - Access right flags.
+ *
+ * Returned Value:
+ *   Pointer to the mapped virtual memory on success; NULL on failure
+ *
+ ****************************************************************************/
+
+static void *map_pages(FAR void **pages, size_t npages, int prot)
+{
+  struct mm_map_entry_s entry;
+  FAR void             *vaddr;
+  size_t                size;
+  int                   ret;
+
+  /* The region size is full pages */
+
+  size = npages << MM_PGSHIFT;
+
+  /* Find a virtual memory area that fits */
+
+  vaddr = gran_alloc(&g_kmm_map_vpages, size);
+  if (!vaddr)
+    {
+      return NULL;
+    }
+
+  /* Map the pages into the kernel page directory */
+
+  ret = up_addrenv_kmap_pages(pages, npages, (uintptr_t)vaddr, prot);
+  if (ret < 0)
+    {
+      goto errout_with_vaddr;
+    }
+
+  entry.vaddr = vaddr;
+  entry.length = size;
+  entry.offset = 0;
+  entry.munmap = NULL;
+
+  ret = mm_map_add(&g_kmm_map, &entry);
+  if (ret < 0)
+    {
+      goto errout_with_pgmap;
+    }
+
+  return vaddr;
+
+errout_with_pgmap:
+  up_addrenv_kunmap_pages((uintptr_t)vaddr, npages);
+errout_with_vaddr:
+  gran_free(&g_kmm_map_vpages, vaddr, size);
+  return NULL;
+}
+
+/****************************************************************************
+ * Name: map_single_user_page
+ *
+ * Description:
+ *   Map a single user page into kernel memory.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer that contains the physical page addresses.
+ *   npages - Amount of pages.
+ *   prot   - Access right flags.
+ *
+ * Returned Value:
+ *   Pointer to the mapped virtual memory on success; NULL on failure
+ *
+ ****************************************************************************/
+
+static void *map_single_user_page(uintptr_t vaddr)
+{
+  FAR struct tcb_s *tcb = nxsched_self();
+  uintptr_t         page;
+
+  /* Find the page associated with this virtual address */
+
+  page = up_addrenv_find_page(&tcb->addrenv_own->addrenv, vaddr);
+  if (!page)
+    {
+      return NULL;
+    }
+
+  vaddr = up_addrenv_page_vaddr(page);
+  return (FAR void *)vaddr;
+}
+
+/****************************************************************************
+ * Name: kmm_map_lock
+ *
+ * Description:
+ *   Get exclusive access to the kernel mm_map
+ *
+ ****************************************************************************/
+
+static int kmm_map_lock(void)
+{
+  return nxrmutex_lock(&g_kmm_map.mm_map_mutex);
+}
+
+/****************************************************************************
+ * Name: kmm_map_unlock
+ *
+ * Description:
+ *   Relinquish exclusive access to the kernel mm_map
+ *
+ ****************************************************************************/
+
+static void kmm_map_unlock(void)
+{
+  DEBUGVERIFY(nxrmutex_unlock(&g_kmm_map.mm_map_mutex));
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: kmm_map_initialize
+ *
+ * Description:
+ *   Initialize the kernel dynamic mapping module.
+ *
+ * Input Parameters:
+ *   None.
+ *
+ * Returned Value:
+ *   None.
+ *
+ ****************************************************************************/
+
+void kmm_map_initialize(void)
+{
+  g_kmm_map_vpages = gran_initialize((FAR void *)CONFIG_ARCH_KMAP_VBASE,
+                                     CONFIG_ARCH_KMAP_NPAGES << MM_PGSHIFT,
+                                     MM_PGSHIFT, MM_PGSHIFT);
+  DEBUGVERIFY(g_kmm_map_vpages);
+  mm_map_initialize(&g_kmm_map, true);
+}
+
+/****************************************************************************
+ * Name: kmm_map_pages
+ *
+ * Description:
+ *   Map pages into kernel virtual memory.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer that contains the physical page addresses.
+ *   npages - Amount of pages.
+ *   prot   - Access right flags.
+ *
+ * Returned Value:
+ *   Pointer to the mapped virtual memory on success; NULL on failure
+ *
+ ****************************************************************************/
+
+FAR void *kmm_map(FAR void **pages, size_t npages, int prot)
+{
+  uintptr_t vaddr;
+
+  if (!pages || !npages || npages > CONFIG_ARCH_KMAP_NPAGES)
+    {
+      return NULL;
+    }
+
+  /* Attempt to map the pages */
+
+  vaddr = (uintptr_t)map_pages(pages, npages, prot);
+  if (!vaddr)
+    {
+      return NULL;
+    }
+
+  return (FAR void *)(vaddr);

Review Comment:
   remove () around vaddr



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pussuw commented on a diff in pull request #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on code in PR #9368:
URL: https://github.com/apache/nuttx/pull/9368#discussion_r1217582262


##########
mm/kmap/kmm_map.c:
##########
@@ -0,0 +1,466 @@
+/****************************************************************************
+ * mm/kmap/kmm_map.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdint.h>
+#include <stddef.h>
+
+#include <nuttx/addrenv.h>
+#include <nuttx/arch.h>
+#include <nuttx/compiler.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/gran.h>
+#include <nuttx/mm/map.h>
+#include <nuttx/pgalloc.h>
+#include <nuttx/sched.h>
+
+#include <sys/mman.h>
+
+#if defined(CONFIG_BUILD_KERNEL)
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static GRAN_HANDLE     g_kmm_map_vpages;
+static struct mm_map_s g_kmm_map;
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: get_user_pages
+ *
+ * Description:
+ *   Get the physical pages mapped to a user virtual memory region.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer where the page addresses are recorded.
+ *   npages - Amount of pages.
+ *   vaddr  - Start address of the user virtual memory region.
+ *
+ * Returned Value:
+ *   Zero (OK) is returned on success; a negated errno value is returned
+ *   on failure.
+ *
+ ****************************************************************************/
+
+static int get_user_pages(FAR void **pages, size_t npages, uintptr_t vaddr)
+{
+  FAR struct tcb_s *tcb = nxsched_self();
+  uintptr_t         page;
+  int               i;
+
+  /* Find the pages associated with the user virtual address space */
+
+  for (i = 0; i < npages; i++, vaddr += MM_PGSIZE)
+    {
+      page = up_addrenv_find_page(&tcb->addrenv_own->addrenv, vaddr);
+      if (!page)
+        {
+          /* Something went wrong, get out */
+
+          return -EINVAL;
+        }
+
+      pages[i] = (FAR void *)page;
+    }
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: map_pages
+ *
+ * Description:
+ *   Map pages into kernel virtual memory.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer that contains the physical page addresses.
+ *   npages - Amount of pages.
+ *   prot   - Access right flags.
+ *
+ * Returned Value:
+ *   Pointer to the mapped virtual memory on success; NULL on failure
+ *
+ ****************************************************************************/
+
+static void *map_pages(FAR void **pages, size_t npages, int prot)
+{
+  struct mm_map_entry_s entry;
+  FAR void             *vaddr;
+  size_t                size;
+  int                   ret;
+
+  /* The region size is full pages */
+
+  size = npages << MM_PGSHIFT;
+
+  /* Find a virtual memory area that fits */
+
+  vaddr = gran_alloc(&g_kmm_map_vpages, size);
+  if (!vaddr)
+    {
+      return NULL;
+    }
+
+  /* Map the pages into the kernel page directory */
+
+  ret = up_addrenv_kmap_pages(pages, npages, (uintptr_t)vaddr, prot);
+  if (ret < 0)
+    {
+      goto errout_with_vaddr;
+    }
+
+  entry.vaddr = vaddr;
+  entry.length = size;
+  entry.offset = 0;
+  entry.munmap = NULL;
+
+  ret = mm_map_add(&g_kmm_map, &entry);
+  if (ret < 0)
+    {
+      goto errout_with_pgmap;
+    }
+
+  return vaddr;
+
+errout_with_pgmap:
+  up_addrenv_kunmap_pages((uintptr_t)vaddr, npages);
+errout_with_vaddr:
+  gran_free(&g_kmm_map_vpages, vaddr, size);
+  return NULL;
+}
+
+/****************************************************************************
+ * Name: map_single_user_page
+ *
+ * Description:
+ *   Map a single user page into kernel memory.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer that contains the physical page addresses.
+ *   npages - Amount of pages.
+ *   prot   - Access right flags.
+ *
+ * Returned Value:
+ *   Pointer to the mapped virtual memory on success; NULL on failure
+ *
+ ****************************************************************************/
+
+static void *map_single_user_page(uintptr_t vaddr)
+{
+  FAR struct tcb_s *tcb = nxsched_self();
+  uintptr_t         page;
+
+  /* Find the page associated with this virtual address */
+
+  page = up_addrenv_find_page(&tcb->addrenv_own->addrenv, vaddr);
+  if (!page)
+    {
+      return NULL;
+    }
+
+  vaddr = up_addrenv_page_vaddr(page);
+  return (FAR void *)vaddr;
+}
+
+/****************************************************************************
+ * Name: kmm_map_lock
+ *
+ * Description:
+ *   Get exclusive access to the kernel mm_map
+ *
+ ****************************************************************************/
+
+static int kmm_map_lock(void)
+{
+  return nxrmutex_lock(&g_kmm_map.mm_map_mutex);
+}
+
+/****************************************************************************
+ * Name: kmm_map_unlock
+ *
+ * Description:
+ *   Relinquish exclusive access to the kernel mm_map
+ *
+ ****************************************************************************/
+
+static void kmm_map_unlock(void)
+{
+  DEBUGVERIFY(nxrmutex_unlock(&g_kmm_map.mm_map_mutex));
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: kmm_map_initialize
+ *
+ * Description:
+ *   Initialize the kernel dynamic mapping module.
+ *
+ * Input Parameters:
+ *   None.
+ *
+ * Returned Value:
+ *   None.
+ *
+ ****************************************************************************/
+
+void kmm_map_initialize(void)
+{
+  g_kmm_map_vpages = gran_initialize((FAR void *)CONFIG_ARCH_KMAP_VBASE,
+                                     CONFIG_ARCH_KMAP_NPAGES << MM_PGSHIFT,
+                                     MM_PGSHIFT, MM_PGSHIFT);
+  DEBUGVERIFY(g_kmm_map_vpages);
+  mm_map_initialize(&g_kmm_map, true);
+}
+
+/****************************************************************************
+ * Name: kmm_map_pages
+ *
+ * Description:
+ *   Map pages into kernel virtual memory.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer that contains the physical page addresses.
+ *   npages - Amount of pages.
+ *   prot   - Access right flags.
+ *
+ * Returned Value:
+ *   Pointer to the mapped virtual memory on success; NULL on failure
+ *
+ ****************************************************************************/
+
+FAR void *kmm_map(FAR void **pages, size_t npages, int prot)
+{
+  uintptr_t vaddr;
+
+  if (!pages || !npages || npages > CONFIG_ARCH_KMAP_NPAGES)
+    {
+      return NULL;
+    }
+
+  /* Attempt to map the pages */
+
+  vaddr = (uintptr_t)map_pages(pages, npages, prot);
+  if (!vaddr)
+    {
+      return NULL;
+    }
+
+  return (FAR void *)(vaddr);
+}
+
+/****************************************************************************
+ * Name: kmm_unmap
+ *
+ * Description:
+ *   Unmap a previously allocated kernel virtual memory area.
+ *
+ * Input Parameters:
+ *   kaddr - The kernel virtual address where the mapping begins.
+ *
+ * Returned Value:
+ *   None.
+ *
+ ****************************************************************************/
+
+void kmm_unmap(FAR void *kaddr)
+{
+  FAR struct mm_map_entry_s *entry;
+  unsigned int               npages;
+  int                        ret;
+
+  /* Lock the mapping list when we fiddle around with it */
+
+  ret = kmm_map_lock();
+  if (ret == OK)
+    {
+      /* Find the entry, it is OK if none found */
+
+      entry = mm_map_find(get_current_mm(), kaddr, 1);
+      if (entry)
+        {
+          npages = MM_NPAGES(entry->length);
+
+          /* Remove the mappings from the page directory */
+
+          up_addrenv_kunmap_pages((uintptr_t)entry->vaddr, npages);
+
+          /* Release the virtual memory area for use */
+
+          gran_free(&g_kmm_map_vpages, entry->vaddr, entry->length);
+
+          /* Remove the mapping from the kernel mapping list */
+
+          mm_map_remove(&g_kmm_map, entry);
+        }
+
+      kmm_map_unlock();
+    }
+}
+
+/****************************************************************************
+ * Name: kmm_user_map
+ *
+ * Description:
+ *   Map a region of user memory (physical pages) for kernel use through
+ *   a continuous virtual memory area.
+ *
+ * Input Parameters:
+ *   uaddr - The user virtual address where mapping begins.
+ *   size  - Size of the region.
+ *
+ * Returned Value:
+ *   Pointer to the virtual memory area, or NULL if out of memory.
+ *
+ ****************************************************************************/
+
+FAR void *kmm_user_map(FAR void *uaddr, size_t size)
+{
+  FAR void *pages;
+  uintptr_t vaddr;
+  uintptr_t offset;
+  size_t    npages;
+  int       ret;
+
+  /* Find the kernel addressable virtual address, or map the user memory */
+
+  vaddr = (uintptr_t)uaddr;
+
+  /* If the memory is not user memory, get out */
+
+  if (!up_addrenv_user_vaddr(vaddr))
+    {
+      /* Not user memory, get out */
+
+      return uaddr;
+    }
+
+  /* How many pages (including partial) does the space encompass ? */
+
+  offset = vaddr & MM_PGMASK;
+  vaddr  = MM_PGALIGNDOWN(vaddr);
+  npages = MM_NPAGES(offset + size);
+
+  /* Does the area fit in 1 page, including page boundary crossings ? */
+
+  if (npages == 1)
+    {
+      /* Yes, can simply return the kernel addressable virtual address */
+
+      vaddr = (uintptr_t)map_single_user_page(vaddr);
+      return (FAR void *)(vaddr + offset);
+    }
+
+  /* No, the area must be mapped into kernel virtual address space */
+
+  pages = kmm_zalloc(npages * sizeof(void *));

Review Comment:
   ~~Unnecessary, the size of void* does not change if declared far~~Hmm it seems like I'm wrong.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pussuw commented on a diff in pull request #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on code in PR #9368:
URL: https://github.com/apache/nuttx/pull/9368#discussion_r1210342337


##########
arch/risc-v/src/common/riscv_mmu.h:
##########
@@ -308,24 +308,40 @@ static inline uintptr_t mmu_pte_to_paddr(uintptr_t pte)
 }
 
 /****************************************************************************
- * Name: mmu_get_satp_pgbase
+ * Name: mmu_satp_to_paddr
  *
  * Description:
- *   Utility function to read the base page table physical address
+ *   Extract physical address from SATP
  *
  * Returned Value:
- *   Physical address of the current base page table
+ *   Physical address from SATP value
  *
  ****************************************************************************/
 
-static inline uintptr_t mmu_get_satp_pgbase(void)
+static inline uintptr_t mmu_satp_to_paddr(uintptr_t satp)
 {
   uintptr_t ppn;
-  ppn = mmu_read_satp();
+  ppn = satp;

Review Comment:
   My suggestion is to keep this as-is for readability. With -Os the compiler sees extra storage for ppn is unnecessary and does not allocate it, verified from output.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pkarashchenko commented on a diff in pull request #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #9368:
URL: https://github.com/apache/nuttx/pull/9368#discussion_r1214683999


##########
arch/risc-v/src/common/riscv_addrenv_pgmap.c:
##########
@@ -0,0 +1,239 @@
+/****************************************************************************
+ * arch/risc-v/src/common/riscv_addrenv_pgmap.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/arch.h>
+#include <nuttx/addrenv.h>
+#include <nuttx/irq.h>
+#include <nuttx/pgalloc.h>
+#include <nuttx/sched.h>
+
+#include <arch/barriers.h>
+
+#include <sys/mman.h>
+
+#include "pgalloc.h"
+#include "riscv_mmu.h"
+
+#ifdef CONFIG_BUILD_KERNEL
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: up_addrenv_find_page
+ *
+ * Description:
+ *   Find physical page mapped to user virtual address from the address
+ *   environment page directory.
+ *
+ * Input Parameters:
+ *   addrenv - The user address environment.
+ *   vaddr   - The user virtual address
+ *
+ * Returned Value:
+ *   Page physical address on success; NULL on failure.
+ *
+ ****************************************************************************/
+
+uintptr_t up_addrenv_find_page(FAR arch_addrenv_t *addrenv, uintptr_t vaddr)

Review Comment:
   Remove FAR



##########
include/nuttx/mm/kmap.h:
##########
@@ -0,0 +1,109 @@
+/****************************************************************************
+ * include/nuttx/mm/kmap.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_MM_KMAP_H_

Review Comment:
   Remove `_` after `H`



##########
mm/kmap/kmm_map.c:
##########
@@ -0,0 +1,466 @@
+/****************************************************************************
+ * mm/kmap/kmm_map.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdint.h>
+#include <stddef.h>
+
+#include <nuttx/addrenv.h>
+#include <nuttx/arch.h>
+#include <nuttx/compiler.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/gran.h>
+#include <nuttx/mm/map.h>
+#include <nuttx/pgalloc.h>
+#include <nuttx/sched.h>
+
+#include <sys/mman.h>
+
+#if defined(CONFIG_BUILD_KERNEL)
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static GRAN_HANDLE     g_kmm_map_vpages;
+static struct mm_map_s g_kmm_map;
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: get_user_pages
+ *
+ * Description:
+ *   Get the physical pages mapped to a user virtual memory region.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer where the page addresses are recorded.
+ *   npages - Amount of pages.
+ *   vaddr  - Start address of the user virtual memory region.
+ *
+ * Returned Value:
+ *   Zero (OK) is returned on success; a negated errno value is returned
+ *   on failure.
+ *
+ ****************************************************************************/
+
+static int get_user_pages(FAR void **pages, size_t npages, uintptr_t vaddr)
+{
+  FAR struct tcb_s *tcb = nxsched_self();
+  uintptr_t         page;
+  int               i;
+
+  /* Find the pages associated with the user virtual address space */
+
+  for (i = 0; i < npages; i++, vaddr += MM_PGSIZE)
+    {
+      page = up_addrenv_find_page(&tcb->addrenv_own->addrenv, vaddr);
+      if (!page)
+        {
+          /* Something went wrong, get out */
+
+          return -EINVAL;
+        }
+
+      pages[i] = (FAR void *)page;
+    }
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: map_pages
+ *
+ * Description:
+ *   Map pages into kernel virtual memory.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer that contains the physical page addresses.
+ *   npages - Amount of pages.
+ *   prot   - Access right flags.
+ *
+ * Returned Value:
+ *   Pointer to the mapped virtual memory on success; NULL on failure
+ *
+ ****************************************************************************/
+
+static void *map_pages(FAR void **pages, size_t npages, int prot)

Review Comment:
   `static FAR void *`



##########
mm/kmap/kmm_map.c:
##########
@@ -0,0 +1,466 @@
+/****************************************************************************
+ * mm/kmap/kmm_map.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdint.h>
+#include <stddef.h>
+
+#include <nuttx/addrenv.h>
+#include <nuttx/arch.h>
+#include <nuttx/compiler.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/gran.h>
+#include <nuttx/mm/map.h>
+#include <nuttx/pgalloc.h>
+#include <nuttx/sched.h>
+
+#include <sys/mman.h>
+
+#if defined(CONFIG_BUILD_KERNEL)
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static GRAN_HANDLE     g_kmm_map_vpages;
+static struct mm_map_s g_kmm_map;
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: get_user_pages
+ *
+ * Description:
+ *   Get the physical pages mapped to a user virtual memory region.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer where the page addresses are recorded.
+ *   npages - Amount of pages.
+ *   vaddr  - Start address of the user virtual memory region.
+ *
+ * Returned Value:
+ *   Zero (OK) is returned on success; a negated errno value is returned
+ *   on failure.
+ *
+ ****************************************************************************/
+
+static int get_user_pages(FAR void **pages, size_t npages, uintptr_t vaddr)
+{
+  FAR struct tcb_s *tcb = nxsched_self();
+  uintptr_t         page;
+  int               i;
+
+  /* Find the pages associated with the user virtual address space */
+
+  for (i = 0; i < npages; i++, vaddr += MM_PGSIZE)
+    {
+      page = up_addrenv_find_page(&tcb->addrenv_own->addrenv, vaddr);
+      if (!page)
+        {
+          /* Something went wrong, get out */
+
+          return -EINVAL;
+        }
+
+      pages[i] = (FAR void *)page;
+    }
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: map_pages
+ *
+ * Description:
+ *   Map pages into kernel virtual memory.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer that contains the physical page addresses.
+ *   npages - Amount of pages.
+ *   prot   - Access right flags.
+ *
+ * Returned Value:
+ *   Pointer to the mapped virtual memory on success; NULL on failure
+ *
+ ****************************************************************************/
+
+static void *map_pages(FAR void **pages, size_t npages, int prot)
+{
+  struct mm_map_entry_s entry;
+  FAR void             *vaddr;
+  size_t                size;
+  int                   ret;
+
+  /* The region size is full pages */
+
+  size = npages << MM_PGSHIFT;
+
+  /* Find a virtual memory area that fits */
+
+  vaddr = gran_alloc(&g_kmm_map_vpages, size);
+  if (!vaddr)
+    {
+      return NULL;
+    }
+
+  /* Map the pages into the kernel page directory */
+
+  ret = up_addrenv_kmap_pages(pages, npages, (uintptr_t)vaddr, prot);
+  if (ret < 0)
+    {
+      goto errout_with_vaddr;
+    }
+
+  entry.vaddr = vaddr;
+  entry.length = size;
+  entry.offset = 0;
+  entry.munmap = NULL;
+
+  ret = mm_map_add(&g_kmm_map, &entry);
+  if (ret < 0)
+    {
+      goto errout_with_pgmap;
+    }
+
+  return vaddr;
+
+errout_with_pgmap:
+  up_addrenv_kunmap_pages((uintptr_t)vaddr, npages);
+errout_with_vaddr:
+  gran_free(&g_kmm_map_vpages, vaddr, size);
+  return NULL;
+}
+
+/****************************************************************************
+ * Name: map_single_user_page
+ *
+ * Description:
+ *   Map a single user page into kernel memory.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer that contains the physical page addresses.
+ *   npages - Amount of pages.
+ *   prot   - Access right flags.
+ *
+ * Returned Value:
+ *   Pointer to the mapped virtual memory on success; NULL on failure
+ *
+ ****************************************************************************/
+
+static void *map_single_user_page(uintptr_t vaddr)

Review Comment:
   `static FAR void *`



##########
mm/kmap/kmm_map.c:
##########
@@ -0,0 +1,466 @@
+/****************************************************************************
+ * mm/kmap/kmm_map.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdint.h>
+#include <stddef.h>
+
+#include <nuttx/addrenv.h>
+#include <nuttx/arch.h>
+#include <nuttx/compiler.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/gran.h>
+#include <nuttx/mm/map.h>
+#include <nuttx/pgalloc.h>
+#include <nuttx/sched.h>
+
+#include <sys/mman.h>
+
+#if defined(CONFIG_BUILD_KERNEL)
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static GRAN_HANDLE     g_kmm_map_vpages;
+static struct mm_map_s g_kmm_map;
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: get_user_pages
+ *
+ * Description:
+ *   Get the physical pages mapped to a user virtual memory region.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer where the page addresses are recorded.
+ *   npages - Amount of pages.
+ *   vaddr  - Start address of the user virtual memory region.
+ *
+ * Returned Value:
+ *   Zero (OK) is returned on success; a negated errno value is returned
+ *   on failure.
+ *
+ ****************************************************************************/
+
+static int get_user_pages(FAR void **pages, size_t npages, uintptr_t vaddr)
+{
+  FAR struct tcb_s *tcb = nxsched_self();
+  uintptr_t         page;
+  int               i;
+
+  /* Find the pages associated with the user virtual address space */
+
+  for (i = 0; i < npages; i++, vaddr += MM_PGSIZE)
+    {
+      page = up_addrenv_find_page(&tcb->addrenv_own->addrenv, vaddr);
+      if (!page)
+        {
+          /* Something went wrong, get out */
+
+          return -EINVAL;
+        }
+
+      pages[i] = (FAR void *)page;
+    }
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: map_pages
+ *
+ * Description:
+ *   Map pages into kernel virtual memory.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer that contains the physical page addresses.
+ *   npages - Amount of pages.
+ *   prot   - Access right flags.
+ *
+ * Returned Value:
+ *   Pointer to the mapped virtual memory on success; NULL on failure
+ *
+ ****************************************************************************/
+
+static void *map_pages(FAR void **pages, size_t npages, int prot)
+{
+  struct mm_map_entry_s entry;
+  FAR void             *vaddr;
+  size_t                size;
+  int                   ret;
+
+  /* The region size is full pages */
+
+  size = npages << MM_PGSHIFT;
+
+  /* Find a virtual memory area that fits */
+
+  vaddr = gran_alloc(&g_kmm_map_vpages, size);
+  if (!vaddr)
+    {
+      return NULL;
+    }
+
+  /* Map the pages into the kernel page directory */
+
+  ret = up_addrenv_kmap_pages(pages, npages, (uintptr_t)vaddr, prot);
+  if (ret < 0)
+    {
+      goto errout_with_vaddr;
+    }
+
+  entry.vaddr = vaddr;
+  entry.length = size;
+  entry.offset = 0;
+  entry.munmap = NULL;
+
+  ret = mm_map_add(&g_kmm_map, &entry);
+  if (ret < 0)
+    {
+      goto errout_with_pgmap;
+    }
+
+  return vaddr;
+
+errout_with_pgmap:
+  up_addrenv_kunmap_pages((uintptr_t)vaddr, npages);
+errout_with_vaddr:
+  gran_free(&g_kmm_map_vpages, vaddr, size);
+  return NULL;
+}
+
+/****************************************************************************
+ * Name: map_single_user_page
+ *
+ * Description:
+ *   Map a single user page into kernel memory.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer that contains the physical page addresses.
+ *   npages - Amount of pages.
+ *   prot   - Access right flags.
+ *
+ * Returned Value:
+ *   Pointer to the mapped virtual memory on success; NULL on failure
+ *
+ ****************************************************************************/
+
+static void *map_single_user_page(uintptr_t vaddr)
+{
+  FAR struct tcb_s *tcb = nxsched_self();
+  uintptr_t         page;
+
+  /* Find the page associated with this virtual address */
+
+  page = up_addrenv_find_page(&tcb->addrenv_own->addrenv, vaddr);
+  if (!page)
+    {
+      return NULL;
+    }
+
+  vaddr = up_addrenv_page_vaddr(page);
+  return (FAR void *)vaddr;
+}
+
+/****************************************************************************
+ * Name: kmm_map_lock
+ *
+ * Description:
+ *   Get exclusive access to the kernel mm_map
+ *
+ ****************************************************************************/
+
+static int kmm_map_lock(void)
+{
+  return nxrmutex_lock(&g_kmm_map.mm_map_mutex);
+}
+
+/****************************************************************************
+ * Name: kmm_map_unlock
+ *
+ * Description:
+ *   Relinquish exclusive access to the kernel mm_map
+ *
+ ****************************************************************************/
+
+static void kmm_map_unlock(void)
+{
+  DEBUGVERIFY(nxrmutex_unlock(&g_kmm_map.mm_map_mutex));
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: kmm_map_initialize
+ *
+ * Description:
+ *   Initialize the kernel dynamic mapping module.
+ *
+ * Input Parameters:
+ *   None.
+ *
+ * Returned Value:
+ *   None.
+ *
+ ****************************************************************************/
+
+void kmm_map_initialize(void)
+{
+  g_kmm_map_vpages = gran_initialize((FAR void *)CONFIG_ARCH_KMAP_VBASE,
+                                     CONFIG_ARCH_KMAP_NPAGES << MM_PGSHIFT,
+                                     MM_PGSHIFT, MM_PGSHIFT);
+  DEBUGVERIFY(g_kmm_map_vpages);
+  mm_map_initialize(&g_kmm_map, true);
+}
+
+/****************************************************************************
+ * Name: kmm_map_pages
+ *
+ * Description:
+ *   Map pages into kernel virtual memory.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer that contains the physical page addresses.
+ *   npages - Amount of pages.
+ *   prot   - Access right flags.
+ *
+ * Returned Value:
+ *   Pointer to the mapped virtual memory on success; NULL on failure
+ *
+ ****************************************************************************/
+
+FAR void *kmm_map(FAR void **pages, size_t npages, int prot)
+{
+  uintptr_t vaddr;
+
+  if (!pages || !npages || npages > CONFIG_ARCH_KMAP_NPAGES)
+    {
+      return NULL;
+    }
+
+  /* Attempt to map the pages */
+
+  vaddr = (uintptr_t)map_pages(pages, npages, prot);
+  if (!vaddr)
+    {
+      return NULL;
+    }
+
+  return (FAR void *)(vaddr);
+}
+
+/****************************************************************************
+ * Name: kmm_unmap
+ *
+ * Description:
+ *   Unmap a previously allocated kernel virtual memory area.
+ *
+ * Input Parameters:
+ *   kaddr - The kernel virtual address where the mapping begins.
+ *
+ * Returned Value:
+ *   None.
+ *
+ ****************************************************************************/
+
+void kmm_unmap(FAR void *kaddr)
+{
+  FAR struct mm_map_entry_s *entry;
+  unsigned int               npages;
+  int                        ret;
+
+  /* Lock the mapping list when we fiddle around with it */
+
+  ret = kmm_map_lock();
+  if (ret == OK)
+    {
+      /* Find the entry, it is OK if none found */
+
+      entry = mm_map_find(get_current_mm(), kaddr, 1);
+      if (entry)
+        {
+          npages = MM_NPAGES(entry->length);
+
+          /* Remove the mappings from the page directory */
+
+          up_addrenv_kunmap_pages((uintptr_t)entry->vaddr, npages);
+
+          /* Release the virtual memory area for use */
+
+          gran_free(&g_kmm_map_vpages, entry->vaddr, entry->length);
+
+          /* Remove the mapping from the kernel mapping list */
+
+          mm_map_remove(&g_kmm_map, entry);
+        }
+
+      kmm_map_unlock();
+    }
+}
+
+/****************************************************************************
+ * Name: kmm_user_map
+ *
+ * Description:
+ *   Map a region of user memory (physical pages) for kernel use through
+ *   a continuous virtual memory area.
+ *
+ * Input Parameters:
+ *   uaddr - The user virtual address where mapping begins.
+ *   size  - Size of the region.
+ *
+ * Returned Value:
+ *   Pointer to the virtual memory area, or NULL if out of memory.
+ *
+ ****************************************************************************/
+
+FAR void *kmm_user_map(FAR void *uaddr, size_t size)
+{
+  FAR void *pages;
+  uintptr_t vaddr;
+  uintptr_t offset;
+  size_t    npages;
+  int       ret;
+
+  /* Find the kernel addressable virtual address, or map the user memory */
+
+  vaddr = (uintptr_t)uaddr;
+
+  /* If the memory is not user memory, get out */
+
+  if (!up_addrenv_user_vaddr(vaddr))
+    {
+      /* Not user memory, get out */
+
+      return uaddr;
+    }
+
+  /* How many pages (including partial) does the space encompass ? */
+
+  offset = vaddr & MM_PGMASK;
+  vaddr  = MM_PGALIGNDOWN(vaddr);
+  npages = MM_NPAGES(offset + size);
+
+  /* Does the area fit in 1 page, including page boundary crossings ? */
+
+  if (npages == 1)
+    {
+      /* Yes, can simply return the kernel addressable virtual address */
+
+      vaddr = (uintptr_t)map_single_user_page(vaddr);
+      return (FAR void *)(vaddr + offset);
+    }
+
+  /* No, the area must be mapped into kernel virtual address space */
+
+  pages = kmm_zalloc(npages * sizeof(void *));

Review Comment:
   `sizeof(FAR void *)`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pussuw commented on pull request #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on PR #9368:
URL: https://github.com/apache/nuttx/pull/9368#issuecomment-1561700119

   > @pussuw is there some test that could be add to ostest to verify if dynamic map pages is working? Also please consider added some basic Documentation/ it will help people discover that NuttX supports it.
   
   @acassis I might be willing to do the documentation but I have absolutely no idea how to do that. Some pointers would be very much appreciated.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pussuw commented on a diff in pull request #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on code in PR #9368:
URL: https://github.com/apache/nuttx/pull/9368#discussion_r1204470213


##########
include/nuttx/arch.h:
##########
@@ -1253,6 +1253,108 @@ int up_addrenv_kstackalloc(FAR struct tcb_s *tcb);
 int up_addrenv_kstackfree(FAR struct tcb_s *tcb);
 #endif
 
+/****************************************************************************
+ * Name: up_addrenv_find_page
+ *
+ * Description:
+ *   Find physical page mapped to user virtual address from the address
+ *   environment page directory.
+ *
+ * Input Parameters:
+ *   addrenv - The user address environment.
+ *   vaddr   - The user virtual address
+ *
+ * Returned Value:
+ *   Page physical address on success; NULL on failure.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_ARCH_ADDRENV
+uintptr_t up_addrenv_find_page(FAR arch_addrenv_t *addrenv, uintptr_t vaddr);
+#endif
+
+/****************************************************************************
+ * Name: up_addrenv_page_vaddr
+ *
+ * Description:
+ *   Find the kernel virtual address associated with physical page.
+ *
+ * Input Parameters:
+ *   page - The page physical address.
+ *
+ * Returned Value:
+ *   Page kernel virtual address on success; NULL on failure.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_ARCH_ADDRENV
+uintptr_t up_addrenv_page_vaddr(uintptr_t page);
+#endif
+
+/****************************************************************************
+ * Name: up_addrenv_user_vaddr
+ *
+ * Description:
+ *   Check if a virtual address is in user virtual address space.
+ *
+ * Input Parameters:
+ *   vaddr - The virtual address.
+ *
+ * Returned Value:
+ *   True if it is; false if it's not
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_ARCH_ADDRENV
+bool up_addrenv_user_vaddr(uintptr_t vaddr);
+#endif
+
+/****************************************************************************
+ * Name: riscv_map_pages

Review Comment:
   Note to self: copy&paste error, both here and in implementation



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pussuw commented on pull request #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on PR #9368:
URL: https://github.com/apache/nuttx/pull/9368#issuecomment-1561768499

   >For both architectures, the page allocator support is in:
   
   >```
   arm/src/armv7-a/arm_pgalloc.c
   nuttx/mm/mm_gran/mm_pgalloc.c
   ```
   
   I implemented the riscv address environment support long ago, it implements exactly the same public API:
   https://github.com/apache/nuttx/blob/master/arch/risc-v/src/common/riscv_pgalloc.c


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] patacongo commented on a diff in pull request #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

Posted by "patacongo (via GitHub)" <gi...@apache.org>.
patacongo commented on code in PR #9368:
URL: https://github.com/apache/nuttx/pull/9368#discussion_r1204558850


##########
arch/risc-v/src/common/addrenv.h:
##########
@@ -44,6 +44,14 @@
 #  define ARCH_KERNEL_STACKSIZE STACK_ALIGN_UP(CONFIG_ARCH_KERNEL_STACKSIZE)
 #endif
 
+/* Base address for address environment */
+
+#if CONFIG_ARCH_TEXT_VBASE != 0
+#  define ARCH_ADDRENV_VBASE    (CONFIG_ARCH_TEXT_VBASE)
+#else
+#  define ARCH_ADDRENV_VBASE    (CONFIG_ARCH_DATA_VBASE)
+#endif
+

Review Comment:
   Is the size of the address environment needed too?  For range checking later?



##########
arch/risc-v/src/common/riscv_mmu.h:
##########
@@ -308,24 +308,40 @@ static inline uintptr_t mmu_pte_to_paddr(uintptr_t pte)
 }
 
 /****************************************************************************
- * Name: mmu_get_satp_pgbase
+ * Name: mmu_satp_to_paddr
  *
  * Description:
- *   Utility function to read the base page table physical address
+ *   Extract physical address from SATP
  *
  * Returned Value:
- *   Physical address of the current base page table
+ *   Physical address from SATP value
  *
  ****************************************************************************/
 
-static inline uintptr_t mmu_get_satp_pgbase(void)
+static inline uintptr_t mmu_satp_to_paddr(uintptr_t satp)
 {
   uintptr_t ppn;
-  ppn = mmu_read_satp();
+  ppn = satp;

Review Comment:
   It should not be necessary to copy satp, it should be preserved by the inline function. 



##########
arch/risc-v/src/common/pgalloc.h:
##########
@@ -78,6 +80,24 @@ static inline uintptr_t riscv_pgvaddr(uintptr_t paddr)
 }
 #endif /* CONFIG_ARCH_PGPOOL_MAPPING */
 
+/****************************************************************************
+ * Name: riscv_uservaddr
+ *
+ * Description:
+ *   Return true if the virtual address, vaddr, lies in the user address
+ *   space.
+ *
+ ****************************************************************************/
+
+static inline bool riscv_uservaddr(uintptr_t vaddr)
+{
+  /* Check if this address is within the range of the virtualized .bss/.data,
+   * heap, or stack regions.
+   */
+
+  return vaddr >= ARCH_ADDRENV_VBASE;
+}

Review Comment:
   This is a one-sided test.  Should there not be an upper boundary as well?  It would be an error if vaddr were greater than that limit, wouldn't it?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pussuw commented on pull request #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on PR #9368:
URL: https://github.com/apache/nuttx/pull/9368#issuecomment-1561758299

   I don't agree this PR is in conflict with the existing APIs, this is something entirely new.
   
   > That is the interface currently used by Used by sbrk, initialization logic, shm interfaces.
   
   sbrk is used to find the limit of the userspace data section (heap). If sbrk is used to increase the user space data section (heap) it _allocates_ memory (pages) for the extended region and maps them into the _user_ page directory. mm_pgalloc is used to allocate pages from the page pool, and sbrk maps them for the user.
   
   What this PR adds is an interface to map (or find the kernel virtual address of) existing pages dynamically into kernel space memory. It does _not_ allocate pages, only maps them (or finds the kernel addressable mapping).
   
   > I don't think we should permit multiple different designs for essentially the same thing in the OS. That would be the wrong way to go
   
   Agree 100.0%, unification is the way to go. I would not be proposing this change if it was just a copy of something already existing.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pkarashchenko commented on a diff in pull request #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #9368:
URL: https://github.com/apache/nuttx/pull/9368#discussion_r1217585206


##########
mm/kmap/kmm_map.c:
##########
@@ -0,0 +1,466 @@
+/****************************************************************************
+ * mm/kmap/kmm_map.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdint.h>
+#include <stddef.h>
+
+#include <nuttx/addrenv.h>
+#include <nuttx/arch.h>
+#include <nuttx/compiler.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/gran.h>
+#include <nuttx/mm/map.h>
+#include <nuttx/pgalloc.h>
+#include <nuttx/sched.h>
+
+#include <sys/mman.h>
+
+#if defined(CONFIG_BUILD_KERNEL)
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static GRAN_HANDLE     g_kmm_map_vpages;
+static struct mm_map_s g_kmm_map;
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: get_user_pages
+ *
+ * Description:
+ *   Get the physical pages mapped to a user virtual memory region.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer where the page addresses are recorded.
+ *   npages - Amount of pages.
+ *   vaddr  - Start address of the user virtual memory region.
+ *
+ * Returned Value:
+ *   Zero (OK) is returned on success; a negated errno value is returned
+ *   on failure.
+ *
+ ****************************************************************************/
+
+static int get_user_pages(FAR void **pages, size_t npages, uintptr_t vaddr)
+{
+  FAR struct tcb_s *tcb = nxsched_self();
+  uintptr_t         page;
+  int               i;
+
+  /* Find the pages associated with the user virtual address space */
+
+  for (i = 0; i < npages; i++, vaddr += MM_PGSIZE)
+    {
+      page = up_addrenv_find_page(&tcb->addrenv_own->addrenv, vaddr);
+      if (!page)
+        {
+          /* Something went wrong, get out */
+
+          return -EINVAL;
+        }
+
+      pages[i] = (FAR void *)page;
+    }
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: map_pages
+ *
+ * Description:
+ *   Map pages into kernel virtual memory.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer that contains the physical page addresses.
+ *   npages - Amount of pages.
+ *   prot   - Access right flags.
+ *
+ * Returned Value:
+ *   Pointer to the mapped virtual memory on success; NULL on failure
+ *
+ ****************************************************************************/
+
+static void *map_pages(FAR void **pages, size_t npages, int prot)
+{
+  struct mm_map_entry_s entry;
+  FAR void             *vaddr;
+  size_t                size;
+  int                   ret;
+
+  /* The region size is full pages */
+
+  size = npages << MM_PGSHIFT;
+
+  /* Find a virtual memory area that fits */
+
+  vaddr = gran_alloc(&g_kmm_map_vpages, size);
+  if (!vaddr)
+    {
+      return NULL;
+    }
+
+  /* Map the pages into the kernel page directory */
+
+  ret = up_addrenv_kmap_pages(pages, npages, (uintptr_t)vaddr, prot);
+  if (ret < 0)
+    {
+      goto errout_with_vaddr;
+    }
+
+  entry.vaddr = vaddr;
+  entry.length = size;
+  entry.offset = 0;
+  entry.munmap = NULL;
+
+  ret = mm_map_add(&g_kmm_map, &entry);
+  if (ret < 0)
+    {
+      goto errout_with_pgmap;
+    }
+
+  return vaddr;
+
+errout_with_pgmap:
+  up_addrenv_kunmap_pages((uintptr_t)vaddr, npages);
+errout_with_vaddr:
+  gran_free(&g_kmm_map_vpages, vaddr, size);
+  return NULL;
+}
+
+/****************************************************************************
+ * Name: map_single_user_page
+ *
+ * Description:
+ *   Map a single user page into kernel memory.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer that contains the physical page addresses.
+ *   npages - Amount of pages.
+ *   prot   - Access right flags.
+ *
+ * Returned Value:
+ *   Pointer to the mapped virtual memory on success; NULL on failure
+ *
+ ****************************************************************************/
+
+static void *map_single_user_page(uintptr_t vaddr)
+{
+  FAR struct tcb_s *tcb = nxsched_self();
+  uintptr_t         page;
+
+  /* Find the page associated with this virtual address */
+
+  page = up_addrenv_find_page(&tcb->addrenv_own->addrenv, vaddr);
+  if (!page)
+    {
+      return NULL;
+    }
+
+  vaddr = up_addrenv_page_vaddr(page);
+  return (FAR void *)vaddr;
+}
+
+/****************************************************************************
+ * Name: kmm_map_lock
+ *
+ * Description:
+ *   Get exclusive access to the kernel mm_map
+ *
+ ****************************************************************************/
+
+static int kmm_map_lock(void)
+{
+  return nxrmutex_lock(&g_kmm_map.mm_map_mutex);
+}
+
+/****************************************************************************
+ * Name: kmm_map_unlock
+ *
+ * Description:
+ *   Relinquish exclusive access to the kernel mm_map
+ *
+ ****************************************************************************/
+
+static void kmm_map_unlock(void)
+{
+  DEBUGVERIFY(nxrmutex_unlock(&g_kmm_map.mm_map_mutex));
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: kmm_map_initialize
+ *
+ * Description:
+ *   Initialize the kernel dynamic mapping module.
+ *
+ * Input Parameters:
+ *   None.
+ *
+ * Returned Value:
+ *   None.
+ *
+ ****************************************************************************/
+
+void kmm_map_initialize(void)
+{
+  g_kmm_map_vpages = gran_initialize((FAR void *)CONFIG_ARCH_KMAP_VBASE,
+                                     CONFIG_ARCH_KMAP_NPAGES << MM_PGSHIFT,
+                                     MM_PGSHIFT, MM_PGSHIFT);
+  DEBUGVERIFY(g_kmm_map_vpages);
+  mm_map_initialize(&g_kmm_map, true);
+}
+
+/****************************************************************************
+ * Name: kmm_map_pages
+ *
+ * Description:
+ *   Map pages into kernel virtual memory.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer that contains the physical page addresses.
+ *   npages - Amount of pages.
+ *   prot   - Access right flags.
+ *
+ * Returned Value:
+ *   Pointer to the mapped virtual memory on success; NULL on failure
+ *
+ ****************************************************************************/
+
+FAR void *kmm_map(FAR void **pages, size_t npages, int prot)
+{
+  uintptr_t vaddr;
+
+  if (!pages || !npages || npages > CONFIG_ARCH_KMAP_NPAGES)
+    {
+      return NULL;
+    }
+
+  /* Attempt to map the pages */
+
+  vaddr = (uintptr_t)map_pages(pages, npages, prot);
+  if (!vaddr)
+    {
+      return NULL;
+    }
+
+  return (FAR void *)(vaddr);
+}
+
+/****************************************************************************
+ * Name: kmm_unmap
+ *
+ * Description:
+ *   Unmap a previously allocated kernel virtual memory area.
+ *
+ * Input Parameters:
+ *   kaddr - The kernel virtual address where the mapping begins.
+ *
+ * Returned Value:
+ *   None.
+ *
+ ****************************************************************************/
+
+void kmm_unmap(FAR void *kaddr)
+{
+  FAR struct mm_map_entry_s *entry;
+  unsigned int               npages;
+  int                        ret;
+
+  /* Lock the mapping list when we fiddle around with it */
+
+  ret = kmm_map_lock();
+  if (ret == OK)
+    {
+      /* Find the entry, it is OK if none found */
+
+      entry = mm_map_find(get_current_mm(), kaddr, 1);
+      if (entry)
+        {
+          npages = MM_NPAGES(entry->length);
+
+          /* Remove the mappings from the page directory */
+
+          up_addrenv_kunmap_pages((uintptr_t)entry->vaddr, npages);
+
+          /* Release the virtual memory area for use */
+
+          gran_free(&g_kmm_map_vpages, entry->vaddr, entry->length);
+
+          /* Remove the mapping from the kernel mapping list */
+
+          mm_map_remove(&g_kmm_map, entry);
+        }
+
+      kmm_map_unlock();
+    }
+}
+
+/****************************************************************************
+ * Name: kmm_user_map
+ *
+ * Description:
+ *   Map a region of user memory (physical pages) for kernel use through
+ *   a continuous virtual memory area.
+ *
+ * Input Parameters:
+ *   uaddr - The user virtual address where mapping begins.
+ *   size  - Size of the region.
+ *
+ * Returned Value:
+ *   Pointer to the virtual memory area, or NULL if out of memory.
+ *
+ ****************************************************************************/
+
+FAR void *kmm_user_map(FAR void *uaddr, size_t size)
+{
+  FAR void *pages;
+  uintptr_t vaddr;
+  uintptr_t offset;
+  size_t    npages;
+  int       ret;
+
+  /* Find the kernel addressable virtual address, or map the user memory */
+
+  vaddr = (uintptr_t)uaddr;
+
+  /* If the memory is not user memory, get out */
+
+  if (!up_addrenv_user_vaddr(vaddr))
+    {
+      /* Not user memory, get out */
+
+      return uaddr;
+    }
+
+  /* How many pages (including partial) does the space encompass ? */
+
+  offset = vaddr & MM_PGMASK;
+  vaddr  = MM_PGALIGNDOWN(vaddr);
+  npages = MM_NPAGES(offset + size);
+
+  /* Does the area fit in 1 page, including page boundary crossings ? */
+
+  if (npages == 1)
+    {
+      /* Yes, can simply return the kernel addressable virtual address */
+
+      vaddr = (uintptr_t)map_single_user_page(vaddr);
+      return (FAR void *)(vaddr + offset);
+    }
+
+  /* No, the area must be mapped into kernel virtual address space */
+
+  pages = kmm_zalloc(npages * sizeof(void *));

Review Comment:
   Just for style consistency.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] patacongo commented on pull request #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

Posted by "patacongo (via GitHub)" <gi...@apache.org>.
patacongo commented on PR #9368:
URL: https://github.com/apache/nuttx/pull/9368#issuecomment-1561548931

   > * When task_create is used to make workers, they can be replaced with pthreads
   
   That most likely works because NuttX uses heavyweight threads, ie., coupled tasks that masquerade as pthreads.  So IPCs like semaphores, legacy signal functions, etc. work okay because PIDs, TIDs, and pthread_t's are are aliased; semaphore shared attributes are ignored; etc.  Certainly the result would not be POSIX compliant and so not a proper solution for a POSIX compliance test.  In order to be a POSIX compliant test, the test it would have to use pthread_sigprocmask() vs. sigprocmask(), for examples.  Signals are dispatched completely differently for pthread_kill() and kill().
   
   It might make more sense to create a separate test for unique kernel build features.  Or maybe two versions of OS test that share as much they can.
   
   Or maybe force the existing OS test execute with a file system and all tasks as files in the file system.  Then perhaps they could be made compatible.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pussuw commented on pull request #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on PR #9368:
URL: https://github.com/apache/nuttx/pull/9368#issuecomment-1561572579

   > Certainly the result would not be POSIX compliant and so not a proper solution for a POSIX compliance test. In order to be a POSIX compliant test, the test it would have to use pthread_sigprocmask() vs. sigprocmask(), for examples. Signals are dispatched completely differently for pthread_kill() and kill().
   
   Yes indeed, using pthreads was a kludge solution (or maybe even a dirty hack) and that is why the PR is pending.
   
   > Or maybe two versions of OS test that share as much they can.
   
   I think this is unavoidable, some split is expected.
   
   > Or maybe force the existing OS test execute with a file system and all tasks as files in the file system. Then perhaps they could be made compatible.
   
   I think kernel mode needs this anyway, due to not having fork() i.e. only spawning from file works. I had some sort of idea to keep the current code pretty much compatible with the flat addressing modes too with wrapping / flagging. Your solution might be better but it does force compiling the tests into loadable files -> need more memory and features enabled to test the os.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pussuw commented on pull request #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on PR #9368:
URL: https://github.com/apache/nuttx/pull/9368#issuecomment-1561691435

   What I mean is with fork() you could do something like overload the implementation of task_spawn (in ostest) like this:
   
   ```
   int task_spawn(FAR const char *name, main_t entry,
          FAR const posix_spawn_file_actions_t *file_actions, /* This can be ignored afaik ? */
          FAR const posix_spawnattr_t *attr, /* This can also be ignored afaik */
          FAR char * const argv[], FAR char * const envp[])
   {
     int pid = fork();
     if (pid == 0)
       {
         /* Child performs the task given to task_spawn() */
         
         entry(argc, argv);
       }
       
     /* Parent returns the child pid to caller */
     
     return pid;
   }
   ```
   Unless I'm missing something this should work (might not do what we want it to do, but works), as the main thread does indeed exist in the same address environment. Maybe there is something else I'm missing.
   
   Using posix_spawn is the best option still, just need to make at least some of the tests into loadable processes and that should do it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] xiaoxiang781216 commented on pull request #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #9368:
URL: https://github.com/apache/nuttx/pull/9368#issuecomment-1561734945

   > > task_spawn() is not available in KERNEL mode either:
   > 
   > My bad, I meant task_create of course.. Has been a long day...
   > 
   > What I mean is with fork() you could do something like overload the implementation of task_create (in ostest) like this:
   > 
   > ```
   > int task_create(FAR const char *name, int priority,
   >                 int stack_size, main_t entry, FAR char * const argv[])
   > {
   >   int pid = fork();
   >   if (pid == 0)
   >     {
   >       /* Child performs the task given to task_spawn() */
   >       ... calculate argc etc ...
   >       entry(argc, argv);
   >     }
   >     
   >   /* Parent returns the child pid to caller */
   >   
   >   return pid;
   > }
   > ```
   > 
   > Unless I'm missing something this should work (might not do what we want it to do, but works), as the main thread does indeed exist in the same address environment. Maybe there is something else I'm missing but as far as I can tell task_create() is in fact quite close to fork() (without the obvious address environment stuff).
   > 
   > Using posix_spawn is the best option still, just need to make at least some of the tests into loadable processes and that should do it.
   > 
   
   We can keep the current ostest structure even switch to posix_spawn:
   
   1. Assign the test case launched by task_create an unique id(string is a good choice)
   2. If ostest is launched with one argument, do the test identified by the unique id
   3. Change task_create to posix_spawn with "ostest" and the unique id
   
   posix_spawn work on all mode(FLAT, PROTECTED and KERNEL), so we can use the same code base. 
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pussuw commented on a diff in pull request #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on code in PR #9368:
URL: https://github.com/apache/nuttx/pull/9368#discussion_r1210329400


##########
arch/risc-v/src/common/addrenv.h:
##########
@@ -44,6 +44,14 @@
 #  define ARCH_KERNEL_STACKSIZE STACK_ALIGN_UP(CONFIG_ARCH_KERNEL_STACKSIZE)
 #endif
 
+/* Base address for address environment */
+
+#if CONFIG_ARCH_TEXT_VBASE != 0
+#  define ARCH_ADDRENV_VBASE    (CONFIG_ARCH_TEXT_VBASE)
+#else
+#  define ARCH_ADDRENV_VBASE    (CONFIG_ARCH_DATA_VBASE)
+#endif
+

Review Comment:
   Done



##########
arch/risc-v/src/common/pgalloc.h:
##########
@@ -78,6 +80,24 @@ static inline uintptr_t riscv_pgvaddr(uintptr_t paddr)
 }
 #endif /* CONFIG_ARCH_PGPOOL_MAPPING */
 
+/****************************************************************************
+ * Name: riscv_uservaddr
+ *
+ * Description:
+ *   Return true if the virtual address, vaddr, lies in the user address
+ *   space.
+ *
+ ****************************************************************************/
+
+static inline bool riscv_uservaddr(uintptr_t vaddr)
+{
+  /* Check if this address is within the range of the virtualized .bss/.data,
+   * heap, or stack regions.
+   */
+
+  return vaddr >= ARCH_ADDRENV_VBASE;
+}

Review Comment:
   Done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pkarashchenko commented on a diff in pull request #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #9368:
URL: https://github.com/apache/nuttx/pull/9368#discussion_r1217586920


##########
mm/kmap/kmm_map.c:
##########
@@ -0,0 +1,466 @@
+/****************************************************************************
+ * mm/kmap/kmm_map.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdint.h>
+#include <stddef.h>
+
+#include <nuttx/addrenv.h>
+#include <nuttx/arch.h>
+#include <nuttx/compiler.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/gran.h>
+#include <nuttx/mm/map.h>
+#include <nuttx/pgalloc.h>
+#include <nuttx/sched.h>
+
+#include <sys/mman.h>
+
+#if defined(CONFIG_BUILD_KERNEL)
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static GRAN_HANDLE     g_kmm_map_vpages;
+static struct mm_map_s g_kmm_map;
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: get_user_pages
+ *
+ * Description:
+ *   Get the physical pages mapped to a user virtual memory region.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer where the page addresses are recorded.
+ *   npages - Amount of pages.
+ *   vaddr  - Start address of the user virtual memory region.
+ *
+ * Returned Value:
+ *   Zero (OK) is returned on success; a negated errno value is returned
+ *   on failure.
+ *
+ ****************************************************************************/
+
+static int get_user_pages(FAR void **pages, size_t npages, uintptr_t vaddr)
+{
+  FAR struct tcb_s *tcb = nxsched_self();
+  uintptr_t         page;
+  int               i;
+
+  /* Find the pages associated with the user virtual address space */
+
+  for (i = 0; i < npages; i++, vaddr += MM_PGSIZE)
+    {
+      page = up_addrenv_find_page(&tcb->addrenv_own->addrenv, vaddr);
+      if (!page)
+        {
+          /* Something went wrong, get out */
+
+          return -EINVAL;
+        }
+
+      pages[i] = (FAR void *)page;
+    }
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: map_pages
+ *
+ * Description:
+ *   Map pages into kernel virtual memory.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer that contains the physical page addresses.
+ *   npages - Amount of pages.
+ *   prot   - Access right flags.
+ *
+ * Returned Value:
+ *   Pointer to the mapped virtual memory on success; NULL on failure
+ *
+ ****************************************************************************/
+
+static void *map_pages(FAR void **pages, size_t npages, int prot)
+{
+  struct mm_map_entry_s entry;
+  FAR void             *vaddr;
+  size_t                size;
+  int                   ret;
+
+  /* The region size is full pages */
+
+  size = npages << MM_PGSHIFT;
+
+  /* Find a virtual memory area that fits */
+
+  vaddr = gran_alloc(&g_kmm_map_vpages, size);
+  if (!vaddr)
+    {
+      return NULL;
+    }
+
+  /* Map the pages into the kernel page directory */
+
+  ret = up_addrenv_kmap_pages(pages, npages, (uintptr_t)vaddr, prot);
+  if (ret < 0)
+    {
+      goto errout_with_vaddr;
+    }
+
+  entry.vaddr = vaddr;
+  entry.length = size;
+  entry.offset = 0;
+  entry.munmap = NULL;
+
+  ret = mm_map_add(&g_kmm_map, &entry);
+  if (ret < 0)
+    {
+      goto errout_with_pgmap;
+    }
+
+  return vaddr;
+
+errout_with_pgmap:
+  up_addrenv_kunmap_pages((uintptr_t)vaddr, npages);
+errout_with_vaddr:
+  gran_free(&g_kmm_map_vpages, vaddr, size);
+  return NULL;
+}
+
+/****************************************************************************
+ * Name: map_single_user_page
+ *
+ * Description:
+ *   Map a single user page into kernel memory.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer that contains the physical page addresses.
+ *   npages - Amount of pages.
+ *   prot   - Access right flags.
+ *
+ * Returned Value:
+ *   Pointer to the mapped virtual memory on success; NULL on failure
+ *
+ ****************************************************************************/
+
+static void *map_single_user_page(uintptr_t vaddr)
+{
+  FAR struct tcb_s *tcb = nxsched_self();
+  uintptr_t         page;
+
+  /* Find the page associated with this virtual address */
+
+  page = up_addrenv_find_page(&tcb->addrenv_own->addrenv, vaddr);
+  if (!page)
+    {
+      return NULL;
+    }
+
+  vaddr = up_addrenv_page_vaddr(page);
+  return (FAR void *)vaddr;
+}
+
+/****************************************************************************
+ * Name: kmm_map_lock
+ *
+ * Description:
+ *   Get exclusive access to the kernel mm_map
+ *
+ ****************************************************************************/
+
+static int kmm_map_lock(void)
+{
+  return nxrmutex_lock(&g_kmm_map.mm_map_mutex);
+}
+
+/****************************************************************************
+ * Name: kmm_map_unlock
+ *
+ * Description:
+ *   Relinquish exclusive access to the kernel mm_map
+ *
+ ****************************************************************************/
+
+static void kmm_map_unlock(void)
+{
+  DEBUGVERIFY(nxrmutex_unlock(&g_kmm_map.mm_map_mutex));
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: kmm_map_initialize
+ *
+ * Description:
+ *   Initialize the kernel dynamic mapping module.
+ *
+ * Input Parameters:
+ *   None.
+ *
+ * Returned Value:
+ *   None.
+ *
+ ****************************************************************************/
+
+void kmm_map_initialize(void)
+{
+  g_kmm_map_vpages = gran_initialize((FAR void *)CONFIG_ARCH_KMAP_VBASE,
+                                     CONFIG_ARCH_KMAP_NPAGES << MM_PGSHIFT,
+                                     MM_PGSHIFT, MM_PGSHIFT);
+  DEBUGVERIFY(g_kmm_map_vpages);
+  mm_map_initialize(&g_kmm_map, true);
+}
+
+/****************************************************************************
+ * Name: kmm_map_pages
+ *
+ * Description:
+ *   Map pages into kernel virtual memory.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer that contains the physical page addresses.
+ *   npages - Amount of pages.
+ *   prot   - Access right flags.
+ *
+ * Returned Value:
+ *   Pointer to the mapped virtual memory on success; NULL on failure
+ *
+ ****************************************************************************/
+
+FAR void *kmm_map(FAR void **pages, size_t npages, int prot)
+{
+  uintptr_t vaddr;
+
+  if (!pages || !npages || npages > CONFIG_ARCH_KMAP_NPAGES)
+    {
+      return NULL;
+    }
+
+  /* Attempt to map the pages */
+
+  vaddr = (uintptr_t)map_pages(pages, npages, prot);
+  if (!vaddr)
+    {
+      return NULL;
+    }
+
+  return (FAR void *)(vaddr);
+}
+
+/****************************************************************************
+ * Name: kmm_unmap
+ *
+ * Description:
+ *   Unmap a previously allocated kernel virtual memory area.
+ *
+ * Input Parameters:
+ *   kaddr - The kernel virtual address where the mapping begins.
+ *
+ * Returned Value:
+ *   None.
+ *
+ ****************************************************************************/
+
+void kmm_unmap(FAR void *kaddr)
+{
+  FAR struct mm_map_entry_s *entry;
+  unsigned int               npages;
+  int                        ret;
+
+  /* Lock the mapping list when we fiddle around with it */
+
+  ret = kmm_map_lock();
+  if (ret == OK)
+    {
+      /* Find the entry, it is OK if none found */
+
+      entry = mm_map_find(get_current_mm(), kaddr, 1);
+      if (entry)
+        {
+          npages = MM_NPAGES(entry->length);
+
+          /* Remove the mappings from the page directory */
+
+          up_addrenv_kunmap_pages((uintptr_t)entry->vaddr, npages);
+
+          /* Release the virtual memory area for use */
+
+          gran_free(&g_kmm_map_vpages, entry->vaddr, entry->length);
+
+          /* Remove the mapping from the kernel mapping list */
+
+          mm_map_remove(&g_kmm_map, entry);
+        }
+
+      kmm_map_unlock();
+    }
+}
+
+/****************************************************************************
+ * Name: kmm_user_map
+ *
+ * Description:
+ *   Map a region of user memory (physical pages) for kernel use through
+ *   a continuous virtual memory area.
+ *
+ * Input Parameters:
+ *   uaddr - The user virtual address where mapping begins.
+ *   size  - Size of the region.
+ *
+ * Returned Value:
+ *   Pointer to the virtual memory area, or NULL if out of memory.
+ *
+ ****************************************************************************/
+
+FAR void *kmm_user_map(FAR void *uaddr, size_t size)
+{
+  FAR void *pages;
+  uintptr_t vaddr;
+  uintptr_t offset;
+  size_t    npages;
+  int       ret;
+
+  /* Find the kernel addressable virtual address, or map the user memory */
+
+  vaddr = (uintptr_t)uaddr;
+
+  /* If the memory is not user memory, get out */
+
+  if (!up_addrenv_user_vaddr(vaddr))
+    {
+      /* Not user memory, get out */
+
+      return uaddr;
+    }
+
+  /* How many pages (including partial) does the space encompass ? */
+
+  offset = vaddr & MM_PGMASK;
+  vaddr  = MM_PGALIGNDOWN(vaddr);
+  npages = MM_NPAGES(offset + size);
+
+  /* Does the area fit in 1 page, including page boundary crossings ? */
+
+  if (npages == 1)
+    {
+      /* Yes, can simply return the kernel addressable virtual address */
+
+      vaddr = (uintptr_t)map_single_user_page(vaddr);
+      return (FAR void *)(vaddr + offset);
+    }
+
+  /* No, the area must be mapped into kernel virtual address space */
+
+  pages = kmm_zalloc(npages * sizeof(void *));

Review Comment:
   Yeah, just rechecked. 16 bits for near and 32 (24) bits for far



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pkarashchenko commented on a diff in pull request #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #9368:
URL: https://github.com/apache/nuttx/pull/9368#discussion_r1217586920


##########
mm/kmap/kmm_map.c:
##########
@@ -0,0 +1,466 @@
+/****************************************************************************
+ * mm/kmap/kmm_map.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdint.h>
+#include <stddef.h>
+
+#include <nuttx/addrenv.h>
+#include <nuttx/arch.h>
+#include <nuttx/compiler.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/gran.h>
+#include <nuttx/mm/map.h>
+#include <nuttx/pgalloc.h>
+#include <nuttx/sched.h>
+
+#include <sys/mman.h>
+
+#if defined(CONFIG_BUILD_KERNEL)
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static GRAN_HANDLE     g_kmm_map_vpages;
+static struct mm_map_s g_kmm_map;
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: get_user_pages
+ *
+ * Description:
+ *   Get the physical pages mapped to a user virtual memory region.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer where the page addresses are recorded.
+ *   npages - Amount of pages.
+ *   vaddr  - Start address of the user virtual memory region.
+ *
+ * Returned Value:
+ *   Zero (OK) is returned on success; a negated errno value is returned
+ *   on failure.
+ *
+ ****************************************************************************/
+
+static int get_user_pages(FAR void **pages, size_t npages, uintptr_t vaddr)
+{
+  FAR struct tcb_s *tcb = nxsched_self();
+  uintptr_t         page;
+  int               i;
+
+  /* Find the pages associated with the user virtual address space */
+
+  for (i = 0; i < npages; i++, vaddr += MM_PGSIZE)
+    {
+      page = up_addrenv_find_page(&tcb->addrenv_own->addrenv, vaddr);
+      if (!page)
+        {
+          /* Something went wrong, get out */
+
+          return -EINVAL;
+        }
+
+      pages[i] = (FAR void *)page;
+    }
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: map_pages
+ *
+ * Description:
+ *   Map pages into kernel virtual memory.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer that contains the physical page addresses.
+ *   npages - Amount of pages.
+ *   prot   - Access right flags.
+ *
+ * Returned Value:
+ *   Pointer to the mapped virtual memory on success; NULL on failure
+ *
+ ****************************************************************************/
+
+static void *map_pages(FAR void **pages, size_t npages, int prot)
+{
+  struct mm_map_entry_s entry;
+  FAR void             *vaddr;
+  size_t                size;
+  int                   ret;
+
+  /* The region size is full pages */
+
+  size = npages << MM_PGSHIFT;
+
+  /* Find a virtual memory area that fits */
+
+  vaddr = gran_alloc(&g_kmm_map_vpages, size);
+  if (!vaddr)
+    {
+      return NULL;
+    }
+
+  /* Map the pages into the kernel page directory */
+
+  ret = up_addrenv_kmap_pages(pages, npages, (uintptr_t)vaddr, prot);
+  if (ret < 0)
+    {
+      goto errout_with_vaddr;
+    }
+
+  entry.vaddr = vaddr;
+  entry.length = size;
+  entry.offset = 0;
+  entry.munmap = NULL;
+
+  ret = mm_map_add(&g_kmm_map, &entry);
+  if (ret < 0)
+    {
+      goto errout_with_pgmap;
+    }
+
+  return vaddr;
+
+errout_with_pgmap:
+  up_addrenv_kunmap_pages((uintptr_t)vaddr, npages);
+errout_with_vaddr:
+  gran_free(&g_kmm_map_vpages, vaddr, size);
+  return NULL;
+}
+
+/****************************************************************************
+ * Name: map_single_user_page
+ *
+ * Description:
+ *   Map a single user page into kernel memory.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer that contains the physical page addresses.
+ *   npages - Amount of pages.
+ *   prot   - Access right flags.
+ *
+ * Returned Value:
+ *   Pointer to the mapped virtual memory on success; NULL on failure
+ *
+ ****************************************************************************/
+
+static void *map_single_user_page(uintptr_t vaddr)
+{
+  FAR struct tcb_s *tcb = nxsched_self();
+  uintptr_t         page;
+
+  /* Find the page associated with this virtual address */
+
+  page = up_addrenv_find_page(&tcb->addrenv_own->addrenv, vaddr);
+  if (!page)
+    {
+      return NULL;
+    }
+
+  vaddr = up_addrenv_page_vaddr(page);
+  return (FAR void *)vaddr;
+}
+
+/****************************************************************************
+ * Name: kmm_map_lock
+ *
+ * Description:
+ *   Get exclusive access to the kernel mm_map
+ *
+ ****************************************************************************/
+
+static int kmm_map_lock(void)
+{
+  return nxrmutex_lock(&g_kmm_map.mm_map_mutex);
+}
+
+/****************************************************************************
+ * Name: kmm_map_unlock
+ *
+ * Description:
+ *   Relinquish exclusive access to the kernel mm_map
+ *
+ ****************************************************************************/
+
+static void kmm_map_unlock(void)
+{
+  DEBUGVERIFY(nxrmutex_unlock(&g_kmm_map.mm_map_mutex));
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: kmm_map_initialize
+ *
+ * Description:
+ *   Initialize the kernel dynamic mapping module.
+ *
+ * Input Parameters:
+ *   None.
+ *
+ * Returned Value:
+ *   None.
+ *
+ ****************************************************************************/
+
+void kmm_map_initialize(void)
+{
+  g_kmm_map_vpages = gran_initialize((FAR void *)CONFIG_ARCH_KMAP_VBASE,
+                                     CONFIG_ARCH_KMAP_NPAGES << MM_PGSHIFT,
+                                     MM_PGSHIFT, MM_PGSHIFT);
+  DEBUGVERIFY(g_kmm_map_vpages);
+  mm_map_initialize(&g_kmm_map, true);
+}
+
+/****************************************************************************
+ * Name: kmm_map_pages
+ *
+ * Description:
+ *   Map pages into kernel virtual memory.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer that contains the physical page addresses.
+ *   npages - Amount of pages.
+ *   prot   - Access right flags.
+ *
+ * Returned Value:
+ *   Pointer to the mapped virtual memory on success; NULL on failure
+ *
+ ****************************************************************************/
+
+FAR void *kmm_map(FAR void **pages, size_t npages, int prot)
+{
+  uintptr_t vaddr;
+
+  if (!pages || !npages || npages > CONFIG_ARCH_KMAP_NPAGES)
+    {
+      return NULL;
+    }
+
+  /* Attempt to map the pages */
+
+  vaddr = (uintptr_t)map_pages(pages, npages, prot);
+  if (!vaddr)
+    {
+      return NULL;
+    }
+
+  return (FAR void *)(vaddr);
+}
+
+/****************************************************************************
+ * Name: kmm_unmap
+ *
+ * Description:
+ *   Unmap a previously allocated kernel virtual memory area.
+ *
+ * Input Parameters:
+ *   kaddr - The kernel virtual address where the mapping begins.
+ *
+ * Returned Value:
+ *   None.
+ *
+ ****************************************************************************/
+
+void kmm_unmap(FAR void *kaddr)
+{
+  FAR struct mm_map_entry_s *entry;
+  unsigned int               npages;
+  int                        ret;
+
+  /* Lock the mapping list when we fiddle around with it */
+
+  ret = kmm_map_lock();
+  if (ret == OK)
+    {
+      /* Find the entry, it is OK if none found */
+
+      entry = mm_map_find(get_current_mm(), kaddr, 1);
+      if (entry)
+        {
+          npages = MM_NPAGES(entry->length);
+
+          /* Remove the mappings from the page directory */
+
+          up_addrenv_kunmap_pages((uintptr_t)entry->vaddr, npages);
+
+          /* Release the virtual memory area for use */
+
+          gran_free(&g_kmm_map_vpages, entry->vaddr, entry->length);
+
+          /* Remove the mapping from the kernel mapping list */
+
+          mm_map_remove(&g_kmm_map, entry);
+        }
+
+      kmm_map_unlock();
+    }
+}
+
+/****************************************************************************
+ * Name: kmm_user_map
+ *
+ * Description:
+ *   Map a region of user memory (physical pages) for kernel use through
+ *   a continuous virtual memory area.
+ *
+ * Input Parameters:
+ *   uaddr - The user virtual address where mapping begins.
+ *   size  - Size of the region.
+ *
+ * Returned Value:
+ *   Pointer to the virtual memory area, or NULL if out of memory.
+ *
+ ****************************************************************************/
+
+FAR void *kmm_user_map(FAR void *uaddr, size_t size)
+{
+  FAR void *pages;
+  uintptr_t vaddr;
+  uintptr_t offset;
+  size_t    npages;
+  int       ret;
+
+  /* Find the kernel addressable virtual address, or map the user memory */
+
+  vaddr = (uintptr_t)uaddr;
+
+  /* If the memory is not user memory, get out */
+
+  if (!up_addrenv_user_vaddr(vaddr))
+    {
+      /* Not user memory, get out */
+
+      return uaddr;
+    }
+
+  /* How many pages (including partial) does the space encompass ? */
+
+  offset = vaddr & MM_PGMASK;
+  vaddr  = MM_PGALIGNDOWN(vaddr);
+  npages = MM_NPAGES(offset + size);
+
+  /* Does the area fit in 1 page, including page boundary crossings ? */
+
+  if (npages == 1)
+    {
+      /* Yes, can simply return the kernel addressable virtual address */
+
+      vaddr = (uintptr_t)map_single_user_page(vaddr);
+      return (FAR void *)(vaddr + offset);
+    }
+
+  /* No, the area must be mapped into kernel virtual address space */
+
+  pages = kmm_zalloc(npages * sizeof(void *));

Review Comment:
   Yeah, just rechecked. 16 bits for near and 32 bits for far



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pussuw commented on a diff in pull request #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on code in PR #9368:
URL: https://github.com/apache/nuttx/pull/9368#discussion_r1217582262


##########
mm/kmap/kmm_map.c:
##########
@@ -0,0 +1,466 @@
+/****************************************************************************
+ * mm/kmap/kmm_map.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdint.h>
+#include <stddef.h>
+
+#include <nuttx/addrenv.h>
+#include <nuttx/arch.h>
+#include <nuttx/compiler.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/gran.h>
+#include <nuttx/mm/map.h>
+#include <nuttx/pgalloc.h>
+#include <nuttx/sched.h>
+
+#include <sys/mman.h>
+
+#if defined(CONFIG_BUILD_KERNEL)
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static GRAN_HANDLE     g_kmm_map_vpages;
+static struct mm_map_s g_kmm_map;
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: get_user_pages
+ *
+ * Description:
+ *   Get the physical pages mapped to a user virtual memory region.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer where the page addresses are recorded.
+ *   npages - Amount of pages.
+ *   vaddr  - Start address of the user virtual memory region.
+ *
+ * Returned Value:
+ *   Zero (OK) is returned on success; a negated errno value is returned
+ *   on failure.
+ *
+ ****************************************************************************/
+
+static int get_user_pages(FAR void **pages, size_t npages, uintptr_t vaddr)
+{
+  FAR struct tcb_s *tcb = nxsched_self();
+  uintptr_t         page;
+  int               i;
+
+  /* Find the pages associated with the user virtual address space */
+
+  for (i = 0; i < npages; i++, vaddr += MM_PGSIZE)
+    {
+      page = up_addrenv_find_page(&tcb->addrenv_own->addrenv, vaddr);
+      if (!page)
+        {
+          /* Something went wrong, get out */
+
+          return -EINVAL;
+        }
+
+      pages[i] = (FAR void *)page;
+    }
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: map_pages
+ *
+ * Description:
+ *   Map pages into kernel virtual memory.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer that contains the physical page addresses.
+ *   npages - Amount of pages.
+ *   prot   - Access right flags.
+ *
+ * Returned Value:
+ *   Pointer to the mapped virtual memory on success; NULL on failure
+ *
+ ****************************************************************************/
+
+static void *map_pages(FAR void **pages, size_t npages, int prot)
+{
+  struct mm_map_entry_s entry;
+  FAR void             *vaddr;
+  size_t                size;
+  int                   ret;
+
+  /* The region size is full pages */
+
+  size = npages << MM_PGSHIFT;
+
+  /* Find a virtual memory area that fits */
+
+  vaddr = gran_alloc(&g_kmm_map_vpages, size);
+  if (!vaddr)
+    {
+      return NULL;
+    }
+
+  /* Map the pages into the kernel page directory */
+
+  ret = up_addrenv_kmap_pages(pages, npages, (uintptr_t)vaddr, prot);
+  if (ret < 0)
+    {
+      goto errout_with_vaddr;
+    }
+
+  entry.vaddr = vaddr;
+  entry.length = size;
+  entry.offset = 0;
+  entry.munmap = NULL;
+
+  ret = mm_map_add(&g_kmm_map, &entry);
+  if (ret < 0)
+    {
+      goto errout_with_pgmap;
+    }
+
+  return vaddr;
+
+errout_with_pgmap:
+  up_addrenv_kunmap_pages((uintptr_t)vaddr, npages);
+errout_with_vaddr:
+  gran_free(&g_kmm_map_vpages, vaddr, size);
+  return NULL;
+}
+
+/****************************************************************************
+ * Name: map_single_user_page
+ *
+ * Description:
+ *   Map a single user page into kernel memory.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer that contains the physical page addresses.
+ *   npages - Amount of pages.
+ *   prot   - Access right flags.
+ *
+ * Returned Value:
+ *   Pointer to the mapped virtual memory on success; NULL on failure
+ *
+ ****************************************************************************/
+
+static void *map_single_user_page(uintptr_t vaddr)
+{
+  FAR struct tcb_s *tcb = nxsched_self();
+  uintptr_t         page;
+
+  /* Find the page associated with this virtual address */
+
+  page = up_addrenv_find_page(&tcb->addrenv_own->addrenv, vaddr);
+  if (!page)
+    {
+      return NULL;
+    }
+
+  vaddr = up_addrenv_page_vaddr(page);
+  return (FAR void *)vaddr;
+}
+
+/****************************************************************************
+ * Name: kmm_map_lock
+ *
+ * Description:
+ *   Get exclusive access to the kernel mm_map
+ *
+ ****************************************************************************/
+
+static int kmm_map_lock(void)
+{
+  return nxrmutex_lock(&g_kmm_map.mm_map_mutex);
+}
+
+/****************************************************************************
+ * Name: kmm_map_unlock
+ *
+ * Description:
+ *   Relinquish exclusive access to the kernel mm_map
+ *
+ ****************************************************************************/
+
+static void kmm_map_unlock(void)
+{
+  DEBUGVERIFY(nxrmutex_unlock(&g_kmm_map.mm_map_mutex));
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: kmm_map_initialize
+ *
+ * Description:
+ *   Initialize the kernel dynamic mapping module.
+ *
+ * Input Parameters:
+ *   None.
+ *
+ * Returned Value:
+ *   None.
+ *
+ ****************************************************************************/
+
+void kmm_map_initialize(void)
+{
+  g_kmm_map_vpages = gran_initialize((FAR void *)CONFIG_ARCH_KMAP_VBASE,
+                                     CONFIG_ARCH_KMAP_NPAGES << MM_PGSHIFT,
+                                     MM_PGSHIFT, MM_PGSHIFT);
+  DEBUGVERIFY(g_kmm_map_vpages);
+  mm_map_initialize(&g_kmm_map, true);
+}
+
+/****************************************************************************
+ * Name: kmm_map_pages
+ *
+ * Description:
+ *   Map pages into kernel virtual memory.
+ *
+ * Input Parameters:
+ *   pages  - Pointer to buffer that contains the physical page addresses.
+ *   npages - Amount of pages.
+ *   prot   - Access right flags.
+ *
+ * Returned Value:
+ *   Pointer to the mapped virtual memory on success; NULL on failure
+ *
+ ****************************************************************************/
+
+FAR void *kmm_map(FAR void **pages, size_t npages, int prot)
+{
+  uintptr_t vaddr;
+
+  if (!pages || !npages || npages > CONFIG_ARCH_KMAP_NPAGES)
+    {
+      return NULL;
+    }
+
+  /* Attempt to map the pages */
+
+  vaddr = (uintptr_t)map_pages(pages, npages, prot);
+  if (!vaddr)
+    {
+      return NULL;
+    }
+
+  return (FAR void *)(vaddr);
+}
+
+/****************************************************************************
+ * Name: kmm_unmap
+ *
+ * Description:
+ *   Unmap a previously allocated kernel virtual memory area.
+ *
+ * Input Parameters:
+ *   kaddr - The kernel virtual address where the mapping begins.
+ *
+ * Returned Value:
+ *   None.
+ *
+ ****************************************************************************/
+
+void kmm_unmap(FAR void *kaddr)
+{
+  FAR struct mm_map_entry_s *entry;
+  unsigned int               npages;
+  int                        ret;
+
+  /* Lock the mapping list when we fiddle around with it */
+
+  ret = kmm_map_lock();
+  if (ret == OK)
+    {
+      /* Find the entry, it is OK if none found */
+
+      entry = mm_map_find(get_current_mm(), kaddr, 1);
+      if (entry)
+        {
+          npages = MM_NPAGES(entry->length);
+
+          /* Remove the mappings from the page directory */
+
+          up_addrenv_kunmap_pages((uintptr_t)entry->vaddr, npages);
+
+          /* Release the virtual memory area for use */
+
+          gran_free(&g_kmm_map_vpages, entry->vaddr, entry->length);
+
+          /* Remove the mapping from the kernel mapping list */
+
+          mm_map_remove(&g_kmm_map, entry);
+        }
+
+      kmm_map_unlock();
+    }
+}
+
+/****************************************************************************
+ * Name: kmm_user_map
+ *
+ * Description:
+ *   Map a region of user memory (physical pages) for kernel use through
+ *   a continuous virtual memory area.
+ *
+ * Input Parameters:
+ *   uaddr - The user virtual address where mapping begins.
+ *   size  - Size of the region.
+ *
+ * Returned Value:
+ *   Pointer to the virtual memory area, or NULL if out of memory.
+ *
+ ****************************************************************************/
+
+FAR void *kmm_user_map(FAR void *uaddr, size_t size)
+{
+  FAR void *pages;
+  uintptr_t vaddr;
+  uintptr_t offset;
+  size_t    npages;
+  int       ret;
+
+  /* Find the kernel addressable virtual address, or map the user memory */
+
+  vaddr = (uintptr_t)uaddr;
+
+  /* If the memory is not user memory, get out */
+
+  if (!up_addrenv_user_vaddr(vaddr))
+    {
+      /* Not user memory, get out */
+
+      return uaddr;
+    }
+
+  /* How many pages (including partial) does the space encompass ? */
+
+  offset = vaddr & MM_PGMASK;
+  vaddr  = MM_PGALIGNDOWN(vaddr);
+  npages = MM_NPAGES(offset + size);
+
+  /* Does the area fit in 1 page, including page boundary crossings ? */
+
+  if (npages == 1)
+    {
+      /* Yes, can simply return the kernel addressable virtual address */
+
+      vaddr = (uintptr_t)map_single_user_page(vaddr);
+      return (FAR void *)(vaddr + offset);
+    }
+
+  /* No, the area must be mapped into kernel virtual address space */
+
+  pages = kmm_zalloc(npages * sizeof(void *));

Review Comment:
   Unnecessary, the size of void* does not change if declared far



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pussuw commented on a diff in pull request #9368: mm/kmm_map: Add support to dynamically map pages into kernel virtual …

Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on code in PR #9368:
URL: https://github.com/apache/nuttx/pull/9368#discussion_r1210342337


##########
arch/risc-v/src/common/riscv_mmu.h:
##########
@@ -308,24 +308,40 @@ static inline uintptr_t mmu_pte_to_paddr(uintptr_t pte)
 }
 
 /****************************************************************************
- * Name: mmu_get_satp_pgbase
+ * Name: mmu_satp_to_paddr
  *
  * Description:
- *   Utility function to read the base page table physical address
+ *   Extract physical address from SATP
  *
  * Returned Value:
- *   Physical address of the current base page table
+ *   Physical address from SATP value
  *
  ****************************************************************************/
 
-static inline uintptr_t mmu_get_satp_pgbase(void)
+static inline uintptr_t mmu_satp_to_paddr(uintptr_t satp)
 {
   uintptr_t ppn;
-  ppn = mmu_read_satp();
+  ppn = satp;

Review Comment:
   My suggestion is to keep this as-is for readability. With -Os the compiler sees extra storage for ppn is unnecessary and does not allocate it.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org