rarias/nixos-riscv
1
0
Fork 1
Code Issues Pull Requests 2 Actions Packages Projects Releases Wiki Activity

Set kernel arguments from host in qemu #2

Open
dbautist wants to merge 1 commits from dbautist/nixos-riscv:pl-enhancement into master
pull from: dbautist/nixos-riscv:pl-enhancement
merge into: rarias:master
Conversation 1 Commits 1 Files Changed 5 +99 -38
dbautist commented 2025-02-17 15:58:47 +01:00
First-time contributor
Copy Link

This merge request introduces the next features to the system:

  • boot.sh now accepts arguments such as "bench2" and passes them as kernel parameters to qemu. The command to do it is "./boot.sh --kernelp bench2"
  • If the bench2 option is used, then the benchmarking phase launches at boot stage 2.
  • If the bench2 option is used, when the execution of the defined set of benchmarks is completed the host machine kills the qemu process. Also, if the execution of one benchmark exits with error status, qemu process is terminated and user is informed that some error has occurred.
This merge request introduces the next features to the system: - boot.sh now accepts arguments such as "bench2" and passes them as kernel parameters to qemu. The command to do it is "./boot.sh --kernelp bench2" - If the bench2 option is used, then the benchmarking phase launches at boot stage 2. - If the bench2 option is used, when the execution of the defined set of benchmarks is completed the host machine kills the qemu process. Also, if the execution of one benchmark exits with error status, qemu process is terminated and user is informed that some error has occurred.
dbautist added 4 commits 2025-02-17 15:58:48 +01:00
rarias requested changes 2025-02-17 16:43:02 +01:00
rarias left a comment
Owner
Copy Link

Thanks, this is more or less what I had in mind. I left some comments so we can keep it more flexible. I would need to enable a pipeline for Gitea to check it can detect a successful execution and a wrong one.

Thanks, this is more or less what I had in mind. I left some comments so we can keep it more flexible. I would need to enable a pipeline for Gitea to check it can detect a successful execution and a wrong one.
boot.sh Outdated
@ -1,6 +1,40 @@
#!/usr/bin/env bash
set -ex
set -e
rarias commented 2025-02-17 16:31:37 +01:00
Owner
Copy Link

Let's leave -x for now, so we see what is happening.

Let's leave -x for now, so we see what is happening.
dbautist commented 2025-02-18 15:52:40 +01:00
Author
First-time contributor
Copy Link

The -x option is set on line 37 to avoid showing the trace of the option parsing commands. Do you want these commands to be also traced?

The -x option is set on line 37 to avoid showing the trace of the option parsing commands. Do you want these commands to be also traced?
rarias commented 2025-02-19 11:41:17 +01:00
Owner
Copy Link

For now yes

For now yes
dbautist marked this conversation as resolved
boot.sh Outdated
@ -4,0 +32,4 @@
break
;;
esac
done
rarias commented 2025-02-17 16:14:15 +01:00
Owner
Copy Link

Use just QEMU_KERNEL_PARAMS="$@", there is no need to add all this complexity.

Use just `QEMU_KERNEL_PARAMS="$@"`, there is no need to add all this complexity.
dbautist commented 2025-02-18 15:08:33 +01:00
Author
First-time contributor
Copy Link

The reason why I did it this way is to enable the possibility of adding new options to boot.sh in the future. Furthermore, the option of passing additional arguments to the qemu command is not erased.

If you still want it the simple way, I'll change it :-)

The reason why I did it this way is to enable the possibility of adding new options to boot.sh in the future. Furthermore, the option of passing additional arguments to the qemu command is not erased. If you still want it the simple way, I'll change it :-)
rarias commented 2025-02-19 11:38:17 +01:00
Owner
Copy Link

Let's keep it simple until we need more complexity.

Let's keep it simple until we need more complexity.
dbautist marked this conversation as resolved
boot.sh Outdated
@ -76,0 +105,4 @@
-serial mon:stdio \
-append "$(cat $system/kernel-params) init=$system/init console=ttyS0 loglevel=7 $QEMU_KERNEL_PARAMS" \
$QEMU_OPTS
"$@" 2>&1
rarias commented 2025-02-17 16:16:06 +01:00
Owner
Copy Link

Why do we need to send stderr to the awk output?, this will hide errors.

Why do we need to send stderr to the awk output?, this will hide errors.
dbautist commented 2025-02-18 15:10:45 +01:00
Author
First-time contributor
Copy Link

Sorry that was an experiment I performed, there's no reason for that to be there... erasing it.

Sorry that was an experiment I performed, there's no reason for that to be there... erasing it.
dbautist marked this conversation as resolved
boot.sh Outdated
@ -76,0 +108,4 @@
"$@" 2>&1
}
if [ $QEMU_KERNEL_PARAMS == "bench2" ]; then
rarias commented 2025-02-17 16:30:11 +01:00
Owner
Copy Link

Use a regex match and quote $QEMU_KERNEL_PARAMS :

if [[ "$QEMU_KERNEL_PARAMS" =~ bench2 ]]; then

Use a regex match and quote `$QEMU_KERNEL_PARAMS` : `if [[ "$QEMU_KERNEL_PARAMS" =~ bench2 ]]; then`
dbautist marked this conversation as resolved
boot.sh Outdated
@ -76,0 +109,4 @@
}
if [ $QEMU_KERNEL_PARAMS == "bench2" ]; then
run_qemu | awk -f $CDIR/verify_rvb.awk
rarias commented 2025-02-17 16:40:41 +01:00
Owner
Copy Link

As this would be used to test the bench2 outcome including other benchmarks I recommend you it rename to verify-bench2.awk.

As this would be used to test the bench2 outcome including other benchmarks I recommend you it rename to `verify-bench2.awk`.
dbautist marked this conversation as resolved
verify_rvb.awk Outdated
@ -0,0 +3,4 @@
}
/^BENCHMARK-SUCESS-STATE/ { bench_success_flag = 1; system("pkill -f -SIGTERM qemu-system-riscv64")}
/^BENCHMARK-ERROR-STATE/ { bench_success_flag = 0; system("pkill -f -SIGTERM qemu-system-riscv64")}
rarias commented 2025-02-17 16:25:42 +01:00
Owner
Copy Link

This will kill any qemu running in parallel by the CI, could this be done by just breaking the pipe? Just exit 0 or 1 instead of the system command.

This will kill any qemu running in parallel by the CI, could this be done by just breaking the pipe? Just exit 0 or 1 instead of the system command.
dbautist commented 2025-02-18 15:25:34 +01:00
Author
First-time contributor
Copy Link

I've previously tried to do it this way. However, breaking the pipe is not sufficient for stopping the qemu process (or at least, not by exiting). I am reseraching why this happens.

I suspect SIGPIPE is ignored by qemu process and continues executing.

I've previously tried to do it this way. However, breaking the pipe is not sufficient for stopping the qemu process (or at least, not by exiting). I am reseraching why this happens. I suspect SIGPIPE is ignored by qemu process and continues executing.
rarias commented 2025-02-19 11:40:34 +01:00
Owner
Copy Link

Okay, then let's leave it as-is for now, but we will have to add a different logic to handle the FPGA case in the future.

Okay, then let's leave it as-is for now, but we will have to add a different logic to handle the FPGA case in the future.
dbautist commented 2025-02-19 12:59:17 +01:00
Author
First-time contributor
Copy Link

I've updated the script and the awk program to only kill the desired qemu process. For this, I save the qemu process PID in a file named qemu.pid which is read by awk when a pattern matches.

Not sure if this is FPGA friendly.

I've updated the script and the awk program to only kill the desired qemu process. For this, I save the qemu process PID in a file named qemu.pid which is read by awk when a pattern matches. Not sure if this is FPGA friendly.
dbautist marked this conversation as resolved
dbautist added 1 commit 2025-02-19 12:56:10 +01:00
dbautist added 1 commit 2025-02-19 12:58:01 +01:00
dbautist added 1 commit 2025-02-19 13:16:51 +01:00
dbautist added 1 commit 2025-02-19 14:26:14 +01:00
rarias changed title from pl-enhancement to Set kernel arguments from host in qemu 2025-02-21 10:50:44 +01:00
rarias commented 2025-02-21 10:52:53 +01:00
Owner
Copy Link

Could you make this run in the Gitea CI? Here is an example: 5da53c69b8

I think we only need to run the same command we are doing in the GitLab CI (https://jungle.bsc.es/git/rarias/nixos-riscv/src/branch/master/.gitlab-ci.yml#L31) but in Gitea.

Otherwise I can take a look later.

Could you make this run in the Gitea CI? Here is an example: https://jungle.bsc.es/git/rarias/nixos-riscv/commit/5da53c69b8b33d2323a363ac269859a89d816579 I think we only need to run the same command we are doing in the GitLab CI (https://jungle.bsc.es/git/rarias/nixos-riscv/src/branch/master/.gitlab-ci.yml#L31) but in Gitea. Otherwise I can take a look later.
dbautist added 1 commit 2025-02-27 10:40:25 +01:00
dbautist added 1 commit 2025-02-27 10:46:15 +01:00
dbautist added 1 commit 2025-02-27 10:51:02 +01:00
dbautist force-pushed pl-enhancement from 4ca9182c75 to 492c540bf5 2025-02-27 11:18:08 +01:00 Compare
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u pl-enhancement:dbautist-pl-enhancement
git checkout dbautist-pl-enhancement
Sign in to join this conversation.
Reviewers
No reviewers
rarias
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: rarias/nixos-riscv#2
No description provided.
Delete Branch "dbautist/nixos-riscv:pl-enhancement"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?