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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id CC82DE7B601 for ; Wed, 4 Oct 2023 12:23:22 +0000 (UTC) Received: from list by lists.xenproject.org with outflank-mailman.612529.952464 (Exim 4.92) (envelope-from ) id 1qo0uE-0001R0-2k; Wed, 04 Oct 2023 12:23:10 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 612529.952464; Wed, 04 Oct 2023 12:23:10 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1qo0uD-0001Qt-Uq; Wed, 04 Oct 2023 12:23:09 +0000 Received: by outflank-mailman (input) for mailman id 612529; Wed, 04 Oct 2023 12:23:09 +0000 Received: from se1-gles-flk1-in.inumbo.com ([94.247.172.50] helo=se1-gles-flk1.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1qo0uD-0001Qk-CB for xen-devel@lists.xenproject.org; Wed, 04 Oct 2023 12:23:09 +0000 Received: from support.bugseng.com (mail.bugseng.com [162.55.131.47]) by se1-gles-flk1.inumbo.com (Halon) with ESMTPS id c5b7f86c-62b0-11ee-9b0d-b553b5be7939; Wed, 04 Oct 2023 14:23:06 +0200 (CEST) Received: from support.bugseng.com (support.bugseng.com [162.55.131.47]) by support.bugseng.com (Postfix) with ESMTPA id 6949A4EE0737; Wed, 4 Oct 2023 14:23:06 +0200 (CEST) X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: c5b7f86c-62b0-11ee-9b0d-b553b5be7939 MIME-Version: 1.0 Date: Wed, 04 Oct 2023 14:23:06 +0200 From: Nicola Vetrini To: Luca Fancellu Cc: Stefano Stabellini , Xen-devel , michal.orzel@amd.com, xenia.ragiadakou@amd.com, Ayan Kumar Halder , consulting@bugseng.com, Jan Beulich , =?UTF-8?Q?Rog?= =?UTF-8?Q?er_Pau_Monn=C3=A9?= , Henry Wang , Simone Ballarin , Doug Goldstein , George Dunlap , Julien Grall , Wei Liu , Andrew Cooper Subject: Re: [XEN PATCH] xen: Add SAF deviations for MISRA C:2012 Rule 7.1 In-Reply-To: References: <338d8e574db943a86c7605e4c6d9a299d45f067d.1696347345.git.nicola.vetrini@bugseng.com> <2894008e8f612296da84267346ae4240@bugseng.com> User-Agent: Roundcube Webmail/1.4.3 Message-ID: X-Sender: nicola.vetrini@bugseng.com Organization: BUGSENG s.r.l. Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 04/10/2023 12:52, Luca Fancellu wrote: >> On 4 Oct 2023, at 11:29, Nicola Vetrini >> wrote: >> >> On 04/10/2023 12:06, Luca Fancellu wrote: >>> Hi Nicola, >>>> On 4 Oct 2023, at 10:56, andrew.cooper3@citrix.com wrote: >>>> On 03/10/2023 9:46 pm, Stefano Stabellini wrote: >>>>> On Tue, 3 Oct 2023, Nicola Vetrini wrote: >>>>>> As specified in rules.rst, these constants can be used >>>>>> in the code. >>>>>> Their deviation is now accomplished by using a SAF comment, >>>>>> rather than an ECLAIR configuration. >>>>>> Signed-off-by: Nicola Vetrini >>>>> "SAF" discussion aside that can be resolved elsewhere: >>>>> Reviewed-by: Stefano Stabellini >>>> Well no. "SAF" aside (and SAF does need fixing before reposting >>>> this patch, otherwise it's just unnecessary churn), ... >>>>>> diff --git a/xen/arch/x86/hvm/svm/svm.h >>>>>> b/xen/arch/x86/hvm/svm/svm.h >>>>>> index d2a781fc3fb5..d0623b72ccfa 100644 >>>>>> --- a/xen/arch/x86/hvm/svm/svm.h >>>>>> +++ b/xen/arch/x86/hvm/svm/svm.h >>>>>> @@ -57,14 +57,23 @@ static inline void svm_invlpga(unsigned long >>>>>> linear, uint32_t asid) >>>>>> #define INSTR_INT3 INSTR_ENC(X86EMUL_OPC( 0, 0xcc), 0) >>>>>> #define INSTR_ICEBP INSTR_ENC(X86EMUL_OPC( 0, 0xf1), 0) >>>>>> #define INSTR_HLT INSTR_ENC(X86EMUL_OPC( 0, 0xf4), 0) >>>>>> +/* SAF-2-safe */ >>>>>> #define INSTR_XSETBV INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0321) >>>>>> +/* SAF-2-safe */ >>>>>> #define INSTR_VMRUN INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0330) >>>>>> +/* SAF-2-safe */ >>>>>> #define INSTR_VMCALL INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0331) >>>>>> +/* SAF-2-safe */ >>>>>> #define INSTR_VMLOAD INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0332) >>>>>> +/* SAF-2-safe */ >>>>>> #define INSTR_VMSAVE INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0333) >>>>>> +/* SAF-2-safe */ >>>>>> #define INSTR_STGI INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0334) >>>>>> +/* SAF-2-safe */ >>>>>> #define INSTR_CLGI INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0335) >>>>>> +/* SAF-2-safe */ >>>>>> #define INSTR_INVLPGA INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0337) >>>>>> +/* SAF-2-safe */ >>>>>> #define INSTR_RDTSCP INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0371) >>>>>> #define INSTR_INVD INSTR_ENC(X86EMUL_OPC(0x0f, 0x08), 0) >>>>>> #define INSTR_WBINVD INSTR_ENC(X86EMUL_OPC(0x0f, 0x09), 0) >>>> ... this has broken a tabulated structure to have comments ahead of >>>> lines with octal numbers, while ... >>>>>> diff --git a/xen/arch/x86/hvm/svm/emulate.c >>>>>> b/xen/arch/x86/hvm/svm/emulate.c >>>>>> index aa2c61c433b3..c5e3341c6316 100644 >>>>>> --- a/xen/arch/x86/hvm/svm/emulate.c >>>>>> +++ b/xen/arch/x86/hvm/svm/emulate.c >>>>>> @@ -90,9 +90,9 @@ unsigned int svm_get_insn_len(struct vcpu *v, >>>>>> unsigned int instr_enc) >>>>>> if ( !instr_modrm ) >>>>>> return emul_len; >>>>>> - if ( modrm_mod == MASK_EXTR(instr_modrm, 0300) && >>>>>> - (modrm_reg & 7) == MASK_EXTR(instr_modrm, 0070) && >>>>>> - (modrm_rm & 7) == MASK_EXTR(instr_modrm, 0007) ) >>>>>> + if ( modrm_mod == MASK_EXTR(instr_modrm, 0300) && /* SAF-2-safe >>>>>> */ >>>>>> + (modrm_reg & 7) == MASK_EXTR(instr_modrm, 0070) && /* SAF-2-safe >>>>>> */ >>>>>> + (modrm_rm & 7) == MASK_EXTR(instr_modrm, 0007) ) /* SAF-2-safe >>>>>> */ >>>>>> return emul_len; >>>>>> } >>>> ... this has comments at the end of lines with octal numbers. >>>> So which is it? >>> I agree with Andrew here in this sense: the in-code comment is >>> supposed to be on the line *before* the violation, >>> not on the same line, so I’m also wondering how it is fixing the very >>> first violation. >>> Cheers, >>> Luca >> > > Hi Nicola, > >> Actually it justifies what is on either the previous line or the same >> because it's >> translated to /* -E> safe MC3R1.R7.1 1 */, where the last number is >> how many lines besides >> the current one are to be deviated (e.g. you can have 0 deviate only >> the current line). > > Just to understand, does this way: > > > /* -E> safe MC3R1.R7.1 1 */ > > > Justifies only line B? Because I thought so, but now I want to be > sure, otherwise it doesn’t act > as intended. > > Yes, line A is untouched. /* -E> safe MC3R1.R7.1 1 */ (deviated) (deviated) -- Nicola Vetrini, BSc Software Engineer, BUGSENG srl (https://bugseng.com)