From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933647AbdBPVNA (ORCPT ); Thu, 16 Feb 2017 16:13:00 -0500 Received: from mail-sn1nam02on0108.outbound.protection.outlook.com ([104.47.36.108]:50736 "EHLO NAM02-SN1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S933096AbdBPVM6 (ORCPT ); Thu, 16 Feb 2017 16:12:58 -0500 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=abanman@hpe.com; Subject: Re: [PATCH 6/6] x86/platform/uv/BAU: Add payload descriptor qualifier To: Thomas Gleixner References: <1487123931-56809-1-git-send-email-abanman@hpe.com> <1487123931-56809-7-git-send-email-abanman@hpe.com> CC: , , , , , , , From: Andrew Banman Message-ID: Date: Thu, 16 Feb 2017 15:12:48 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [192.48.192.5] X-ClientProxiedBy: BY2PR1001CA0008.namprd10.prod.outlook.com (10.164.163.146) To TU4PR84MB0271.NAMPRD84.PROD.OUTLOOK.COM (10.162.186.17) X-MS-Office365-Filtering-Correlation-Id: e7b9a977-f87c-41d2-8774-08d456b09495 X-MS-Office365-Filtering-HT: Tenant X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001)(48565401081);SRVR:TU4PR84MB0271; X-Microsoft-Exchange-Diagnostics: 1;TU4PR84MB0271;3:UEsbPyWBXxBwQduDaI75vVJtxo9R3cbvPDXeFDY3otNdDXaEdJRAJ+3nnXFyjxyFuEXuX16WGQtL7EPQzIu+3+Avn1KGtyT0c3erYD5urMySK3aiTo/hfbSbp1X/MoJvMPQH3g+9e2pAuyuR7FV8pGpRobOioVybEeL2ZYw3Ho6zNbopRzNTcjfnbLfgX32ov25OvVILeFTQrjfhAwmk8AWf8WlAkhutwpusigJb3C3IkrUzzvQKwNEhvwYFhkgR3WyZMwLJss4xEJFaCSwN4ieSufd4IFkxnpPgoQCMsaQ=;25:jgtzpDPYJcAwFqNoKMSYroocDRWzUwT0XOJFHj9yAksUudDqMdXBZyCyJze1FdyuHAoD+Uc1HxgdbS9U4hHqYEoMGCMOWr3zMb40aR52GS6rPCBOAX8/y8AiMeG9vZwdyrgcULZ2rR+WXwMWHNQ3AWafoQCvSQSWTgaYnnrQqLRqzUdhLDRfmWPNKNaEoA94ZuEnncc6Zo0FqPfqwZfemZHQ5HtrSuqXz+5P97Q4bEtHhjGNqvbCqpH0wMTLXGJ3Hm7F5KUj3+/jqh0Mw28pYF4pvleHDDUDISkoOPgeOVuSjAPSQvkLWcY4aRzBIGR/NcRAzSLyu9ri1wCXhgc6/b4it+ZuX1iI3mrNs2JVCTfL3qI4I2G6kLqehOwkFbitqF2OM5MfY+QHSo6ON4m5A3DTJZ09LKpd0Wm/R2x2/bvbY3Kqd6Z9fhuKzAzRm+p6+KP1UMeep+mKeqeZNy3LCA== X-Microsoft-Exchange-Diagnostics: 1;TU4PR84MB0271;31:/8HI8mSGR0K+d+dGvVx9HYqLtV32U+4Y86u4lVjv9/AOG8s6olapOcZDevHfJNAxBJN0gBx88uvqMMJCWT/c7GRJFxk7OASPiGy2l/E4lDzXzhxN3nNq3DPXx8WA2eWA54uAvK6J9UfJX7PIkyIF4DRR0VFYV21NB2SjnqAq6mXhResM182VyD/Du9/zrXvAeigXaznAiFureeea7TGM0BHSMdxjd8XCayadw8UBqlI1NOleO0vQX3hya7UvJriv;20:S9ZCcQLv25GQxA6sbl2AxuI5+qxP1ZardLEeB8TV6CAVS6k6HBClYWIRfF9OO9wZ3V0UNmNJ8FrsmGNE2QL6bKWyl5EOE9CcmWYZNtlbJ80DoUW+TLyS1GdN/vZAB4/pHl2r4nvuhXeMkRRzs2B8xPFucbayYHYc46KAH/132/sBVfTBWr3WqKPLnVMQ+2YPSIvgMcXdZNzHiEcPH5Al0De0Raufp7tO1zXMNOBwsMrp9Yw5l5SALs4yBwK4bh7WQv4L3TkHKTG4rmPaRDHBGRbwy2miPIrk26fZNkEeF06MvbyBkTquG7040MzrfrcJqgfG2KmDjFeFSF51BY71rS6BwF42IdkXaLrRjI5gqXJ+FnW9HnfSJnHuN1M8ycxBiM0gGNIY4lzcU8IPbMEUXAkw7l1kzn/9K4dyHpfEGMEKgpT0jAioDLHnbjczbC3EfRffW0izT8UPqOX+5YrFfcYl3Th8CCyFiMw6xkD59oLRqz/NK9xT2ElixQKu98sI X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(788757137089); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040375)(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001)(6055026)(6041248)(20161123555025)(20161123562025)(20161123564025)(20161123558025)(20161123560025)(6072148);SRVR:TU4PR84MB0271;BCL:0;PCL:0;RULEID:;SRVR:TU4PR84MB0271; X-Microsoft-Exchange-Diagnostics: 1;TU4PR84MB0271;4:ZLBPyHnPXkcNTatNYuoKvFkQemn2FiRvcKl09IgXDFr+uL1n8GcHFVu/YbPGU+SvEcmaDL3rMUzVdC7YypmidcEDtvGbYm4YcRkYun1eGJzpfmAHKsQVS7zJN5EcGc7pGnjiWRMOkNwrSTli2R41ibStFHYtGx9HP6hNOG0ezcUXYkqTWDyQwcOPTB3ZJMXh33Rt/3a2HHshPY9itKq748PoibIU6o18BFZyniN3rOyxLubNoqcg9rNYo0yh6rA0R6RkCgGfxPBJGcPIFvJVjpABqT3+psBf/QPjuGRB+ZOwgOK1r1yBmLuPF6MeNtscqA04t+LFj2zRZbYpfK+z6QWjX5jeCBuiObwvVsxun2osbNxDviwWVbLzD8oDWKxc59Xfxl6Lc90LYWzqX1k42EoKFCx1pFZ7zx1HLJA+wCenWJYMUpQ8IVQWiyKH3g1GYvu8dkdHcZoGuiwcvsV0vEIATLDMbHs3cvuiAZ3t33FC+Ifpd8HK7LZ89sIQtKALt7+au1ifpnw/J6CuCGc4PaqoGSW9i4WKkRkzqM7jmYG5+VS43gY3ozAayelTcHFl3WC9HkOR8fWt41Mf7KZXsPwE248IzouWGrIJ43oTXF7gxGYg0DPB0tMiqpVq1KUaFRioOiBpoI2P1+HSPzJvJBikWEWb05EWkudi35qCPhc= X-Forefront-PRVS: 0220D4B98D X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6049001)(6009001)(7916002)(39840400002)(39410400002)(39860400002)(39850400002)(39450400003)(377454003)(24454002)(199003)(189002)(33646002)(54906002)(230700001)(53936002)(68736007)(110136004)(2906002)(6246003)(6666003)(97736004)(389900003)(53546006)(6916009)(38730400002)(189998001)(4001350100001)(86362001)(2950100002)(65826007)(64126003)(50986999)(305945005)(6486002)(106356001)(50466002)(105586002)(3846002)(36756003)(6116002)(23746002)(81166006)(42186005)(81156014)(7736002)(101416001)(65956001)(66066001)(83506001)(65806001)(229853002)(31696002)(54356999)(76176999)(31686004)(47776003)(92566002)(8676002)(5660300001)(4326007)(345774005);DIR:OUT;SFP:1102;SCL:1;SRVR:TU4PR84MB0271;H:[128.162.233.168];FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?Windows-1252?Q?1;TU4PR84MB0271;23:E8UWvGuUTmhPBKH0frB5W1PplfBUZkxCBWCUP?= =?Windows-1252?Q?qN2G06NqP1oblSK4ivcBb2Nqp6Xwl79PR5wp1pTfIeJI4vPp1p85vAlm?= =?Windows-1252?Q?wskIy7w62SmvUZQlOFA5WUk6HSGbOfGZSnzuKXNBw/OMmhcatYe+V6Zd?= =?Windows-1252?Q?VdWCjgIvrn4hdKTxvngqHvjLcMxvoeGbxEwipY85dUfD5FjYN4gkLRc4?= =?Windows-1252?Q?bOqGCrqpenQcJvnLrD1ceX3KrCGXAco4mEztvaER+yYOkiKqMtyUdrNL?= =?Windows-1252?Q?FUMEsw2QvOL/Zi3rsOs4INKEmVweDTUxjjfCH8/wrHpxFnIsHiqxzjYo?= =?Windows-1252?Q?D25oaX2V+6iU7ua12OQUVEQMN+H4fVtfGqN0qlKAkX+zyb/dRWAo7pmb?= =?Windows-1252?Q?rVsJijHfGJGUmb9p8UV+N2WvRkohkGkNqTnGX5WdsAoEDOvtSHMakMAR?= =?Windows-1252?Q?aEIAHoYivKT80m8gW5wTv/6eaUfMMR7sW8mBVkyZEAYU/jUXJGoKAhaY?= =?Windows-1252?Q?yXlf9727oW5NfamTHDDSLMSH4k74r32BpJAXk3xwKh4SoXZIw2GUrtXs?= =?Windows-1252?Q?T/LC6eQGqFHzUvxTVP9fSwQ/t+VK7e4kCP8uzZNHTWEwUaTQsC/1gDTF?= =?Windows-1252?Q?qTXWJ7g4AqwoDKRX/yOtapiY3soSMRxUxO9Bu3Xy2f0SF9CWORxylu7g?= =?Windows-1252?Q?7XXkkGYPzYOPta8a0KC9+vHMOMEAEPMB2z2PSaGPuqlxyYzWIUB8DsW6?= =?Windows-1252?Q?Ml+MCa+ufiLWssVskdZRPzOYZ9PAHrDO7YepnZyiX2itS0gHtb8cer6g?= =?Windows-1252?Q?x2bQnHVw1Z5Z8DG1eCIiKiArJ3COsJqGU0Eo5mr9LjDg1Oz/vzEr8iW/?= =?Windows-1252?Q?rrQe39VkcvJrnSq6QzQ9onORJh91OYY6AGfLLaZxZnBasyDapNjlrDme?= =?Windows-1252?Q?FnxHJNpNUsCMVAPhax+N4UpFNIW5pu8oerO3BEynV25iXe/h1Q+nM4wV?= =?Windows-1252?Q?vEwzxN91Tct4wlHFe7CYaq7+4mf2QBxNQOHXSgdnXrhqLWmgk1LDvNXF?= =?Windows-1252?Q?s8SADJjfoAz6JAubpGshqNQaKi315+3M+KmJd1nktEAD+NGLUoLqvPfn?= =?Windows-1252?Q?mOAbrbtYk4SlDXptVPc+UlYHuwk99ddR9E6O4vDdHK/z4Q5NkaZrIOSi?= =?Windows-1252?Q?qKfVI+pTPhVuJ7HY3O8yvvZmStTvI2CMMMsR7EF+t8aj8Gmczxj9u1cT?= =?Windows-1252?Q?2coQYcBWFvy1adu3tgv20hpRl+LuqAMoi8vrvJpj7GBDOwkpl5278rtv?= =?Windows-1252?Q?SYhXF7Jt4TcfuDnOe2C8LAYR/908fv2q/+0Z/0PQbZfYXkDTCkO7yFSQ?= =?Windows-1252?Q?KEhbraOCxnDzrlbXMDMTywYm7BZEO75GCMh5qdvSLtQ8OVQYlNLyNjeR?= =?Windows-1252?Q?9MBzvdpCFnIlL9qfJAch/Mm0wVoTNQjBRn9GskjFsUieRsCWwhdeSX0U?= =?Windows-1252?Q?qvJw1V5RH5jBxTXoRpdS30xDX7GY1FmAFi2oKVDSZT7QHKq9kr+UOfhj?= =?Windows-1252?Q?dp7bifVVfhTOci6SwkycXG6VI5cn8QxW50TVM75Ni9FX2ywrGXqqN+Kh?= =?Windows-1252?Q?JZMJ+gYWXDeGaJF4z9mS0w=3D?= X-Microsoft-Exchange-Diagnostics: 1;TU4PR84MB0271;6:tjJJJN54BAs/TQSFoFMIbx2wihOEbwLfviIh3q7PYFsGW7S/JIDquVQ1HdjhF0GTporJwteJa7SAJRQEWAeCcsBsPFS4OQdyPAj3XkNwqbTZhq0B8BltXUQYhEj8I1OyaHuCyNGODxvBYYTXO6MTYzh16zZy61mfozsT+AludnafjW6cEmyyjcFwBo2jJ6n5HJmCcalGQnqKPdNdj36fGNJvncigpCTyn1OOO2s5AgByrErXxTsWii3xYPh8Ks46EaiTQ7Y45j5RiFSztfB8PwYi1M3AzBAC9hVeU5rRDCWdOwA81PtaCagAFq7AEqbTXAgKwNed0ZwCyhWMtuzTYQLqDPa3sQ86z8z+4RgSW3ZBdIRJQ1R+uPT07WH2zx5W2XM9fN5709d2KfuNVn74JTyVnGOqqjYmTjJtHz/2jcA=;5:tpACRqsmwsoELX9vVw/VKELeWGefB8jabDffTRWvybw/esaXyvbVBbRdKu/0T7dnzYSSaYlYqYqjNBFStjyMuYj2zfdnSBk/k6p7nICq1j2foF5OhhOUAqfc6q48yZ4f0Or4rSzhPhc7fWTztSk+xhw7ei/FfjtoT7OPdN1m2Vk=;24:m7HGZRNv4gBUAusLvP4Q71XwA2V24c4e3KMWFnzrwQ+y4EMspZlntz82IwKKzvIFY9Tnkjv0KXSx7KePIffA5RIMFzo0UcIrf24uIz3DA10= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;TU4PR84MB0271;7:dg5hJO6Vpa8/AaWgXgSuDXnJchmJ8ti1zIQXW7Jpn9mCt4rLnsUbEriySCEKF7NKdIOk5j82HpXEqlh6Re/Llam7AKokks/E4uSlf6eEbN+ffoNCzbLZoevff6rSOWcWc4vnX/B5eeOo3eQnNc2Fxc7yjQEpU2HTh7+r8mWM3qix+Cci1MBR+7HNjVXEhnj9P075XQX+d7jvf/fadPxnK8dEdF5sT2lBy6aIs+qirdvbbES37EdymJF6e9/0vH+2vVby/NSB723OJyaIqfbtR1wEhusV3eJs33QlxvO3l8/QoAidd2dyRAavQWSbSOS+e7KOsdbU7l5bditj3dpsSA== X-OriginatorOrg: hpe.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Feb 2017 21:12:56.2204 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: TU4PR84MB0271 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org I will resubmit this patch and [Patch 5/6] as separate patches. They are both independent of the wait_completion feature. I agree with your suggestions here. The updated version will have KernelDoc comments and proper u** integer types. I'd also like to add a preparatory patch to implement the hub version enum constants. This patch will replace all the magic numbers used to check uvhub_version. Like so: [Patch 1/2] x86/platform/uv/BAU: Add uv_bau_version enumerated constants [PATCH 2/2] x86/platform/uv/BAU: Add payload descriptor qualifier Thank you for the suggestions! Andrew Banman On 02/16/2017 12:42 PM, Thomas Gleixner wrote: > On Tue, 14 Feb 2017, Andrew Banman wrote: > >> On UV4, the destination agent verifies each message by checking the >> descriptor qualifier field of the message payload. Messages without this >> field set to 0x534749 will cause a hub error to assert. > > Ok. > > What's missing here is: > > Seperate the message structs for uv123 and uv4. > >> Make this the default action for future architectures, anticipating they >> will have the same requirement. > > That's a guarantee to cause issues when uv5 comes around. The way better > solution for this is to do: > > enum uv_bau_version { > UV_BAU_V1 = 1, > UV_BAU_V2, > UV_BAU_V3, > UV_BAU_V4, > }; > > Make bau->uvhub_version type uv_bau_version and use the enum constants in > the switch case. That way the compiler will catch you when you add > UV_BAU_V5 and forgot to update that switch case. That's probably handy to > have that in a few other places which switch on the bau version. > >> -struct bau_msg_payload { >> +struct uv1_2_3_bau_msg_payload { >> unsigned long address; /* signifies a page or all >> TLB's of the cpu */ >> /* 64 bits */ >> @@ -236,6 +238,20 @@ struct bau_msg_payload { >> unsigned int reserved1:32; /* not usable */ >> }; >> >> +struct uv4_bau_msg_payload { >> + unsigned long address; /* signifies a page or all >> + * TLB's of the cpu >> + */ > > Please get rid of these tail comments. Either document the struct members > with a comment above the member or even better use the KernelDoc comment > format above the struct to document it. > >> + /* 64 bits */ > > And these are horrible. I had to look twice where this belongs to. I know > you copied existing crap, but that does not make it any better. And really, > if you want to express the size of a member here because you have to talk > to hardware then use the proper types we have for this: u64, u32, u16 .... > >> + unsigned short sending_cpu; /* filled in by sender */ >> + /* 16 bits */ >> + unsigned short acknowledge_count; /* filled in by destination */ >> + /* 16 bits */ >> + unsigned int reserved1:8; /* not usable */ >> + unsigned int qualifier:24; /* descriptor qualifier filled >> + * in by sender > > >> @@ -1200,6 +1200,7 @@ const struct cpumask *uv_flush_tlb_other >> struct bau_control *bcp; >> unsigned long descriptor_status; >> unsigned long status; >> + unsigned long address; > > Same types can go into a single line. No value in wasting lines. > > Thanks, > > tglx >