From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id B6604E748E9 for ; Sun, 1 Oct 2023 06:20:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=N4wyqvLHFGYn7Xjq8oB07FNVPWsSSQJlohkOM4lIVig=; b=DH6odR7i0SFkXV p/ThIKZscxIr+uVGN/gbH4FA0TlOc6SpRTqhSmmFT7XCHA87/LaUSv8auqZFPGGUJyMv3Ra9QVzu0 ASwK1qY4eAFilDYEwyz9KivKkBKME2S+mVBJK7Z7cG6GyknCB4cpWkawTzLMnZpHaeisKx4WbVLDF iF34mahn2dDHMyrEjH+AfRfNfOHs5Hbf/nQtFv7xDdq1ohgyW/eqDoer51i0cuwXkm7xWjrdHl32o T6P+pUEv/bCPTBzm/sw+BCGP3o0R+StiiNYFgUs5Koq9UaO2NXxLyoWmuv5UhVQc8AeJlr2VuqTk3 e2RpLyN6BK2VhS3+UZcw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qmpnm-00Af93-1i; Sun, 01 Oct 2023 06:19:38 +0000 Received: from ams.source.kernel.org ([2604:1380:4601:e00::1]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qmpni-00Af7x-1i for linux-arm-kernel@lists.infradead.org; Sun, 01 Oct 2023 06:19:36 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by ams.source.kernel.org (Postfix) with ESMTP id C9C14B808C0; Sun, 1 Oct 2023 06:19:31 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E1221C433C7; Sun, 1 Oct 2023 06:19:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1696141170; bh=HCPDJuB+VMwSxIKjZO86qo5UoEDCRGeYi9qs9U712Rk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=V1tJ4TJboCGp7/y9JWb9M49g9jcwUtV8cgBkmuBful5NJDpuM2utscf1o1NzLll4a Hgr9J811M/9jsP2eJoReuyfljPha7iYKEMnhZLMiVfB7O9Q3MG3DdYR6A1Ru0dod/P UGRNmRrHHacfULccI+eASPQOUtOWXFBTL6Ef259A= Date: Sun, 1 Oct 2023 08:19:27 +0200 From: Greg KH To: Wei Liu Cc: Nuno Das Neves , 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, 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 13/15] uapi: hyperv: Add mshv driver headers defining hypervisor ABIs Message-ID: <2023100154-ferret-rift-acef@gregkh> References: <1696010501-24584-1-git-send-email-nunodasneves@linux.microsoft.com> <1696010501-24584-14-git-send-email-nunodasneves@linux.microsoft.com> <2023093057-eggplant-reshoot-8513@gregkh> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230930_231934_835582_BC9D9469 X-CRM114-Status: GOOD ( 31.53 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Sat, Sep 30, 2023 at 10:01:58PM +0000, Wei Liu wrote: > On Sat, Sep 30, 2023 at 08:09:19AM +0200, Greg KH wrote: > > On Fri, Sep 29, 2023 at 11:01:39AM -0700, Nuno Das Neves wrote: > > > These must be in uapi because they will be used in the mshv ioctl API. > > > > > > Version numbers for each file: > > > hvhdk.h 25212 > > > hvhdk_mini.h 25294 > > > hvgdk.h 25125 > > > hvgdk_mini.h 25294 > > > > what are version numbers? > > These are internal version numbers for the hypervisor headers. We keep > track of them so that we can detect if there are any breakages in the > ABI, and thus either ask them to be fixed or we come up with ways to > maintain compatibility. People outside of Microsoft don't need to worry > about this. If you don't think this information belongs in the commit > message, we can drop it. Internal numbers to a single company that have no relevance to anyone else do not belong in a changelog comment. Would you want to see this in any other kernel changelog message for any other portion of the kernel? > > > diff --git a/include/uapi/hyperv/hvgdk.h b/include/uapi/hyperv/hvgdk.h > > > new file mode 100644 > > > index 000000000000..9bcbb7d902b2 > > > --- /dev/null > > > +++ b/include/uapi/hyperv/hvgdk.h > > > @@ -0,0 +1,41 @@ > > > +/* SPDX-License-Identifier: MIT */ > > > > That's usually not a good license for a new uapi .h file, why did you > > choose this one? > > > > This is chosen so that other Microsoft developers who don't normally > work on Linux can review this code. Sorry, but that's not how kernel development is done. Please fix your internal review processes and use the correct uapi header file license. If your lawyers insist on this license, that's fine, but please have them provide a signed-off-by on the patch that adds it and have it documented why it is this license in the changelog AND in a comment in the file so we can understand what is going on with it. > > > +/* Define connection identifier type. */ > > > +union hv_connection_id { > > > + __u32 asu32; > > > + struct { > > > + __u32 id:24; > > > + __u32 reserved:8; > > > + } __packed u; > > > > bitfields will not work properly in uapi .h files, please never do that. > > Can you clarify a bit more why it wouldn't work? Endianess? Alignment? Yes to both. Did you all read the documentation for how to write a kernel api? If not, please do so. I think it mentions bitfields, but it not, it really should as of course, this will not work properly with different endian systems or many compilers. thanks, greg k-h _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel