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 <brijesh.si...@amd.com>
>>>> ---
>>>> 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
