From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pg1-f202.google.com (mail-pg1-f202.google.com [209.85.215.202]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 594F01E3DCD for ; Thu, 5 Mar 2026 04:21:08 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.215.202 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772684469; cv=none; b=TNih4W8E8vBEtvHES/ps55mDC7QcYykfipAVanmGhYJm7nvJ8Lnh0C8WViMk/8k17KVb7MrkEU0QpBiW1xkJyMEDV19IqWB1Ojsl9dXZsm2LjXmJ3q9fA9EKPHiHZWLwOOpFkBf3IVVcdPRS1KPpt1J4zxcgv3HZRz/jnaykQlY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772684469; c=relaxed/simple; bh=mCfnJLaJLg+daVBBNaHa3/R5EF5xE+Cjpkd0CDVtjtw=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=BFPT/3N2G5o3oPRtaE/bUJiykB2lc9LJlm+qCydTefU7nsr5vh9t4c7PbHFnkSo60cMPAd43Xg5ecM0cYJfROR+BXQzZB1LiJAq51AWnYDQEWMEB1Ld8adVQc/NeLxr/x4oE3oNoCCnPwYbT/6EffNnDQZBt0A4Zk6Bp0z1mLIM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=LlBae1wC; arc=none smtp.client-ip=209.85.215.202 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="LlBae1wC" Received: by mail-pg1-f202.google.com with SMTP id 41be03b00d2f7-c7380305a9aso659937a12.1 for ; Wed, 04 Mar 2026 20:21:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1772684468; x=1773289268; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=ceZ3dTJkwg4Hdhcsp9fwJ65sqpITXml71eHPvJD5bDs=; b=LlBae1wC9wjNODUto144Notsr2tHNx2rbqw7S/rdKhcNeKbR2YRCn2fye3OEcuK7F6 1QkZMWzeV4vH6k4u7m/p5+3/VN9TiVz/Uoa2WIfAPSNx7xIaJNNk/EwJw/ga4Uda9PfD 0sUU5cmGvcTz5Vd96fjYTWd1QJ5K6kCQB+6ZW5ioJoeofyD1oUBt51DSX+V4CfgQ9MnI 9MeHPbPBNLv3iF9PuCm80qcJWkfI5iUYx81xZGLFohEqmjGm/bxYkJlLWNj07is1zz2w w8pdcYK7CJIima0YJ5fLJCSXcgdxIOcmFkGhNvQph6HgDWwaju0nmDLbYgSEMNBSJjkq FIOw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1772684468; x=1773289268; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ceZ3dTJkwg4Hdhcsp9fwJ65sqpITXml71eHPvJD5bDs=; b=bwyP23Jk6zRuLfEVxI9evE6ob5cjf33kek9F5yKKN1GEPI1JoaCyvnVG0+hBU2F1y8 DaGM72FUbiHgu99/IX+sijnu6Ol4un5fWopw24RhCeF9htC4nSOY/wn4qeAWfQnTk7Wo 1nOfS/gfwgfw3GY8rtbv6I+QeHHipcq0Ra2J+PEakOfmPmWq0ZNq+u6vcg9nL1dtr+Cv aGzOLTQ1Zm/msO6Ce0qZDBOKQp8+O+QfvePrMJ3XlqBIpLXjmbcJA/odOVPwUmdssCXH /2nEHg6CXwb3TCDNHMZIFUOsPx/VfPG/0phe6g+kHTTIyeRumfSvOBzh7YATkxF4lMdr BrvQ== X-Forwarded-Encrypted: i=1; AJvYcCVWe7zVGS7mXwETfy72K83aoJ0AzO8NNPPjWhjRf1inLpjANYwHN3ZkhDH4f9/I2koNFTZYMcjYHCimXIw=@vger.kernel.org X-Gm-Message-State: AOJu0YysVXkQpHTUYYjyLWCTgGAj4wKohp3iNjG5080mR5nDMUoGXQID GrUaGJ17ncgPFXlKSKnsJX/Gn3h+E3+0qyFh+4i1Hgh6LieGw0ZyZsHGHrWYF/lcY8fHfLYNDDm ti3HeGA== X-Received: from pgww16.prod.google.com ([2002:a05:6a02:2c90:b0:c6e:18d8:7280]) (user=seanjc job=prod-delivery.src-stubby-dispatcher) by 2002:a05:6a20:d509:b0:38e:974e:af00 with SMTP id adf61e73a8af0-39842626e0bmr1012163637.38.1772684467465; Wed, 04 Mar 2026 20:21:07 -0800 (PST) Date: Wed, 4 Mar 2026 20:21:05 -0800 In-Reply-To: <20260112235408.168200-5-chang.seok.bae@intel.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20260112235408.168200-1-chang.seok.bae@intel.com> <20260112235408.168200-5-chang.seok.bae@intel.com> Message-ID: Subject: Re: [PATCH v2 04/16] KVM: VMX: Introduce unified instruction info structure From: Sean Christopherson To: "Chang S. Bae" Cc: pbonzini@redhat.com, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, chao.gao@intel.com Content-Type: text/plain; charset="us-ascii" On Mon, Jan 12, 2026, Chang S. Bae wrote: > Define a unified data structure that can represent both the legacy and > extended VMX instruction information formats. > > VMX provides per-instruction metadata for VM exits to help decode the > attributes of the instruction that triggered the exit. The legacy format, > however, only supports up to 16 GPRs and thus cannot represent EGPRs. To > support these new registers, VMX introduces an extended 64-bit layout. > > Instead of maintaining separate storage for each format, a single > union structure makes the overall handling simple. The field names are > consistent across both layouts. While the presence of certain fields > depends on the instruction type, the offsets remain fixed within each > format. > > Signed-off-by: Chang S. Bae > --- > arch/x86/kvm/vmx/vmx.h | 61 ++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 61 insertions(+) > > diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h > index bc3ed3145d7e..567320115a5a 100644 > --- a/arch/x86/kvm/vmx/vmx.h > +++ b/arch/x86/kvm/vmx/vmx.h > @@ -311,6 +311,67 @@ struct kvm_vmx { > u64 *pid_table; > }; > > +/* > + * 32-bit layout of the legacy instruction information field. This format > + * supports the 16 legacy GPRs. > + */ > +struct base_insn_info { > + u32 scale : 2; /* Scaling factor */ > + u32 reserved1 : 1; > + u32 reg1 : 4; /* First register index */ > + u32 asize : 3; /* Address size */ > + u32 is_reg : 1; /* 0: memory, 1: register */ > + u32 osize : 2; /* Operand size */ > + u32 reserved2 : 2; > + u32 seg : 3; /* Segment register index */ > + u32 index : 4; /* Index register index */ > + u32 index_invalid : 1; /* 0: valid, 1: invalid */ > + u32 base : 4; /* Base register index */ > + u32 base_invalid : 1; /* 0: valid, 1: invalid */ > + u32 reg2 : 4; /* Second register index */ > +}; > + > +/* > + * 64-bit layout of the extended instruction information field, which > + * supports EGPRs. > + */ > +struct ext_insn_info { > + u64 scale : 2; /* Scaling factor */ > + u64 asize : 2; /* Address size */ > + u64 is_reg : 1; /* 0: memory, 1: register */ > + u64 osize : 2; /* Operand size */ > + u64 seg : 3; /* Segment register index */ > + u64 index_invalid : 1; /* 0: valid, 1: invalid */ > + u64 base_invalid : 1; /* 0: valid, 1: invalid */ > + u64 reserved1 : 4; > + u64 reg1 : 5; /* First register index */ > + u64 reserved2 : 3; > + u64 index : 5; /* Index register index */ > + u64 reserved3 : 3; > + u64 base : 5; /* Base register index */ > + u64 reserved4 : 3; > + u64 reg2 : 5; /* Second register index */ > + u64 reserved5 : 19; > +}; > + > +/* Union for accessing either the legacy or extended format. */ > +union insn_info { > + struct base_insn_info base; > + struct ext_insn_info ext; > + u32 word; > + u64 dword; word is 16 bits, dword is 32 bits, qword is 64 bits. > +}; > + > +/* > + * Wrapper structure combining the instruction info and a flag indicating > + * whether the extended layout is in use. > + */ > +struct vmx_insn_info { > + /* true if using the extended layout */ > + bool extended; > + union insn_info info; > +}; Absolutely not. I despise bit fields, as they're extremely difficult to review, don't help developers/debuggers understand the expected layout (finding flags and whatnot in .h files is almost always faster than searching the SDM), and they often generate suboptimal code. This is also infrastructure overkill. Two bitfields, a union, and another struct, just to track a 64-bit value. And the macros added later on only add to the obfuscation. Even worse, saving the "extended" flag on the stack and passing it around turns a static branch into a dynamic branch. I don't see any reason to do anything more complicated than: static inline u64 vmx_get_insn_info(void) { if (vmx_insn_info_extended()) return vmcs_read64(EXTENDED_INSTRUCTION_INFO); return vmcs_read32(VMX_INSTRUCTION_INFO); } static inline int vmx_get_insn_info_reg(u64 insn_info) { return vmx_insn_info_extended() ? (insn_info >> ??) & 0x1f : (insn_info >> 3) & 0xf; }