Re: Field names inside ms_hyperv_info

[Vitaly Kuznetsov <vkuznets@redhat.com>]

Wei Liu <wei.liu@kernel.org> writes:

> Hi all
>
> During my work to make Linux boot as root partition on MSHV, I found out
> that a privilege mask was not collected in ms_hyperv_info.
>
> Looking at the code, the field names of ms_hyperv_info are not
> consistent with the names defined in TLFS. That makes it difficult to
> choose a name for the field that stores the privilege mask.
>
> I've got a local patch to make the existing fields closer to TLFS. The
> suffix "_a" means the value comes from EAX.
>
> Given that this structure is also used on ARM, so having x86 suffix is
> probably not the best idea. Do people care?
>
> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> index c57799684170..913af5e93cce 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -26,9 +26,9 @@
>  #include <asm/hyperv-tlfs.h>
>
>  struct ms_hyperv_info {
> -       u32 features;
> -       u32 misc_features;
> -       u32 hints;
> +       u32 features_a;
> +       u32 features_d;
> +       u32 recommendations;
>         u32 nested_features;
>         u32 max_vp_index;
>         u32 max_lp_index;
>
> Any comment on this? I'm normally bad at naming things so any suggestion
> is welcomed.

My take: let's avoid ambiguous '_a', '_d' and use full register names,
it's only three letters after all. Let's also avoid suffix-less names as
eventually we'll need to add non-eax parts. That is:

       u32 features_eax;
       u32 features_edx;
       u32 recommendations_eax;
       u32 nested_features_eax;
...

I would also feel comfortable with these names sortened,

       u32 feat_eax;
       u32 feat_edx;
       u32 recomm_eax;
       u32 nested_feat_eax;
...

-- 
Vitaly