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 5170CE7C4E9 for ; Wed, 4 Oct 2023 18:17:18 +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-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To:Subject: MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=pBOqejZbwlX3czEY8lYrtggjIYrRvrNGayNIAY6eC7c=; b=J/cmA67nLDm9RT gyNGf2uFYTJcA9qIgmt5jDd1pkuorVlul/MvJ9wXNB+uassVq8MQdkD0gLen0U7tJI3FgezWQhtY9 8iAdbz6laE222EYi7CllCtFP3H/r6i2SBlJkku49wrYxT4H8+NFDwWe+bYb9xDsUefA3/VJr/qjQ8 JC/dzNm7rn1Wy7m70p8kWLUhh1Ofn8ldSU9uch+Ov5GhqAWw6uqwpAxVTcEJng48N5rD027Tu4I/+ H7oM3P7cl8j/lJsgghJU7kvkFMpXr/RN/dqrjarJOYtXXh3InrlhRR8MfEmDusD84tjAObiZx7yyo KoL0JKztZPIFZ4AxQTNA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qo6QX-000g3r-2B; Wed, 04 Oct 2023 18:16:53 +0000 Received: from linux.microsoft.com ([13.77.154.182]) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qo6QU-000g2x-0X for linux-arm-kernel@lists.infradead.org; Wed, 04 Oct 2023 18:16:51 +0000 Received: from [10.0.0.178] (c-76-135-56-23.hsd1.wa.comcast.net [76.135.56.23]) by linux.microsoft.com (Postfix) with ESMTPSA id D440620B74C2; Wed, 4 Oct 2023 11:16:46 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com D440620B74C2 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1696443407; bh=cb3Cn5uFmmhvezEjsgPr/7TAsXnOt8mw0WVyDVVkoNA=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=hQ5yb0Z3u+VTo0KbZX//PdVbLhCSFC3mhN67YsPEJy0IakC7G13dj12ECZ/+iFXKL 5RmIamHk1JCosfQJxlUOL/PeHl/M+f68XzUQyqtoNaGuXMdH370qibhMf07jSTguq9 WauZnyAwIRzLwbbRJO2zePDtjT9w415VjegYCqqo= Message-ID: Date: Wed, 4 Oct 2023 11:16:46 -0700 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v4 13/15] uapi: hyperv: Add mshv driver headers defining hypervisor ABIs Content-Language: en-US To: Greg KH , Dexuan Cui Cc: Wei Liu , "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" , "Michael Kelley (LINUX)" , KY Srinivasan , Haiyang Zhang , "apais@linux.microsoft.com" , Tianyu Lan , "ssengar@linux.microsoft.com" , MUKESH RATHOR , "stanislav.kinsburskiy@gmail.com" , "jinankjain@linux.microsoft.com" , vkuznets , "tglx@linutronix.de" , "mingo@redhat.com" , "bp@alien8.de" , "dave.hansen@linux.intel.com" , "hpa@zytor.com" , "will@kernel.org" , "catalin.marinas@arm.com" 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> <2023100154-ferret-rift-acef@gregkh> <2023100443-wrinkly-romp-79d9@gregkh> <2023100415-diving-clapper-a2a7@gregkh> From: Nuno Das Neves In-Reply-To: <2023100415-diving-clapper-a2a7@gregkh> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231004_111650_325344_82616E74 X-CRM114-Status: GOOD ( 23.86 ) 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 10/4/2023 10:50 AM, Greg KH wrote: > On Wed, Oct 04, 2023 at 05:36:32PM +0000, Dexuan Cui wrote: >>> From: Greg KH >>> Sent: Tuesday, October 3, 2023 11:10 PM >>> [...] >>> On Tue, Oct 03, 2023 at 04:37:01PM -0700, Nuno Das Neves wrote: >>>> On 9/30/2023 11:19 PM, Greg KH wrote: >>>>> 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: >>>>>>>> +/* Define connection identifier type. */ >>>>>>>> +union hv_connection_id { >>>>>>>> + __u32 asu32; >>>>>>>> + struct { >>>>>>>> + __u32 id:24; >>>>>>>> + __u32 reserved:8; >>>>>>>> + } __packed u; >> >> IMO the "__packed" is unnecessary. >> >>>>>>> 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. >>>> >>>> Yes, in >>> https://docs.k/ >>> ernel.org%2Fdriver- >>> api%2Fioctl.html&data=05%7C01%7Cdecui%40microsoft.com%7Ce404769e0f >>> 85493f0aa108dbc4a08a27%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C >>> 0%7C638319966071263290%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLj >>> AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C% >>> 7C%7C&sdata=RiLNA5DRviWBQK6XXhxC4m77raSDBb%2F0BB6BDpFPUJY%3D >>> &reserved=0 it says that it is >>>> "better to avoid" bitfields. >>>> >>>> Unfortunately bitfields are used in the definition of the hypervisor >>>> ABI. We import these definitions directly from the hypervisor code. >>> >>> So why do you feel you have to use this specific format for your >>> user/kernel api? That is not what is going to the hypervisor. >> These *are* going to the hypervisor - we use these same definitions in our driver for the kernel/hypervisor API. This is so we don't have to maintain two separate definitions for user/kernel and kernel/hypervisor APIs. >> If it's hard to avoid bitfield here, maybe we can refer to the definition of >> struct iphdr in include/uapi/linux/ip.h > > It is not hard to avoid using bitfields, just use the proper definitions > to make this portable for all compilers. And ick, ip.h is not a good > thing to follow :) > Greg, there is nothing making us use bitfields. It just makes the work of porting the hypervisor definitions to Linux easier - aided by the fact that in practice, all the compilers in our stack produce the same code for these. If that stops being true, or we need to support some other scenario, then I can see the value in changing it. Right now it just feels like pointless work. Just a reminder - we are the only consumers of this code right now; no one else can meaningfully use this interface yet. That all said, if you really insist on changing it, then please say so. And please point to an example of how it should be done so there is no confusion on the best path forward. Thanks, Nuno > thanks, > > greg k-h _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel