qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Michael S. Tsirkin
Subject: Re: [Qemu-devel] [RFC PATCH v1 20/22] fw_cfg: sev: disable dma in real mode
Date: Wed, 14 Sep 2016 16:14:58 +0300

On Wed, Sep 14, 2016 at 03:01:51PM +0200, Paolo Bonzini wrote:
> 
> 
> On 14/09/2016 14:09, Eduardo Habkost wrote:
> > 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.
> 
> And the attestation is what lets the guest check that the memory
> contents are indeed what the guest owner expects.
> 
> Paolo

So the cover letter says hypervisor is benign, and then people turn
around and start discussing guest owner checking memory as if hypervisor
is malicious and might load something unexpected there.  Makes no sense
to me.

I suggest we just drop this attestation thing in v1. Try to merge
something minimal that actually works first.

You can always extend this to add more features later:
whether there's any value in guest checking its own memory
is something that would need a separate discussion at
a higher level. I'm not saying there isn't.
Let's do one thing at a time though.

-- 
MST



reply via email to

[Prev in Thread] Current Thread [Next in Thread]