Re: [PATCH v19 027/130] KVM: TDX: Define TDX architectural definitions

Isaku Yamahata
2024-04-03T16:30:12Z

On Wed, Apr 03, 2024 at 08:04:03AM -0700,
Sean Christopherson <seanjc@google.com> wrote:

> On Mon, Feb 26, 2024, isaku.yamahata@intel.com wrote:
> > +union tdx_vcpu_state_details {
> > + struct {
> > + u64 vmxip : 1;
> > + u64 reserved : 63;
> > + };
> > + u64 full;
> > +};
>
> No unions please. KVM uses unions in a few places where they are the lesser of
> all evils, but in general, unions are frowned upon. Bitfields in particular are
> strongly discourage, as they are a nightmare to read/review and tend to generate
> bad code.
>
> E.g. for this one, something like (names aren't great)
>
> static inline bool tdx_has_pending_virtual_interrupt(struct kvm_vcpu *vcpu)
> {
> return <get "non arch field"> & TDX_VCPU_STATE_VMXIP;
> }

Sure, let me replace them with mask and shift.
--
Isaku Yamahata <isaku.yamahata@intel.com>