diff mbox series

[RFC,v2,1/9] Add Rust SEV library as subproject

Message ID 20231004203418.56508-2-tfanelli@redhat.com
State New
Headers show
Series i386/sev: Use C API of Rust SEV library | expand

Commit Message

Tyler Fanelli Oct. 4, 2023, 8:34 p.m. UTC
The Rust sev library provides a C API for the AMD SEV launch ioctls, as
well as the ability to build with meson. Add the Rust sev library as a
QEMU subproject with the goal of outsourcing all SEV launch ioctls to C
APIs provided by it.

Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
---
 meson.build                   | 8 ++++++++
 meson_options.txt             | 2 ++
 scripts/meson-buildoptions.sh | 3 +++
 subprojects/sev.wrap          | 6 ++++++
 target/i386/meson.build       | 2 +-
 5 files changed, 20 insertions(+), 1 deletion(-)
 create mode 100644 subprojects/sev.wrap

Comments

Philippe Mathieu-Daudé Oct. 5, 2023, 6:03 a.m. UTC | #1
Hi Tyler,

On 4/10/23 22:34, Tyler Fanelli wrote:
> The Rust sev library provides a C API for the AMD SEV launch ioctls, as
> well as the ability to build with meson. Add the Rust sev library as a
> QEMU subproject with the goal of outsourcing all SEV launch ioctls to C
> APIs provided by it.
> 
> Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
> ---
>   meson.build                   | 8 ++++++++
>   meson_options.txt             | 2 ++
>   scripts/meson-buildoptions.sh | 3 +++
>   subprojects/sev.wrap          | 6 ++++++
>   target/i386/meson.build       | 2 +-
>   5 files changed, 20 insertions(+), 1 deletion(-)
>   create mode 100644 subprojects/sev.wrap


> diff --git a/subprojects/sev.wrap b/subprojects/sev.wrap
> new file mode 100644
> index 0000000000..5be1faccf6
> --- /dev/null
> +++ b/subprojects/sev.wrap
> @@ -0,0 +1,6 @@
> +[wrap-git]
> +url = https://github.com/tylerfanelli/sev
> +revision = b81b1da5df50055600a5b0349b0c4afda677cccb

Why use your tree instead of the mainstream one?

Before this gets merged we need to mirror the subproject
on our GitLab namespace, then use the mirror URL here.
Stefan Hajnoczi Oct. 5, 2023, 3:54 p.m. UTC | #2
On Wed, Oct 04, 2023 at 04:34:10PM -0400, Tyler Fanelli wrote:
> The Rust sev library provides a C API for the AMD SEV launch ioctls, as
> well as the ability to build with meson. Add the Rust sev library as a
> QEMU subproject with the goal of outsourcing all SEV launch ioctls to C
> APIs provided by it.
> 
> Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
> ---
>  meson.build                   | 8 ++++++++
>  meson_options.txt             | 2 ++
>  scripts/meson-buildoptions.sh | 3 +++
>  subprojects/sev.wrap          | 6 ++++++
>  target/i386/meson.build       | 2 +-
>  5 files changed, 20 insertions(+), 1 deletion(-)
>  create mode 100644 subprojects/sev.wrap
> 
> diff --git a/meson.build b/meson.build
> index 20ceeb8158..8a17c29de8 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -960,6 +960,13 @@ if not get_option('slirp').auto() or have_system
>    endif
>  endif
>  
> +sev = not_found
> +if not get_option('sev').auto()

When 'sev' is auto, then it won't be built. That seems strange. The
auto-detection part is missing! I did you test this on a system that
doesn't have libsev installed system-wide?

I guess the auto-detection would look something like:

  cargo = find_program('cargo', required: true)

  if not get_option('sev').auto() or cargo.found()
      ...

That way 'sev' is only built automatically on systems that have cargo
installed.

> +  sev = dependency('sev',
> +                   method: 'pkg-config',
> +                   required: get_option('sev'))
> +endif

If you update the auto logic, see the documentation about fallbacks to
subprojects for optional dependencies:
https://mesonbuild.com/Wrap-dependency-system-manual.html#provide-section

It might be necessary to add dependency(..., fallback='sev').

> +
>  vde = not_found
>  if not get_option('vde').auto() or have_system or have_tools
>    vde = cc.find_library('vdeplug', has_headers: ['libvdeplug.h'],
> @@ -4331,6 +4338,7 @@ summary_info += {'libudev':           libudev}
>  # Dummy dependency, keep .found()
>  summary_info += {'FUSE lseek':        fuse_lseek.found()}
>  summary_info += {'selinux':           selinux}
> +summary_info += {'sev':               sev}
>  summary_info += {'libdw':             libdw}
>  summary(summary_info, bool_yn: true, section: 'Dependencies')
>  
> diff --git a/meson_options.txt b/meson_options.txt
> index 57e265c871..5b8d283717 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -204,6 +204,8 @@ option('sdl_image', type : 'feature', value : 'auto',
>         description: 'SDL Image support for icons')
>  option('seccomp', type : 'feature', value : 'auto',
>         description: 'seccomp support')
> +option('sev', type : 'feature', value : 'auto',
> +        description: 'Rust AMD SEV library')
>  option('smartcard', type : 'feature', value : 'auto',
>         description: 'CA smartcard emulation support')
>  option('snappy', type : 'feature', value : 'auto',
> diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
> index e4b46d5715..e585a548fa 100644
> --- a/scripts/meson-buildoptions.sh
> +++ b/scripts/meson-buildoptions.sh
> @@ -161,6 +161,7 @@ meson_options_help() {
>    printf "%s\n" '  sdl-image       SDL Image support for icons'
>    printf "%s\n" '  seccomp         seccomp support'
>    printf "%s\n" '  selinux         SELinux support in qemu-nbd'
> +  printf "%s\n" '  sev             SEV library support'
>    printf "%s\n" '  slirp           libslirp user mode network backend support'
>    printf "%s\n" '  slirp-smbd      use smbd (at path --smbd=*) in slirp networking'
>    printf "%s\n" '  smartcard       CA smartcard emulation support'
> @@ -440,6 +441,8 @@ _meson_option_parse() {
>      --disable-seccomp) printf "%s" -Dseccomp=disabled ;;
>      --enable-selinux) printf "%s" -Dselinux=enabled ;;
>      --disable-selinux) printf "%s" -Dselinux=disabled ;;
> +    --enable-sev) printf "%s" -Dsev=enabled ;;
> +    --disable-sev) printf "%s" -Dsev=disabled ;;
>      --enable-slirp) printf "%s" -Dslirp=enabled ;;
>      --disable-slirp) printf "%s" -Dslirp=disabled ;;
>      --enable-slirp-smbd) printf "%s" -Dslirp_smbd=enabled ;;
> diff --git a/subprojects/sev.wrap b/subprojects/sev.wrap
> new file mode 100644
> index 0000000000..5be1faccf6
> --- /dev/null
> +++ b/subprojects/sev.wrap
> @@ -0,0 +1,6 @@
> +[wrap-git]
> +url = https://github.com/tylerfanelli/sev
> +revision = b81b1da5df50055600a5b0349b0c4afda677cccb
> +
> +[provide]
> +sev = sev_dep
> diff --git a/target/i386/meson.build b/target/i386/meson.build
> index 6f1036d469..8972a4fb17 100644
> --- a/target/i386/meson.build
> +++ b/target/i386/meson.build
> @@ -20,7 +20,7 @@ i386_system_ss.add(files(
>    'monitor.c',
>    'cpu-sysemu.c',
>  ))
> -i386_system_ss.add(when: 'CONFIG_SEV', if_true: files('sev.c'), if_false: files('sev-sysemu-stub.c'))
> +i386_system_ss.add(when: 'CONFIG_SEV', if_true: [sev, files('sev.c')], if_false: files('sev-sysemu-stub.c'))
>  
>  i386_user_ss = ss.source_set()
>  
> -- 
> 2.40.1
>
Tyler Fanelli Oct. 5, 2023, 11:41 p.m. UTC | #3
On 10/5/23 2:03 AM, Philippe Mathieu-Daudé wrote:
> Hi Tyler,
>
> On 4/10/23 22:34, Tyler Fanelli wrote:
>> The Rust sev library provides a C API for the AMD SEV launch ioctls, as
>> well as the ability to build with meson. Add the Rust sev library as a
>> QEMU subproject with the goal of outsourcing all SEV launch ioctls to C
>> APIs provided by it.
>>
>> Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
>> ---
>>   meson.build                   | 8 ++++++++
>>   meson_options.txt             | 2 ++
>>   scripts/meson-buildoptions.sh | 3 +++
>>   subprojects/sev.wrap          | 6 ++++++
>>   target/i386/meson.build       | 2 +-
>>   5 files changed, 20 insertions(+), 1 deletion(-)
>>   create mode 100644 subprojects/sev.wrap
>
>
>> diff --git a/subprojects/sev.wrap b/subprojects/sev.wrap
>> new file mode 100644
>> index 0000000000..5be1faccf6
>> --- /dev/null
>> +++ b/subprojects/sev.wrap
>> @@ -0,0 +1,6 @@
>> +[wrap-git]
>> +url = https://github.com/tylerfanelli/sev
>> +revision = b81b1da5df50055600a5b0349b0c4afda677cccb
>
> Why use your tree instead of the mainstream one?
>
> Before this gets merged we need to mirror the subproject
> on our GitLab namespace, then use the mirror URL here.
>
The required meson changes for the sev library are still in review, so 
I'm still working on a personal branch. Those patches are a blocker for 
this series right now.

This is moreso another RFC to get feedback on building Rust libraries as 
QEMU subprojects (and if this is the proper way to do so).


Tyler
Tyler Fanelli Oct. 11, 2023, 3:05 a.m. UTC | #4
On 10/5/23 2:03 AM, Philippe Mathieu-Daudé wrote:
> Hi Tyler,
>
> On 4/10/23 22:34, Tyler Fanelli wrote:
>> The Rust sev library provides a C API for the AMD SEV launch ioctls, as
>> well as the ability to build with meson. Add the Rust sev library as a
>> QEMU subproject with the goal of outsourcing all SEV launch ioctls to C
>> APIs provided by it.
>>
>> Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
>> ---
>>   meson.build                   | 8 ++++++++
>>   meson_options.txt             | 2 ++
>>   scripts/meson-buildoptions.sh | 3 +++
>>   subprojects/sev.wrap          | 6 ++++++
>>   target/i386/meson.build       | 2 +-
>>   5 files changed, 20 insertions(+), 1 deletion(-)
>>   create mode 100644 subprojects/sev.wrap
>
>
>> diff --git a/subprojects/sev.wrap b/subprojects/sev.wrap
>> new file mode 100644
>> index 0000000000..5be1faccf6
>> --- /dev/null
>> +++ b/subprojects/sev.wrap
>> @@ -0,0 +1,6 @@
>> +[wrap-git]
>> +url = https://github.com/tylerfanelli/sev
>> +revision = b81b1da5df50055600a5b0349b0c4afda677cccb
>
> Why use your tree instead of the mainstream one?
>
> Before this gets merged we need to mirror the subproject
> on our GitLab namespace, then use the mirror URL here.
>
Hi Philippe,

Why must the subproject be mirrored on qemu's GitLab namespace? With the 
changes being accepted in the upstream sev repository, meson will be 
able to fetch it from there. I see that libblkio (another Rust project) 
is not mirrored in the GitLab namespace [0] (assuming I'm looking in the 
right place) and that meson also fetches it from its upstream repo [1].

[0] https://gitlab.com/qemu-project

[1] 
https://gitlab.com/qemu-project/qemu/-/blob/master/subprojects/libblkio.wrap?ref_type=heads#L2


Tyler
Tyler Fanelli Oct. 11, 2023, 3:10 a.m. UTC | #5
On 10/5/23 11:54 AM, Stefan Hajnoczi wrote:
> On Wed, Oct 04, 2023 at 04:34:10PM -0400, Tyler Fanelli wrote:
>> The Rust sev library provides a C API for the AMD SEV launch ioctls, as
>> well as the ability to build with meson. Add the Rust sev library as a
>> QEMU subproject with the goal of outsourcing all SEV launch ioctls to C
>> APIs provided by it.
>>
>> Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
>> ---
>>   meson.build                   | 8 ++++++++
>>   meson_options.txt             | 2 ++
>>   scripts/meson-buildoptions.sh | 3 +++
>>   subprojects/sev.wrap          | 6 ++++++
>>   target/i386/meson.build       | 2 +-
>>   5 files changed, 20 insertions(+), 1 deletion(-)
>>   create mode 100644 subprojects/sev.wrap
>>
>> diff --git a/meson.build b/meson.build
>> index 20ceeb8158..8a17c29de8 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -960,6 +960,13 @@ if not get_option('slirp').auto() or have_system
>>     endif
>>   endif
>>   
>> +sev = not_found
>> +if not get_option('sev').auto()
> When 'sev' is auto, then it won't be built. That seems strange. The
> auto-detection part is missing! I did you test this on a system that
> doesn't have libsev installed system-wide?

My testing environment had libsev installed system-wide. Thanks for 
pointing this out.

>
> I guess the auto-detection would look something like:
>
>    cargo = find_program('cargo', required: true)
>
>    if not get_option('sev').auto() or cargo.found()
>        ...
>
> That way 'sev' is only built automatically on systems that have cargo
> installed.
>
>> +  sev = dependency('sev',
>> +                   method: 'pkg-config',
>> +                   required: get_option('sev'))
>> +endif
> If you update the auto logic, see the documentation about fallbacks to
> subprojects for optional dependencies:
> https://mesonbuild.com/Wrap-dependency-system-manual.html#provide-section
>
> It might be necessary to add dependency(..., fallback='sev').

Noted. Thanks!

>
>> +
>>   vde = not_found
>>   if not get_option('vde').auto() or have_system or have_tools
>>     vde = cc.find_library('vdeplug', has_headers: ['libvdeplug.h'],
>> @@ -4331,6 +4338,7 @@ summary_info += {'libudev':           libudev}
>>   # Dummy dependency, keep .found()
>>   summary_info += {'FUSE lseek':        fuse_lseek.found()}
>>   summary_info += {'selinux':           selinux}
>> +summary_info += {'sev':               sev}
>>   summary_info += {'libdw':             libdw}
>>   summary(summary_info, bool_yn: true, section: 'Dependencies')
>>   
>> diff --git a/meson_options.txt b/meson_options.txt
>> index 57e265c871..5b8d283717 100644
>> --- a/meson_options.txt
>> +++ b/meson_options.txt
>> @@ -204,6 +204,8 @@ option('sdl_image', type : 'feature', value : 'auto',
>>          description: 'SDL Image support for icons')
>>   option('seccomp', type : 'feature', value : 'auto',
>>          description: 'seccomp support')
>> +option('sev', type : 'feature', value : 'auto',
>> +        description: 'Rust AMD SEV library')
>>   option('smartcard', type : 'feature', value : 'auto',
>>          description: 'CA smartcard emulation support')
>>   option('snappy', type : 'feature', value : 'auto',
>> diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
>> index e4b46d5715..e585a548fa 100644
>> --- a/scripts/meson-buildoptions.sh
>> +++ b/scripts/meson-buildoptions.sh
>> @@ -161,6 +161,7 @@ meson_options_help() {
>>     printf "%s\n" '  sdl-image       SDL Image support for icons'
>>     printf "%s\n" '  seccomp         seccomp support'
>>     printf "%s\n" '  selinux         SELinux support in qemu-nbd'
>> +  printf "%s\n" '  sev             SEV library support'
>>     printf "%s\n" '  slirp           libslirp user mode network backend support'
>>     printf "%s\n" '  slirp-smbd      use smbd (at path --smbd=*) in slirp networking'
>>     printf "%s\n" '  smartcard       CA smartcard emulation support'
>> @@ -440,6 +441,8 @@ _meson_option_parse() {
>>       --disable-seccomp) printf "%s" -Dseccomp=disabled ;;
>>       --enable-selinux) printf "%s" -Dselinux=enabled ;;
>>       --disable-selinux) printf "%s" -Dselinux=disabled ;;
>> +    --enable-sev) printf "%s" -Dsev=enabled ;;
>> +    --disable-sev) printf "%s" -Dsev=disabled ;;
>>       --enable-slirp) printf "%s" -Dslirp=enabled ;;
>>       --disable-slirp) printf "%s" -Dslirp=disabled ;;
>>       --enable-slirp-smbd) printf "%s" -Dslirp_smbd=enabled ;;
>> diff --git a/subprojects/sev.wrap b/subprojects/sev.wrap
>> new file mode 100644
>> index 0000000000..5be1faccf6
>> --- /dev/null
>> +++ b/subprojects/sev.wrap
>> @@ -0,0 +1,6 @@
>> +[wrap-git]
>> +url = https://github.com/tylerfanelli/sev
>> +revision = b81b1da5df50055600a5b0349b0c4afda677cccb
>> +
>> +[provide]
>> +sev = sev_dep
>> diff --git a/target/i386/meson.build b/target/i386/meson.build
>> index 6f1036d469..8972a4fb17 100644
>> --- a/target/i386/meson.build
>> +++ b/target/i386/meson.build
>> @@ -20,7 +20,7 @@ i386_system_ss.add(files(
>>     'monitor.c',
>>     'cpu-sysemu.c',
>>   ))
>> -i386_system_ss.add(when: 'CONFIG_SEV', if_true: files('sev.c'), if_false: files('sev-sysemu-stub.c'))
>> +i386_system_ss.add(when: 'CONFIG_SEV', if_true: [sev, files('sev.c')], if_false: files('sev-sysemu-stub.c'))
>>   
>>   i386_user_ss = ss.source_set()
>>   
>> -- 
>> 2.40.1
>>
Tyler
Manos Pitsidianakis Oct. 13, 2023, 6:09 p.m. UTC | #6
Hello Tyler!

With Rust stable 1.72.1, I get:

error: unneeded `return` statement
   --> tests/launch.rs:103:26
    |
103 |         VcpuExit::Hlt => return,
    |                          ^^^^^^
    |
    = help: for further information visit
https://rust-lang.github.io/rust-clippy/master/index.html#needless_return
    = note: `-D clippy::needless-return` implied by `-D warnings`
help: replace `return` with a unit value
    |
103 |         VcpuExit::Hlt => (),
    |                          ~~

error: could not compile `sev` (test "launch") due to previous error
warning: build failed, waiting for other jobs to finish...


When doing make.

--
Manos

On Wed, 11 Oct 2023 at 06:11, Tyler Fanelli <tfanelli@redhat.com> wrote:
>
> On 10/5/23 11:54 AM, Stefan Hajnoczi wrote:
> > On Wed, Oct 04, 2023 at 04:34:10PM -0400, Tyler Fanelli wrote:
> >> The Rust sev library provides a C API for the AMD SEV launch ioctls, as
> >> well as the ability to build with meson. Add the Rust sev library as a
> >> QEMU subproject with the goal of outsourcing all SEV launch ioctls to C
> >> APIs provided by it.
> >>
> >> Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
> >> ---
> >>   meson.build                   | 8 ++++++++
> >>   meson_options.txt             | 2 ++
> >>   scripts/meson-buildoptions.sh | 3 +++
> >>   subprojects/sev.wrap          | 6 ++++++
> >>   target/i386/meson.build       | 2 +-
> >>   5 files changed, 20 insertions(+), 1 deletion(-)
> >>   create mode 100644 subprojects/sev.wrap
> >>
> >> diff --git a/meson.build b/meson.build
> >> index 20ceeb8158..8a17c29de8 100644
> >> --- a/meson.build
> >> +++ b/meson.build
> >> @@ -960,6 +960,13 @@ if not get_option('slirp').auto() or have_system
> >>     endif
> >>   endif
> >>
> >> +sev = not_found
> >> +if not get_option('sev').auto()
> > When 'sev' is auto, then it won't be built. That seems strange. The
> > auto-detection part is missing! I did you test this on a system that
> > doesn't have libsev installed system-wide?
>
> My testing environment had libsev installed system-wide. Thanks for
> pointing this out.
>
> >
> > I guess the auto-detection would look something like:
> >
> >    cargo = find_program('cargo', required: true)
> >
> >    if not get_option('sev').auto() or cargo.found()
> >        ...
> >
> > That way 'sev' is only built automatically on systems that have cargo
> > installed.
> >
> >> +  sev = dependency('sev',
> >> +                   method: 'pkg-config',
> >> +                   required: get_option('sev'))
> >> +endif
> > If you update the auto logic, see the documentation about fallbacks to
> > subprojects for optional dependencies:
> > https://mesonbuild.com/Wrap-dependency-system-manual.html#provide-section
> >
> > It might be necessary to add dependency(..., fallback='sev').
>
> Noted. Thanks!
>
> >
> >> +
> >>   vde = not_found
> >>   if not get_option('vde').auto() or have_system or have_tools
> >>     vde = cc.find_library('vdeplug', has_headers: ['libvdeplug.h'],
> >> @@ -4331,6 +4338,7 @@ summary_info += {'libudev':           libudev}
> >>   # Dummy dependency, keep .found()
> >>   summary_info += {'FUSE lseek':        fuse_lseek.found()}
> >>   summary_info += {'selinux':           selinux}
> >> +summary_info += {'sev':               sev}
> >>   summary_info += {'libdw':             libdw}
> >>   summary(summary_info, bool_yn: true, section: 'Dependencies')
> >>
> >> diff --git a/meson_options.txt b/meson_options.txt
> >> index 57e265c871..5b8d283717 100644
> >> --- a/meson_options.txt
> >> +++ b/meson_options.txt
> >> @@ -204,6 +204,8 @@ option('sdl_image', type : 'feature', value : 'auto',
> >>          description: 'SDL Image support for icons')
> >>   option('seccomp', type : 'feature', value : 'auto',
> >>          description: 'seccomp support')
> >> +option('sev', type : 'feature', value : 'auto',
> >> +        description: 'Rust AMD SEV library')
> >>   option('smartcard', type : 'feature', value : 'auto',
> >>          description: 'CA smartcard emulation support')
> >>   option('snappy', type : 'feature', value : 'auto',
> >> diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
> >> index e4b46d5715..e585a548fa 100644
> >> --- a/scripts/meson-buildoptions.sh
> >> +++ b/scripts/meson-buildoptions.sh
> >> @@ -161,6 +161,7 @@ meson_options_help() {
> >>     printf "%s\n" '  sdl-image       SDL Image support for icons'
> >>     printf "%s\n" '  seccomp         seccomp support'
> >>     printf "%s\n" '  selinux         SELinux support in qemu-nbd'
> >> +  printf "%s\n" '  sev             SEV library support'
> >>     printf "%s\n" '  slirp           libslirp user mode network backend support'
> >>     printf "%s\n" '  slirp-smbd      use smbd (at path --smbd=*) in slirp networking'
> >>     printf "%s\n" '  smartcard       CA smartcard emulation support'
> >> @@ -440,6 +441,8 @@ _meson_option_parse() {
> >>       --disable-seccomp) printf "%s" -Dseccomp=disabled ;;
> >>       --enable-selinux) printf "%s" -Dselinux=enabled ;;
> >>       --disable-selinux) printf "%s" -Dselinux=disabled ;;
> >> +    --enable-sev) printf "%s" -Dsev=enabled ;;
> >> +    --disable-sev) printf "%s" -Dsev=disabled ;;
> >>       --enable-slirp) printf "%s" -Dslirp=enabled ;;
> >>       --disable-slirp) printf "%s" -Dslirp=disabled ;;
> >>       --enable-slirp-smbd) printf "%s" -Dslirp_smbd=enabled ;;
> >> diff --git a/subprojects/sev.wrap b/subprojects/sev.wrap
> >> new file mode 100644
> >> index 0000000000..5be1faccf6
> >> --- /dev/null
> >> +++ b/subprojects/sev.wrap
> >> @@ -0,0 +1,6 @@
> >> +[wrap-git]
> >> +url = https://github.com/tylerfanelli/sev
> >> +revision = b81b1da5df50055600a5b0349b0c4afda677cccb
> >> +
> >> +[provide]
> >> +sev = sev_dep
> >> diff --git a/target/i386/meson.build b/target/i386/meson.build
> >> index 6f1036d469..8972a4fb17 100644
> >> --- a/target/i386/meson.build
> >> +++ b/target/i386/meson.build
> >> @@ -20,7 +20,7 @@ i386_system_ss.add(files(
> >>     'monitor.c',
> >>     'cpu-sysemu.c',
> >>   ))
> >> -i386_system_ss.add(when: 'CONFIG_SEV', if_true: files('sev.c'), if_false: files('sev-sysemu-stub.c'))
> >> +i386_system_ss.add(when: 'CONFIG_SEV', if_true: [sev, files('sev.c')], if_false: files('sev-sysemu-stub.c'))
> >>
> >>   i386_user_ss = ss.source_set()
> >>
> >> --
> >> 2.40.1
> >>
> Tyler
>
>
Tyler Fanelli Oct. 13, 2023, 6:20 p.m. UTC | #7
Hi Manos,

Thanks for the heads up, I was using rust 1.71.1. Will update the series 
with 1.72.1

Stefan, Philippe, or Daniel: is there a specific policy for the Rust 
version we should be developing on for crates in qemu?

Tyler

On 10/13/23 2:09 PM, Manos Pitsidianakis wrote:
> Hello Tyler!
>
> With Rust stable 1.72.1, I get:
>
> error: unneeded `return` statement
>     --> tests/launch.rs:103:26
>      |
> 103 |         VcpuExit::Hlt => return,
>      |                          ^^^^^^
>      |
>      = help: for further information visit
> https://rust-lang.github.io/rust-clippy/master/index.html#needless_return
>      = note: `-D clippy::needless-return` implied by `-D warnings`
> help: replace `return` with a unit value
>      |
> 103 |         VcpuExit::Hlt => (),
>      |                          ~~
>
> error: could not compile `sev` (test "launch") due to previous error
> warning: build failed, waiting for other jobs to finish...
>
>
> When doing make.
>
> --
> Manos
>
> On Wed, 11 Oct 2023 at 06:11, Tyler Fanelli <tfanelli@redhat.com> wrote:
>> On 10/5/23 11:54 AM, Stefan Hajnoczi wrote:
>>> On Wed, Oct 04, 2023 at 04:34:10PM -0400, Tyler Fanelli wrote:
>>>> The Rust sev library provides a C API for the AMD SEV launch ioctls, as
>>>> well as the ability to build with meson. Add the Rust sev library as a
>>>> QEMU subproject with the goal of outsourcing all SEV launch ioctls to C
>>>> APIs provided by it.
>>>>
>>>> Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
>>>> ---
>>>>    meson.build                   | 8 ++++++++
>>>>    meson_options.txt             | 2 ++
>>>>    scripts/meson-buildoptions.sh | 3 +++
>>>>    subprojects/sev.wrap          | 6 ++++++
>>>>    target/i386/meson.build       | 2 +-
>>>>    5 files changed, 20 insertions(+), 1 deletion(-)
>>>>    create mode 100644 subprojects/sev.wrap
>>>>
>>>> diff --git a/meson.build b/meson.build
>>>> index 20ceeb8158..8a17c29de8 100644
>>>> --- a/meson.build
>>>> +++ b/meson.build
>>>> @@ -960,6 +960,13 @@ if not get_option('slirp').auto() or have_system
>>>>      endif
>>>>    endif
>>>>
>>>> +sev = not_found
>>>> +if not get_option('sev').auto()
>>> When 'sev' is auto, then it won't be built. That seems strange. The
>>> auto-detection part is missing! I did you test this on a system that
>>> doesn't have libsev installed system-wide?
>> My testing environment had libsev installed system-wide. Thanks for
>> pointing this out.
>>
>>> I guess the auto-detection would look something like:
>>>
>>>     cargo = find_program('cargo', required: true)
>>>
>>>     if not get_option('sev').auto() or cargo.found()
>>>         ...
>>>
>>> That way 'sev' is only built automatically on systems that have cargo
>>> installed.
>>>
>>>> +  sev = dependency('sev',
>>>> +                   method: 'pkg-config',
>>>> +                   required: get_option('sev'))
>>>> +endif
>>> If you update the auto logic, see the documentation about fallbacks to
>>> subprojects for optional dependencies:
>>> https://mesonbuild.com/Wrap-dependency-system-manual.html#provide-section
>>>
>>> It might be necessary to add dependency(..., fallback='sev').
>> Noted. Thanks!
>>
>>>> +
>>>>    vde = not_found
>>>>    if not get_option('vde').auto() or have_system or have_tools
>>>>      vde = cc.find_library('vdeplug', has_headers: ['libvdeplug.h'],
>>>> @@ -4331,6 +4338,7 @@ summary_info += {'libudev':           libudev}
>>>>    # Dummy dependency, keep .found()
>>>>    summary_info += {'FUSE lseek':        fuse_lseek.found()}
>>>>    summary_info += {'selinux':           selinux}
>>>> +summary_info += {'sev':               sev}
>>>>    summary_info += {'libdw':             libdw}
>>>>    summary(summary_info, bool_yn: true, section: 'Dependencies')
>>>>
>>>> diff --git a/meson_options.txt b/meson_options.txt
>>>> index 57e265c871..5b8d283717 100644
>>>> --- a/meson_options.txt
>>>> +++ b/meson_options.txt
>>>> @@ -204,6 +204,8 @@ option('sdl_image', type : 'feature', value : 'auto',
>>>>           description: 'SDL Image support for icons')
>>>>    option('seccomp', type : 'feature', value : 'auto',
>>>>           description: 'seccomp support')
>>>> +option('sev', type : 'feature', value : 'auto',
>>>> +        description: 'Rust AMD SEV library')
>>>>    option('smartcard', type : 'feature', value : 'auto',
>>>>           description: 'CA smartcard emulation support')
>>>>    option('snappy', type : 'feature', value : 'auto',
>>>> diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
>>>> index e4b46d5715..e585a548fa 100644
>>>> --- a/scripts/meson-buildoptions.sh
>>>> +++ b/scripts/meson-buildoptions.sh
>>>> @@ -161,6 +161,7 @@ meson_options_help() {
>>>>      printf "%s\n" '  sdl-image       SDL Image support for icons'
>>>>      printf "%s\n" '  seccomp         seccomp support'
>>>>      printf "%s\n" '  selinux         SELinux support in qemu-nbd'
>>>> +  printf "%s\n" '  sev             SEV library support'
>>>>      printf "%s\n" '  slirp           libslirp user mode network backend support'
>>>>      printf "%s\n" '  slirp-smbd      use smbd (at path --smbd=*) in slirp networking'
>>>>      printf "%s\n" '  smartcard       CA smartcard emulation support'
>>>> @@ -440,6 +441,8 @@ _meson_option_parse() {
>>>>        --disable-seccomp) printf "%s" -Dseccomp=disabled ;;
>>>>        --enable-selinux) printf "%s" -Dselinux=enabled ;;
>>>>        --disable-selinux) printf "%s" -Dselinux=disabled ;;
>>>> +    --enable-sev) printf "%s" -Dsev=enabled ;;
>>>> +    --disable-sev) printf "%s" -Dsev=disabled ;;
>>>>        --enable-slirp) printf "%s" -Dslirp=enabled ;;
>>>>        --disable-slirp) printf "%s" -Dslirp=disabled ;;
>>>>        --enable-slirp-smbd) printf "%s" -Dslirp_smbd=enabled ;;
>>>> diff --git a/subprojects/sev.wrap b/subprojects/sev.wrap
>>>> new file mode 100644
>>>> index 0000000000..5be1faccf6
>>>> --- /dev/null
>>>> +++ b/subprojects/sev.wrap
>>>> @@ -0,0 +1,6 @@
>>>> +[wrap-git]
>>>> +url = https://github.com/tylerfanelli/sev
>>>> +revision = b81b1da5df50055600a5b0349b0c4afda677cccb
>>>> +
>>>> +[provide]
>>>> +sev = sev_dep
>>>> diff --git a/target/i386/meson.build b/target/i386/meson.build
>>>> index 6f1036d469..8972a4fb17 100644
>>>> --- a/target/i386/meson.build
>>>> +++ b/target/i386/meson.build
>>>> @@ -20,7 +20,7 @@ i386_system_ss.add(files(
>>>>      'monitor.c',
>>>>      'cpu-sysemu.c',
>>>>    ))
>>>> -i386_system_ss.add(when: 'CONFIG_SEV', if_true: files('sev.c'), if_false: files('sev-sysemu-stub.c'))
>>>> +i386_system_ss.add(when: 'CONFIG_SEV', if_true: [sev, files('sev.c')], if_false: files('sev-sysemu-stub.c'))
>>>>
>>>>    i386_user_ss = ss.source_set()
>>>>
>>>> --
>>>> 2.40.1
>>>>
>> Tyler
>>
>>
Daniel P. Berrangé Oct. 16, 2023, 9:16 a.m. UTC | #8
On Fri, Oct 13, 2023 at 02:20:16PM -0400, Tyler Fanelli wrote:
> Hi Manos,
> 
> Thanks for the heads up, I was using rust 1.71.1. Will update the series
> with 1.72.1
> 
> Stefan, Philippe, or Daniel: is there a specific policy for the Rust version
> we should be developing on for crates in qemu?

There are a couple of dimensions to this.

First is the matter of what operating system and architecture pairs are
supported as targets for the Rust toolchain, and the standard library.
We'll need both to work of course.

Second there is the matter of what versions of Rust are shipped in the
various operating systems currently.

In a previous discussion there was a wiki page fleshed out with this
info:

   https://wiki.qemu.org/RustInQemu

but the min versions are certainly out of date now.

Third there is the question of whether distros have facility for pulling
in newer toolchain versions, and if so should we be willing to use them.
This is relevant for the long life distros like RHEL, which might ship
with a variety of Rust versions. Historically we've been very conservative
but with Python last year we adopted a more aggressive policy of being
willing to take any newer version available from the distro vendor, not
merely the oldest baseline. I suspect we'll want a similar approach with
rust.

Anyhow, I think you could probably start by updatnig that RustInQemu
wiki page so that it reflects the current state of the world in terms
of support tiers and versions.

> 
> Tyler
> 
> On 10/13/23 2:09 PM, Manos Pitsidianakis wrote:
> > Hello Tyler!
> > 
> > With Rust stable 1.72.1, I get:
> > 
> > error: unneeded `return` statement
> >     --> tests/launch.rs:103:26
> >      |
> > 103 |         VcpuExit::Hlt => return,
> >      |                          ^^^^^^
> >      |
> >      = help: for further information visit
> > https://rust-lang.github.io/rust-clippy/master/index.html#needless_return
> >      = note: `-D clippy::needless-return` implied by `-D warnings`
> > help: replace `return` with a unit value
> >      |
> > 103 |         VcpuExit::Hlt => (),
> >      |                          ~~
> > 
> > error: could not compile `sev` (test "launch") due to previous error
> > warning: build failed, waiting for other jobs to finish...
> > 
> > 
> > When doing make.
> > 
> > --
> > Manos
> > 
> > On Wed, 11 Oct 2023 at 06:11, Tyler Fanelli <tfanelli@redhat.com> wrote:
> > > On 10/5/23 11:54 AM, Stefan Hajnoczi wrote:
> > > > On Wed, Oct 04, 2023 at 04:34:10PM -0400, Tyler Fanelli wrote:
> > > > > The Rust sev library provides a C API for the AMD SEV launch ioctls, as
> > > > > well as the ability to build with meson. Add the Rust sev library as a
> > > > > QEMU subproject with the goal of outsourcing all SEV launch ioctls to C
> > > > > APIs provided by it.
> > > > > 
> > > > > Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
> > > > > ---
> > > > >    meson.build                   | 8 ++++++++
> > > > >    meson_options.txt             | 2 ++
> > > > >    scripts/meson-buildoptions.sh | 3 +++
> > > > >    subprojects/sev.wrap          | 6 ++++++
> > > > >    target/i386/meson.build       | 2 +-
> > > > >    5 files changed, 20 insertions(+), 1 deletion(-)
> > > > >    create mode 100644 subprojects/sev.wrap
> > > > > 
> > > > > diff --git a/meson.build b/meson.build
> > > > > index 20ceeb8158..8a17c29de8 100644
> > > > > --- a/meson.build
> > > > > +++ b/meson.build
> > > > > @@ -960,6 +960,13 @@ if not get_option('slirp').auto() or have_system
> > > > >      endif
> > > > >    endif
> > > > > 
> > > > > +sev = not_found
> > > > > +if not get_option('sev').auto()
> > > > When 'sev' is auto, then it won't be built. That seems strange. The
> > > > auto-detection part is missing! I did you test this on a system that
> > > > doesn't have libsev installed system-wide?
> > > My testing environment had libsev installed system-wide. Thanks for
> > > pointing this out.
> > > 
> > > > I guess the auto-detection would look something like:
> > > > 
> > > >     cargo = find_program('cargo', required: true)
> > > > 
> > > >     if not get_option('sev').auto() or cargo.found()
> > > >         ...
> > > > 
> > > > That way 'sev' is only built automatically on systems that have cargo
> > > > installed.
> > > > 
> > > > > +  sev = dependency('sev',
> > > > > +                   method: 'pkg-config',
> > > > > +                   required: get_option('sev'))
> > > > > +endif
> > > > If you update the auto logic, see the documentation about fallbacks to
> > > > subprojects for optional dependencies:
> > > > https://mesonbuild.com/Wrap-dependency-system-manual.html#provide-section
> > > > 
> > > > It might be necessary to add dependency(..., fallback='sev').
> > > Noted. Thanks!
> > > 
> > > > > +
> > > > >    vde = not_found
> > > > >    if not get_option('vde').auto() or have_system or have_tools
> > > > >      vde = cc.find_library('vdeplug', has_headers: ['libvdeplug.h'],
> > > > > @@ -4331,6 +4338,7 @@ summary_info += {'libudev':           libudev}
> > > > >    # Dummy dependency, keep .found()
> > > > >    summary_info += {'FUSE lseek':        fuse_lseek.found()}
> > > > >    summary_info += {'selinux':           selinux}
> > > > > +summary_info += {'sev':               sev}
> > > > >    summary_info += {'libdw':             libdw}
> > > > >    summary(summary_info, bool_yn: true, section: 'Dependencies')
> > > > > 
> > > > > diff --git a/meson_options.txt b/meson_options.txt
> > > > > index 57e265c871..5b8d283717 100644
> > > > > --- a/meson_options.txt
> > > > > +++ b/meson_options.txt
> > > > > @@ -204,6 +204,8 @@ option('sdl_image', type : 'feature', value : 'auto',
> > > > >           description: 'SDL Image support for icons')
> > > > >    option('seccomp', type : 'feature', value : 'auto',
> > > > >           description: 'seccomp support')
> > > > > +option('sev', type : 'feature', value : 'auto',
> > > > > +        description: 'Rust AMD SEV library')
> > > > >    option('smartcard', type : 'feature', value : 'auto',
> > > > >           description: 'CA smartcard emulation support')
> > > > >    option('snappy', type : 'feature', value : 'auto',
> > > > > diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
> > > > > index e4b46d5715..e585a548fa 100644
> > > > > --- a/scripts/meson-buildoptions.sh
> > > > > +++ b/scripts/meson-buildoptions.sh
> > > > > @@ -161,6 +161,7 @@ meson_options_help() {
> > > > >      printf "%s\n" '  sdl-image       SDL Image support for icons'
> > > > >      printf "%s\n" '  seccomp         seccomp support'
> > > > >      printf "%s\n" '  selinux         SELinux support in qemu-nbd'
> > > > > +  printf "%s\n" '  sev             SEV library support'
> > > > >      printf "%s\n" '  slirp           libslirp user mode network backend support'
> > > > >      printf "%s\n" '  slirp-smbd      use smbd (at path --smbd=*) in slirp networking'
> > > > >      printf "%s\n" '  smartcard       CA smartcard emulation support'
> > > > > @@ -440,6 +441,8 @@ _meson_option_parse() {
> > > > >        --disable-seccomp) printf "%s" -Dseccomp=disabled ;;
> > > > >        --enable-selinux) printf "%s" -Dselinux=enabled ;;
> > > > >        --disable-selinux) printf "%s" -Dselinux=disabled ;;
> > > > > +    --enable-sev) printf "%s" -Dsev=enabled ;;
> > > > > +    --disable-sev) printf "%s" -Dsev=disabled ;;
> > > > >        --enable-slirp) printf "%s" -Dslirp=enabled ;;
> > > > >        --disable-slirp) printf "%s" -Dslirp=disabled ;;
> > > > >        --enable-slirp-smbd) printf "%s" -Dslirp_smbd=enabled ;;
> > > > > diff --git a/subprojects/sev.wrap b/subprojects/sev.wrap
> > > > > new file mode 100644
> > > > > index 0000000000..5be1faccf6
> > > > > --- /dev/null
> > > > > +++ b/subprojects/sev.wrap
> > > > > @@ -0,0 +1,6 @@
> > > > > +[wrap-git]
> > > > > +url = https://github.com/tylerfanelli/sev
> > > > > +revision = b81b1da5df50055600a5b0349b0c4afda677cccb
> > > > > +
> > > > > +[provide]
> > > > > +sev = sev_dep
> > > > > diff --git a/target/i386/meson.build b/target/i386/meson.build
> > > > > index 6f1036d469..8972a4fb17 100644
> > > > > --- a/target/i386/meson.build
> > > > > +++ b/target/i386/meson.build
> > > > > @@ -20,7 +20,7 @@ i386_system_ss.add(files(
> > > > >      'monitor.c',
> > > > >      'cpu-sysemu.c',
> > > > >    ))
> > > > > -i386_system_ss.add(when: 'CONFIG_SEV', if_true: files('sev.c'), if_false: files('sev-sysemu-stub.c'))
> > > > > +i386_system_ss.add(when: 'CONFIG_SEV', if_true: [sev, files('sev.c')], if_false: files('sev-sysemu-stub.c'))
> > > > > 
> > > > >    i386_user_ss = ss.source_set()
> > > > > 
> > > > > --
> > > > > 2.40.1
> > > > > 
> > > Tyler
> > > 
> > > 
> 

With regards,
Daniel
Philippe Mathieu-Daudé Oct. 16, 2023, 1:38 p.m. UTC | #9
On 16/10/23 11:16, Daniel P. Berrangé wrote:
> On Fri, Oct 13, 2023 at 02:20:16PM -0400, Tyler Fanelli wrote:
>> Hi Manos,
>>
>> Thanks for the heads up, I was using rust 1.71.1. Will update the series
>> with 1.72.1
>>
>> Stefan, Philippe, or Daniel: is there a specific policy for the Rust version
>> we should be developing on for crates in qemu?
> 
> There are a couple of dimensions to this.
> 
> First is the matter of what operating system and architecture pairs are
> supported as targets for the Rust toolchain, and the standard library.
> We'll need both to work of course.
> 
> Second there is the matter of what versions of Rust are shipped in the
> various operating systems currently.
> 
> In a previous discussion there was a wiki page fleshed out with this
> info:
> 
>     https://wiki.qemu.org/RustInQemu
> 
> but the min versions are certainly out of date now.
> 
> Third there is the question of whether distros have facility for pulling
> in newer toolchain versions, and if so should we be willing to use them.
> This is relevant for the long life distros like RHEL, which might ship
> with a variety of Rust versions. Historically we've been very conservative
> but with Python last year we adopted a more aggressive policy of being
> willing to take any newer version available from the distro vendor, not
> merely the oldest baseline. I suspect we'll want a similar approach with
> rust.

Cc'ing distrib package maintainers to have their feedback on this.

> Anyhow, I think you could probably start by updatnig that RustInQemu
> wiki page so that it reflects the current state of the world in terms
> of support tiers and versions.
Stefan Hajnoczi Oct. 16, 2023, 1:51 p.m. UTC | #10
On Mon, 16 Oct 2023 at 05:17, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Fri, Oct 13, 2023 at 02:20:16PM -0400, Tyler Fanelli wrote:
> > Hi Manos,
> >
> > Thanks for the heads up, I was using rust 1.71.1. Will update the series
> > with 1.72.1
> >
> > Stefan, Philippe, or Daniel: is there a specific policy for the Rust version
> > we should be developing on for crates in qemu?
>
> There are a couple of dimensions to this.
>
> First is the matter of what operating system and architecture pairs are
> supported as targets for the Rust toolchain, and the standard library.
> We'll need both to work of course.
>
> Second there is the matter of what versions of Rust are shipped in the
> various operating systems currently.
>
> In a previous discussion there was a wiki page fleshed out with this
> info:
>
>    https://wiki.qemu.org/RustInQemu
>
> but the min versions are certainly out of date now.
>
> Third there is the question of whether distros have facility for pulling
> in newer toolchain versions, and if so should we be willing to use them.
> This is relevant for the long life distros like RHEL, which might ship
> with a variety of Rust versions. Historically we've been very conservative
> but with Python last year we adopted a more aggressive policy of being
> willing to take any newer version available from the distro vendor, not
> merely the oldest baseline. I suspect we'll want a similar approach with
> rust.
>
> Anyhow, I think you could probably start by updatnig that RustInQemu
> wiki page so that it reflects the current state of the world in terms
> of support tiers and versions.

I have two specific scenarios in mind that should build successfully:
1. On the oldest operating system version supported by QEMU where Rust
code previously built successfully. In other words, once QEMU Rust
code starts building, it keeps building on that operating system
version with the distro's Rust toolchain until QEMU increases the
minimum supported distro version. The rationale here is for both
end-users that build from source and for distro packagers to be able
to build QEMU easily.
2. On the latest Rust stable toolchain from rustup. The rationale is
that developers often use rustup instead of the distro toolchain, so
it's nice to support it as a convenience.

Stefan
Daniel P. Berrangé March 5, 2024, 1:47 p.m. UTC | #11
On Wed, Oct 04, 2023 at 04:34:10PM -0400, Tyler Fanelli wrote:
> The Rust sev library provides a C API for the AMD SEV launch ioctls, as
> well as the ability to build with meson. Add the Rust sev library as a
> QEMU subproject with the goal of outsourcing all SEV launch ioctls to C
> APIs provided by it.
> 
> Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
> ---
>  meson.build                   | 8 ++++++++
>  meson_options.txt             | 2 ++
>  scripts/meson-buildoptions.sh | 3 +++
>  subprojects/sev.wrap          | 6 ++++++
>  target/i386/meson.build       | 2 +-
>  5 files changed, 20 insertions(+), 1 deletion(-)
>  create mode 100644 subprojects/sev.wrap
> 
> diff --git a/meson.build b/meson.build
> index 20ceeb8158..8a17c29de8 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -960,6 +960,13 @@ if not get_option('slirp').auto() or have_system
>    endif
>  endif
>  
> +sev = not_found
> +if not get_option('sev').auto()
> +  sev = dependency('sev',
> +                   method: 'pkg-config',
> +                   required: get_option('sev'))
> +endif
> +

I've revisited this series and tested it now. As Stefan already
mentioned, this logic is flawed.

Currently QEMU is self-contained for SEV support. If we swap to the
sev crate, then we introduce libsev.so as a build time system library
dependancy, and it is highly unlikely that many existing distros will
add the package. IOW we'll cause a regression for users.

Thus we need to be able to *statically* link to the sev crate when it
is not available in the system.

I had a crack at changing this patch to support that and came up with
this diff on top of your patch here:

diff --git a/meson.build b/meson.build
index 1beb9e9f40..d6aba3fd7d 100644
--- a/meson.build
+++ b/meson.build
@@ -1116,12 +1116,6 @@ if not get_option('slirp').auto() or have_system
   endif
 endif
 
-sev = not_found
-if not get_option('sev').auto()
-  sev = dependency('sev',
-                   method: 'pkg-config',
-                   required: get_option('sev'))
-endif
 
 vde = not_found
 if not get_option('vde').auto() or have_system or have_tools
@@ -3003,6 +2997,7 @@ ignored = [ 'TARGET_XML_FILES', 'TARGET_ABI_DIR', 'TARGET_ARCH' ]
 default_targets = 'CONFIG_DEFAULT_TARGETS' in config_host
 actual_target_dirs = []
 fdt_required = []
+sev_required = []
 foreach target : target_dirs
   config_target = { 'TARGET_NAME': target.split('-')[0] }
   if target.endswith('linux-user')
@@ -3124,6 +3119,9 @@ foreach target : target_dirs
     foreach k, v: config_devices
       config_devices_data.set(k, 1)
     endforeach
+    if 'CONFIG_SEV' in config_devices
+        sev_required += target
+    endif
     config_devices_mak_list += config_devices_mak
     config_devices_h += {target: configure_file(output: target + '-config-devices.h',
                                                 configuration: config_devices_data)}
@@ -3206,6 +3204,39 @@ if have_libvduse
   libvduse = libvduse_proj.get_variable('libvduse_dep')
 endif
 
+sev = not_found
+sev_opt = get_option('sev')
+if sev_required.length() > 0 or sev_opt == 'enabled'
+  if sev_opt == 'disabled'
+    error('sev disabled but required by targets ' + ', '.join(fdt_required))
+  endif
+
+  if sev_opt in ['enabled', 'auto', 'system']
+      if get_option('wrap_mode') == 'nodownload'
+          sev_opt = 'system'
+      endif
+      sev = dependency('sev',
+                       method: 'pkg-config',
+                       required: sev_opt == 'system')
+      if sev.found()
+          sev_opt = 'system'
+      elif sev_opt == 'system'
+          error('system libsev requested')
+      else
+          sev_opt = 'internal'
+          sev = not_found
+      endif
+  endif
+  if not sev.found()
+      assert(sev_opt == 'internal')
+      libsev_proj = subproject('sev', required: true,
+                               default_options: ['default_library=static'])
+      sev = libsev_proj.get_variable('sev_dep')
+  endif
+else
+  sev_opt = 'disabled'
+endif
+
 #####################
 # Generated sources #
 #####################
@@ -4453,7 +4484,7 @@ summary_info += {'libudev':           libudev}
 # Dummy dependency, keep .found()
 summary_info += {'FUSE lseek':        fuse_lseek.found()}
 summary_info += {'selinux':           selinux}
-summary_info += {'sev':               sev}
+summary_info += {'sev support':       sev_opt == 'disabled' ? false : sev_opt}
 summary_info += {'libdw':             libdw}
 if host_os == 'freebsd'
   summary_info += {'libinotify-kqueue': inotify}
diff --git a/meson_options.txt b/meson_options.txt
index 749fc87fd7..405d1abfd4 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -208,8 +208,6 @@ option('sdl_image', type : 'feature', value : 'auto',
        description: 'SDL Image support for icons')
 option('seccomp', type : 'feature', value : 'auto',
        description: 'seccomp support')
-option('sev', type : 'feature', value : 'auto',
-        description: 'Rust AMD SEV library')
 option('smartcard', type : 'feature', value : 'auto',
        description: 'CA smartcard emulation support')
 option('snappy', type : 'feature', value : 'auto',
@@ -313,6 +311,9 @@ option('capstone', type: 'feature', value: 'auto',
 option('fdt', type: 'combo', value: 'auto',
        choices: ['disabled', 'enabled', 'auto', 'system', 'internal'],
        description: 'Whether and how to find the libfdt library')
+option('sev', type: 'combo', value: 'auto',
+       choices: ['disabled', 'enabled', 'auto', 'system', 'internal'],
+       description: 'Whether and how to find the libsev library')
 
 option('selinux', type: 'feature', value: 'auto',
        description: 'SELinux support in qemu-nbd')




The caveat is that this does not actually work because the way the
meson rules in the sev crate are written prevents building it as a
static library:

$ cd sev
$ meson build -Ddefault_library=static
The Meson build system
Version: 1.2.3

...snip...

src/meson.build:18:6: ERROR: Cannot link_whole a custom or Rust target 'libsev.a' into a static library 'sev'. Instead, pass individual object files with the "objects:" keyword argument if possible.


The problem is that it is running 'cargo-build.sh' to compile the
SEV crate, which creates a libsev.a static library. It is then
using that as an input to "library('sev',....)" which will also
want to create a new static library with the contents of the first
static library. Never mind that the 2nd library will also be called
libsev.so, meson simply doesn't support linking static libraries
into static libraries AFAICT. There's an open RFE for it that has
no recent attention.


I observe, however, that if cargo-build.sh has already created a
libsev.a static library, then there's no need to tell meson to
create another static library from that. We should be able to use
the first libsev.a directly.

So I tried this change to the 'sev' crate:

diff --git a/meson.build b/meson.build
index 2bf68c3..915ebfb 100644
--- a/meson.build
+++ b/meson.build
@@ -16,5 +16,10 @@ subdir('docs')
 subdir('include')
 subdir('src') # requires: include
 
-sev_dep = declare_dependency(include_directories: inc,
-                             link_with: lib)
+if get_option('default_library') != 'static'
+  sev_dep = declare_dependency(include_directories: inc,
+                               link_with: libsev_so)
+else
+  sev_dep = declare_dependency(include_directories: inc,
+                               link_with: libsev_a)
+endif
diff --git a/src/meson.build b/src/meson.build
index 30bfe49..0632303 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -2,25 +2,27 @@ cargo_build = find_program('cargo-build.sh')
 
 v = meson.project_version().split('.')
 
-staticlib_name = 'libsev.a'
+libsev_a_name = 'libsev.a'
 
-staticlib_target = custom_target(staticlib_name,
+libsev_a = custom_target(libsev_a_name,
   build_by_default : true,
   build_always_stale : true,
   command : [cargo_build, get_option('debug').to_string(),
              get_option('optimization'), meson.current_build_dir() / 'target',
              '@OUTPUT@'],
   console : true,
-  output : [staticlib_name])
+  output : [libsev_a_name])
 
 math = meson.get_compiler('c').find_library('m', required: true)
 
-lib = library('sev',
-  link_whole: staticlib_target,
-  dependencies: [math],
-  install: true,
-  soversion: meson.project_version())
-
+if get_option('default_library') != 'static'
+  libsev_so = shared_library('sev',
+    link_whole: libsev_a,
+    dependencies: [math],
+    install: true,
+    soversion: meson.project_version())
+endif
+  
 # generate pkg-config file
 
 import('pkgconfig').generate(libraries : ['-lsev'],


It is a little bit gross, but it seems to work making it possible to
static link to the sev crate from QEMU with my QEMU patch earlier.


Now, the second issue is that my patch to QEMU's meson.build where
I look for "CONFIG_SEV" is wrong. I've not tested whether it behaves
correctly on non-x86 hosts - basically I'm hoping that CONFIG_SEV is
*NOT* present if building qemu-system-x86_64 on an aarch64 host.

Assuming we get this logic correct though, this unblocks one issue
with getting this merged - Rust platform support.

We've said we want Rust platform support to be a match for QEMU's
platform support. We're probably pretty close, but still it is a
review stumbling block.

If, however, we demonstrate that we /only/ try to use libsev crate
when building on an x86_64 host, then we don't need to think about
Rust platform support in any detail. We know Rust is fully supported
on x86_64 on Linux, and we're not introducing any Rust dependency
for QEMU on other build target arches.

With regards,
Daniel
Philippe Mathieu-Daudé March 5, 2024, 3:40 p.m. UTC | #12
On 5/3/24 14:47, Daniel P. Berrangé wrote:
> On Wed, Oct 04, 2023 at 04:34:10PM -0400, Tyler Fanelli wrote:
>> The Rust sev library provides a C API for the AMD SEV launch ioctls, as
>> well as the ability to build with meson. Add the Rust sev library as a
>> QEMU subproject with the goal of outsourcing all SEV launch ioctls to C
>> APIs provided by it.
>>
>> Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
>> ---
>>   meson.build                   | 8 ++++++++
>>   meson_options.txt             | 2 ++
>>   scripts/meson-buildoptions.sh | 3 +++
>>   subprojects/sev.wrap          | 6 ++++++
>>   target/i386/meson.build       | 2 +-
>>   5 files changed, 20 insertions(+), 1 deletion(-)
>>   create mode 100644 subprojects/sev.wrap


> Now, the second issue is that my patch to QEMU's meson.build where
> I look for "CONFIG_SEV" is wrong. I've not tested whether it behaves
> correctly on non-x86 hosts - basically I'm hoping that CONFIG_SEV is
> *NOT* present if building qemu-system-x86_64 on an aarch64 host.

See hw/i386/Kconfig:

   config SEV
       bool
       ...
       depends on KVM

and meson.build ($cpu is the host):

   ...
elif cpu == 'x86'
   host_arch = 'i386'

if cpu in ['x86', 'x86_64']
   kvm_targets = ['i386-softmmu', 'x86_64-softmmu']

So SEV is only on selectable on x86 hosts, with KVM enabled.

> Assuming we get this logic correct though, this unblocks one issue
> with getting this merged - Rust platform support.
> 
> We've said we want Rust platform support to be a match for QEMU's
> platform support. We're probably pretty close, but still it is a
> review stumbling block.
> 
> If, however, we demonstrate that we /only/ try to use libsev crate
> when building on an x86_64 host, then we don't need to think about
> Rust platform support in any detail. We know Rust is fully supported
> on x86_64 on Linux, and we're not introducing any Rust dependency
> for QEMU on other build target arches.
> 
> With regards,
> Daniel
diff mbox series

Patch

diff --git a/meson.build b/meson.build
index 20ceeb8158..8a17c29de8 100644
--- a/meson.build
+++ b/meson.build
@@ -960,6 +960,13 @@  if not get_option('slirp').auto() or have_system
   endif
 endif
 
+sev = not_found
+if not get_option('sev').auto()
+  sev = dependency('sev',
+                   method: 'pkg-config',
+                   required: get_option('sev'))
+endif
+
 vde = not_found
 if not get_option('vde').auto() or have_system or have_tools
   vde = cc.find_library('vdeplug', has_headers: ['libvdeplug.h'],
@@ -4331,6 +4338,7 @@  summary_info += {'libudev':           libudev}
 # Dummy dependency, keep .found()
 summary_info += {'FUSE lseek':        fuse_lseek.found()}
 summary_info += {'selinux':           selinux}
+summary_info += {'sev':               sev}
 summary_info += {'libdw':             libdw}
 summary(summary_info, bool_yn: true, section: 'Dependencies')
 
diff --git a/meson_options.txt b/meson_options.txt
index 57e265c871..5b8d283717 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -204,6 +204,8 @@  option('sdl_image', type : 'feature', value : 'auto',
        description: 'SDL Image support for icons')
 option('seccomp', type : 'feature', value : 'auto',
        description: 'seccomp support')
+option('sev', type : 'feature', value : 'auto',
+        description: 'Rust AMD SEV library')
 option('smartcard', type : 'feature', value : 'auto',
        description: 'CA smartcard emulation support')
 option('snappy', type : 'feature', value : 'auto',
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index e4b46d5715..e585a548fa 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -161,6 +161,7 @@  meson_options_help() {
   printf "%s\n" '  sdl-image       SDL Image support for icons'
   printf "%s\n" '  seccomp         seccomp support'
   printf "%s\n" '  selinux         SELinux support in qemu-nbd'
+  printf "%s\n" '  sev             SEV library support'
   printf "%s\n" '  slirp           libslirp user mode network backend support'
   printf "%s\n" '  slirp-smbd      use smbd (at path --smbd=*) in slirp networking'
   printf "%s\n" '  smartcard       CA smartcard emulation support'
@@ -440,6 +441,8 @@  _meson_option_parse() {
     --disable-seccomp) printf "%s" -Dseccomp=disabled ;;
     --enable-selinux) printf "%s" -Dselinux=enabled ;;
     --disable-selinux) printf "%s" -Dselinux=disabled ;;
+    --enable-sev) printf "%s" -Dsev=enabled ;;
+    --disable-sev) printf "%s" -Dsev=disabled ;;
     --enable-slirp) printf "%s" -Dslirp=enabled ;;
     --disable-slirp) printf "%s" -Dslirp=disabled ;;
     --enable-slirp-smbd) printf "%s" -Dslirp_smbd=enabled ;;
diff --git a/subprojects/sev.wrap b/subprojects/sev.wrap
new file mode 100644
index 0000000000..5be1faccf6
--- /dev/null
+++ b/subprojects/sev.wrap
@@ -0,0 +1,6 @@ 
+[wrap-git]
+url = https://github.com/tylerfanelli/sev
+revision = b81b1da5df50055600a5b0349b0c4afda677cccb
+
+[provide]
+sev = sev_dep
diff --git a/target/i386/meson.build b/target/i386/meson.build
index 6f1036d469..8972a4fb17 100644
--- a/target/i386/meson.build
+++ b/target/i386/meson.build
@@ -20,7 +20,7 @@  i386_system_ss.add(files(
   'monitor.c',
   'cpu-sysemu.c',
 ))
-i386_system_ss.add(when: 'CONFIG_SEV', if_true: files('sev.c'), if_false: files('sev-sysemu-stub.c'))
+i386_system_ss.add(when: 'CONFIG_SEV', if_true: [sev, files('sev.c')], if_false: files('sev-sysemu-stub.c'))
 
 i386_user_ss = ss.source_set()