Re: [PATCH] KVM: SVM: Fix reserved fields of struct sev_es_save_area

[Carlos Bilbao]

On 10/4/22 11:29, Sean Christopherson wrote:
> On Tue, Oct 04, 2022, Carlos Bilbao wrote:
> > On 10/4/22 09:05, Carlos Bilbao wrote:
> >
> > > Reserved fields of struct sev_es_save_area are named by their order of
> > > appearance, but right now they jump from reserved_5 to reserved_7. Rename
> > > them with the correct order.
> > >
> > > Fixes: 6d3b3d34e39eb ("KVM: SVM: Update the SEV-ES save area mapping")
> > Actually, there is no bug, so this Fix tag could go. Thanks!!
> Fixes: is appropriate, if we think it's worth fixing.  Personally, I don't think
> it's worth the churn/effort to keep the reserved numbers accurate, e.g. if the
> two bytes in reserved_1 are used, then every other field will need to be updated
> just to accomodate a tiny change.  We'll find ourselves in a similar situation if
> field is added in the middle of reserved_3,
>
> If we really want to the number to have any kind of meaning without needing a pile
> of churn for every update, the best idea I can think of is to name them reserved_<offset>.
> That way only the affected reserved field needs to be modified when adding new
> legal fields.  But that has it's own flavor of maintenance burden as calculating
> and verifying the offset is a waste of everyone's time.
>
> TL;DR: I vote to sweep this under the rug and live with arbitrary/bad numbers.
Well, the discussion on what is the most appropriate way to name reserved
fields is orthogonal to this patch, IMO.

This change just follows the prior approach (reserved_x), but correctly.
Keep in mind that the existence of reserved_{1,5} and reserved_{7,11}
implies there's a reserved_6 (there isn't). Why knowingly keep something
that's wrong, even if small?

If the maintainers think this is worth changing, I will submit a new patch
without the "Fix" tag (nothing was broken) and with a new subject line
(instead of "Fix reserved fields" I will use "Order correctly reserved
fields").

Thanks,
Carlos