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.

Sign up for GitHub

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

Jump to bottom

Add riscv64 support #5198

Merged
merged 1 commit into from
Feb 26, 2024
Merged

Add riscv64 support #5198

merged 1 commit into from
Feb 26, 2024

Conversation

U2FsdGVkX1
Copy link
Contributor

tested in RISC-V 64.

please correct me if any problems

@pep8speaks
Copy link

pep8speaks commented Sep 22, 2023 •
edited
Loading

Hello @U2FsdGVkX1! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-02-24 03:25:46 UTC

@github-actions github-actions bot added the f40 label Sep 22, 2023
VladimirSlavik
VladimirSlavik approved these changes Sep 22, 2023
View reviewed changes
Copy link
Contributor

@VladimirSlavik VladimirSlavik left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thank you!

@VladimirSlavik VladimirSlavik added the release note required Write a release note for this change. label Sep 22, 2023
@VladimirSlavik
Copy link
Contributor

VladimirSlavik commented Sep 22, 2023 •
edited
Loading

Can you please add also a release note? Make a copy of docs/release-notes/template.rst and fill it in.

That will shut up the failing Release note check.

@VladimirSlavik
Copy link
Contributor

/kickstart-test --testtype smoke

@VladimirSlavik
Copy link
Contributor

Unit tests also need some adjusting...

=========================== short test summary info ============================
FAILED unit_tests/pyanaconda_tests/modules/storage/test_platform.py::PlatformTestCase::test_efi
FAILED unit_tests/pyanaconda_tests/modules/storage/test_platform.py::PlatformTestCase::test_unsupported_platform
====== 2 failed, 2029 passed, 1 xfailed, 18 warnings in 295.86s (0:04:55) ======
FAIL unit_tests/unit_tests.sh (exit status: 1)

@U2FsdGVkX1
Copy link
Contributor Author

Unit tests also need some adjusting...

=========================== short test summary info ============================
FAILED unit_tests/pyanaconda_tests/modules/storage/test_platform.py::PlatformTestCase::test_efi
FAILED unit_tests/pyanaconda_tests/modules/storage/test_platform.py::PlatformTestCase::test_unsupported_platform
====== 2 failed, 2029 passed, 1 xfailed, 18 warnings in 295.86s (0:04:55) ======
FAIL unit_tests/unit_tests.sh (exit status: 1)

I have added test and release note, please recheck, thanks!

@VladimirSlavik
Copy link
Contributor

/kickstart-test --testtype smoke

VladimirSlavik
VladimirSlavik reviewed Sep 25, 2023
View reviewed changes
docs/release-notes/riscv64-support.rst Outdated
Comment on lines 5 to 6
In storage stage, partitions cannot be completed
and maybe UEFI is not recognized
Copy link
Contributor

Choose a reason for hiding this comment

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

Um, so this is not really support, but only some part of it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, maybe we need someone from the non-mainstream architecture are on Fedora to chime in ? CCing @rwmjones and @nullr0ute

Copy link

Choose a reason for hiding this comment

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

Adding @davidlt

Copy link

Choose a reason for hiding this comment

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

I'm not at all clear on what this comment in the documentation means, but I can say that RISC-V does support partitions (obviously!) and that there is at least preliminary UEFI support although it is not widely used.

Copy link

Choose a reason for hiding this comment

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

We do run some Pungi composes that. That does incl. booting riscv64 VM with EDK2 and running anaconda. I have a patch 1+ week old. Let me cross-check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Um, so this is not really support, but only some part of it?

No, I mean, before that anaconda couldn't be partitioned, now it's fine (maybe I should rewrite the release note)
and UEFI I need more test.

@davidlt
Copy link

davidlt commented Sep 26, 2023 •
edited
Loading

In Fedora/RISCV Koji I mainly have EFI variant, and that code looks almost exactly the same minus one line. This part in Pungi composes succeeds (i.e. anaconda does not fail), but still don't have full Pungi phases passing to verify the final product.

The only different is listing grub2-efi-riscv64 package which holds GRUB2 EFI binaries for riscv64.

@github-actions GitHub Actions
Copy link

This PR is stale because it has been open 60 days with no activity.
Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the stale label Nov 27, 2023
@U2FsdGVkX1
Copy link
Contributor Author

Hi, are you there?

@github-actions github-actions bot removed the stale label Nov 28, 2023
@github-actions GitHub Actions
Copy link

This PR is stale because it has been open 60 days with no activity.
Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the stale label Jan 28, 2024
@U2FsdGVkX1
Add riscv64 support
fed051f 
Signed-off-by: Songsong Zhang <U2FsdGVkX1@gmail.com>
Reviewed-by: Wei Fu <wefu@redhat.com>
@github-actions github-actions bot removed the stale label Feb 25, 2024
@KKoukiou
Copy link
Contributor

/kickstart-test --testtype smoke

@KKoukiou KKoukiou merged commit e57e905 into rhinstaller:master Feb 26, 2024
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@VladimirSlavik VladimirSlavik VladimirSlavik approved these changes

@M4rtinK M4rtinK M4rtinK left review comments

@rwmjones rwmjones rwmjones left review comments

@davidlt davidlt davidlt left review comments

@vojtechtrefny vojtechtrefny vojtechtrefny approved these changes

Assignees
No one assigned
Labels
f40 release note required Write a release note for this change.
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@U2FsdGVkX1 @pep8speaks @VladimirSlavik @davidlt @KKoukiou @rwmjones @M4rtinK @vojtechtrefny