On Wed, 2025-01-15 at 20:36 +0100, Paolo Bonzini wrote: > WRT hkid, I interpreted "I'd personally probably just keep 'hkid' as an > int everywhere until the point where it gets shoved into the TDX module > ABI" as "it can be u16 in the SEAMCALLs and in mk_keyed_paddr" (as the > latter builds an argument to the SEAMCALLs). > > I understood his objection to be more about > tdx_guest_keyid_alloc/tdx_guest_keyid_free and struct kvm_tdx: > > > Oh, and casts like this: > > > > > static inline void tdx_disassociate_vp(struct kvm_vcpu *vcpu) > > > @@ -2354,7 +2354,8 @@ static int __tdx_td_init(struct kvm *kvm, struct td_params *td_params, > > > ret = tdx_guest_keyid_alloc(); > > > if (ret < 0) > > > return ret; > > > - kvm_tdx->hkid = ret; > > > + kvm_tdx->hkid = (u16)ret; > > > + kvm_tdx->hkid_assigned = true; > > > > are a bit silly, don't you think? > > so I didn't change tdx_guest_keyid_alloc(). There was a related comment on the GPA union Yan was suggesting: https://lore.kernel.org/kvm/753cd9f1-5eb7-480f-ae4f-d263aaecdd6c@xxxxxxxxx/ Basically that the bit fields have subtle behavior when you shift them (ironically the exact bug that happened with u16 keyid). But I think your reasoning seems valid, especially since Dave has since quoted that function without commenting on that aspect. So let's leave it.
