Message ID | 1696010501-24584-15-git-send-email-nunodasneves@linux.microsoft.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:cae8:0:b0:403:3b70:6f57 with SMTP id r8csp4240626vqu; Fri, 29 Sep 2023 11:51:11 -0700 (PDT) X-Google-Smtp-Source: AGHT+IER9GrDIIJMFS4HUDj+VtLQERIHf4xqRHKp433Rqc8pkkhPqIOQu+qeKxfymIZjDfmPedYh X-Received: by 2002:a05:6808:1911:b0:3a1:d075:348b with SMTP id bf17-20020a056808191100b003a1d075348bmr6079312oib.59.1696013471077; Fri, 29 Sep 2023 11:51:11 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1696013471; cv=none; d=google.com; s=arc-20160816; b=PY7n0ke/xgk9UY8erN4jXuh4WPFnCWkekxur/N4mNnmkWGcSRYzWHkZ4z/1U13JjVX UH8Bw8AjxKbhwVHkM3cvlxjf8OyfjXC9fFMEQ/FiSqzv0L0xBlX5SsmopIG1WSl5KZ7M i85O4jturgge/sv66M0kZPt4XI19YKBOvRILJ3KaCHynsgDuR+tE2APb2gxhPWSWmM9R HyPEJVtYxUCJ4O5sJLzkOOu91hGHIqa2xT0cVRt2Z2YRpYNLyUUO33lU1cxPpsWJaQiW HbGF1rJ8agwKEIhxxu67OLuL4a/IMAZyi/VATs4LmAAjZG3rbN5KNPDElJA7QljmjPbw dXPg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:references:in-reply-to:message-id:date:subject :cc:to:from:dkim-signature:dkim-filter; bh=kBs0ffdpHMIIhbU0gkxsqFmjG+eS/wfhbZzz7iGIYls=; fh=rrENblR9DW+P8f8MSnFC4QkpxyTeAbSNxgPLVTXlWGs=; b=MrKxMfjziUUSevCElVkbvOs8rYpIhd2UcK8eXK3EF6VYZZAfVYB0jaPIRnhORJlngQ Crf3o4oHHbi9yCVIFlHtkwRto5Mi6ZkuBCRUYBK+SiVUHtDwMWaMEl1/ezX5lY0Z/7WA R02xzQCCjdn49BNYHfrRNtV1DmxmRkSUfE7hHiLmuJA0MCfshUiGsaW3D87M/Qkp9gLF ERkW1NfXqBmxfF69IxS3z2/Zv5R1GwHZxZIc5RQadL5IGMRolOmb4O3bdZwIWT3u68li pHP6gP3iCS9E7BxhDqmAW5m3XQ2lmDAo/CtmBmOgpMEaJveF6pvhbzpdYJBrHoZ+6ESS c0pg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linux.microsoft.com header.s=default header.b=epWxzT8n; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linux.microsoft.com Received: from lipwig.vger.email (lipwig.vger.email. [2620:137:e000::3:3]) by mx.google.com with ESMTPS id x33-20020a634861000000b00578f4ca8b8fsi21715746pgk.260.2023.09.29.11.51.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 29 Sep 2023 11:51:11 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3; Authentication-Results: mx.google.com; dkim=pass header.i=@linux.microsoft.com header.s=default header.b=epWxzT8n; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linux.microsoft.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by lipwig.vger.email (Postfix) with ESMTP id D196381DC600; Fri, 29 Sep 2023 11:04:10 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at lipwig.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233892AbjI2SCd (ORCPT <rfc822;pwkd43@gmail.com> + 19 others); Fri, 29 Sep 2023 14:02:33 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56224 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233559AbjI2SCJ (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 29 Sep 2023 14:02:09 -0400 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id C981ECE5; Fri, 29 Sep 2023 11:02:05 -0700 (PDT) Received: from linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net (linux.microsoft.com [13.77.154.182]) by linux.microsoft.com (Postfix) with ESMTPSA id D8BA620B74DA; Fri, 29 Sep 2023 11:01:57 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com D8BA620B74DA DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1696010517; bh=kBs0ffdpHMIIhbU0gkxsqFmjG+eS/wfhbZzz7iGIYls=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=epWxzT8nag++BMbDL9pI/Zo3parOf+xcBXotuu+DkNyyS4/fZujZvghn4YS2InzpY o/6/sS4UO4b41UzIuLpMkfaxbiAhPoxTyUtArwQfhNpfCTjqOY7sNnLOPzrw0XTJXQ gbjlGc5Hru0bKtywyfyPKO62isHLQRZRfqNB9sJc= From: Nuno Das Neves <nunodasneves@linux.microsoft.com> To: linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org, x86@kernel.org, linux-arm-kernel@lists.infradead.org, linux-arch@vger.kernel.org Cc: 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: [PATCH v4 14/15] asm-generic: hyperv: Use new Hyper-V headers conditionally. Date: Fri, 29 Sep 2023 11:01:40 -0700 Message-Id: <1696010501-24584-15-git-send-email-nunodasneves@linux.microsoft.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1696010501-24584-1-git-send-email-nunodasneves@linux.microsoft.com> References: <1696010501-24584-1-git-send-email-nunodasneves@linux.microsoft.com> X-Spam-Status: No, score=-8.4 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Fri, 29 Sep 2023 11:04:10 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1778399021441140470 X-GMAIL-MSGID: 1778399021441140470 |
Series |
Introduce /dev/mshv drivers
|
|
Commit Message
Nuno Das Neves
Sept. 29, 2023, 6:01 p.m. UTC
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
Comments
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 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. Would you consider: 1) Updating hyperv-tlfs with all newly documented TLFS fields that are in the HDK headers? 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 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. 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 > >
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? If these are needed elsewhere in the kernel, they can just be added to hyperv-tlfs.h. > 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. > > 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. > 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. 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 >> >>
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 > >> > >> >
On 10/5/2023 12:52 PM, Alex Ionescu wrote: >>> 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 > I don't know the current plans for updating the TLFS. But yes, assuming a new TLFS doc has something that is needed in the kernel, hyperv-tlfs.h would be updated to support that. Thanks, Nuno
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 */