You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2021/11/26 14:05:03 UTC

[GitHub] [incubator-nuttx] davids5 opened a new pull request #4900: Add Board IOCTL for DMA buffer allocation

davids5 opened a new pull request #4900:
URL: https://github.com/apache/incubator-nuttx/pull/4900


   ## Summary
   
   mkfatfs was failing on some stm32F7, stm32H7 and stm32F4.
   
   The diversity of requirements for address space limitation and alignment for SDMMC drivers requires properly aligned and located memory, that only the board config sets and can support.  
   
   For example an H7 SDMMC1 can not DMA to RAM123,4 but SDMMC2 can. 
   
   early and later malloc allocation may get memory from one of the non-accessible memory regions. 
   
   The fat DMA allocator solves this problem OS side as it is facilitated by the board logic and can be configured to choose the correct memory region and granularity.  
   
   This PR will optionally allow the mkfatfs app to use the fat DMA allocator vir board_ioctl.
   
   See ..
   
   ## Impact
   
   None: If disabled.
   
   If enabled on systems with the  fat DMA allocator Fixes mkfatfs 
   
   ## Testing
   PX4 Test rack [stm32F7, stm32F4, stm32H7](http://px4-jenkins.dagar.ca:8080/blue/organizations/jenkins/PX4-Autopilot/detail/pr-nuttx_mkfatfs_contd/7/pipeline/1095/)
   
   see stanza:
   ![image](https://user-images.githubusercontent.com/1945821/143592067-22e25694-46f9-4288-9a2b-dd9b39240591.png)
   
   


-- 
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] [incubator-nuttx] hartmannathan commented on pull request #4900: Add Board IOCTL for DMA buffer allocation

Posted by GitBox <gi...@apache.org>.
hartmannathan commented on pull request #4900:
URL: https://github.com/apache/incubator-nuttx/pull/4900#issuecomment-981170518


   > From the INVIOLABLES.md:
   > 
   > ```
   >  25 ## Strict POSIX compliance
   >  31   - The portable interface must never be compromised only for the sake of
   >  32     expediency.
   >  33   - Expediency or even improved performance are not justifications for
   >  34     violation of the strict POSIX interface.
   > ```
   
   Yes. I'm not suggesting to do otherwise. I'm suggesting to solve the DMA buffers issue within the architecture logic and in a consistent format that all architectures can follow. The internal workings between lower-half driver code and arch code will not expose non-POSIX interfaces to the userland. In fact userland should never have to know how or where DMA buffers are allocated.


-- 
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] [incubator-nuttx] patacongo commented on pull request #4900: Add Board IOCTL for DMA buffer allocation

Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #4900:
URL: https://github.com/apache/incubator-nuttx/pull/4900#issuecomment-981105744


   This seems like a very strange user-space OS access.  I don't think Linux would permit this.
   
   DMA memory is normally well protected for security reasons.  Would this work in KERNEL and PROTECTED modes where DMA memory is allocated from a supervisor mode memory pool?


-- 
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] [incubator-nuttx] davids5 commented on pull request #4900: Add Board IOCTL for DMA buffer allocation

Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #4900:
URL: https://github.com/apache/incubator-nuttx/pull/4900#issuecomment-981106401


   @patacongo do you have a design in mind to solve the problem?


-- 
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] [incubator-nuttx] patacongo commented on pull request #4900: Add Board IOCTL for DMA buffer allocation

Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #4900:
URL: https://github.com/apache/incubator-nuttx/pull/4900#issuecomment-981130621


   > I was thinking of situations needing highly-optimized, zero-copy transfers.
   
   I don't think formatting an SD card requires high performance.  OS managed data transfers, yes, formatting, no.


-- 
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] [incubator-nuttx] davids5 closed pull request #4900: Add Board IOCTL for DMA buffer allocation

Posted by GitBox <gi...@apache.org>.
davids5 closed pull request #4900:
URL: https://github.com/apache/incubator-nuttx/pull/4900


   


-- 
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] [incubator-nuttx] xiaoxiang781216 commented on pull request #4900: Add Board IOCTL for DMA buffer allocation

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4900:
URL: https://github.com/apache/incubator-nuttx/pull/4900#issuecomment-981236285


   > > At least one block buffer is required. It looks better to improve the bounce buffer to the block level cache which resolve DMA limitation in the universal way and improve the performance at the same time.
   > 
   > @xiaoxiang781216 can you help flesh this out with some more details?
   > 
   > ```
   > Buffer would be in the device driver that understands the HW limitation or where?
   > ```
   > 
   > > which resolve DMA limitation in the universal way
   > 
   > What do you propose as a universal way?
   
   For example, we can add a block level buffer(could reuse rwbuffer_s here) to mmcsd_state_s, and let's arch mmc driver initialize the buffer from DMAable memory region. Any File system or BCH access could cache in this buffer before sending to MMC/SD card.


-- 
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] [incubator-nuttx] davids5 commented on pull request #4900: Add Board IOCTL for DMA buffer allocation

Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #4900:
URL: https://github.com/apache/incubator-nuttx/pull/4900#issuecomment-980134775


   >At least one block buffer is required. It looks better to improve the bounce buffer to the block level cache which resolve DMA limitation in the universal way and improve the performance at the same time.
   
   @xiaoxiang781216 can you help flesh this out with some more details?
   
       Buffer would be in the device driver that understands the HW limitation or where?
   
   >    which resolve DMA limitation in the universal way
   
   What do you propose as a universal way?
        


-- 
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] [incubator-nuttx] hartmannathan commented on pull request #4900: Add Board IOCTL for DMA buffer allocation

Posted by GitBox <gi...@apache.org>.
hartmannathan commented on pull request #4900:
URL: https://github.com/apache/incubator-nuttx/pull/4900#issuecomment-981137020


   > > I was thinking of situations needing highly-optimized, zero-copy transfers.
   > 
   > I don't think formatting an SD card requires high performance. OS managed data transfers, yes, formatting, no.
   
   I meant other situations. The present issue with SD cards is, I believe, a symptom of a larger issue, which is that each architecture has its own idiosyncrasies with DMA. If there isn't an approach to deal with it in a systematic way then we'll keep finding more edge cases that need workarounds. I think that pushing this out to boards will be a mistake since this is an arch issue, not a board issue. This is different than, say, clocking, which is expected to vary by board depending on whether a clock is installed and its type and frequency.


-- 
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] [incubator-nuttx] xiaoxiang781216 commented on pull request #4900: Add Board IOCTL for DMA buffer allocation

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4900:
URL: https://github.com/apache/incubator-nuttx/pull/4900#issuecomment-980033338


   Why not use some bounce buffer in SDMMC driver or switch to PIO mode to migrate the hardware limitation? You can't fix all location which access SD or EMMC card.


-- 
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] [incubator-nuttx] xiaoxiang781216 commented on pull request #4900: Add Board IOCTL for DMA buffer allocation

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4900:
URL: https://github.com/apache/incubator-nuttx/pull/4900#issuecomment-980066203


   > > Why not use some bounce buffer in SDMMC driver or switch to PIO mode to migrate the hardware limitation? You can't fix all location which access SD or EMMC card.
   > 
   > The FAT already has a mechanism for this. Performance and wasting resources.
   > 
   
   But, can you modify all file system(e.g. add SMARTFS_DMAMEMORY, SPIFFS_DMAMEMORY....) supported by NuttX? Even you modify all file system, user space application can still access block device directly. mkfatfs is one but not the last one.
   
   > How would you envision the bounce buffer implementation usage? Would it take more resources?
   
   At least one block buffer is required. It looks better to improve the bounce buffer to the block level cache which resolve DMA limitation in the universal way and improve the performance at the same time.


-- 
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] [incubator-nuttx] davids5 commented on pull request #4900: Add Board IOCTL for DMA buffer allocation

Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #4900:
URL: https://github.com/apache/incubator-nuttx/pull/4900#issuecomment-981894285


   Thank you for all the feedback. 


-- 
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] [incubator-nuttx] hartmannathan commented on pull request #4900: Add Board IOCTL for DMA buffer allocation

Posted by GitBox <gi...@apache.org>.
hartmannathan commented on pull request #4900:
URL: https://github.com/apache/incubator-nuttx/pull/4900#issuecomment-981103849


   This seems more like an architecture issue than a board issue. It will affect all boards using STM32F7, H7, F4. Also problems related to memory regions and DMA will keep coming back again and again unless there is a universal solution. See for example PR #3198: https://github.com/apache/incubator-nuttx/pull/3198 where I ran into issues because SPI6 can only DMA to/from RAM4.
   
   I think memory allocation restrictions only apply to DMA, regardless of architecture. Some architectures can DMA all DMA-aware peripherals to all their memory (e.g., TM4C12x), others can't.
   
   So I think what we need is a special OS-level memory allocator specifically for allocating DMA buffers. It would take some kind of flags indicating which peripheral(s) and direction(s) (memory-to-peripheral, peripheral-to-memory, both), and in turn it would call an arch-specific allocator that knows all the constraints for that arch and therefore can allocate memory in the correct region and alignment.
   
   Then this can be hooked up to a boardctl or ioctl, allowing applications to allocate memory that fits the constraints.
   
   Thoughts?


-- 
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] [incubator-nuttx] davids5 commented on pull request #4900: Add Board IOCTL for DMA buffer allocation

Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #4900:
URL: https://github.com/apache/incubator-nuttx/pull/4900#issuecomment-980039508


   > Why not use some bounce buffer in SDMMC driver or switch to PIO mode to migrate the hardware limitation? You can't fix all location which access SD or EMMC card.
   
    The FAT already has a mechanism for this. Performance and wasting resources.
   
   How would you envision the bounce buffer implementation usage? Would it take more resources?


-- 
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] [incubator-nuttx] patacongo commented on pull request #4900: Add Board IOCTL for DMA buffer allocation

Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #4900:
URL: https://github.com/apache/incubator-nuttx/pull/4900#issuecomment-981139500


   From the INVIOLABLES.md:
   
        25 ## Strict POSIX compliance
        31   - The portable interface must never be compromised only for the sake of
        32     expediency.
        33   - Expediency or even improved performance are not justifications for
        34     violation of the strict POSIX interface.
   


-- 
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] [incubator-nuttx] hartmannathan commented on pull request #4900: Add Board IOCTL for DMA buffer allocation

Posted by GitBox <gi...@apache.org>.
hartmannathan commented on pull request #4900:
URL: https://github.com/apache/incubator-nuttx/pull/4900#issuecomment-981127474


   > This seems like a very strange user-space OS access. I don't think Linux would permit this.
   > 
   > DMA memory is normally well protected for security reasons. Would this work in KERNEL and PROTECTED modes where DMA memory is allocated from a supervisor mode memory pool?
   
   Yes come to think of it, my earlier thought to expose it to applications via an ioctl is not necessary, but I think the other parts of my comment are valid. (I was thinking of situations needing highly-optimized, zero-copy transfers.)


-- 
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] [incubator-nuttx] pkarashchenko commented on a change in pull request #4900: Add Board IOCTL for DMA buffer allocation

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #4900:
URL: https://github.com/apache/incubator-nuttx/pull/4900#discussion_r757902423



##########
File path: include/sys/boardctl.h
##########
@@ -404,6 +420,18 @@ struct boardioc_boot_info_s
 };
 #endif
 
+#ifdef CONFIG_BOARDCTL_FATDMAMEMORY
+/* Arguments passed with the BOARDIOC_FATDMAMEM command */
+
+struct boardioc_dmafatmemory_ioctl_s
+{
+  size_t  size;                   /* Allocation size */
+  uint8_t *pmemory;               /* pointer to allocated mem on NULL
+                                   * non-null to be freed
+                                   */

Review comment:
       ```suggestion
                                      * non-null to be freed */
   ```

##########
File path: boards/boardctl.c
##########
@@ -777,6 +781,41 @@ int boardctl(unsigned int cmd, uintptr_t arg)
         break;
 #endif
 
+#ifdef CONFIG_BOARDCTL_FATDMAMEMORY
+        /* CMD:           BOARDIOC_FATDMAMEM
+         * DESCRIPTION:   Allocate or free a DMA capable buffer.
+         * ARG:           A pointer to an boardioc_dmafatmemory_ioctl_s
+         *                object.
+         *                If the pmemory is NULL an allocation of size is
+         *                returned in pmemory.
+         *                If the pmemory is non-NULL the block is freed.
+         * CONFIGURATION: CONFIG_BOARDCTL_FATDMAMEMORY
+         * DEPENDENCIES:  Board specific logic provides
+         *                   void *fat_dma_alloc(size_t size)
+         *                   void fat_dma_free(FAR void *memory, size_t size)

Review comment:
       ```suggestion
            *                   void *fat_dma_alloc(size_t size);
            *                   void fat_dma_free(FAR void *memory, size_t size);
   ```

##########
File path: boards/boardctl.c
##########
@@ -777,6 +781,41 @@ int boardctl(unsigned int cmd, uintptr_t arg)
         break;
 #endif
 
+#ifdef CONFIG_BOARDCTL_FATDMAMEMORY
+        /* CMD:           BOARDIOC_FATDMAMEM
+         * DESCRIPTION:   Allocate or free a DMA capable buffer.
+         * ARG:           A pointer to an boardioc_dmafatmemory_ioctl_s
+         *                object.
+         *                If the pmemory is NULL an allocation of size is
+         *                returned in pmemory.
+         *                If the pmemory is non-NULL the block is freed.
+         * CONFIGURATION: CONFIG_BOARDCTL_FATDMAMEMORY
+         * DEPENDENCIES:  Board specific logic provides
+         *                   void *fat_dma_alloc(size_t size)
+         *                   void fat_dma_free(FAR void *memory, size_t size)

Review comment:
       or remove `;` at file include/sys/boardctl.h line 197 and 198




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