Re: [PATCH v2 53/63] KVM: x86: Use struct_group() to zero decode cache

From: Sean Christopherson
Date: Wed Aug 18 2021 - 11:11:41 EST


On Tue, Aug 17, 2021, Kees Cook wrote:
> arch/x86/kvm/emulate.c | 3 +--
> arch/x86/kvm/kvm_emulate.h | 19 +++++++++++--------
> 2 files changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 2837110e66ed..2608a047e769 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -5377,8 +5377,7 @@ static int fastop(struct x86_emulate_ctxt *ctxt, fastop_t fop)
>
> void init_decode_cache(struct x86_emulate_ctxt *ctxt)
> {
> - memset(&ctxt->rip_relative, 0,
> - (void *)&ctxt->modrm - (void *)&ctxt->rip_relative);
> + memset(&ctxt->decode_cache, 0, sizeof(ctxt->decode_cache));
>
> ctxt->io_read.pos = 0;
> ctxt->io_read.end = 0;
> diff --git a/arch/x86/kvm/kvm_emulate.h b/arch/x86/kvm/kvm_emulate.h
> index 68b420289d7e..9b8afcb8ad39 100644
> --- a/arch/x86/kvm/kvm_emulate.h
> +++ b/arch/x86/kvm/kvm_emulate.h
> @@ -341,14 +341,17 @@ struct x86_emulate_ctxt {
> * the rest are initialized unconditionally in x86_decode_insn
> * or elsewhere
> */
> - bool rip_relative;
> - u8 rex_prefix;
> - u8 lock_prefix;
> - u8 rep_prefix;
> - /* bitmaps of registers in _regs[] that can be read */
> - u32 regs_valid;
> - /* bitmaps of registers in _regs[] that have been written */
> - u32 regs_dirty;
> + struct_group(decode_cache,

This is somewhat misleading because half of this struct is the so called "decode
cache", not just these six fields.

KVM's "optimization" is quite ridiculous as this has never been such a hot path
that saving a few mov instructions would be noticeable. And hilariously, the
"optimization" is completely unnecessary because both gcc and clang are clever
enough to batch the first five into a movq even when zeroing the fields individually.

So, I would much prefer to go with the following: