Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

extmod/modplatform: Expose CPU features/extensions. #15268

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

agatti
Copy link
Contributor

@agatti agatti commented Jun 12, 2024

This PR adds the ability to expose CPU-specific features/extensions to scripts when the platform module is compiled in.

Right now this is only available on RISC-V RV32, but I looked into extending it for other architectures. Some Cortex CPUs have a pair of registers called ID_PFR but no flags are defined in CMSIS. For x86/x64 accessing those flags would be OS-dependent, but still doable if you limit yourself to Linux and Windows. Finally, for Aarch64 the ID_AA64PFR registers have documented flags, but I have no Armv8 to test an eventual implementation on.

The function name is still somewhat tentative as different architectures may have different names for the same thing after all.

For RISC-V, it is not implemented as a singleton with precomputed values like other functions in the platform module because it is technically possible to disable extensions by clearing the bits in the MISA special register rather than reading them.

Copy link

codecov bot commented Jun 12, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 98.42%. Comparing base (e9c898c) to head (090c922).

Current head 090c922 differs from pull request most recent head 8390cc7

Please upload reports for the commit 8390cc7 to get more accurate results.

Files Patch % Lines
extmod/modplatform.c 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #15268      +/-   ##
==========================================
- Coverage   98.42%   98.42%   -0.01%     
==========================================
  Files         161      161              
  Lines       21204    21205       +1     
==========================================
  Hits        20870    20870              
- Misses        334      335       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

github-actions bot commented Jun 12, 2024

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:  +184 +0.022% standard[incl +64(data)]
      stm32:   +28 +0.007% PYBV10
     mimxrt:   +48 +0.013% TEENSY40
        rp2:   +48 +0.005% RPI_PICO_W
       samd:   +40 +0.015% ADAFRUIT_ITSYBITSY_M4_EXPRESS

@dpgeorge
Copy link
Member

The platform module is a CPython standard module, so we can't add custom extensions to it.

Existing CPython functions such as platform.machine() and platform.processor() may be good places to put this info.

Otherwise, we already do have sys.implementation._machine which could also store this info. Eg on stm32 it stores the full MCU name, such as:

>>> import sys
>>> sys.implementation
(name='micropython', version=(1, 24, 0, 'preview'), _machine='PYBD-SF6W with STM32F767IIK', _mpy=8966)

This adds the ability to expose CPU-specific features/extensions to
scripts when the `platform` module is compiled in, by implementing
`platform.processor()`.   Right now this is only available on RISC-V
RV32, on other CPUs it will return an empty string.

Signed-off-by: Alessandro Gatti <a.gatti@frob.it>
@agatti agatti force-pushed the extmod-platform-extensions branch from 090c922 to 8390cc7 Compare June 18, 2024 20:05
@agatti
Copy link
Contributor Author

agatti commented Jun 18, 2024

Understood. I moved it to platform.processor(), and it now returns an empty string if it's not running on a RV32 processor. This should be in line with CPython's behaviour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants