[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6d2a6bd4-a7cf-4672-9fb0-975acdc8ed31@linux.microsoft.com>
Date: Wed, 13 Nov 2024 15:32:32 -0800
From: Nuno Das Neves <nunodasneves@...ux.microsoft.com>
To: Michael Kelley <mhklinux@...look.com>,
"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"kvm@...r.kernel.org" <kvm@...r.kernel.org>,
"iommu@...ts.linux.dev" <iommu@...ts.linux.dev>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
"linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
"virtualization@...ts.linux.dev" <virtualization@...ts.linux.dev>
Cc: "kys@...rosoft.com" <kys@...rosoft.com>,
"haiyangz@...rosoft.com" <haiyangz@...rosoft.com>,
"wei.liu@...nel.org" <wei.liu@...nel.org>,
"decui@...rosoft.com" <decui@...rosoft.com>,
"catalin.marinas@....com" <catalin.marinas@....com>,
"will@...nel.org" <will@...nel.org>, "luto@...nel.org" <luto@...nel.org>,
"tglx@...utronix.de" <tglx@...utronix.de>,
"mingo@...hat.com" <mingo@...hat.com>, "bp@...en8.de" <bp@...en8.de>,
"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
"x86@...nel.org" <x86@...nel.org>, "hpa@...or.com" <hpa@...or.com>,
"seanjc@...gle.com" <seanjc@...gle.com>,
"pbonzini@...hat.com" <pbonzini@...hat.com>,
"peterz@...radead.org" <peterz@...radead.org>,
"daniel.lezcano@...aro.org" <daniel.lezcano@...aro.org>,
"joro@...tes.org" <joro@...tes.org>,
"robin.murphy@....com" <robin.murphy@....com>,
"davem@...emloft.net" <davem@...emloft.net>,
"edumazet@...gle.com" <edumazet@...gle.com>,
"kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com"
<pabeni@...hat.com>, "lpieralisi@...nel.org" <lpieralisi@...nel.org>,
"kw@...ux.com" <kw@...ux.com>, "robh@...nel.org" <robh@...nel.org>,
"bhelgaas@...gle.com" <bhelgaas@...gle.com>, "arnd@...db.de"
<arnd@...db.de>, "sgarzare@...hat.com" <sgarzare@...hat.com>,
"jinankjain@...ux.microsoft.com" <jinankjain@...ux.microsoft.com>,
"muminulrussell@...il.com" <muminulrussell@...il.com>,
"skinsburskii@...ux.microsoft.com" <skinsburskii@...ux.microsoft.com>,
"mukeshrathor@...rosoft.com" <mukeshrathor@...rosoft.com>,
"vkuznets@...hat.com" <vkuznets@...hat.com>,
"ssengar@...ux.microsoft.com" <ssengar@...ux.microsoft.com>,
"apais@...ux.microsoft.com" <apais@...ux.microsoft.com>
Subject: Re: [PATCH v2 3/4] hyperv: Add new Hyper-V headers in include/hyperv
On 11/11/2024 11:31 AM, Michael Kelley wrote:
> From: Nuno Das Neves <nunodasneves@...ux.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@...ux.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.
>
Agreed, thanks for helping clarify the points.
>>>>
>>>> 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. :-)
>
Ok, great. I'll use that for the next version then.
Thanks again!
Nuno
> 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
>>
Powered by blists - more mailing lists