linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Alex Ionescu <aionescu@gmail.com>
To: Nuno Das Neves <nunodasneves@linux.microsoft.com>
Cc: linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
	x86@kernel.org,  linux-arm-kernel@lists.infradead.org,
	linux-arch@vger.kernel.org,  patches@lists.linux.dev,
	mikelley@microsoft.com, kys@microsoft.com,  wei.liu@kernel.org,
	gregkh@linuxfoundation.org, haiyangz@microsoft.com,
	 decui@microsoft.com, apais@linux.microsoft.com,
	Tianyu.Lan@microsoft.com,  ssengar@linux.microsoft.com,
	mukeshrathor@microsoft.com,  stanislav.kinsburskiy@gmail.com,
	jinankjain@linux.microsoft.com,  vkuznets@redhat.com,
	tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
	 dave.hansen@linux.intel.com, hpa@zytor.com, will@kernel.org,
	 catalin.marinas@arm.com
Subject: Re: [PATCH v4 14/15] asm-generic: hyperv: Use new Hyper-V headers conditionally.
Date: Thu, 5 Oct 2023 15:52:04 -0400	[thread overview]
Message-ID: <CAJ-90NL8S5xnJbiwCHAGs4QeiJ3DHUL0Obi1snqsTDJEpQRnsg@mail.gmail.com> (raw)
In-Reply-To: <749f477a-1e7a-495e-bea1-e3abe8da7fb9@linux.microsoft.com>

Hi Nuno,

Best regards,
Alex Ionescu

On Mon, Oct 2, 2023 at 8:41 PM Nuno Das Neves
<nunodasneves@linux.microsoft.com> wrote:
>
> Hi Alex,
>
> On 10/2/2023 12:35 PM, Alex Ionescu wrote:
> > Hi Nuno,
> >
> > I understand the requirement to have
> > undocumented/non-standard/non-TLFS-published information in the HDK
> > headers, however, the current state of this patch is that for any
> > other code that's not in the kernel today, or in this upcoming driver,
> > the hyperv-tlfs definitions are incomplete, because some *documented*
> > TLFS fields are only in HDK headers. Similarly, it is also impossible
>
> If I understand correctly, you are saying there are documented
> definitions (in the TLFS document), which are NOT in hyperv-tlfs.h, but
> ARE in these new HDK headers, correct?

Correct.

>
> If these are needed elsewhere in the kernel, they can just be added to
> hyperv-tlfs.h.

OK, great, As the need arises I will submit patches here to do so (and
source the TLFS page/paragraph as needed to help provide they're in
there). Thank you!

>
> > to only use the HDK headers for other use cases, because some basic
> > documented, standard defines only exist in hyperv-tlfs. So there is no
> > "logical" relationship between the two -- HDK headers are not _just_
> > undocumented information, but also documented information, but also
> > not complete documented information.
>
> That is correct - they are meant to be independently compileable.
> The new HDK headers only serve as a replacement *in our driver* when we
> need some definitions like do_hypercall() etc in mshyperv.h.

Understood.

>
> >
> > Would you consider:
> >
> > 1) Updating hyperv-tlfs with all newly documented TLFS fields that are
> > in the HDK headers?
>
> I think this can be done on an as-needed basis, as I outlined above.

Sounds good.

>
> > OR
> > 2) Updating the new HDK headers you're adding here to also include
> > previously-documented information from hyperv-tlfs? This way, someone
> > can include the HDK headers and get everything they need
>
> The new HDK headers are only intended for the new mshv driver.
>
> > OR
> > 3) Truly making hypertv-tlfs the "documented" header, and then > removing any duplication from HDK so that it remains the
> > "undocumented" header file. In this manner, one would include
> > hyperv-tlfs to use the stable ABI, and they would include HDK (which
> > would include hyperv-tlfs) to use the unstable+stable ABI.
>
> hyperv-tlfs.h is remaining the "documented" header.
>
> But, we can't make the HDK header depend on hyperv-tlfs.h, for 2 primary
> reasons:
> 1. We need to put the new HDK headers in uapi so that we can use them in
> our IOCTL interface. As a result, we can't include hyperv-tlfs.h (unless
> we put it in uapi as well).
> 2. The HDK headers not only duplicate, but also MODIFY some structures
> in hyperv-tlfs.h. e.g., The struct is in hyperv-tlfs.h, but a particular
> field or bitfield is not.

#2 was something I was worried about. Do you know if the
standards/docs team is planning on updating the TLFS at some point
with updates on their end? At which point I'd assume you'd be OK with
patches to add them to hyperv-tlfs.h

Thanks a lot!

--
Best regards,
Alex Ionescu

>
> Thanks,
> Nuno
>
> >
> > Thank you for your consideration.
> >
> > Best regards,
> > Alex Ionescu
> >
> > On Fri, Sep 29, 2023 at 2:02 PM Nuno Das Neves
> > <nunodasneves@linux.microsoft.com> wrote:
> >>
> >> Add asm-generic/hyperv-defs.h. It includes hyperv-tlfs.h or hvhdk.h
> >> depending on compile-time constant HV_HYPERV_DEFS which will be defined in
> >> the mshv driver.
> >>
> >> This is needed to keep unstable Hyper-V interfaces independent of
> >> hyperv-tlfs.h. This ensures hvhdk.h replaces hyperv-tlfs.h in the mshv
> >> driver, even via indirect includes.
> >>
> >> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> >> Acked-by: Wei Liu <wei.liu@kernel.org>
> >> ---
> >>   arch/arm64/include/asm/mshyperv.h |  2 +-
> >>   arch/x86/include/asm/mshyperv.h   |  3 +--
> >>   drivers/hv/hyperv_vmbus.h         |  1 -
> >>   include/asm-generic/hyperv-defs.h | 26 ++++++++++++++++++++++++++
> >>   include/asm-generic/mshyperv.h    |  2 +-
> >>   include/linux/hyperv.h            |  2 +-
> >>   6 files changed, 30 insertions(+), 6 deletions(-)
> >>   create mode 100644 include/asm-generic/hyperv-defs.h
> >>
> >> diff --git a/arch/arm64/include/asm/mshyperv.h b/arch/arm64/include/asm/mshyperv.h
> >> index 20070a847304..8ec14caf3d4f 100644
> >> --- a/arch/arm64/include/asm/mshyperv.h
> >> +++ b/arch/arm64/include/asm/mshyperv.h
> >> @@ -20,7 +20,7 @@
> >>
> >>   #include <linux/types.h>
> >>   #include <linux/arm-smccc.h>
> >> -#include <asm/hyperv-tlfs.h>
> >> +#include <asm-generic/hyperv-defs.h>
> >>
> >>   /*
> >>    * Declare calls to get and set Hyper-V VP register values on ARM64, which
> >> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> >> index e3768d787065..bb1b97106cd3 100644
> >> --- a/arch/x86/include/asm/mshyperv.h
> >> +++ b/arch/x86/include/asm/mshyperv.h
> >> @@ -6,10 +6,9 @@
> >>   #include <linux/nmi.h>
> >>   #include <linux/msi.h>
> >>   #include <linux/io.h>
> >> -#include <asm/hyperv-tlfs.h>
> >>   #include <asm/nospec-branch.h>
> >>   #include <asm/paravirt.h>
> >> -#include <asm/mshyperv.h>
> >> +#include <asm-generic/hyperv-defs.h>
> >>
> >>   /*
> >>    * Hyper-V always provides a single IO-APIC at this MMIO address.
> >> diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> >> index 09792eb4ffed..0e4bc18a13fa 100644
> >> --- a/drivers/hv/hyperv_vmbus.h
> >> +++ b/drivers/hv/hyperv_vmbus.h
> >> @@ -15,7 +15,6 @@
> >>   #include <linux/list.h>
> >>   #include <linux/bitops.h>
> >>   #include <asm/sync_bitops.h>
> >> -#include <asm/hyperv-tlfs.h>
> >>   #include <linux/atomic.h>
> >>   #include <linux/hyperv.h>
> >>   #include <linux/interrupt.h>
> >> diff --git a/include/asm-generic/hyperv-defs.h b/include/asm-generic/hyperv-defs.h
> >> new file mode 100644
> >> index 000000000000..ac6fcba35c8c
> >> --- /dev/null
> >> +++ b/include/asm-generic/hyperv-defs.h
> >> @@ -0,0 +1,26 @@
> >> +/* SPDX-License-Identifier: GPL-2.0 */
> >> +#ifndef _ASM_GENERIC_HYPERV_DEFS_H
> >> +#define _ASM_GENERIC_HYPERV_DEFS_H
> >> +
> >> +/*
> >> + * There are cases where Microsoft Hypervisor ABIs are needed which may not be
> >> + * stable or present in the Hyper-V TLFS document. E.g. the mshv_root driver.
> >> + *
> >> + * As these interfaces are unstable and may differ from hyperv-tlfs.h, they
> >> + * must be kept separate and independent.
> >> + *
> >> + * However, code from files that depend on hyperv-tlfs.h (such as mshyperv.h)
> >> + * is still needed, so work around the issue by conditionally including the
> >> + * correct definitions.
> >> + *
> >> + * Note: Since they are independent of each other, there are many definitions
> >> + * duplicated in both hyperv-tlfs.h and uapi/hyperv/hv*.h files.
> >> + */
> >> +#ifdef HV_HYPERV_DEFS
> >> +#include <uapi/hyperv/hvhdk.h>
> >> +#else
> >> +#include <asm/hyperv-tlfs.h>
> >> +#endif
> >> +
> >> +#endif /* _ASM_GENERIC_HYPERV_DEFS_H */
> >> +
> >> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> >> index d832852d0ee7..6bef0d59d1b7 100644
> >> --- a/include/asm-generic/mshyperv.h
> >> +++ b/include/asm-generic/mshyperv.h
> >> @@ -25,7 +25,7 @@
> >>   #include <linux/cpumask.h>
> >>   #include <linux/nmi.h>
> >>   #include <asm/ptrace.h>
> >> -#include <asm/hyperv-tlfs.h>
> >> +#include <asm-generic/hyperv-defs.h>
> >>
> >>   #define VTPM_BASE_ADDRESS 0xfed40000
> >>
> >> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> >> index 4d5a5e39d76c..722a8cf23d87 100644
> >> --- a/include/linux/hyperv.h
> >> +++ b/include/linux/hyperv.h
> >> @@ -24,7 +24,7 @@
> >>   #include <linux/mod_devicetable.h>
> >>   #include <linux/interrupt.h>
> >>   #include <linux/reciprocal_div.h>
> >> -#include <asm/hyperv-tlfs.h>
> >> +#include <asm-generic/hyperv-defs.h>
> >>
> >>   #define MAX_PAGE_BUFFER_COUNT                          32
> >>   #define MAX_MULTIPAGE_BUFFER_COUNT                     32 /* 128K */
> >> --
> >> 2.25.1
> >>
> >>
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2023-10-05 19:52 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-29 18:01 [PATCH v4 00/15] Introduce /dev/mshv drivers Nuno Das Neves
2023-09-29 18:01 ` [PATCH v4 01/15] hyperv-tlfs: Change shared HV_REGISTER_* defines to HV_MSR_* Nuno Das Neves
2023-09-29 18:01 ` [PATCH v4 02/15] mshyperv: Introduce hv_get_hypervisor_version function Nuno Das Neves
2023-09-29 18:01 ` [PATCH v4 03/15] mshyperv: Introduce numa_node_to_proximity_domain_info Nuno Das Neves
2023-09-29 18:01 ` [PATCH v4 04/15] asm-generic/mshyperv: Introduce hv_recommend_using_aeoi() Nuno Das Neves
2023-09-29 18:01 ` [PATCH v4 05/15] hyperv: Move hv_connection_id to hyperv-tlfs Nuno Das Neves
2023-09-29 18:01 ` [PATCH v4 06/15] hyperv-tlfs: Introduce hv_status_to_string and hv_status_to_errno Nuno Das Neves
2023-09-29 18:01 ` [PATCH v4 07/15] Drivers: hv: Move hv_call_deposit_pages and hv_call_create_vp to common code Nuno Das Neves
2023-10-03  2:51   ` kernel test robot
2023-09-29 18:01 ` [PATCH v4 08/15] Drivers: hv: Introduce per-cpu event ring tail Nuno Das Neves
2023-09-30 11:26   ` kernel test robot
2023-09-29 18:01 ` [PATCH v4 09/15] Drivers: hv: Introduce hv_output_arg_exists in hv_common.c Nuno Das Neves
2023-10-02 19:29   ` Alex Ionescu
2023-10-04 18:27     ` Nuno Das Neves
2023-10-06 18:13       ` Long Li
2023-09-29 18:01 ` [PATCH v4 10/15] x86: hyperv: Add mshv_handler irq handler and setup function Nuno Das Neves
2023-09-29 18:01 ` [PATCH v4 11/15] Drivers: hv: export vmbus_isr, hv_context and hv_post_message Nuno Das Neves
2023-09-29 18:01 ` [PATCH v4 12/15] Documentation: Reserve ioctl number for mshv driver Nuno Das Neves
2023-09-29 18:01 ` [PATCH v4 13/15] uapi: hyperv: Add mshv driver headers defining hypervisor ABIs Nuno Das Neves
2023-09-30  6:09   ` Greg KH
2023-09-30 22:01     ` Wei Liu
2023-10-01  6:19       ` Greg KH
2023-10-03 23:29         ` Wei Liu
2023-10-04  6:11           ` Greg KH
2023-10-04 18:14             ` Wei Liu
2023-10-05  3:59             ` Wei Liu
2023-10-03 23:37         ` Nuno Das Neves
2023-10-04  6:09           ` Greg KH
2023-10-04 17:36             ` Dexuan Cui
2023-10-04 17:50               ` Greg KH
2023-10-04 18:16                 ` Nuno Das Neves
2023-10-05  6:26                   ` Greg KH
2023-09-30 16:33   ` kernel test robot
2023-10-05 10:27   ` Greg KH
2023-09-29 18:01 ` [PATCH v4 14/15] asm-generic: hyperv: Use new Hyper-V headers conditionally Nuno Das Neves
2023-10-02 19:35   ` Alex Ionescu
2023-10-03  0:41     ` Nuno Das Neves
2023-10-05 19:52       ` Alex Ionescu [this message]
2023-10-11 23:32         ` Nuno Das Neves
     [not found] ` <1696010501-24584-16-git-send-email-nunodasneves@linux.microsoft.com>
2023-09-30  6:02   ` [PATCH v4 15/15] Drivers: hv: Add modules to expose /dev/mshv to VMMs running on Hyper-V kernel test robot
2023-09-30  6:11   ` Greg KH
2023-09-30 18:11     ` Wei Liu
2023-09-30 18:31       ` Greg KH
2023-09-30 21:13         ` Wei Liu
2023-10-01  6:20           ` Greg KH
2023-10-03 23:35             ` Wei Liu
2023-11-06  4:03   ` kernel test robot
2023-11-06 11:51   ` kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAJ-90NL8S5xnJbiwCHAGs4QeiJ3DHUL0Obi1snqsTDJEpQRnsg@mail.gmail.com \
    --to=aionescu@gmail.com \
    --cc=Tianyu.Lan@microsoft.com \
    --cc=apais@linux.microsoft.com \
    --cc=bp@alien8.de \
    --cc=catalin.marinas@arm.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=decui@microsoft.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=haiyangz@microsoft.com \
    --cc=hpa@zytor.com \
    --cc=jinankjain@linux.microsoft.com \
    --cc=kys@microsoft.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-hyperv@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mikelley@microsoft.com \
    --cc=mingo@redhat.com \
    --cc=mukeshrathor@microsoft.com \
    --cc=nunodasneves@linux.microsoft.com \
    --cc=patches@lists.linux.dev \
    --cc=ssengar@linux.microsoft.com \
    --cc=stanislav.kinsburskiy@gmail.com \
    --cc=tglx@linutronix.de \
    --cc=vkuznets@redhat.com \
    --cc=wei.liu@kernel.org \
    --cc=will@kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).