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

Setup SNP on the RHEL host #15

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

LakshmiSaiHarika
Copy link
Contributor

Updated snp.sh to install dependencies, build SNP latest kernel package and set the default grub menu to the SNP latest kernel version in RHEL (Tested this on RHEL 9.4)

docs/snp.md Outdated
@@ -51,6 +52,11 @@ Download the script and add the execute permission:
wget https://github.com/amd/sev-utils/raw/main/tools/snp.sh
chmod +x snp.sh
```
For RHEL OS distribution, export the following environment variables for using your RHEL subscription manager credentials for installing the package dependencies:
Copy link
Collaborator

Choose a reason for hiding this comment

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

This shouldn't be necessary. If you're installing packages on the host, let it error out if subscription manager isn't enabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, I removed this portion

Copy link
Collaborator

Choose a reason for hiding this comment

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

I still see the variables and method to register the subscription manager.

tools/snp.sh Outdated
@@ -90,6 +90,10 @@ QEMU_CMDLINE_FILE="${QEMU_CMDLINE:-${LAUNCH_WORKING_DIR}/qemu.cmdline}"
IMAGE="${IMAGE:-${LAUNCH_WORKING_DIR}/${GUEST_NAME}.img}"
GENERATED_INITRD_BIN="${SETUP_WORKING_DIR}/initrd.img"

# Redhat Sensitive Credentials -- User must specify, no default value set
Copy link
Collaborator

Choose a reason for hiding this comment

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

See docs comment. Shouldn't be needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, I removed this line

tools/snp.sh Outdated
@@ -214,7 +215,41 @@ install_sev_snp_measure() {
pip install sev-snp-measure==${SEV_SNP_MEASURE_VERSION}
}

install_dependencies() {
check_if_redhat_credentials_set(){
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead, maybe have a function to check if the subscription manager is registered and working before proceeding.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created check_rhel_subscription_mgr_status() to check the status, if status is in unregister keyterms, then register RHEL subscription with the given RHEL env variables

tools/snp.sh Outdated
#sudo apt install -y nasm

# Remove any default nasm package
sudo apt purge nasm
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this line as this works on fresh os image

tools/snp.sh Outdated
@@ -268,10 +319,110 @@ install_dependencies() {

# pip needed for sev-snp-measure
sudo apt install -y python3-pip
}

check_if_redhat_credentials_set(){
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you have this method twice?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.. I cleaned this portion

tools/snp.sh Outdated
# genisoimage utility supported until RedHat version 8
# For genisoimage utility support in RedHat 9, using epel
sudo dnf install -y https://dl.fedoraproject.org/pub/epel/epel-release-latest-9.noarch.rpm
sudo dnf install -y genisoimage
Copy link
Collaborator

Choose a reason for hiding this comment

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

we need to avoid adding a repo (esp epel) unless completely necessary. Let's try to resolve this to an existing package. If not, I'll help you find a solution.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right now removing genisoimage from this PR (RHEL setup host PR) as this package is used in the launch-guest process..

I will update further dependencies for the SNP guest launch in the future PR

}


Copy link
Collaborator

Choose a reason for hiding this comment

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

remove

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.. I cleaned this up

tools/snp.sh Outdated
@@ -1113,6 +1264,7 @@ main() {
;;

setup-host)
identify_linux_distribution_type
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we run this, only when needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.. I updated this

tools/snp.sh Outdated
AMDSEV_URL="https://github.com/ryansavino/AMDSEV.git"
AMDSEV_DEFAULT_BRANCH="snp-latest-fixes"
AMDSEV_URL="https://github.com/LakshmiSaiHarika/AMDSEV.git"
AMDSEV_DEFAULT_BRANCH="rhel-snp-latest-fixes"
Copy link
Collaborator

Choose a reason for hiding this comment

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

what changes here? If not too involved maybe we can merge into my existing fork.

Copy link
Contributor Author

@LakshmiSaiHarika LakshmiSaiHarika Sep 5, 2024

Choose a reason for hiding this comment

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

AMDESE changes: In your fork, I added few fixes fir RHEL in build and install script

I raised a PR (below link) to merge in your AMDESE fork:
ryansavino/amdese-amdsev#1

tools/snp.sh Outdated
@@ -465,6 +465,28 @@ set_grub_default_snp() {
sudo update-grub
}

grubby_to_set_grub_default_snp(){
Copy link
Collaborator

Choose a reason for hiding this comment

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

grubby_set...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed grubby from this script

Modified RHEL grub configuration and reloaded settings in "rhel_set_grub_default_snp"

tools/snp.sh Outdated
rhel)
LINUX_TYPE='rhel'

if [[ "$UPM" = false ]]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

This check doesn't belong here. You can run a similar failure case outside this function somewhere if you want to make sure rhel doesn't proceed.

Missing break for rhel case.

indent under each category.

docs/snp.md Outdated
@@ -51,6 +52,11 @@ Download the script and add the execute permission:
wget https://github.com/amd/sev-utils/raw/main/tools/snp.sh
chmod +x snp.sh
```
For RHEL OS distribution, export the following environment variables for using your RHEL subscription manager credentials for installing the package dependencies:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I still see the variables and method to register the subscription manager.

Added RHEL 9.4 OS as one of the tested OS distribution for sev-utils

Signed-off-by: Harika Nittala <lnittala@amd.com>
AMDSEV_URL="https://github.com/ryansavino/AMDSEV.git"
AMDSEV_DEFAULT_BRANCH="snp-latest-fixes"
AMDSEV_URL="https://github.com/LakshmiSaiHarika/AMDSEV.git"
AMDSEV_DEFAULT_BRANCH="rhel-setup-host"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ryansavino Please review and merge my PR for adding minor RHEL fixes into your AMDESE fork
PR link: here

Renamed old install_dependencies feature to ubuntu_install_dependencies
Updated install_dependencies to perform installation of dependencies based on identified linux distribution and their package manager command

Signed-off-by: Harika Nittala <lnittala@amd.com>
Removed ubuntu nasm package uninstallation step from the nasm source installation process

Signed-off-by: Harika Nittala <lnittala@amd.com>
…build

Added RHEL dependencies to build the SNP kernel rpm package for the host and guest

Signed-off-by: Harika Nittala <lnittala@amd.com>
Generalized set_default_grub_menu across various OS distribution
Renamed ubuntu_set_default_grub_menu to set specific default ubuntu grub menu to the built SNP kernel
Added a function to set the RHEL default grub menu to the built SNP kernel

Signed-off-by: Harika Nittala <lnittala@amd.com>
Modified AMDSEV URL to add RHEL fixes for the latest SNP kernel patches, qemu build on the RHEL OS
Requested code merge into ryan's latest fix issues branch for the updated AMD_SEV_URL in sev-utils

Signed-off-by: Harika Nittala <lnittala@amd.com>
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.

2 participants