You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "yf13 (via GitHub)" <gi...@apache.org> on 2024/04/03 04:30:46 UTC

[PR] mm/arch: add user space device memory mapping related defs [nuttx]

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

   
   ## Summary
   This patch introduces definitions for user space device memory regions and mappings management such as `MM_UDEV`, `ARCH_UDEV_MAPPING` and `ARCH_UDEV_VBASE` etc. It also defines MCU interfaces for the mapping.
   
   ## Impact
   None
   
   ## Testing
   CI checks
   


-- 
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


Re: [PR] mm/arch: user-space device mapping support [nuttx]

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


##########
include/nuttx/addrenv.h:
##########
@@ -190,9 +190,8 @@
 #    define CONFIG_ARCH_SHM_NPAGES 1
 #  endif
 
-#  define ARCH_SHM_MAXPAGES   (CONFIG_ARCH_SHM_NPAGES * CONFIG_ARCH_SHM_MAXREGIONS)
-#  define ARCH_SHM_REGIONSIZE (CONFIG_ARCH_SHM_NPAGES * CONFIG_MM_PGSIZE)
-#  define ARCH_SHM_SIZE       (CONFIG_ARCH_SHM_MAXREGIONS * ARCH_SHM_REGIONSIZE)
+#  define ARCH_SHM_MAXPAGES   (CONFIG_ARCH_SHM_NPAGES)

Review Comment:
   remove



-- 
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


Re: [PR] mm/arch: user space device mapping defs [nuttx]

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

   @pussuw, @xiaoxiang781216 and @jlaitine,assuming gran allocation limitation will be removed, I've updated patch description above and the commit itself. Please take a look.
   


-- 
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


Re: [PR] mm/arch: user space device mapping defs [nuttx]

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


##########
include/nuttx/addrenv.h:
##########
@@ -193,7 +193,9 @@
 #  define ARCH_SHM_MAXPAGES   (CONFIG_ARCH_SHM_NPAGES * CONFIG_ARCH_SHM_MAXREGIONS)
 #  define ARCH_SHM_REGIONSIZE (CONFIG_ARCH_SHM_NPAGES * CONFIG_MM_PGSIZE)
 #  define ARCH_SHM_SIZE       (CONFIG_ARCH_SHM_MAXREGIONS * ARCH_SHM_REGIONSIZE)

Review Comment:
   @xiaoxiang781216  and @pussuw
   
   Okay, I will drop UDEV_NPAGES and leave SHM stuffs for now and only revise Kconfig help and prompt messages. Maybe later we can use mail list or an issue to discuess about how to fix those SHM confusions.



-- 
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


Re: [PR] mm/arch: user-space device mapping support [nuttx]

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


##########
include/nuttx/addrenv.h:
##########
@@ -193,7 +193,9 @@
 #  define ARCH_SHM_MAXPAGES   (CONFIG_ARCH_SHM_NPAGES * CONFIG_ARCH_SHM_MAXREGIONS)
 #  define ARCH_SHM_REGIONSIZE (CONFIG_ARCH_SHM_NPAGES * CONFIG_MM_PGSIZE)
 #  define ARCH_SHM_SIZE       (CONFIG_ARCH_SHM_MAXREGIONS * ARCH_SHM_REGIONSIZE)

Review Comment:
   @xiaoxiang781216  and @pussuw
   
   Okay, I will discard UDEV_NPAGES and revise comments for SHM_NPAGES and SHM_MAXREGIONS and also remove the latter from the region size calculations. Hope this can help to reduce confusions.



-- 
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


Re: [PR] mm/arch: user space device mapping defs [nuttx]

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

   > > For kernel mapped memory sure, but for user mappings, I would not do it.
   > 
   > @pussuw Is this due to user space page table design limiations (e.g. some non-leaf page tables might be shared among processes) or for some other reasons?
   > 
   > @pussuw and @xiaoxiang781216, on the other side, it seems that we still need wrap page-oriented `up_shmat()` to byte oriented function like `xxx_map(uintptr_t vaddr, uinptr_t paddr, size_t size)` as it is more convenient to use in drivers like `video/fb.c` or alike. Where should we put this wrapped function? I don't think that we need duplicate the wrapping code everywhere.
   
   tthe unit of mmap is page size, why do we need byte unit?


-- 
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


Re: [PR] mm/arch: user space device mapping defs [nuttx]

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


##########
include/nuttx/addrenv.h:
##########
@@ -193,7 +193,9 @@
 #  define ARCH_SHM_MAXPAGES   (CONFIG_ARCH_SHM_NPAGES * CONFIG_ARCH_SHM_MAXREGIONS)
 #  define ARCH_SHM_REGIONSIZE (CONFIG_ARCH_SHM_NPAGES * CONFIG_MM_PGSIZE)
 #  define ARCH_SHM_SIZE       (CONFIG_ARCH_SHM_MAXREGIONS * ARCH_SHM_REGIONSIZE)

Review Comment:
   > 
   > You define `# define ARCH_SHM_VEND (CONFIG_ARCH_SHM_VBASE + ARCH_UMAP_SIZE - 1)` which proves that the UMAP region is just a region of SHM. If your goal is to remove naming confusion, this does not help in my opinion.
   
   This will be `ARCH_UMAP_VEND` later if we agree that `UMAP` region is composed of `SHM` and `UDEV`.
   
   > Why not just use CONFIG_ARCH_SHM_NPAGES for the UDEV/UMAP area as well ? 
   
   To reuse existing SHM configs, my confusion comes from the ARCH_SHM_MAXREGIONS thing as it affects the total virtual region size. Can we remove it from the ARCH_SHM_VEND calculations and simply let SHM_NPAGES defines the size for the generalized SHM region in virtual memory space?
   



-- 
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


Re: [PR] mm/arch: add user space device memory mapping related defs [nuttx]

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

   @pussuw  and @xiaoxiang781216 as for `up_addrenv_map()`, I am fine to drop it from `nuttx/arch.h`. This however implies that we may need a similar thing likely in `mm/map` to wrap the `up_shmat()` as it can be shared between framebuffer and other drivers. Actually the `up_shmat()` parameter model lacks the `prot` argument and it is may be hard for implementation to use larger PTE entries as its input pages might be incontinuous pages. Maybe having `up_addrenv_map()` there it also worth as its parameter model is more generic and implementations can choose to use non-leaf PTE entries if the size is bigger?
   


-- 
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


Re: [PR] mm/arch: user space device mapping defs [nuttx]

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


##########
include/nuttx/addrenv.h:
##########
@@ -193,7 +193,9 @@
 #  define ARCH_SHM_MAXPAGES   (CONFIG_ARCH_SHM_NPAGES * CONFIG_ARCH_SHM_MAXREGIONS)
 #  define ARCH_SHM_REGIONSIZE (CONFIG_ARCH_SHM_NPAGES * CONFIG_MM_PGSIZE)
 #  define ARCH_SHM_SIZE       (CONFIG_ARCH_SHM_MAXREGIONS * ARCH_SHM_REGIONSIZE)

Review Comment:
   By adding one UDEV_NPAGES option, we can keep most SHM_xxx unchanged, and `device mapping` and `SHM` can be two indenpendent choices, this sounds more flexible. There is no other `UDEV_xxx` stuffs needed.



-- 
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


Re: [PR] mm/arch: user-space device mapping support [nuttx]

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


##########
include/nuttx/mm/map.h:
##########
@@ -193,6 +193,45 @@ void vm_release_region(FAR struct mm_map_s *mm, FAR void *vaddr,
 
 #endif
 
+#ifdef CONFIG_ARCH_VMA_MAPPING
+
+/****************************************************************************
+ * Name: vm_map_region
+ *
+ * Description:
+ *   Allocate virtual memory and maps given physical memory into user space
+ *   of the current process.
+ *
+ * Input Parameters:
+ *   paddr - Starting physical address
+ *   size  - Size of the address range
+ *
+ * Returned Value:
+ *   Virtual address if success, or NULL if error
+ *
+ ****************************************************************************/
+
+FAR char *vm_map_region(uintptr_t paddr, size_t size);

Review Comment:
   ```suggestion
   FAR void *vm_map_region(uintptr_t paddr, size_t size);
   ```



##########
include/nuttx/mm/map.h:
##########
@@ -193,6 +193,45 @@ void vm_release_region(FAR struct mm_map_s *mm, FAR void *vaddr,
 
 #endif
 
+#ifdef CONFIG_ARCH_VMA_MAPPING
+
+/****************************************************************************
+ * Name: vm_map_region
+ *
+ * Description:
+ *   Allocate virtual memory and maps given physical memory into user space
+ *   of the current process.
+ *
+ * Input Parameters:
+ *   paddr - Starting physical address
+ *   size  - Size of the address range
+ *
+ * Returned Value:
+ *   Virtual address if success, or NULL if error
+ *
+ ****************************************************************************/
+
+FAR char *vm_map_region(uintptr_t paddr, size_t size);
+
+/****************************************************************************
+ * Name: vm_unmap_region
+ *
+ * Description:
+ *   Unmap previously mapped userspace device and release the virtual memory.
+ *
+ * Input Parameters:
+ *   vaddr - Starting virtual address of the mapped device
+ *   size - Size of the address range
+ *
+ * Returned Value:
+ *   OK for success or negative value for error
+ *
+ ****************************************************************************/
+
+int vm_unmap_region(FAR char *vaddr, size_t size);

Review Comment:
   ```suggestion
   int vm_unmap_region(FAR void *vaddr, size_t size);
   ```



##########
mm/map/vm_map.c:
##########
@@ -0,0 +1,97 @@
+/****************************************************************************
+ * mm/map/vm_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 <debug.h>
+
+#include <nuttx/arch.h>
+#include <nuttx/mm/map.h>
+#include <nuttx/pgalloc.h>
+#include <nuttx/sched.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+#ifdef CONFIG_ARCH_VMA_MAPPING
+
+/* map physical region to userspace */
+
+FAR char *vm_map_region(uintptr_t paddr, size_t size)
+{
+  FAR char *vaddr;
+  size_t    npages  = MM_NPAGES(size);
+  int       ret     = OK;
+  uint      i       = 0;
+  uintptr_t tvaddr;
+  uintptr_t tpaddr;
+
+  DEBUGASSERT(npages > 0);
+  DEBUGASSERT(MM_ISALIGNED(paddr));
+
+  vaddr = vm_alloc_region(get_current_mm(), 0, size);
+  if (vaddr)
+    {
+      tvaddr    = (uintptr_t)vaddr;

Review Comment:
   ```suggestion
         tvaddr = (uintptr_t)vaddr;
   ```



##########
mm/map/vm_map.c:
##########
@@ -0,0 +1,97 @@
+/****************************************************************************
+ * mm/map/vm_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 <debug.h>
+
+#include <nuttx/arch.h>
+#include <nuttx/mm/map.h>
+#include <nuttx/pgalloc.h>
+#include <nuttx/sched.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+#ifdef CONFIG_ARCH_VMA_MAPPING
+
+/* map physical region to userspace */
+
+FAR char *vm_map_region(uintptr_t paddr, size_t size)
+{
+  FAR char *vaddr;

Review Comment:
   ```suggestion
     FAR void *vaddr;
   ```



##########
mm/map/vm_map.c:
##########
@@ -0,0 +1,97 @@
+/****************************************************************************
+ * mm/map/vm_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 <debug.h>
+
+#include <nuttx/arch.h>
+#include <nuttx/mm/map.h>
+#include <nuttx/pgalloc.h>
+#include <nuttx/sched.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+#ifdef CONFIG_ARCH_VMA_MAPPING
+
+/* map physical region to userspace */
+
+FAR char *vm_map_region(uintptr_t paddr, size_t size)
+{
+  FAR char *vaddr;
+  size_t    npages  = MM_NPAGES(size);
+  int       ret     = OK;
+  uint      i       = 0;
+  uintptr_t tvaddr;
+  uintptr_t tpaddr;
+
+  DEBUGASSERT(npages > 0);
+  DEBUGASSERT(MM_ISALIGNED(paddr));
+
+  vaddr = vm_alloc_region(get_current_mm(), 0, size);
+  if (vaddr)
+    {
+      tvaddr    = (uintptr_t)vaddr;
+      tpaddr    = (uintptr_t)paddr;
+      for (; i < npages; i++, tvaddr += MM_PGSIZE, tpaddr += MM_PGSIZE)
+        {
+          ret = up_shmat(&tpaddr, 1, tvaddr);
+          if (ret)
+            {
+              goto errorout;
+            }
+        }
+    }
+
+  return vaddr;
+
+errorout:
+  if (vaddr)
+    {
+      if (i)   /* undo mapped pages */
+        {
+          up_shmdt((uintptr_t)vaddr, i);
+        }
+
+      vm_release_region(get_current_mm(), (FAR void *)vaddr, size);

Review Comment:
   ```suggestion
         vm_release_region(get_current_mm(), vaddr, size);
   ```



##########
mm/map/vm_map.c:
##########
@@ -0,0 +1,97 @@
+/****************************************************************************
+ * mm/map/vm_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 <debug.h>
+
+#include <nuttx/arch.h>
+#include <nuttx/mm/map.h>
+#include <nuttx/pgalloc.h>
+#include <nuttx/sched.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+#ifdef CONFIG_ARCH_VMA_MAPPING
+
+/* map physical region to userspace */
+
+FAR char *vm_map_region(uintptr_t paddr, size_t size)
+{
+  FAR char *vaddr;
+  size_t    npages  = MM_NPAGES(size);

Review Comment:
   ```suggestion
     size_t    npages = MM_NPAGES(size);
   ```



##########
mm/map/vm_map.c:
##########
@@ -0,0 +1,97 @@
+/****************************************************************************
+ * mm/map/vm_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 <debug.h>
+
+#include <nuttx/arch.h>
+#include <nuttx/mm/map.h>
+#include <nuttx/pgalloc.h>
+#include <nuttx/sched.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+#ifdef CONFIG_ARCH_VMA_MAPPING
+
+/* map physical region to userspace */
+
+FAR char *vm_map_region(uintptr_t paddr, size_t size)
+{
+  FAR char *vaddr;
+  size_t    npages  = MM_NPAGES(size);
+  int       ret     = OK;
+  uint      i       = 0;
+  uintptr_t tvaddr;
+  uintptr_t tpaddr;
+
+  DEBUGASSERT(npages > 0);
+  DEBUGASSERT(MM_ISALIGNED(paddr));
+
+  vaddr = vm_alloc_region(get_current_mm(), 0, size);
+  if (vaddr)
+    {
+      tvaddr    = (uintptr_t)vaddr;
+      tpaddr    = (uintptr_t)paddr;
+      for (; i < npages; i++, tvaddr += MM_PGSIZE, tpaddr += MM_PGSIZE)
+        {
+          ret = up_shmat(&tpaddr, 1, tvaddr);
+          if (ret)
+            {
+              goto errorout;
+            }
+        }
+    }
+
+  return vaddr;
+
+errorout:
+  if (vaddr)
+    {
+      if (i)   /* undo mapped pages */
+        {
+          up_shmdt((uintptr_t)vaddr, i);
+        }
+
+      vm_release_region(get_current_mm(), (FAR void *)vaddr, size);
+    }
+
+  return 0;
+}
+
+/* unmap userspace device pointer */
+
+int vm_unmap_region(FAR char *vaddr, size_t size)
+{
+  size_t npages = MM_NPAGES(size);
+  int ret;
+
+  DEBUGASSERT(MM_ISALIGNED(vaddr));
+  DEBUGASSERT(npages);
+  ret = up_shmdt((uintptr_t)vaddr, npages);
+  vm_release_region(get_current_mm(), (FAR void *)vaddr, size);

Review Comment:
   ```suggestion
     vm_release_region(get_current_mm(), vaddr, size);
   ```



-- 
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


Re: [PR] mm/arch: add user space device memory mapping related defs [nuttx]

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


##########
arch/Kconfig:
##########
@@ -801,6 +801,28 @@ config ARCH_KMAP_NPAGES
 		maximum number of pages per region, and the configured size of
 		each page.
 
+config ARCH_UDEV_MAPPING
+	bool "Support mapping devices memory into user space"

Review Comment:
   That is a detail of shmfs, you can map any memory to user virtual memory with up_shmat()
   
   The intent of the shared memory area was to hold any dynamic mappings for the user, like shared memory objects, ad-hoc mappings, shared libraries and so forth. User devices can go there as well, just remember to reserve the space from the shared memory pool i.e. calling vm_alloc_region().
   
   Why is it called "shared memory area" ? That is the name we came up with, there is no really good / descriptive name for it IMO.
   



-- 
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


Re: [PR] mm/arch: user space device mapping defs [nuttx]

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


##########
include/nuttx/mm/map.h:
##########
@@ -193,6 +193,42 @@ void vm_release_region(FAR struct mm_map_s *mm, FAR void *vaddr,
 
 #endif
 
+#ifdef CONFIG_MM_UMAP
+
+/****************************************************************************
+ * Name: vm_map_region
+ *
+ * Description:
+ *   Allocate virtual memory and maps physical memory range into user space
+ *   of the current process.
+ *
+ * Input Parameters:
+ *   paddr - Starting physical address
+ *   size  - Size of the address range
+ *
+ * Returned Value:
+ *   Virtual address of the range, or NULL if error
+ *
+ ****************************************************************************/
+
+uintptr_t vm_map_region(uintptr_t paddr, size_t size);

Review Comment:
   This is for interfaces definitions only as they are likely to change during reviews, implementations will be added later once the defs are accepted, they can be simply based on `vm_alloc_region()` and `up_shmat()`.



-- 
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


Re: [PR] mm/arch: user space device mapping defs [nuttx]

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

   > 
   > Yes, the caller pass byte unit, but it is round up to page unit internally.
   
   This helper is for callers like `fb_mmap()` to use, where byte unit looks more natural. The helper can do VM allocation, unit conversion and do looping call to up_shmat internally, these processing may take a dozen of LOCs. By wrapping them in the helper, the caller side can be simpler.
   


-- 
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


Re: [PR] mm/arch: add user space device memory mapping related defs [nuttx]

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

   A third option is to use something else than the granule allocator for user vm area bookkeeping. It was selected because it was available and very easy to use, also it's a super fast allocator.


-- 
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


Re: [PR] mm/arch: add user space device memory mapping related defs [nuttx]

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

   @pussuw the limitation is [that no more than 32 granules](https://github.com/apache/nuttx/blob/46b1eb44b6b7a9f217a5ef1fbe347baef5e1e127/mm/mm_gran/mm_granalloc.c#L74) can be allocated 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


Re: [PR] mm/arch: user space device mapping defs [nuttx]

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

   > tthe unit of mmap is page size, why do we need byte unit?
   
   This helper function is for device drivers to use, where using byte unit is  more natural. The "mmap()" function defined in <sys/mman.h> is also using byte unit for length parameter. using byte unit is more convenient for the callers.
   
   


-- 
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


Re: [PR] mm/arch: user space device mapping defs [nuttx]

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


##########
include/nuttx/addrenv.h:
##########
@@ -193,7 +193,9 @@
 #  define ARCH_SHM_MAXPAGES   (CONFIG_ARCH_SHM_NPAGES * CONFIG_ARCH_SHM_MAXREGIONS)
 #  define ARCH_SHM_REGIONSIZE (CONFIG_ARCH_SHM_NPAGES * CONFIG_MM_PGSIZE)
 #  define ARCH_SHM_SIZE       (CONFIG_ARCH_SHM_MAXREGIONS * ARCH_SHM_REGIONSIZE)

Review Comment:
   By adding one UDEV_NPAGES option, we can keep most SHM_xxx unchanged, and `device mapping` and `SHM` can be two indenpendent choices, this sounds more flexible. There is no other `UDEV_xxx` stuffs needed. 
   
   Keeping most SHM_xxx unchanged also reduces concerns from existing SHM feature users.
   
   Yet another reason for not simply renaming SHM_xxx to be more general as the SHM region structure is very specific to SHM,  which can not be simply renamed to general purpose.
   



-- 
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


Re: [PR] mm/arch: add user space device memory mapping related defs [nuttx]

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

   > @pussuw thanks for the explanations.
   > 
   > did you mean that we
   > 
   >     * reuse the `vm_alloc_region()` and `vm_alloc_release()` for virtual address allocation.
   > 
   >     * reuse the `up_shmat()` and `up_shmdt()` for mapping/unmapping.
   > 
   >     * don't use anything in `mm/shm` or `fs/shm`.
   > 
   > 
   Yes, this should work. It is not clear to me what you are trying to do in user space, but I think you will use mmap() to map some device ?
   > The memory mapping virtual address zone is defined by `ARCH_SHM_NPAGES` and `ARCH_SHM_MAXREGIONS` etc, as long as this zone is big enough, we can use it for both shared memory and device mappings. We also don't need worry about affecting the shared memory use cases as the device mappings don't occupy any shmid. Is this correct?
   > 
   Yes, especially you do not need to worry about `SHM_MAXREGIONS`, that is only used by SysV shared memory, maybe you can familiarize yourself with the implementation of that, I'm not too familiar with the details. I obviously encountered it when shared memory support was implemented for risc-v. `ARCH_SHM_NPAGES` defines the amount of _virtual memory_ that is allocated for shared memory objects (really, for ANY objects mapped to user virtual memory).
   > As `vm_alloc_region()` has limitations now and can't do 640x480x4=1228800 frame buffer yet, maybe we can use static virtual address plan for devices? If so, should the static address space be separated from the above space for dynamic mapping?
   What are the limitations ? I don't understand why this is an issue ?
   


-- 
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


Re: [PR] mm/arch: user space device mapping defs [nuttx]

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


##########
include/nuttx/addrenv.h:
##########
@@ -193,7 +193,9 @@
 #  define ARCH_SHM_MAXPAGES   (CONFIG_ARCH_SHM_NPAGES * CONFIG_ARCH_SHM_MAXREGIONS)
 #  define ARCH_SHM_REGIONSIZE (CONFIG_ARCH_SHM_NPAGES * CONFIG_MM_PGSIZE)
 #  define ARCH_SHM_SIZE       (CONFIG_ARCH_SHM_MAXREGIONS * ARCH_SHM_REGIONSIZE)

Review Comment:
   @xiaoxiang781216  and @pussuw
   
   Okay, I will drop UDEV_NPAGES and leave SHM stuffs untouched for now. Maybe later we can use mail list or an issue to discuess about how to fix the confusions of SHM stuffs.



-- 
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


Re: [PR] mm/arch: user space device mapping defs [nuttx]

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


##########
include/nuttx/addrenv.h:
##########
@@ -193,7 +193,9 @@
 #  define ARCH_SHM_MAXPAGES   (CONFIG_ARCH_SHM_NPAGES * CONFIG_ARCH_SHM_MAXREGIONS)
 #  define ARCH_SHM_REGIONSIZE (CONFIG_ARCH_SHM_NPAGES * CONFIG_MM_PGSIZE)
 #  define ARCH_SHM_SIZE       (CONFIG_ARCH_SHM_MAXREGIONS * ARCH_SHM_REGIONSIZE)

Review Comment:
   Changing the name of the SHM region is unnecessary, what would be a better name. Doing so would also create a regression for downstream projects, so this is not preferred by me.
   
   Why not just use CONFIG_ARCH_SHM_NPAGES for the UDEV/UMAP area as well ? There is no distinction between the two from memory segmentation point of view. The SHM area has a confusing name I agree, but it is really just a general "memory mappings region". Anything that is mapped by mmap can go here; shm objects, shared libraries, udev mappings, whatever. Here is a good description of a process's memory map: https://i.stack.imgur.com/epGfE.png.
   Semantically, the SHM region in that figure is the "Memory Mapping Region".
   
   You define `# define ARCH_SHM_VEND (CONFIG_ARCH_SHM_VBASE + ARCH_UMAP_SIZE - 1)` which proves that the UMAP region is just a region of SHM. If your goal is to remove naming confusion, this does not help in my opinion.



##########
include/nuttx/addrenv.h:
##########
@@ -193,7 +193,9 @@
 #  define ARCH_SHM_MAXPAGES   (CONFIG_ARCH_SHM_NPAGES * CONFIG_ARCH_SHM_MAXREGIONS)
 #  define ARCH_SHM_REGIONSIZE (CONFIG_ARCH_SHM_NPAGES * CONFIG_MM_PGSIZE)
 #  define ARCH_SHM_SIZE       (CONFIG_ARCH_SHM_MAXREGIONS * ARCH_SHM_REGIONSIZE)

Review Comment:
   Changing the name of the SHM region is unnecessary, what would be a better name? Doing so would also create a regression for downstream projects, so this is not preferred by me.
   
   Why not just use CONFIG_ARCH_SHM_NPAGES for the UDEV/UMAP area as well ? There is no distinction between the two from memory segmentation point of view. The SHM area has a confusing name I agree, but it is really just a general "memory mappings region". Anything that is mapped by mmap can go here; shm objects, shared libraries, udev mappings, whatever. Here is a good description of a process's memory map: https://i.stack.imgur.com/epGfE.png.
   Semantically, the SHM region in that figure is the "Memory Mapping Region".
   
   You define `# define ARCH_SHM_VEND (CONFIG_ARCH_SHM_VBASE + ARCH_UMAP_SIZE - 1)` which proves that the UMAP region is just a region of SHM. If your goal is to remove naming confusion, this does not help in my opinion.



-- 
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


Re: [PR] mm/arch: user space device mapping defs [nuttx]

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


##########
include/nuttx/addrenv.h:
##########
@@ -193,7 +193,9 @@
 #  define ARCH_SHM_MAXPAGES   (CONFIG_ARCH_SHM_NPAGES * CONFIG_ARCH_SHM_MAXREGIONS)
 #  define ARCH_SHM_REGIONSIZE (CONFIG_ARCH_SHM_NPAGES * CONFIG_MM_PGSIZE)
 #  define ARCH_SHM_SIZE       (CONFIG_ARCH_SHM_MAXREGIONS * ARCH_SHM_REGIONSIZE)

Review Comment:
   > 
   > You define `# define ARCH_SHM_VEND (CONFIG_ARCH_SHM_VBASE + ARCH_UMAP_SIZE - 1)` which proves that the UMAP region is just a region of SHM. If your goal is to remove naming confusion, this does not help in my opinion.
   
   This will be `ARCH_UMAP_VEND` later if we agree that `UMAP` region is composed of `SHM` and `UDEV`.
   
   To reuse existing SHM configs, my confusion comes from the ARCH_SHM_MAXREGIONS thing as it affects the total virtual region size. Can we remove it from the ARCH_SHM_VEND calculations and simply let SHM_NPAGES defines the size?
   
   



-- 
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


Re: [PR] mm/arch: user space device mapping defs [nuttx]

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


##########
include/nuttx/mm/map.h:
##########
@@ -193,6 +193,42 @@ void vm_release_region(FAR struct mm_map_s *mm, FAR void *vaddr,
 
 #endif
 
+#ifdef CONFIG_MM_UMAP
+
+/****************************************************************************
+ * Name: vm_map_region
+ *
+ * Description:
+ *   Allocate virtual memory and maps physical memory range into user space
+ *   of the current process.
+ *
+ * Input Parameters:
+ *   paddr - Starting physical address
+ *   size  - Size of the address range
+ *
+ * Returned Value:
+ *   Virtual address of the range, or NULL if error
+ *
+ ****************************************************************************/
+
+uintptr_t vm_map_region(uintptr_t paddr, size_t size);

Review Comment:
   where is this function implemented?



##########
include/nuttx/addrenv.h:
##########
@@ -193,7 +193,9 @@
 #  define ARCH_SHM_MAXPAGES   (CONFIG_ARCH_SHM_NPAGES * CONFIG_ARCH_SHM_MAXREGIONS)
 #  define ARCH_SHM_REGIONSIZE (CONFIG_ARCH_SHM_NPAGES * CONFIG_MM_PGSIZE)
 #  define ARCH_SHM_SIZE       (CONFIG_ARCH_SHM_MAXREGIONS * ARCH_SHM_REGIONSIZE)

Review Comment:
   why not rename SHM_xxx to general name directly, but add UDEV_xxx?



-- 
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


Re: [PR] mm/arch: user space device mapping defs [nuttx]

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


##########
include/nuttx/addrenv.h:
##########
@@ -193,7 +193,9 @@
 #  define ARCH_SHM_MAXPAGES   (CONFIG_ARCH_SHM_NPAGES * CONFIG_ARCH_SHM_MAXREGIONS)
 #  define ARCH_SHM_REGIONSIZE (CONFIG_ARCH_SHM_NPAGES * CONFIG_MM_PGSIZE)
 #  define ARCH_SHM_SIZE       (CONFIG_ARCH_SHM_MAXREGIONS * ARCH_SHM_REGIONSIZE)

Review Comment:
   @xiaoxiang781216  and @pussuw
   
   Okay, I will drop UDEV_NPAGES and revise comments for SHM_NPAGES and SHM_MAXREGIONS and also remove the latter from the region size calculations. Hope this can help to reduce confusions.



-- 
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


Re: [PR] mm/arch: add user space device memory mapping related defs [nuttx]

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

   > > A third option is to use something else than the granule allocator for user vm area bookkeeping.
   > 
   > Do you see such a thing? I was thinking to remove the limitation.
   
   Whatever you prefer of course, there are many options for implementing such an allocator.


-- 
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


Re: [PR] mm/arch: add user space device memory mapping related defs [nuttx]

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


##########
arch/Kconfig:
##########
@@ -801,6 +801,28 @@ config ARCH_KMAP_NPAGES
 		maximum number of pages per region, and the configured size of
 		each page.
 
+config ARCH_UDEV_MAPPING
+	bool "Support mapping devices memory into user space"

Review Comment:
   That is a detail of shmfs, you can map any memory to user virtual memory with up_shmat()
   
   The intent of the shared memory are was to hold any dynamic mappings for the user, like shared memory objects, ad-hoc mappings, shared libraries and so forth. User devices can go there as well, just remember to reserve the space from the shared memory pool i.e. calling vm_alloc_region().
   
   



-- 
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


Re: [PR] mm/arch: user space device mapping defs [nuttx]

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


##########
include/nuttx/addrenv.h:
##########
@@ -193,7 +193,9 @@
 #  define ARCH_SHM_MAXPAGES   (CONFIG_ARCH_SHM_NPAGES * CONFIG_ARCH_SHM_MAXREGIONS)
 #  define ARCH_SHM_REGIONSIZE (CONFIG_ARCH_SHM_NPAGES * CONFIG_MM_PGSIZE)
 #  define ARCH_SHM_SIZE       (CONFIG_ARCH_SHM_MAXREGIONS * ARCH_SHM_REGIONSIZE)

Review Comment:
   > 
   > You define `# define ARCH_SHM_VEND (CONFIG_ARCH_SHM_VBASE + ARCH_UMAP_SIZE - 1)` which proves that the UMAP region is just a region of SHM. If your goal is to remove naming confusion, this does not help in my opinion.
   
   This will be `ARCH_UMAP_VEND` later if we agree that `UMAP` region is composed of `SHM` and `UDEV`.
   
   > Why not just use CONFIG_ARCH_SHM_NPAGES for the UDEV/UMAP area as well ? 
   
   To reuse existing SHM configs, my confusion comes from the ARCH_SHM_MAXREGIONS thing as it affects the total virtual region size. Can we remove it from the ARCH_SHM_VEND calculations and simply let SHM_NPAGES defines the size?
   



-- 
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


Re: [PR] mm/arch: user space device mapping defs [nuttx]

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


##########
include/nuttx/addrenv.h:
##########
@@ -193,7 +193,9 @@
 #  define ARCH_SHM_MAXPAGES   (CONFIG_ARCH_SHM_NPAGES * CONFIG_ARCH_SHM_MAXREGIONS)
 #  define ARCH_SHM_REGIONSIZE (CONFIG_ARCH_SHM_NPAGES * CONFIG_MM_PGSIZE)
 #  define ARCH_SHM_SIZE       (CONFIG_ARCH_SHM_MAXREGIONS * ARCH_SHM_REGIONSIZE)

Review Comment:
   ARCH_SHM_MAXREGIONS is SysV shared memory stuff. I'm not sure what to do about that. I'm 100% sure no one uses that feature, it is obsolete and a better alternative (shmfs) exists.



-- 
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


Re: [PR] mm/arch: user space device mapping defs [nuttx]

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


##########
include/nuttx/addrenv.h:
##########
@@ -193,7 +193,9 @@
 #  define ARCH_SHM_MAXPAGES   (CONFIG_ARCH_SHM_NPAGES * CONFIG_ARCH_SHM_MAXREGIONS)
 #  define ARCH_SHM_REGIONSIZE (CONFIG_ARCH_SHM_NPAGES * CONFIG_MM_PGSIZE)
 #  define ARCH_SHM_SIZE       (CONFIG_ARCH_SHM_MAXREGIONS * ARCH_SHM_REGIONSIZE)

Review Comment:
   Yes, SysV shared memory is better to implement on top of shmfs and remove all related config, as I suggest before.



-- 
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


Re: [PR] mm/arch: user space device mapping defs [nuttx]

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


##########
include/nuttx/mm/map.h:
##########
@@ -193,6 +193,42 @@ void vm_release_region(FAR struct mm_map_s *mm, FAR void *vaddr,
 
 #endif
 
+#ifdef CONFIG_MM_UMAP
+
+/****************************************************************************
+ * Name: vm_map_region
+ *
+ * Description:
+ *   Allocate virtual memory and maps physical memory range into user space
+ *   of the current process.
+ *
+ * Input Parameters:
+ *   paddr - Starting physical address
+ *   size  - Size of the address range
+ *
+ * Returned Value:
+ *   Virtual address of the range, or NULL if error
+ *
+ ****************************************************************************/
+
+uintptr_t vm_map_region(uintptr_t paddr, size_t size);

Review Comment:
   This is for interfaces definitions only as they are likely to change during reviews, implementations will be added later once the defs are accepted, they can be based on `vm_alloc_region()` and `up_shmat()`.



-- 
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


Re: [PR] mm/arch: user space device mapping defs [nuttx]

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

   @xiaoxiang781216 and @pussuw, I've updated the patch, please take a look.


-- 
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


Re: [PR] mm/arch: user space device mapping defs [nuttx]

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


##########
include/nuttx/addrenv.h:
##########
@@ -193,7 +193,9 @@
 #  define ARCH_SHM_MAXPAGES   (CONFIG_ARCH_SHM_NPAGES * CONFIG_ARCH_SHM_MAXREGIONS)
 #  define ARCH_SHM_REGIONSIZE (CONFIG_ARCH_SHM_NPAGES * CONFIG_MM_PGSIZE)
 #  define ARCH_SHM_SIZE       (CONFIG_ARCH_SHM_MAXREGIONS * ARCH_SHM_REGIONSIZE)

Review Comment:
   > 
   > You define `# define ARCH_SHM_VEND (CONFIG_ARCH_SHM_VBASE + ARCH_UMAP_SIZE - 1)` which proves that the UMAP region is just a region of SHM. If your goal is to remove naming confusion, this does not help in my opinion.
   
   This will be `ARCH_UMAP_VEND` later if we agree that `UMAP` region is composed of `SHM` and `UDEV`.
   
   > Why not just use CONFIG_ARCH_SHM_NPAGES for the UDEV/UMAP area as well ? 
   
   To reuse existing SHM configs, my confusion comes from the ARCH_SHM_MAXREGIONS thing as it affects the total virtual region size. Can we remove it from the ARCH_SHM_VEND calculations and simply use SHM_NPAGES for the size of general SHM region that includes all SHM, UDEV etc?
   



-- 
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


Re: [PR] mm/arch: user space device mapping defs [nuttx]

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


##########
include/nuttx/mm/map.h:
##########
@@ -193,6 +193,42 @@ void vm_release_region(FAR struct mm_map_s *mm, FAR void *vaddr,
 
 #endif
 
+#ifdef CONFIG_MM_UMAP
+
+/****************************************************************************
+ * Name: vm_map_region
+ *
+ * Description:
+ *   Allocate virtual memory and maps physical memory range into user space
+ *   of the current process.
+ *
+ * Input Parameters:
+ *   paddr - Starting physical address
+ *   size  - Size of the address range
+ *
+ * Returned Value:
+ *   Virtual address of the range, or NULL if error
+ *
+ ****************************************************************************/
+
+uintptr_t vm_map_region(uintptr_t paddr, size_t size);

Review Comment:
   This patch is for interfaces definitions only as we are highly to change based on review discussions. The implementations will be based on `vm_alloc_region()` and `up_shmat()`. This is simply a wrapper function for device drivers to use, I am unsure if there is better place in the source tree to put them.



-- 
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


Re: [PR] mm/arch: user space device mapping defs [nuttx]

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


##########
include/nuttx/mm/map.h:
##########
@@ -193,6 +193,42 @@ void vm_release_region(FAR struct mm_map_s *mm, FAR void *vaddr,
 
 #endif
 
+#ifdef CONFIG_MM_UMAP
+
+/****************************************************************************
+ * Name: vm_map_region
+ *
+ * Description:
+ *   Allocate virtual memory and maps physical memory range into user space
+ *   of the current process.
+ *
+ * Input Parameters:
+ *   paddr - Starting physical address
+ *   size  - Size of the address range
+ *
+ * Returned Value:
+ *   Virtual address of the range, or NULL if error
+ *
+ ****************************************************************************/
+
+uintptr_t vm_map_region(uintptr_t paddr, size_t size);

Review Comment:
   but, the implementation doesn't provide in this patch? I would suggest organizing your change that one patch fix one problem completely.



-- 
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


Re: [PR] mm/arch: user space device mapping defs [nuttx]

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

   @pussuw, @xiaoxiang781216 and @jlaitine I've updated patch description above as well as the commit by assuming gran allocation limitation is removed. Please take a look.


-- 
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


Re: [PR] mm/arch: user space device mapping defs [nuttx]

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


##########
include/nuttx/addrenv.h:
##########
@@ -193,7 +193,9 @@
 #  define ARCH_SHM_MAXPAGES   (CONFIG_ARCH_SHM_NPAGES * CONFIG_ARCH_SHM_MAXREGIONS)
 #  define ARCH_SHM_REGIONSIZE (CONFIG_ARCH_SHM_NPAGES * CONFIG_MM_PGSIZE)
 #  define ARCH_SHM_SIZE       (CONFIG_ARCH_SHM_MAXREGIONS * ARCH_SHM_REGIONSIZE)

Review Comment:
   By adding one UDEV_NPAGES option, we can keep most SHM_xxx unchanged, and `device mapping` and `SHM` can be two indenpendent choices, this sounds more flexible. There is no other `UDEV_xxx` stuffs needed. 
   
   Yet another reason for not simply renaming SHM_xxx to be more general as the SHM region structure is very specific to SHM,  which can not be simply renamed to general purpose.
   
   Keeping most SHM_xxx unchanged also reduces concerns from existing SHM feature users. 



##########
include/nuttx/addrenv.h:
##########
@@ -193,7 +193,9 @@
 #  define ARCH_SHM_MAXPAGES   (CONFIG_ARCH_SHM_NPAGES * CONFIG_ARCH_SHM_MAXREGIONS)
 #  define ARCH_SHM_REGIONSIZE (CONFIG_ARCH_SHM_NPAGES * CONFIG_MM_PGSIZE)
 #  define ARCH_SHM_SIZE       (CONFIG_ARCH_SHM_MAXREGIONS * ARCH_SHM_REGIONSIZE)

Review Comment:
   By adding one UDEV_NPAGES option, we can keep most SHM_xxx unchanged, and `device mapping` and `SHM` can be two indenpendent choices, this sounds more flexible. There is no other `UDEV_xxx` stuffs needed. 
   
   Yet another reason for not simply renaming SHM_xxx to be more general as the SHM region structure is very specific to SHM,  which can not be simply renamed to general purpose.
   
   Keeping most SHM_xxx unchanged also reduces concerns from existing SHM users. 



-- 
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


Re: [PR] mm/arch: user-space device mapping support [nuttx]

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


-- 
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


Re: [PR] mm/arch: add user space device memory mapping related defs [nuttx]

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

    > A third option is to use something else than the granule allocator for user vm area bookkeeping.
   Do you see such a thing?


-- 
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


Re: [PR] mm/arch: add user space device memory mapping related defs [nuttx]

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

   When you mmap a device, the driver needs to implement mmap, this is where the call to up_shmat etc should go to ?
   
   As per the "prot" parameter, what is this needed for ? You can always simply add it, or overload the functions, whichever you prefer.
   
   > as its input pages might be scattered pages.
   
   Of course, the function does not expect them to be contiguous ?
   
   >  implementations can also choose to use non-leaf PTE entries if the size is bigger
   
   You mean to use giga/megapages instead of 4K pages for mapping ? I don't think this is a good idea. For kernel mapped memory sure, but for user mappings, I would not 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


Re: [PR] mm/arch: add user space device memory mapping related defs [nuttx]

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


##########
arch/Kconfig:
##########
@@ -801,6 +801,28 @@ config ARCH_KMAP_NPAGES
 		maximum number of pages per region, and the configured size of
 		each page.
 
+config ARCH_UDEV_MAPPING
+	bool "Support mapping devices memory into user space"

Review Comment:
   can we reuse share memory virtual space?



-- 
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


Re: [PR] mm/arch: add user space device memory mapping related defs [nuttx]

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


##########
arch/Kconfig:
##########
@@ -801,6 +801,28 @@ config ARCH_KMAP_NPAGES
 		maximum number of pages per region, and the configured size of
 		each page.
 
+config ARCH_UDEV_MAPPING
+	bool "Support mapping devices memory into user space"

Review Comment:
   It seems that shared memory are different. For example,  they are backed by physical allocations and they use GRAN allocation in user space which is size limited. By having independent regions, the two features can be independently selected.
   



-- 
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


Re: [PR] mm/arch: add user space device memory mapping related defs [nuttx]

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

   @pussuw static allocation can be done via Kconfig like:
   - System wide a virtual zone for device mapping can be defined by `ARCH_UDEV_VBASE` and `ARCH_UDEV_NPAGES`.
   - Then a device's range can be defined by `FB_VBASE` and constrained in the static zone.
   
   This isn't scalable but not so hard. Later once the granule limitation is removed, all static configuration can be optional.
   


-- 
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


Re: [PR] mm/arch: add user space device memory mapping related defs [nuttx]

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

   > For kernel mapped memory sure, but for user mappings, I would not do it.
   
   Is this due to user space page table design limiations (e.g. some non-leaf page tables are shared among processes) or for some other reasons?
   


-- 
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


Re: [PR] mm/arch: add user space device memory mapping related defs [nuttx]

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

   @pussuw thanks for the explanations.
   
   did you mean that we only
   - reuse the `vm_alloc_region()` and `vm_alloc_release()` for virtual address allocation.
   - reuse the `up_shmat()` and `up_shmdt()` for mapping/unmapping.
   and don't use anything in "mm/shm"?
   
   The memory mapping zone is defined by `ARCH_SHM_NPAGES` and `ARCH_SHM_MAXREGIONS` etc, as long as the virtual zone is big enough to hold both shared memory and device mapping requirements, we don't need worry about affecting the shared memory use cases as the device memory mappings don't occupy any shmid. Is this correct?
   
   As `vm_alloc_region()` has limitations and can't do 640x480x4=1228800 frame buffer yet, maybe we can use static virtual address plan for devices now? If so, should the static address space be separated from the dynamic space?
   


-- 
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


Re: [PR] mm/arch: add user space device memory mapping related defs [nuttx]

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

   > @pussuw
   > 
   > Yes I plan to use `mmap()` from user space to gain access to the device memory range, like gaining a pointer to frame buffer and drawing the pixels.
   > 
   > The limitation is [that no more than 32 granules](https://github.com/apache/nuttx/blob/46b1eb44b6b7a9f217a5ef1fbe347baef5e1e127/mm/mm_gran/mm_granalloc.c#L74) can be allocated now. If only dynamic virtual address is allowed, then we must remove the limitation.
   
   Of course, I forgot gran_alloc uses a bitmask. I'm not sure what you mean by static allocation, currently the granule allocator's bitmask is the only information about virtual memory areas that are free / used. How did you plan to add static allocations on top of this ? It sounds quite unscalable to me, but then again I don't know what you mean.


-- 
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


Re: [PR] mm/arch: user space device mapping defs [nuttx]

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


##########
include/nuttx/addrenv.h:
##########
@@ -193,7 +193,9 @@
 #  define ARCH_SHM_MAXPAGES   (CONFIG_ARCH_SHM_NPAGES * CONFIG_ARCH_SHM_MAXREGIONS)
 #  define ARCH_SHM_REGIONSIZE (CONFIG_ARCH_SHM_NPAGES * CONFIG_MM_PGSIZE)
 #  define ARCH_SHM_SIZE       (CONFIG_ARCH_SHM_MAXREGIONS * ARCH_SHM_REGIONSIZE)

Review Comment:
   > 
   > You define `# define ARCH_SHM_VEND (CONFIG_ARCH_SHM_VBASE + ARCH_UMAP_SIZE - 1)` which proves that the UMAP region is just a region of SHM. If your goal is to remove naming confusion, this does not help in my opinion.
   
   This will be `ARCH_UMAP_VEND` later if we agree that `UMAP` region is composed of `SHM` and `UDEV`.
   
   > Why not just use CONFIG_ARCH_SHM_NPAGES for the UDEV/UMAP area as well ? 
   
   To reuse existing SHM configs, my confusion comes from the ARCH_SHM_MAXREGIONS thing as it affects the total virtual region size. Can we remove it from the ARCH_SHM_VEND calculations and simply let SHM_NPAGES defines the size of generalized SHM region in virtual memory space?
   



-- 
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


Re: [PR] mm/arch: user space device mapping defs [nuttx]

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

   @pussuw, @xiaoxiang781216 and @jlaitine I've updated patch description above as well as the commit by assuming gran allocation limitation is removed. Please take a look. many thanks!
   


-- 
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


Re: [PR] mm/arch: user space device mapping defs [nuttx]

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


##########
include/nuttx/mm/map.h:
##########
@@ -193,6 +193,42 @@ void vm_release_region(FAR struct mm_map_s *mm, FAR void *vaddr,
 
 #endif
 
+#ifdef CONFIG_MM_UMAP
+
+/****************************************************************************
+ * Name: vm_map_region
+ *
+ * Description:
+ *   Allocate virtual memory and maps physical memory range into user space
+ *   of the current process.
+ *
+ * Input Parameters:
+ *   paddr - Starting physical address
+ *   size  - Size of the address range
+ *
+ * Returned Value:
+ *   Virtual address of the range, or NULL if error
+ *
+ ****************************************************************************/
+
+uintptr_t vm_map_region(uintptr_t paddr, size_t size);

Review Comment:
   This patch is for interfaces definitions only as they are likely to change during reviews, I don't want rush into implementations. 
   But the implementations can be based on `vm_alloc_region()` and `up_shmat()`.



-- 
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


Re: [PR] mm/arch: user space device mapping defs [nuttx]

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

   > > tthe unit of mmap is page size, why do we need byte unit?
   > 
   > This helper function is for device drivers to use,using byte unit there seems more conveninent. The "mmap()" function in <sys/mman.h> is also using byte unit.
   
   Yes, the caller pass byte unit, but it is round up to page unit internally.


-- 
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


Re: [PR] mm/arch: user space device mapping defs [nuttx]

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


##########
include/nuttx/addrenv.h:
##########
@@ -193,7 +193,9 @@
 #  define ARCH_SHM_MAXPAGES   (CONFIG_ARCH_SHM_NPAGES * CONFIG_ARCH_SHM_MAXREGIONS)
 #  define ARCH_SHM_REGIONSIZE (CONFIG_ARCH_SHM_NPAGES * CONFIG_MM_PGSIZE)
 #  define ARCH_SHM_SIZE       (CONFIG_ARCH_SHM_MAXREGIONS * ARCH_SHM_REGIONSIZE)

Review Comment:
   @pussuw what's your opinion?



-- 
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


Re: [PR] mm/arch: user-space device mapping support [nuttx]

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


##########
mm/map/vm_map.c:
##########
@@ -0,0 +1,97 @@
+/****************************************************************************
+ * mm/map/vm_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 <debug.h>
+
+#include <nuttx/arch.h>
+#include <nuttx/mm/map.h>
+#include <nuttx/pgalloc.h>
+#include <nuttx/sched.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+#ifdef CONFIG_ARCH_VMA_MAPPING
+
+/* map physical region to userspace */
+
+FAR char *vm_map_region(FAR char *paddr, size_t size)

Review Comment:
   ```suggestion
   FAR char *vm_map_region(uintptr_t paddr, size_t size)
   ```



-- 
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


Re: [PR] mm/arch: user-space device mapping support [nuttx]

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


##########
mm/Kconfig:
##########
@@ -194,6 +194,15 @@ config MM_SHM
 		Build in support for the shared memory interfaces shmget(), shmat(),
 		shmctl(), and shmdt().
 
+config MM_UMAP

Review Comment:
   let's reuse MM_SHM config? the new functions doesn't have any side effect if nobody call them.



-- 
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