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 X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E7647C433F7 for ; Wed, 15 Jul 2020 08:35:10 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id BAF502067D for ; Wed, 15 Jul 2020 08:35:10 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=citrix.com header.i=@citrix.com header.b="c3vSpRcr" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org BAF502067D Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=citrix.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1jvcsS-00059a-Em; Wed, 15 Jul 2020 08:34:56 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1jvcsQ-00059V-Vt for xen-devel@lists.xenproject.org; Wed, 15 Jul 2020 08:34:55 +0000 X-Inumbo-ID: 0d0e30c2-c676-11ea-bb8b-bc764e2007e4 Received: from esa4.hc3370-68.iphmx.com (unknown [216.71.155.144]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 0d0e30c2-c676-11ea-bb8b-bc764e2007e4; Wed, 15 Jul 2020 08:34:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=citrix.com; s=securemail; t=1594802093; h=date:from:to:cc:subject:message-id:references: mime-version:content-transfer-encoding:in-reply-to; bh=m1zYlnWCGeigBRLopOjVVCPrIjQTIp4NpqO1z7cdwF4=; b=c3vSpRcrchH2MGhfiZvcH1S4LCkmxE+Qbx9C2m6wBm8O7tsHgSgwf0ND PqqL1rLTujN1lDWNTi7kn/k5gYbHd9PKjl6i7Pde6lbyxwwHClnrQbBAL FfdyEqCv5RXcQxplegZzobFLImJYHHk5QWhELHGK8/ZVN/Oaeh1VwOON6 s=; Authentication-Results: esa4.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none IronPort-SDR: gpXS6aAanqMBJpS65vS253WVzDpztu8kXwN+gYoGnX3JitLB9lQLZIAmPz9YOoJl+lLComVGv+ M3xnMFQ02IPwTh0t0mTFCQ1WyMTahyxo3RRaLc65CzY2oSxsA5Pua4Mk4yfqB4+HQgSm9Qx0Gx kZo6sK/nfXiEIo4bocXLeI0xhV5s2Yx5CcwJceYJVJ3PyvEDhwGNk62muZfiJ4v2qQvfbeOisG hYRFWZ1WKOHo+kA8OsJ0LbmMwEwomlsBUfu1TmG7roxGHND7eOH6Aocxz/JtsuVoMIbZe/dMiq 6bE= X-SBRS: 2.7 X-MesageID: 23256205 X-Ironport-Server: esa4.hc3370-68.iphmx.com X-Remote-IP: 162.221.158.21 X-Policy: $RELAYED X-IronPort-AV: E=Sophos;i="5.75,354,1589256000"; d="scan'208";a="23256205" Date: Wed, 15 Jul 2020 10:34:41 +0200 From: Roger Pau =?utf-8?B?TW9ubsOp?= To: Jan Beulich Subject: Re: [PATCH v2 5/7] x86: generalize padding field handling Message-ID: <20200715083441.GR7191@Air-de-Roger> References: <83274416-2812-53c9-f8cb-23ebdf73782e@suse.com> <20200714142948.GK7191@Air-de-Roger> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-ClientProxiedBy: AMSPEX02CAS02.citrite.net (10.69.22.113) To AMSPEX02CL02.citrite.net (10.69.22.126) X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: Stefano Stabellini , Julien Grall , Wei Liu , Paul Durrant , George Dunlap , Andrew Cooper , Ian Jackson , "xen-devel@lists.xenproject.org" Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" On Wed, Jul 15, 2020 at 08:36:10AM +0200, Jan Beulich wrote: > On 14.07.2020 16:29, Roger Pau Monné wrote: > > On Wed, Jul 01, 2020 at 12:27:37PM +0200, Jan Beulich wrote: > >> The original intention was to ignore padding fields, but the pattern > >> matched only ones whose names started with an underscore. Also match > >> fields whose names are in line with the C spec by not having a leading > >> underscore. (Note that the leading ^ in the sed regexps was pointless > >> and hence get dropped.) > >> > >> This requires adjusting some vNUMA macros, to avoid triggering > >> "enumeration value ... not handled in switch" warnings, which - due to > >> -Werror - would cause the build to fail. (I have to admit that I find > >> these padding fields odd, when translation of the containing structure > >> is needed anyway.) > >> > >> Signed-off-by: Jan Beulich > > > > Reviewed-by: Roger Pau Monné > > Thanks. > > >> --- > >> While for translation macros skipping padding fields pretty surely is a > >> reasonable thing to do, we may want to consider not ignoring them when > >> generating checking macros. > > (note this remark, towards your question at the end) > > >> --- a/xen/common/compat/memory.c > >> +++ b/xen/common/compat/memory.c > >> @@ -354,10 +354,13 @@ int compat_memory_op(unsigned int cmd, X > >> return -EFAULT; > >> > >> #define XLAT_vnuma_topology_info_HNDL_vdistance_h(_d_, _s_) \ > >> + case XLAT_vnuma_topology_info_vdistance_pad: \ > >> guest_from_compat_handle((_d_)->vdistance.h, (_s_)->vdistance.h) > >> #define XLAT_vnuma_topology_info_HNDL_vcpu_to_vnode_h(_d_, _s_) \ > >> + case XLAT_vnuma_topology_info_vcpu_to_vnode_pad: \ > >> guest_from_compat_handle((_d_)->vcpu_to_vnode.h, (_s_)->vcpu_to_vnode.h) > >> #define XLAT_vnuma_topology_info_HNDL_vmemrange_h(_d_, _s_) \ > >> + case XLAT_vnuma_topology_info_vmemrange_pad: \ > >> guest_from_compat_handle((_d_)->vmemrange.h, (_s_)->vmemrange.h) > > > > I find this quite ugly, would it be better to just handle them with a > > default case in the XLAT_ macros? > > Default cases explicitly do not get added to be able to spot missing > case labels, as most compilers will warn about such when the controlling > expression is of enum type. As you say on the comment above, ignoring those for translation macros would be better, and would avoid the ugliness of having to add the _pad cases here. > > AFAICT it will also set (_d_)->vmemrange.h twice? > > I'm not seeing it (and if it was, I'd then also wonder why not for the > other two handles above). This is the generated macro: > > #define XLAT_vnuma_topology_info(_d_, _s_) do { \ > (_d_)->domid = (_s_)->domid; \ > (_d_)->nr_vnodes = (_s_)->nr_vnodes; \ > (_d_)->nr_vcpus = (_s_)->nr_vcpus; \ > (_d_)->nr_vmemranges = (_s_)->nr_vmemranges; \ > switch (vdistance) { \ > case XLAT_vnuma_topology_info_vdistance_h: \ > XLAT_vnuma_topology_info_HNDL_vdistance_h(_d_, _s_); \ > break; \ > } \ > switch (vcpu_to_vnode) { \ > case XLAT_vnuma_topology_info_vcpu_to_vnode_h: \ > XLAT_vnuma_topology_info_HNDL_vcpu_to_vnode_h(_d_, _s_); \ > break; \ > } \ > switch (vmemrange) { \ > case XLAT_vnuma_topology_info_vmemrange_h: \ > XLAT_vnuma_topology_info_HNDL_vmemrange_h(_d_, _s_); \ > break; \ > } \ > } while (0) > > Am I overlooking any further aspect? No, vdistance, vcpu_to_vnode and vmemrange are set by the caller, so the enums will never have the _pad value, and hence the assignation will be done only once, you are right. Thanks, Roger.