Re: [PATCH 06/21] KVM: TDX: Add accessors VMX VMCS helpers

From: Edgecombe, Rick P
Date: Mon Sep 09 2024 - 17:29:54 EST


On Mon, 2024-09-09 at 16:19 +0200, Paolo Bonzini wrote:
> On 9/4/24 05:07, Rick Edgecombe wrote:
> > +static __always_inline void td_##lclass##_clearbit##bits(struct vcpu_tdx
> > *tdx, \
> > +                                                        u32 field, u64
> > bit)    \
> > +{                                                                          
> >     \
> > +       u64
> > err;                                                                \
> > +                                                                           
> >     \
> > +       tdvps_##lclass##_check(field,
> > bits);                                    \
> > +       err = tdh_vp_wr(tdx, TDVPS_##uclass(field), 0,
> > bit);                    \
> > +       if (KVM_BUG_ON(err, tdx-
> > >vcpu.kvm))                                     \
> > +               pr_err("TDH_VP_WR["#uclass".0x%x] &= ~0x%llx failed:
> > 0x%llx\n", \
> > +                      field, bit, 
> > err);                                       \
>
> Maybe a bit large when inlined?  Maybe
>
>         if (unlikely(err))
>                 tdh_vp_wr_failed(tdx, field, bit, err);
>
> and add tdh_vp_wr_failed to tdx.c.

There is a tiny bit of difference between the messages:
pr_err("TDH_VP_WR["#uclass".0x%x] = 0x%llx failed: 0x%llx\n", ...
pr_err("TDH_VP_WR["#uclass".0x%x] |= 0x%llx failed: 0x%llx\n", ...
pr_err("TDH_VP_WR["#uclass".0x%x] &= ~0x%llx failed: 0x%llx\n", ...

We can parameterize that part of the message, but it gets a bit tortured. Or
just lose that bit of detail. We can take a look. Thanks.