| Message ID | 20201204054415.579042-7-david@gibson.dropbear.id.au (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Generalize memory encryption models | expand |
On 12/4/20 6:44 AM, David Gibson wrote: > The kvm_memcrypt_enabled() and kvm_memcrypt_encrypt_data() helper functions > don't conceptually have any connection to KVM (although it's not possible > in practice to use them without it). > > They also rely on looking at the global KVMState. But the same information > is available from the machine, and the only existing callers have natural > access to the machine state. > > Therefore, move and rename them to helpers in securable-guest-memory.h, > taking an explicit machine parameter. > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > --- > accel/kvm/kvm-all.c | 27 -------------------- > accel/stubs/kvm-stub.c | 10 -------- > hw/i386/pc_sysfw.c | 6 +++-- > include/exec/securable-guest-memory.h | 36 +++++++++++++++++++++++++++ > include/sysemu/kvm.h | 17 ------------- > 5 files changed, 40 insertions(+), 56 deletions(-) ... > +static inline int securable_guest_memory_encrypt(MachineState *machine, > + uint8_t *ptr, uint64_t len) > +{ > + SecurableGuestMemory *sgm = machine->sgm; > + > + if (sgm) { > + SecurableGuestMemoryClass *sgmc = SECURABLE_GUEST_MEMORY_GET_CLASS(sgm); > + > + if (sgmc->encrypt_data) { Can this ever happen? Maybe use assert(sgmc->encrypt_data) instead? Otherwise: Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > + return sgmc->encrypt_data(sgm, ptr, len); > + } > + } > + > + return 1; > +}
On Mon, Jan 11, 2021 at 07:13:27PM +0100, Philippe Mathieu-Daudé wrote: > On 12/4/20 6:44 AM, David Gibson wrote: > > The kvm_memcrypt_enabled() and kvm_memcrypt_encrypt_data() helper functions > > don't conceptually have any connection to KVM (although it's not possible > > in practice to use them without it). > > > > They also rely on looking at the global KVMState. But the same information > > is available from the machine, and the only existing callers have natural > > access to the machine state. > > > > Therefore, move and rename them to helpers in securable-guest-memory.h, > > taking an explicit machine parameter. > > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > > --- > > accel/kvm/kvm-all.c | 27 -------------------- > > accel/stubs/kvm-stub.c | 10 -------- > > hw/i386/pc_sysfw.c | 6 +++-- > > include/exec/securable-guest-memory.h | 36 +++++++++++++++++++++++++++ > > include/sysemu/kvm.h | 17 ------------- > > 5 files changed, 40 insertions(+), 56 deletions(-) > ... > > > +static inline int securable_guest_memory_encrypt(MachineState *machine, > > + uint8_t *ptr, uint64_t len) > > +{ > > + SecurableGuestMemory *sgm = machine->sgm; > > + > > + if (sgm) { > > + SecurableGuestMemoryClass *sgmc = SECURABLE_GUEST_MEMORY_GET_CLASS(sgm); > > + > > + if (sgmc->encrypt_data) { > > Can this ever happen? Maybe use assert(sgmc->encrypt_data) instead? It's made moot by changes in the next spin. > > Otherwise: > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > > + return sgmc->encrypt_data(sgm, ptr, len); > > + } > > + } > > + > > + return 1; > > +} >
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index 92a49b328a..c6bd7b9d02 100644 --- a/accel/kvm/kvm-all.c +++ b/accel/kvm/kvm-all.c @@ -121,9 +121,6 @@ struct KVMState KVMMemoryListener memory_listener; QLIST_HEAD(, KVMParkedVcpu) kvm_parked_vcpus; - /* securable guest memory (e.g. by guest memory encryption) */ - SecurableGuestMemory *sgm; - /* For "info mtree -f" to tell if an MR is registered in KVM */ int nr_as; struct KVMAs { @@ -222,28 +219,6 @@ int kvm_get_max_memslots(void) return s->nr_slots; } -bool kvm_memcrypt_enabled(void) -{ - if (kvm_state && kvm_state->sgm) { - return true; - } - - return false; -} - -int kvm_memcrypt_encrypt_data(uint8_t *ptr, uint64_t len) -{ - SecurableGuestMemory *sgm = kvm_state->sgm; - - if (sgm) { - SecurableGuestMemoryClass *sgmc = SECURABLE_GUEST_MEMORY_GET_CLASS(sgm); - - return sgmc->encrypt_data(sgm, ptr, len); - } - - return 1; -} - /* Called with KVMMemoryListener.slots_lock held */ static KVMSlot *kvm_get_free_slot(KVMMemoryListener *kml) { @@ -2213,8 +2188,6 @@ static int kvm_init(MachineState *ms) if (ret < 0) { goto err; } - - kvm_state->sgm = ms->sgm; } ret = kvm_arch_init(ms, s); diff --git a/accel/stubs/kvm-stub.c b/accel/stubs/kvm-stub.c index 680e099463..0f17acfac0 100644 --- a/accel/stubs/kvm-stub.c +++ b/accel/stubs/kvm-stub.c @@ -81,16 +81,6 @@ int kvm_on_sigbus(int code, void *addr) return 1; } -bool kvm_memcrypt_enabled(void) -{ - return false; -} - -int kvm_memcrypt_encrypt_data(uint8_t *ptr, uint64_t len) -{ - return 1; -} - #ifndef CONFIG_USER_ONLY int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev) { diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c index b6c0822fe3..439ac78970 100644 --- a/hw/i386/pc_sysfw.c +++ b/hw/i386/pc_sysfw.c @@ -38,6 +38,7 @@ #include "sysemu/sysemu.h" #include "hw/block/flash.h" #include "sysemu/kvm.h" +#include "exec/securable-guest-memory.h" /* * We don't have a theoretically justifiable exact lower bound on the base @@ -201,10 +202,11 @@ static void pc_system_flash_map(PCMachineState *pcms, pc_isa_bios_init(rom_memory, flash_mem, size); /* Encrypt the pflash boot ROM */ - if (kvm_memcrypt_enabled()) { + if (securable_guest_memory_enabled(MACHINE(pcms))) { flash_ptr = memory_region_get_ram_ptr(flash_mem); flash_size = memory_region_size(flash_mem); - ret = kvm_memcrypt_encrypt_data(flash_ptr, flash_size); + ret = securable_guest_memory_encrypt(MACHINE(pcms), + flash_ptr, flash_size); if (ret) { error_report("failed to encrypt pflash rom"); exit(1); diff --git a/include/exec/securable-guest-memory.h b/include/exec/securable-guest-memory.h index 4e2ae27040..7325b504ba 100644 --- a/include/exec/securable-guest-memory.h +++ b/include/exec/securable-guest-memory.h @@ -21,6 +21,7 @@ #ifndef CONFIG_USER_ONLY #include "qom/object.h" +#include "hw/boards.h" #define TYPE_SECURABLE_GUEST_MEMORY "securable-guest-memory" #define SECURABLE_GUEST_MEMORY(obj) \ @@ -43,6 +44,41 @@ typedef struct SecurableGuestMemoryClass { int (*encrypt_data)(SecurableGuestMemory *, uint8_t *, uint64_t); } SecurableGuestMemoryClass; +/** + * securable_guest_memory_enabled - return whether guest memory is protected + * from hypervisor access (with memory + * encryption or otherwise) + * Returns: true guest memory is not directly accessible to qemu + * false guest memory is directly accessible to qemu + */ +static inline bool securable_guest_memory_enabled(MachineState *machine) +{ + return !!machine->sgm; +} + +/** + * securable_guest_memory_encrypt: encrypt the memory range to make + * it guest accessible + * + * Return: 1 failed to encrypt the range + * 0 succesfully encrypted memory region + */ +static inline int securable_guest_memory_encrypt(MachineState *machine, + uint8_t *ptr, uint64_t len) +{ + SecurableGuestMemory *sgm = machine->sgm; + + if (sgm) { + SecurableGuestMemoryClass *sgmc = SECURABLE_GUEST_MEMORY_GET_CLASS(sgm); + + if (sgmc->encrypt_data) { + return sgmc->encrypt_data(sgm, ptr, len); + } + } + + return 1; +} + #endif /* !CONFIG_USER_ONLY */ #endif /* QEMU_SECURABLE_GUEST_MEMORY_H */ diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h index bb5d5cf497..0e163c2c9d 100644 --- a/include/sysemu/kvm.h +++ b/include/sysemu/kvm.h @@ -233,23 +233,6 @@ int kvm_has_intx_set_mask(void); */ bool kvm_arm_supports_user_irq(void); -/** - * kvm_memcrypt_enabled - return boolean indicating whether memory encryption - * is enabled - * Returns: 1 memory encryption is enabled - * 0 memory encryption is disabled - */ -bool kvm_memcrypt_enabled(void); - -/** - * kvm_memcrypt_encrypt_data: encrypt the memory range - * - * Return: 1 failed to encrypt the range - * 0 succesfully encrypted memory region - */ -int kvm_memcrypt_encrypt_data(uint8_t *ptr, uint64_t len); - - #ifdef NEED_CPU_H #include "cpu.h"