Re: [RFC PATCH v2 1/9] Add Rust SEV library as subproject

["Daniel P. Berrangé" <berrange@redhat.com>]

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
-- 
|: 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 :|