qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Tyler Fanelli <tfanelli@redhat.com>
To: "Stefan Hajnoczi" <stefanha@gmail.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>
Cc: qemu-devel@nongnu.org, pbonzini@redhat.com, mtosatti@redhat.com,
	stefanha@redhat.com
Subject: Re: [RFC PATCH 0/8] i386/sev: Use C API of Rust SEV library
Date: Fri, 15 Sep 2023 13:08:07 -0400	[thread overview]
Message-ID: <6e626dba-8be2-5cfe-4f81-75de19febd6e@redhat.com> (raw)
In-Reply-To: <CAJSP0QUA--fpGAjj9JJ2BW7VSunw6mb7=8NDmTt7+k71OOAd1g@mail.gmail.com>

On 9/15/23 7:33 AM, Stefan Hajnoczi wrote:
> On Fri, 15 Sept 2023 at 05:54, Daniel P. Berrangé <berrange@redhat.com> wrote:
>> On Thu, Sep 14, 2023 at 01:58:27PM -0400, Tyler Fanelli wrote:
>>> These patches are submitted as an RFC mainly because I'm a relative
>>> newcomer to QEMU with no knowledge of the community's views on
>>> including Rust code, nor it's preference of using library APIs for
>>> ioctls that were previously implemented in QEMU directly.
>> We've talked about Rust alot, but thus far most focus has been on
>> areas peripheral to QEMU. Projects that might have been part of
>> QEMU in the past, and now being done as separate efforts, and
>> have bene taking advantage of Rust. eg virtiofsd Rust replacing
>> QEMU's in -tree C impl. eg passt providing an alternative to
>> slirp. eg the dbus display in QEMU allowing a remote display
>> frontend to be provided, written in rust. eg libblkio providing
>> a block backend in Rust.
>>
>> The libblkio work is likely closest to what you've proposed
>> here, in that it is a Rust create exposed as a C shared library
>> for apps to consume. In theory apps don't need to care that it
>> is written in Rust, as it is opaque.
>>
>> The one key difference though is that it was not replacing
>> existing functionality, it was adding a new feature. So users
>> who didn't have libblkio or whom want to avoid Rust dependancies
>> didn't loose anything they were already using.
>>
>> If we use the libsev.so we create a hard dependancy on the Rust
>> sev crate, otherwise users loose the SEV feature in QEMU. Right
>> now the sev crate C library is not present in *any* distro that
>> I can see.

Yes, the C API is very new and not packaged in any distro at the moment.

>>
>> If we treat 'sev' as just another opaque 3rd party library to be
>> provided by the distro, this creates a problem. Our support
>> policy is that we usually won't drop features in existing distros,
>> but that is what would happen if we applied this patchset today.
>> We did bend that rule slightly with virtiofsd, but that was already
>> a separate binary and we followed our deprecation path before
>> deleting it, giving distros time to adapt.
>>
>>
>> If we rollback the curtain, however, and decide to expose Rust
>> directly to QEMU we could address this problem. We could bundle
>> the dependant Rust crates directly with QEMU tarballs, and
>> generate the FFI C library as part of QEMU build and static
>> link the library. Distros would not have todo anything, though
>> they could have the choice of dyn linking if they really wanted
>> to.
>>
>> If we directly exposed the notion of Rust to QEMU, then we are
>> also not limited by whether a Rust crate provides a C FFI itself.
>> QEMU could provide C FFI glue for any Rust crate it sees as
>> useful to its code.
>>
>> This all forces us, however, to have the difficult discussion
>> about whether we're willing to make Rust a mandatory dependancy
>> of QEMU and permit (or even welcome) its use /anywhere/ in the
>> QEMU tree that looks relevant.
>>
>> We've already queried whether Rust will actually benefit the
>> core QEMU codebase, or whether we'll end up punching too many
>> holes in its safety net to make it worthwhile. My opinion is
>> that we probably shouldn't obsess over that as I think it is
>> hard to predict the future, it has a habit of surprising us.
>> Your patch series here doesn't demonstrate an obvious safety
>> benefit, since we have existing working code and that code is
>> not especially complex.

Correct, there isn't any new features being added here. SEV on QEMU 
should work _exactly_ how it did before these patches.

>> Once we open the doors to Rust code
>> in QEMU though, we will probably surprise ourselves with the
>> range of benefits we'll see 2, 3, 5 years down the road.
>>
>> IOW, we shouldn't judge future benefits based on this patch
>> series. It is great that this series is actually quite simple,
>> because it lets us focus on how we might integrate Rust more
>> directly into QEMU, without worrying much about the actual
>> code being replaced.

It also shows that much of Rust's security benefits in QEMU would depend 
on Rust being integrated more directly, rather than just using C FFI. 
Most of the code in the libsev C API is unsafe Rust anyway (it must be 
in order to interact with C). Therefore there is not much of an added 
security benefit here. However, if Rust can be further expanded in QEMU, 
much of the unsafe bits can be removed entirely (i.e. by bypassing the C 
API).

>>
>>> This series looks to explore the possibility of using the library and
>>> show a bit of what it would look like. I'm looking for comments
>>> regarding if this feature is desired.
>> My summary, is that I'd personally be in favour of opening the door
>> to Rust code as a mandatory pre-requisite for QEMU, at the very least
>> for system emulators. Not because this particular series is compelling,
>> but because I think Rust could be more beneficial to QEMU over the long
>> term than we expect. In terms of consuming it though, if we're going
>> to replace existing QEMU functionality, then I think we need to bundle
>> the Rust code and natively integrate it into the build system, as we
>> have recently started doing with our python deps, to detach ourselves
>> from the limits of what distros ship.
> I support using Rust directly within QEMU.
>
> David Gibson looked at Rust's operating system and CPU architecture
> coverage a few years ago:
> https://wiki.qemu.org/RustInQemu
>
> Please update that support matrix to check that depending on Rust in
> core QEMU code really works everywhere where QEMU is supported today.
> This is probably just a formality at this stage since Rust has become
> widely used over the past few years.
>
> The library approach worked well for libblkio but the overhead of
> creating a separate shared library and shipping it is significant.
> When QEMU is the only user of some code, then it should definitely be
> part of QEMU. Also, when QEMU needs early access to code that isn't
> widely available yet, then bundling it inside QEMU until packages are
> available also seems reasonable to me (I think we already do that for
> libvfio-user and maybe other libraries).
>
> I would prefer it if we minimize Rust wrappers for C APIs and instead
> focus on using Rust to build new subsystems. Writing and maintaing two
> sets of the same API is expensive and I hope we don't get bogged down
> keeping C and Rust APIs in sync.

It would also allow QEMU to take advantage of other Rust crates without 
having to add some type of shim layer to facilitate the interaction.


Tyler

> That said, I think there's an
> argument for wrapping core QEMU APIs needed for device emulation (e.g.
> DeviceState, PCIDevice) because of the security benefits of writing
> new device emulation code in Rust.
>
> Stefan
>



  parent reply	other threads:[~2023-09-15 17:08 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-14 17:58 [RFC PATCH 0/8] i386/sev: Use C API of Rust SEV library Tyler Fanelli
2023-09-14 17:58 ` [RFC PATCH 1/8] Add SEV Rust library as dependency with CONFIG_SEV Tyler Fanelli
2023-09-14 17:58 ` [RFC PATCH 2/8] i386/sev: Replace INIT and ES_INIT ioctls with sev library equivalents Tyler Fanelli
2023-09-14 17:58 ` [RFC PATCH 3/8] i386/sev: Replace LAUNCH_START ioctl with sev library equivalent Tyler Fanelli
2023-09-14 17:58 ` [RFC PATCH 4/8] i386/sev: Replace UPDATE_DATA " Tyler Fanelli
2023-09-14 17:58 ` [RFC PATCH 5/8] i386/sev: Replace LAUNCH_UPDATE_VMSA " Tyler Fanelli
2023-09-14 17:58 ` [RFC PATCH 6/8] i386/sev: Replace LAUNCH_MEASURE " Tyler Fanelli
2023-09-14 17:58 ` [RFC PATCH 7/8] i386/sev: Replace LAUNCH_SECRET " Tyler Fanelli
2023-09-14 17:58 ` [RFC PATCH 8/8] i386/sev: Replace LAUNCH_FINISH " Tyler Fanelli
2023-09-14 19:04 ` [RFC PATCH 0/8] i386/sev: Use C API of Rust SEV library Philippe Mathieu-Daudé
2023-09-15  0:36   ` Tyler Fanelli
2023-09-15  9:53 ` Daniel P. Berrangé
2023-09-15 11:33   ` Stefan Hajnoczi
2023-09-15 12:02     ` Daniel P. Berrangé
2023-09-15 17:08     ` Tyler Fanelli [this message]
2023-09-15 13:49   ` Peter Maydell
2023-09-15 17:09     ` Stefan Hajnoczi
2023-09-25 12:53       ` Marc-André Lureau
  -- strict thread matches above, loose matches on Subject: below --
2023-09-14 16:33 Tyler Fanelli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6e626dba-8be2-5cfe-4f81-75de19febd6e@redhat.com \
    --to=tfanelli@redhat.com \
    --cc=berrange@redhat.com \
    --cc=mtosatti@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@gmail.com \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).