On 5/12/20 7:52 PM, Brijesh Singh wrote:
On 5/12/20 10:32 AM, Erik Skultety wrote:
On Tue, May 12, 2020 at 09:08:38AM +0200, Boris Fiuczynski wrote:
On 5/11/20 9:40 PM, Brijesh Singh wrote:
Thanks for the patch Paulo, Few comments.
On 5/11/20 11:41 AM, Boris Fiuczynski wrote:
From: Paulo de Rezende Pinatti <ppinatti@xxxxxxxxxxxxx>
Implement secure guest check for AMD SEV (Secure Encrypted
Virtualization) in order to invalidate the qemu capabilities
cache in case the availability of the feature changed.
For AMD SEV the verification consists of:
- checking if /sys/module/kvm_amd/parameters/sev contains the
value '1': meaning SEV is enabled in the host kernel;
- checking if the kernel cmdline contains 'mem_encrypt=on': meaning
SME memory encryption feature on the host is enabled
In addition to the kernel module parameter, we probably also need to
check whether QEMU supports the feature. e.g, what if user has newer
kernel but older qemu. e.g kernel > 4.18 but Qemu < 2.12. To check the
SEV feature support, we should verify the following conditions:
1) check kernel module parameter is set
Paulo implemented the checks following the documentation in
docs/kbase/launch_security_sev.rst. The check for the module parameter sev
is included. Is the check for the kernel cmdline parameter "mem_encrypt" not
necessary? Or would that be covered by your suggested check in 2)?
Brijesh correct me here please, but IIRC having mem_encrypt set is merely
a good security recommendation but is orthogonal with kvm.amd_sev feature, IOW
SEV should work without SME. If my memory serves well here, we don't need
and also should not parse the kernel cmdline in this case.
Yes, that is correct. mem_encrypt=on is not required for the SEV to
work. The mem_encrypt=on option is meant to enable the SME feature on
the host machine. In addition to the guest, if customer wants to protect
the Hypervsior memory from physical attacks then they can use SME or
TSME. The SME can be enabled using mem_encrypt=on, whereas the TSME can
be enabled in the BIOS and does not require any kernel changes. AMD is
mostly recommending TSME to protect against the physical attacks.
2) check if /dev/sev device exist (aka firmware is detected)
This seems reasonable. Shouldn't it have been documented in
docs/kbase/launch_security_sev.rst?
Sure, we can add a mention about this. Although, doesn't 1 imply 2? IOW can
you have the kernel module parameter set to 1 and yet kernel doesn't expose the
/dev/sev node?
Currently, 1 does not imply 2, KVM driver does not initialize the
firmware during the feature probe (i.e does not access the /dev/sev).
The firmware initialization is delayed until the first guest launch. So
only sane way to know whether firmware is been detected is check the
existence of the /dev/sev or issue a query-sev command . The query-sev
command will send the platform_status request to the firmware, if the
firmware is not ready then this command will fail.
Just to summarize the checks you want implemented for AMD SEV support:
1) check kernel module parameter is set (as already implemented)
2) check if /dev/sev device exist
Please note that these checks will also go into virt-host-validate in
patch 5.
3) Check if Qemu supports SEV feature (maybe we can detect the existence
of the query-sev QMP command or detect Qemu version >= 2.12)
^This is already achieved by qemuMonitorJSONGetSEVCapabilities
The idea is to check the host capabilities to detect if qemus capabilities
need to be reprobed. Therefore this would be a result if checks 1+2 change
but it would not be a cache invalidation trigger.
I agree in the sense that the SEV support that is currently being reported in
QEMU capabilities wasn't a good choice because it reports only platform data
which are constant to the host and have nothing to do with QEMU. It would be
okay to just say <sev supported="yes|no"/> in qemu capabilities and report the
rest in host capabilities. I haven't added this info into host capabilities
when I realized the mistake because I didn't want to duplicate the platform
info on 2 places. For the IBM secure guest feature, it makes total sense to
report support within host capabilities, I'm just not sure whether we should do
the same for SEV and try really hard to "fix" the mess.
Regards,
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
