RE: [PATCH v2 3/4] hyperv: Add new Hyper-V headers in include/hyperv

Michael Kelley
2024-11-11T19:31:08Z

From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Monday, November 11, 2024 10:45 AM
>
> On 11/10/2024 8:13 PM, Michael Kelley wrote:
> > From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Thursday,
> November 7, 2024 2:32 PM
> >>
> >> These headers contain definitions for regular Hyper-V guests (as in
> >> hyperv-tlfs.h), as well as interfaces for more privileged guests like
> >> Dom0.
> >
> > See my comment on Patch 0/4 about use of "dom0" terminology.
> >
>
> Thanks, noted.
>
> >>
> >> These files are derived from headers exported from Hyper-V, rather than
> >> being derived from the TLFS document. (Although, to preserve
> >> compatibility with existing Linux code, some definitions are copied
> >> directly from hyperv-tlfs.h too).
> >>
> >> The new files follow a naming convention according to their original
> >> use:
> >> - hdk "host development kit"
> >> - gdk "guest development kit"
> >> With postfix "_mini" implying userspace-only headers, and "_ext" for
> >> extended hypercalls.
> >>
> >> These names should be considered a rough guide only - since there are
> >> many places already where both host and guest code are in the same
> >> place, hvhdk.h (which includes everything) can be used most of the time.
> >
> > Just curious -- are there really cases where hvhdk.h can't be used?
> > If so, could you summarize why?
> >
>
> No, there aren't cases where it "can't" be used. I suppose if someone
> doesn't want to include everything, perhaps they could just include
> hvgdk.h, for example. It doesn't really matter though.
>
> > I ask because it would be nice to expand slightly on your paragraph
> > below, as follows: (if indeed what I've added is correct)
> >
> > The use of multiple files and their original names is primarily to
> > keep the provenance of exactly where they came from in Hyper-V
> > code, which is helpful for manual maintenance and extension
> > of these definitions. Microsoft maintainers importing new definitions
> > should take care to put them in the right file. However, Linux kernel code
> > that uses any of the definitions need not be aware of the multiple files
> > or assign any meaning to the new names. Linux kernel uses should
> > always just include hvhdk.h
> >
>
> Thanks, I think that additional sentence helps clarify things. I'll
> include it in the next version, and I think I can probably omit the prior
> paragraph: "These names should be considered a rough guide only...".
>

Omitting that prior paragraph is OK with me. The key thoughts from my
standpoint are:
* The separation into multiple files and the file names come from
the Windows Hyper-V world and are maintained to ease bringing
the definitions over from that world

* Linux code can ignore the multiple files and their names. Just
#include hvhdk.h.

> >>
> >> The original names are kept intact primarily to keep the provenance of
> >> exactly where they came from in Hyper-V code, which is helpful for
> >> manual maintenance and extension of these definitions. Microsoft
> >> maintainers importing new definitions should take care to put them in
> >> the right file.
> >>
> >> Note also that the files contain both arm64 and x86_64 code guarded by
> >> \#ifdefs, which is how the definitions originally appear in Hyper-V.
> >
> > Spurious backslash?
> >
>
> Indeed, thanks.
>
> > I would suggest some additional clarification: The #ifdef guards are
> > employed minimally where necessary to prevent conflicts due to
> > different definitions for the same thing on x86_64 and arm64. Where
> > there are no conflicts, the union of x86_64 definitions and arm64
> > definitions is visible when building for either architecture. In other
> > words, not all definitions specific to x86_64 are protected by #ifdef
> > x86_64. Such unprotected definitions may be visible when building
> > for arm64. And vice versa.
> >
>
> Is there a reason you specifically want to point out that "Such
> unprotected definitions may be visible when building for arm64. And vice
> versa."? I think, in all the cases where #ifdefs are not used, an
> arch-specific prefix is used - hv_x64_ or hv_arm64_.
>
> The main thing I wanted to call out here was the reasoning for not
> splitting arch-specific definitions into separate files in arch/x86/
> and arch/arm64/ as is typical in Linux.
>
> Maybe this is a bit clearer:
> "
> Note the new headers contain both arm64 and x86_64 definitions. Some are
> guarded by #ifdefs, and some are instead prefixed with the architecture,
> e.g. hv_x64_*. These conventions are kept from Hyper-V code as another
> tactic to simplify the process of importing and maintaining the
> definitions, rather than splitting them up into their own files in
> arch/x86/ and arch/arm64/.
> "

Yes, your new paragraph works for me. Your original statement was
"the files contain both arm64 and x86_64 code guarded by #ifdefs",
which sounds like the more typical Linux approach of using #ifdefs
to segregate into x86-specific, arm64-specific, and common. I was
just trying to be explicit that full segregation isn't done, and isn't a
goal, because of wanting to maintain alignment with the original
Hyper-V definitions.

It's "Hey, we know we're not handling this in the typical Linux way,
and here's why". Your revised paragraph covers that in a less
heavyweight way than what I wrote. :-)

Michael

>
> I hope it's reasonably clear that it's a good tradeoff to go against
> Linux convention in this case, to make it easy to import and maintain
> Hyper-V definitions.
>
> Thanks
> Nuno
>