[PATCH 0/2] examples: systemtap: Fix/update 'amd-sev-es-vmsa.stp'
Peter Krempa (2): examples: systemtap: Warn users to properly update 'amd-sev-es-vmsa.stp' examples: systemtap: Update to linux-6.3 examples/systemtap/amd-sev-es-vmsa.stp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) -- 2.39.2
The script references a very specific line in the kernel source code and thus may need to be updated to work properly. Add a warning into the comment and also print it to warn users. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- examples/systemtap/amd-sev-es-vmsa.stp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/examples/systemtap/amd-sev-es-vmsa.stp b/examples/systemtap/amd-sev-es-vmsa.stp index 551ed739b7..14bfb01c10 100644 --- a/examples/systemtap/amd-sev-es-vmsa.stp +++ b/examples/systemtap/amd-sev-es-vmsa.stp @@ -19,6 +19,9 @@ # A script that captures the VMSA blob for the boot vCPU and # first additional vCPU, when a KVM guest is booted with SEV-ES # +# NOTE: This directly references specific structures and places in the +# kernel source code. If the code changes this example will break. +# # The captured VMSA will be printed to the console in hex format, # and can be converted to the required binary format by feeding # it through @@ -27,6 +30,7 @@ # probe begin { + printf("WARNING: Make sure this script is updated according to your kernel source!\n") printf("Running\n") } -- 2.39.2
On Mon, Mar 06, 2023 at 11:21:53AM +0100, Peter Krempa wrote:
The script references a very specific line in the kernel source code and thus may need to be updated to work properly. Add a warning into the comment and also print it to warn users.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- examples/systemtap/amd-sev-es-vmsa.stp | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/examples/systemtap/amd-sev-es-vmsa.stp b/examples/systemtap/amd-sev-es-vmsa.stp index 551ed739b7..14bfb01c10 100644 --- a/examples/systemtap/amd-sev-es-vmsa.stp +++ b/examples/systemtap/amd-sev-es-vmsa.stp @@ -19,6 +19,9 @@ # A script that captures the VMSA blob for the boot vCPU and # first additional vCPU, when a KVM guest is booted with SEV-ES # +# NOTE: This directly references specific structures and places in the +# kernel source code. If the code changes this example will break.
Replace "If the code changes this example will break" with "It is expected that this example will need to be editted to match the kenrel you intend to run it against."
# The captured VMSA will be printed to the console in hex format, # and can be converted to the required binary format by feeding # it through @@ -27,6 +30,7 @@ #
probe begin { + printf("WARNING: Make sure this script is updated according to your kernel source!\n")
I would expect it to generally fail to compile, so rarely would it get as far as running this method. For the change you proposed in the next match I would have expected a compile error.
printf("Running\n") }
-- 2.39.2
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
The 'vmsa' struct was moved out of 'struct vcpu_svm' in linux commit: commit b67a4cc35c9f726999fa29880713ce72d4e39e8d Author: Peter Gonda <pgonda@google.com> Date: Thu Oct 21 10:42:59 2021 -0700 KVM: SEV: Refactor out sev_es_state struct Move SEV-ES vCPU metadata into new sev_es_state struct from vcpu_svm. Also update the line reference for linux-6.3. NB: I strongly considered removing the example as it's impossible to keep in sync. With the warning added by previous commit I think we can give it one more chance. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2175598 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- examples/systemtap/amd-sev-es-vmsa.stp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/examples/systemtap/amd-sev-es-vmsa.stp b/examples/systemtap/amd-sev-es-vmsa.stp index 14bfb01c10..ab2f202681 100644 --- a/examples/systemtap/amd-sev-es-vmsa.stp +++ b/examples/systemtap/amd-sev-es-vmsa.stp @@ -46,7 +46,7 @@ function dump_vmsa(addr:long) { # is the one beween the call to clflush_cache_range(...) and the # call to sev_issue_cmd(kvm, SEV_CMD_LAUNCH_UPDATE...). # -# Line 632 is correct for Linux v6.0 -probe module("kvm_amd").statement("__sev_launch_update_vmsa@arch/x86/kvm/svm/sev.c:632") { - dump_vmsa($svm->vmsa) +# Line 635 is correct for Linux v6.3 +probe module("kvm_amd").statement("__sev_launch_update_vmsa@arch/x86/kvm/svm/sev.c:635") { + dump_vmsa($svm->sev_es->vmsa) } -- 2.39.2
On Mon, Mar 06, 2023 at 11:21:54AM +0100, Peter Krempa wrote:
The 'vmsa' struct was moved out of 'struct vcpu_svm' in linux commit:
commit b67a4cc35c9f726999fa29880713ce72d4e39e8d Author: Peter Gonda <pgonda@google.com> Date: Thu Oct 21 10:42:59 2021 -0700
KVM: SEV: Refactor out sev_es_state struct
Move SEV-ES vCPU metadata into new sev_es_state struct from vcpu_svm.
Also update the line reference for linux-6.3.
NB: I strongly considered removing the example as it's impossible to keep in sync. With the warning added by previous commit I think we can give it one more chance.
It sucks, but it sucks less than what every was doing before I suggested systemtap, which is to tell people to edit their kernel source to add printfs and then rebuild their kernel Ideally the kernel would expose the pristine expected VMSA data in some sane interface like debugfs.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2175598
In general such bugs should be closed WONTFIX. There is no expectation that this demo works on RHEL kernels. At most I would worry about upstream kernel, but even that's not very critical. This is just a demo of the approach you should take not a supported script we expect to work out of the box.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- examples/systemtap/amd-sev-es-vmsa.stp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/examples/systemtap/amd-sev-es-vmsa.stp b/examples/systemtap/amd-sev-es-vmsa.stp index 14bfb01c10..ab2f202681 100644 --- a/examples/systemtap/amd-sev-es-vmsa.stp +++ b/examples/systemtap/amd-sev-es-vmsa.stp @@ -46,7 +46,7 @@ function dump_vmsa(addr:long) { # is the one beween the call to clflush_cache_range(...) and the # call to sev_issue_cmd(kvm, SEV_CMD_LAUNCH_UPDATE...). # -# Line 632 is correct for Linux v6.0 -probe module("kvm_amd").statement("__sev_launch_update_vmsa@arch/x86/kvm/svm/sev.c:632") { - dump_vmsa($svm->vmsa) +# Line 635 is correct for Linux v6.3 +probe module("kvm_amd").statement("__sev_launch_update_vmsa@arch/x86/kvm/svm/sev.c:635") { + dump_vmsa($svm->sev_es->vmsa) } -- 2.39.2
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Mon, Mar 06, 2023 at 10:37:54 +0000, Daniel P. Berrangé wrote:
On Mon, Mar 06, 2023 at 11:21:54AM +0100, Peter Krempa wrote:
The 'vmsa' struct was moved out of 'struct vcpu_svm' in linux commit:
commit b67a4cc35c9f726999fa29880713ce72d4e39e8d Author: Peter Gonda <pgonda@google.com> Date: Thu Oct 21 10:42:59 2021 -0700
KVM: SEV: Refactor out sev_es_state struct
Move SEV-ES vCPU metadata into new sev_es_state struct from vcpu_svm.
Also update the line reference for linux-6.3.
NB: I strongly considered removing the example as it's impossible to keep in sync. With the warning added by previous commit I think we can give it one more chance.
It sucks, but it sucks less than what every was doing before I suggested systemtap, which is to tell people to edit their kernel source to add printfs and then rebuild their kernel
Ideally the kernel would expose the pristine expected VMSA data in some sane interface like debugfs.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2175598
In general such bugs should be closed WONTFIX. There is no expectation that this demo works on RHEL kernels. At most I would worry about upstream kernel, but even that's not very critical. This is just a demo of the approach you should take not a supported script we expect to work out of the box.
I agree. I'll close this one.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- examples/systemtap/amd-sev-es-vmsa.stp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/examples/systemtap/amd-sev-es-vmsa.stp b/examples/systemtap/amd-sev-es-vmsa.stp index 14bfb01c10..ab2f202681 100644 --- a/examples/systemtap/amd-sev-es-vmsa.stp +++ b/examples/systemtap/amd-sev-es-vmsa.stp @@ -46,7 +46,7 @@ function dump_vmsa(addr:long) { # is the one beween the call to clflush_cache_range(...) and the # call to sev_issue_cmd(kvm, SEV_CMD_LAUNCH_UPDATE...). # -# Line 632 is correct for Linux v6.0 -probe module("kvm_amd").statement("__sev_launch_update_vmsa@arch/x86/kvm/svm/sev.c:632") { - dump_vmsa($svm->vmsa) +# Line 635 is correct for Linux v6.3 +probe module("kvm_amd").statement("__sev_launch_update_vmsa@arch/x86/kvm/svm/sev.c:635") { + dump_vmsa($svm->sev_es->vmsa)
At least the change to this line IMO makes sense from upstream's POV going forward as the code was refactored and thus the old example will never work again with new kernels.
On Mon, Mar 06, 2023 at 12:26:24PM +0100, Peter Krempa wrote:
On Mon, Mar 06, 2023 at 10:37:54 +0000, Daniel P. Berrangé wrote:
On Mon, Mar 06, 2023 at 11:21:54AM +0100, Peter Krempa wrote:
The 'vmsa' struct was moved out of 'struct vcpu_svm' in linux commit:
commit b67a4cc35c9f726999fa29880713ce72d4e39e8d Author: Peter Gonda <pgonda@google.com> Date: Thu Oct 21 10:42:59 2021 -0700
KVM: SEV: Refactor out sev_es_state struct
Move SEV-ES vCPU metadata into new sev_es_state struct from vcpu_svm.
Also update the line reference for linux-6.3.
NB: I strongly considered removing the example as it's impossible to keep in sync. With the warning added by previous commit I think we can give it one more chance.
It sucks, but it sucks less than what every was doing before I suggested systemtap, which is to tell people to edit their kernel source to add printfs and then rebuild their kernel
Ideally the kernel would expose the pristine expected VMSA data in some sane interface like debugfs.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2175598
In general such bugs should be closed WONTFIX. There is no expectation that this demo works on RHEL kernels. At most I would worry about upstream kernel, but even that's not very critical. This is just a demo of the approach you should take not a supported script we expect to work out of the box.
I agree. I'll close this one.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- examples/systemtap/amd-sev-es-vmsa.stp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/examples/systemtap/amd-sev-es-vmsa.stp b/examples/systemtap/amd-sev-es-vmsa.stp index 14bfb01c10..ab2f202681 100644 --- a/examples/systemtap/amd-sev-es-vmsa.stp +++ b/examples/systemtap/amd-sev-es-vmsa.stp @@ -46,7 +46,7 @@ function dump_vmsa(addr:long) { # is the one beween the call to clflush_cache_range(...) and the # call to sev_issue_cmd(kvm, SEV_CMD_LAUNCH_UPDATE...). # -# Line 632 is correct for Linux v6.0 -probe module("kvm_amd").statement("__sev_launch_update_vmsa@arch/x86/kvm/svm/sev.c:632") { - dump_vmsa($svm->vmsa) +# Line 635 is correct for Linux v6.3 +probe module("kvm_amd").statement("__sev_launch_update_vmsa@arch/x86/kvm/svm/sev.c:635") { + dump_vmsa($svm->sev_es->vmsa)
At least the change to this line IMO makes sense from upstream's POV going forward as the code was refactored and thus the old example will never work again with new kernels.
Yes, I'm fine with changes to this demo program if they're presented simply as following LKML latest git impl. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (2)
-
Daniel P. Berrangé -
Peter Krempa