Re: [Qemu-devel] [RFC PATCH v1 20/22] fw_cfg: sev: disable dma in real mode

[Eduardo Habkost]

On Wed, Sep 14, 2016 at 05:33:09AM +0300, Michael S. Tsirkin wrote:
> On Wed, Sep 14, 2016 at 12:53:36AM +0200, Paolo Bonzini wrote:
> > 
> > 
> > On 13/09/2016 16:50, Brijesh Singh wrote:
> > > In SEV-enabled guest dma should be performed on shared pages. Since
> > > the SeaBIOS executes in non PAE mode and does not have access to C-bit
> > > to create a shared page hence disable the dma operation when reading
> > > from fw_cfg interface.
> > > 
> > > Signed-off-by: Brijesh Singh <address@hidden>
> > > ---
> > >  hw/nvram/fw_cfg.c |    6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> > > index 6a68e59..aca99e9 100644
> > > --- a/hw/nvram/fw_cfg.c
> > > +++ b/hw/nvram/fw_cfg.c
> > > @@ -24,6 +24,7 @@
> > >  #include "qemu/osdep.h"
> > >  #include "hw/hw.h"
> > >  #include "sysemu/sysemu.h"
> > > +#include "sysemu/kvm.h"
> > >  #include "sysemu/dma.h"
> > >  #include "hw/boards.h"
> > >  #include "hw/isa/isa.h"
> > > @@ -1009,6 +1010,11 @@ static void fw_cfg_io_realize(DeviceState *dev, 
> > > Error **errp)
> > >      FWCfgIoState *s = FW_CFG_IO(dev);
> > >      SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> > >  
> > > +    /* disable dma on fw_cfg when SEV is enabled */
> > > +    if (kvm_sev_enabled()) {
> > > +        qdev_prop_set_bit(dev, "dma_enabled", false);
> > > +    }
> > > +
> > >      /* when using port i/o, the 8-bit data register ALWAYS overlaps
> > >       * with half of the 16-bit control register. Hence, the total size
> > >       * of the i/o region used is FW_CFG_CTL_SIZE */
> > > 
> > > 
> > > 
> > 
> > As Michael said, a workaround is possible using -global.  However, I
> > really think that SEV should be implemented in UEFI firmware.  Because
> > it runs in 64-bit mode, it would be able to run in paged mode and it
> > would not have to encrypt everything before the SEV launch command.
> > 
> > For example secure boot can be used to authenticate the kernel against
> > keys provided by the owner in encrypted flash, or GRUB2 can be placed in
> > the firmware by the owner and used to boot from a LUKS-encrypted /boot
> > partition.
> > 
> > Paolo
> 
> Frankly I don't understand why do you need to mess with boot at all.
> Quoting the cover letter:
> 
>       SEV is designed to protect guest VMs from a benign but vulnerable
>       (i.e. not fully malicious) hypervisor. In particular, it reduces the
>       attack
>       surface of guest VMs and can prevent certain types of VM-escape bugs
>       (e.g. hypervisor read-anywhere) from being used to steal guest data.
> 
> it seems highly unlikely that any secret data is used during boot.
> So just let guest boot normally, and encrypt afterwards.

After boot seems too late for the attestation part (see Section
1.3.1: Launch in the spec), unless you can ensure the memory
contents will always be exactly what the guest owner expects
after every boot.

-- 
Eduardo