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

The holy grail of PRs #1

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
162 changes: 162 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
# Byte-compiled / optimized / DLL files
__pycache__/
*.py[cod]
*$py.class

# C extensions
*.so

# Distribution / packaging
.Python
build/
develop-eggs/
dist/
downloads/
eggs/
.eggs/
lib/
lib64/
parts/
sdist/
var/
wheels/
share/python-wheels/
*.egg-info/
.installed.cfg
*.egg
MANIFEST

# PyInstaller
# Usually these files are written by a python script from a template
# before PyInstaller builds the exe, so as to inject date/other infos into it.
*.manifest
*.spec

# Installer logs
pip-log.txt
pip-delete-this-directory.txt

# Unit test / coverage reports
htmlcov/
.tox/
.nox/
.coverage
.coverage.*
.cache
nosetests.xml
coverage.xml
*.cover
*.py,cover
.hypothesis/
.pytest_cache/
cover/

# Translations
*.mo
*.pot

# Django stuff:
*.log
local_settings.py
db.sqlite3
db.sqlite3-journal

# Flask stuff:
instance/
.webassets-cache

# Scrapy stuff:
.scrapy

# Sphinx documentation
docs/_build/

# PyBuilder
.pybuilder/
target/

# Jupyter Notebook
.ipynb_checkpoints

# IPython
profile_default/
ipython_config.py

# pyenv
# For a library or package, you might want to ignore these files since the code is
# intended to run in multiple environments; otherwise, check them in:
# .python-version

# pipenv
# According to pypa/pipenv#598, it is recommended to include Pipfile.lock in version control.
# However, in case of collaboration, if having platform-specific dependencies or dependencies
# having no cross-platform support, pipenv may install dependencies that don't work, or not
# install all needed dependencies.
#Pipfile.lock

# poetry
# Similar to Pipfile.lock, it is generally recommended to include poetry.lock in version control.
# This is especially recommended for binary packages to ensure reproducibility, and is more
# commonly ignored for libraries.
# https://python-poetry.org/docs/basic-usage/#commit-your-poetrylock-file-to-version-control
#poetry.lock

# pdm
# Similar to Pipfile.lock, it is generally recommended to include pdm.lock in version control.
#pdm.lock
# pdm stores project-wide configurations in .pdm.toml, but it is recommended to not include it
# in version control.
# https://pdm.fming.dev/latest/usage/project/#working-with-version-control
.pdm.toml
.pdm-python
.pdm-build/

# PEP 582; used by e.g. github.com/David-OConnor/pyflow and github.com/pdm-project/pdm
__pypackages__/

# Celery stuff
celerybeat-schedule
celerybeat.pid

# SageMath parsed files
*.sage.py

# Environments
.env
.venv
env/
venv/
ENV/
env.bak/
venv.bak/

# Spyder project settings
.spyderproject
.spyproject

# Rope project settings
.ropeproject

# mkdocs documentation
/site

# mypy
.mypy_cache/
.dmypy.json
dmypy.json

# Pyre type checker
.pyre/

# pytype static type analyzer
.pytype/

# Cython debug symbols
cython_debug/

# PyCharm
# JetBrains specific template is maintained in a separate JetBrains.gitignore that can
# be found at https://github.com/github/gitignore/blob/main/Global/JetBrains.gitignore
# and can be added to the global gitignore or merged into this file. For a more nuclear
# option (not recommended) you can uncomment the following to ignore the entire idea folder.
#.idea/
67 changes: 67 additions & 0 deletions DOCS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
# Kernel module data spec

This part will describe details of what data need to be passed to insert_kernel_module role.

Example:
```yaml
kernel_modules:
- name: amd_energy
git_repo: https://github.com/amd/amd_energy.git
git_repo_tag: b1033832b817e69f9df49a6a538d5fd2e1f10f6c
force_build: true
cpu_constraints:
cpu_inclusions:
- family: 0x19
model: 0x55
cpu_exclusions:
- family: 0x20
model: 0x56
patches:
- amd_energy_kernel_version.patch
```


| Attribute | Required | Type | Description | Example |
| --------------- | -------- | ---- | -------------------------------------------------------------------------------------------------- | ------------------------------------- |
| name | true | str | Name of the kernel module to be loaded | amd_energy |
| git_repo | false | str | Repository from whichto build the off-tree module | https://github.com/amd/amd_energy.git |
| git_repo_tag | false | str | Tag, branch or hash to checkout to when cloning git repo. | develop |
| force_build | false | bool | Forces build in case the module is already built/loaded. | True |
| cpu_constraints | false | dict | List of requirements that the CPU must satisfy on order for the module to be installed to the dost | example later |
| patches | false | list | List of names of patches from the files/ directory that will be applied to the fit repo. | ["fix1.patch", "fix2.patch"] |

## Example of cpu_constraints

cpu_constraints has two main keys - cpu_inclusions adn cpu_exclusions.
When cpu_inclusion is not met, the module will not be loaded/built.
When cpu_exclusion is met, the module will not be loaded/built.
The same format of family/model is used for cpu_inclusions and for cpu_exclusions.

When no contraints are defined, the module will be loaded on all hosts!

```yaml
cpu_constraints:
cpu_inclusions:
# Single values
- family: 0x19
model: 0x3F
# Closed ranges
- family:
- range:
start: 0x19
end: 0x55
model:
- range:
start: 0x10
end: 0x1F
- range:
start: 0xA0
end: 0xAF
# Open ranges
- family:
- range:
end: 0x04
model:
- range:
start: 0x0F
```
76 changes: 76 additions & 0 deletions filter_plugins/insert_kernel_modules.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
#!/usr/bin/python3
Copy link
Collaborator

@martin-mat martin-mat Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

filename: insert_kernel_modules.py is misleading. Better cpu_constraints.py?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed in e522a51

import cpuinfo


def get_cpu_info():
info = cpuinfo.get_cpu_info()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 points:

  • ansible filters are for data manipulation. They are not intended to execute any actions / data collection on hosts
  • code of filters are executed centrally on ansible controller, not on target hosts. Hence this will not work

cpu information will be needed to collecto on role level, like:
handle_module: "{{ cpu_constraints | check_cpu_constraints(cpu_family, cpu_model) }}"

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the feedback. Should be addressed in 33036e3

return info["family"], info["model"]


def is_in_range(value, start, end):
return start <= value <= end


def match_single_constraint(value, constraint):
# Single value specified
if isinstance(constraint, int):
if value == constraint:
return True

# List of ranges specified
elif isinstance(constraint, list):
for item in constraint:
range = item.get("range")
if not range:
raise ValueError(
"Constraints needs to be int or list of ranges!"
)
if is_in_range(value, range.get("start", 0), range.get("end", 255)):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to cap the range to 255? It looks like unnecessary conctraint to me.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If i understood correctly, the model number is one byte (0x00 to 0xFF) - thus the range should be from 0 to 255.

But I can make additional logic that would be used in case the range is open either from left or right side, so we don't need to cap the range.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in ff0daa1 . The limits of 0 and 255 were replaced by -inf and inf

return True

return False


def match_cpu_constraints(cpu_family, cpu_model, constraints):
"""Return True if CPU family and model are a match in any of the ranges."""
family_match = False
model_match = False

for constraint in constraints:
if match_single_constraint(cpu_family, constraint["family"]):
family_match = True
if match_single_constraint(cpu_model, constraint["model"]):
model_match = True

# Both model and family need to be a match to the constraints
return family_match and model_match


def check_cpu_constraints(kernel_module):
"""Return True if the checks for constraints are positive and a match."""

# Get list of constraints from kernel module data
checks = kernel_module.get("cpu_constraints")

# No constraints were defined
if not checks:
return True

cpu_family, cpu_model = get_cpu_info()

constraint_match = match_cpu_constraints(
cpu_family, cpu_model, checks.get("cpu_inclusions", list()))
exclusion_match = match_cpu_constraints(
cpu_family, cpu_model, checks.get("cpu_exclusions", list()))

if exclusion_match:
return False

return constraint_match


class FilterModule:
def filters(self):
return {
"check_cpu_constraints": check_cpu_constraints
}
38 changes: 35 additions & 3 deletions group_vars/all.yml
Original file line number Diff line number Diff line change
@@ -1,19 +1,51 @@
kernel_modules:
# Example of a builtin module
# Example of a builtin module
# What should happen: it will not be loaded, because it already is loaded
- name: acpi-cpufreq
kernel_build_opt: CONFIG_X86_ACPI_CPUFREQ

- name: amd_pstate
kernel_build_opt: CONFIG_X86_AMD_PSTATE

# Example of a pluggable module that ships with kernel
# Example of a pluggable module that ships with kernel
# What should happen: it will be loaded
- name: cpuid
kernel_build_opt: CONFIG_X86_CPUIDA
kernel_build_opt: CONFIG_X86_CPUID

# Example of modules that need to be built
# What should happen: it will be built and then loaded
- name: amd_hsmp
git_repo: https://github.com/amd/amd_hsmp.git
git_repo_tag: f34b98fb70b67a7908f32d4d6da5a2ee89857b08

- name: amd_energy
git_repo: https://github.com/amd/amd_energy.git
force_build: true
cpu_constraints:
cpu_inclusions:
- family: 0x19
model:
- range:
start: 0x00
end: 0x0F
- range:
start: 0x30
end: 0x3F
- family: 0x19
model:
- range:
start: 0x10
end: 0x1F
- range:
start: 0xA0
end: 0xAF
- range:
start: 0x90
end: 0x9F
- family: 0x1A
model:
- range:
start: 0x00
end: 0x1F
patches:
- amd_energy_kernel_version.patch
25 changes: 0 additions & 25 deletions group_vars/family_19h/main.yml

This file was deleted.

Loading