[v1] kbase: sev: Provide more details on virtio-net configuration

[libvirt PATCH] kbase: sev: Provide more details on virtio-net configuration

Posted by Erik Skultety 3 years, 8 months ago

With virtio-net further configuration settings are required, so document
them and while at it, fix the Q35 machine XML example which wouldn't
work with SEV because of not disabling vhost and the option boot ROM.

Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Erik Skultety <eskultet@redhat.com>
---
 docs/kbase/launch_security_sev.rst | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/docs/kbase/launch_security_sev.rst b/docs/kbase/launch_security_sev.rst
index cfdc2a6120..9df4178aac 100644
--- a/docs/kbase/launch_security_sev.rst
+++ b/docs/kbase/launch_security_sev.rst
@@ -291,8 +291,9 @@ can still perform DoS on each other.
 Virtio
 ------
 
-In order to make virtio devices work, we need to enable emulated IOMMU
-on the devices so that virtual DMA can work.
+In order to make virtio devices work, we need to use
+``<driver iommu='on'/>`` inside the given device XML element in order
+to enable DMA API in the virtio driver.
 
 ::
 
@@ -337,6 +338,26 @@ model, which means that virtio GPU cannot be used.
      ...
    </domain>
 
+Virtio-net
+~~~~~~~~~~
+With virtio-net it's also necessary to disable the iPXE option ROM on the
+device as well as disable the vhost protocol as SEV doesn't support either
+(at the time of this writing). This translates to the following XML:
+
+::
+
+   <domain>
+     ...
+     <interface type='network'>
+        ...
+       <model type='virtio'/>
+       <driver name='qemu' iommu='on'/>
+       <rom enabled='no'/>
+     </interface>
+     ...
+   <domain>
+
+
 Checking SEV from within the guest
 ==================================
 
@@ -423,7 +444,8 @@ Q35 machine
          <mac address='52:54:00:cc:56:90'/>
          <source network='default'/>
          <model type='virtio'/>
-         <driver iommu='on'/>
+         <driver name='qemu' iommu='on'/>
+         <rom enabled='no'/>
        </interface>
        <graphics type='spice' autoport='yes'>
          <listen type='address'/>
-- 
2.26.2

Re: [libvirt PATCH] kbase: sev: Provide more details on virtio-net configuration

Posted by Laszlo Ersek 3 years, 8 months ago

On 08/07/20 13:21, Erik Skultety wrote:
> With virtio-net further configuration settings are required, so document
> them and while at it, fix the Q35 machine XML example which wouldn't
> work with SEV because of not disabling vhost and the option boot ROM.

(1) Please drop:

  not disabling vhost and

(2) please replace

  the option boot ROM

with

  the iPXE option ROM

(more details below)

> 
> Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Erik Skultety <eskultet@redhat.com>
> ---
>  docs/kbase/launch_security_sev.rst | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/kbase/launch_security_sev.rst b/docs/kbase/launch_security_sev.rst
> index cfdc2a6120..9df4178aac 100644
> --- a/docs/kbase/launch_security_sev.rst
> +++ b/docs/kbase/launch_security_sev.rst
> @@ -291,8 +291,9 @@ can still perform DoS on each other.
>  Virtio
>  ------
>  
> -In order to make virtio devices work, we need to enable emulated IOMMU
> -on the devices so that virtual DMA can work.
> +In order to make virtio devices work, we need to use
> +``<driver iommu='on'/>`` inside the given device XML element in order
> +to enable DMA API in the virtio driver.
>  
>  ::
>  

This hunk looks good.

> @@ -337,6 +338,26 @@ model, which means that virtio GPU cannot be used.
>       ...
>     </domain>
>  
> +Virtio-net
> +~~~~~~~~~~
> +With virtio-net it's also necessary to disable the iPXE option ROM on the
> +device as well as disable the vhost protocol

(3) Please break these items into separate sentences.

(4) Please restrict the latter (the vhost disablement) to QEMU version
v2.12.0 exactly.

(Per another part in this document, SEV appeared in QEMU v2.12.0, so we
need not consider anything earlier. And the vhost disablement is
unneeded with both v3.0.0 and v2.12.1, due to QEMU commits d542800d1edc
and 2f2b18923502, respectively. So the only QEMU version that needs the
vhost disablement is v2.12.0.)

> as SEV doesn't support either
> +(at the time of this writing).

(5) This statement is not correct:

First, vhost does support SEV, only QEMU had a small bug (see the
above-named commits) that prevented vhost from working with SEV. It's
not a "total lack of support".

Second, regarding iPXE, it's not that SEV doesn't support iPXE; it's
iPXE that is unaware of SEV, at the time of this writing.

> This translates to the following XML:
> +
> +::
> +
> +   <domain>
> +     ...
> +     <interface type='network'>
> +        ...
> +       <model type='virtio'/>
> +       <driver name='qemu' iommu='on'/>
> +       <rom enabled='no'/>
> +     </interface>
> +     ...
> +   <domain>
> +
> +
>  Checking SEV from within the guest
>  ==================================
>  

(6) So the @name='qemu' attribute for the <driver> element should be
removed, assuming we intend to provide an example XML fragment only for
the latest QEMU version (at the time of this writing).

> @@ -423,7 +444,8 @@ Q35 machine
>           <mac address='52:54:00:cc:56:90'/>
>           <source network='default'/>
>           <model type='virtio'/>
> -         <driver iommu='on'/>
> +         <driver name='qemu' iommu='on'/>
> +         <rom enabled='no'/>
>         </interface>
>         <graphics type='spice' autoport='yes'>
>           <listen type='address'/>
> 

(7) Same as (6).

... Ultimately, if any distro uses a v2.12-based QEMU, perhaps we can
expect that distro to use the latest stable release in the v2.12
"release stream". If we do have that expectation of distros, then we
should simply drop all mentions of "vhost".

Thanks!
Laszlo

Re: [libvirt PATCH] kbase: sev: Provide more details on virtio-net configuration

Posted by Erik Skultety 3 years, 8 months ago

On Fri, Aug 07, 2020 at 07:05:22PM +0200, Laszlo Ersek wrote:
> On 08/07/20 13:21, Erik Skultety wrote:
> > With virtio-net further configuration settings are required, so document
> > them and while at it, fix the Q35 machine XML example which wouldn't
> > work with SEV because of not disabling vhost and the option boot ROM.
>
> (1) Please drop:
>
>   not disabling vhost and
>
> (2) please replace
>
>   the option boot ROM
>
> with
>
>   the iPXE option ROM
>
> (more details below)
>
> >
> > Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > Signed-off-by: Erik Skultety <eskultet@redhat.com>
> > ---
> >  docs/kbase/launch_security_sev.rst | 28 +++++++++++++++++++++++++---
> >  1 file changed, 25 insertions(+), 3 deletions(-)
> >
> > diff --git a/docs/kbase/launch_security_sev.rst b/docs/kbase/launch_security_sev.rst
> > index cfdc2a6120..9df4178aac 100644
> > --- a/docs/kbase/launch_security_sev.rst
> > +++ b/docs/kbase/launch_security_sev.rst
> > @@ -291,8 +291,9 @@ can still perform DoS on each other.
> >  Virtio
> >  ------
> >
> > -In order to make virtio devices work, we need to enable emulated IOMMU
> > -on the devices so that virtual DMA can work.
> > +In order to make virtio devices work, we need to use
> > +``<driver iommu='on'/>`` inside the given device XML element in order
> > +to enable DMA API in the virtio driver.
> >
> >  ::
> >
>
> This hunk looks good.
>
> > @@ -337,6 +338,26 @@ model, which means that virtio GPU cannot be used.
> >       ...
> >     </domain>
> >
> > +Virtio-net
> > +~~~~~~~~~~
> > +With virtio-net it's also necessary to disable the iPXE option ROM on the
> > +device as well as disable the vhost protocol
>
> (3) Please break these items into separate sentences.
>
> (4) Please restrict the latter (the vhost disablement) to QEMU version
> v2.12.0 exactly.
>
> (Per another part in this document, SEV appeared in QEMU v2.12.0, so we
> need not consider anything earlier. And the vhost disablement is
> unneeded with both v3.0.0 and v2.12.1, due to QEMU commits d542800d1edc
> and 2f2b18923502, respectively. So the only QEMU version that needs the
> vhost disablement is v2.12.0.)
>
> > as SEV doesn't support either
> > +(at the time of this writing).
>
> (5) This statement is not correct:
>
> First, vhost does support SEV, only QEMU had a small bug (see the
> above-named commits) that prevented vhost from working with SEV. It's
> not a "total lack of support".
>
> Second, regarding iPXE, it's not that SEV doesn't support iPXE; it's
> iPXE that is unaware of SEV, at the time of this writing.
>
> > This translates to the following XML:
> > +
> > +::
> > +
> > +   <domain>
> > +     ...
> > +     <interface type='network'>
> > +        ...
> > +       <model type='virtio'/>
> > +       <driver name='qemu' iommu='on'/>
> > +       <rom enabled='no'/>
> > +     </interface>
> > +     ...
> > +   <domain>
> > +
> > +
> >  Checking SEV from within the guest
> >  ==================================
> >
>
> (6) So the @name='qemu' attribute for the <driver> element should be
> removed, assuming we intend to provide an example XML fragment only for
> the latest QEMU version (at the time of this writing).
>
> > @@ -423,7 +444,8 @@ Q35 machine
> >           <mac address='52:54:00:cc:56:90'/>
> >           <source network='default'/>
> >           <model type='virtio'/>
> > -         <driver iommu='on'/>
> > +         <driver name='qemu' iommu='on'/>
> > +         <rom enabled='no'/>
> >         </interface>
> >         <graphics type='spice' autoport='yes'>
> >           <listen type='address'/>
> >
>
> (7) Same as (6).
>
>
> ... Ultimately, if any distro uses a v2.12-based QEMU, perhaps we can
> expect that distro to use the latest stable release in the v2.12
> "release stream". If we do have that expectation of distros, then we
> should simply drop all mentions of "vhost".

I double checked with repology.org, whether there's any distro mentioning qemu
2.12.0 and still falling into our platform support promise with CentOS-8 being
the only one. However, the module build available in CentOS-8 is
qemu-kvm-2.12.0-99.module_el8.2.0, while commit d542800d1edc
appeared in qemu-kvm-2.12.0-83.module+el8.1.0. Therefore, indeed, we can drop
the "vhost" mentions.

As usual, thanks Laszlo for your comments, I'll incorporate them and send a v2.

Erik