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 56ED7EB64D9 for ; Fri, 7 Jul 2023 08:04:45 +0000 (UTC) Received: from list by lists.xenproject.org with outflank-mailman.560256.876024 (Exim 4.92) (envelope-from ) id 1qHgS0-0001RA-Tx; Fri, 07 Jul 2023 08:04:24 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 560256.876024; Fri, 07 Jul 2023 08:04:24 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1qHgS0-0001R3-RM; Fri, 07 Jul 2023 08:04:24 +0000 Received: by outflank-mailman (input) for mailman id 560256; Fri, 07 Jul 2023 08:04:23 +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 1qHgRz-0001Qx-IV for xen-devel@lists.xenproject.org; Fri, 07 Jul 2023 08:04:23 +0000 Received: from support.bugseng.com (mail.bugseng.com [162.55.131.47]) by se1-gles-flk1.inumbo.com (Halon) with ESMTPS id e0ecd79e-1c9c-11ee-8611-37d641c3527e; Fri, 07 Jul 2023 10:04:21 +0200 (CEST) Received: from mail-vs1-f43.google.com (mail-vs1-f43.google.com [209.85.217.43]) by support.bugseng.com (Postfix) with ESMTPSA id D28C54EE0C81 for ; Fri, 7 Jul 2023 10:04:20 +0200 (CEST) Received: by mail-vs1-f43.google.com with SMTP id ada2fe7eead31-4435fa903f2so461218137.1 for ; Fri, 07 Jul 2023 01:04:20 -0700 (PDT) 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: e0ecd79e-1c9c-11ee-8611-37d641c3527e X-Gm-Message-State: ABy/qLYOezixCXspRO7VZuj4cAb3CngAUx7xEbKseMHrMRaupj1Nulh0 xaDVKwLb6rRcDU1U7ScrpE6weEc8jpGNN6pm9ss= X-Google-Smtp-Source: APBJJlHJt/ErQDGaFXVOzwKh40ls2rwgzcKDTlwc65574Q5c4UNx7l3WZ6j2Bfz71WCQSWvnC+zS71ZUZQZLl+G+9OM= X-Received: by 2002:a67:d095:0:b0:443:8a2a:9481 with SMTP id s21-20020a67d095000000b004438a2a9481mr1824643vsi.2.1688717059564; Fri, 07 Jul 2023 01:04:19 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Simone Ballarin Date: Fri, 7 Jul 2023 10:04:08 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [XEN PATCH v2 12/13] xen/x86: fix violations of MISRA C:2012 Rule 7.2 To: Jan Beulich Cc: consulting@bugseng.com, Gianluca Luparini , Andrew Cooper , =?UTF-8?Q?Roger_Pau_Monn=C3=A9?= , Wei Liu , Paul Durrant , Stefano Stabellini , Michal Orzel , Xenia Ragiadakou , Ayan Kumar Halder , xen-devel@lists.xenproject.org Content-Type: multipart/alternative; boundary="000000000000762c8405ffe11458" --000000000000762c8405ffe11458 Content-Type: text/plain; charset="UTF-8" Il giorno ven 7 lug 2023 alle ore 09:04 Jan Beulich ha scritto: > On 07.07.2023 08:50, Simone Ballarin wrote: > > Il giorno gio 6 lug 2023 alle ore 18:22 Jan Beulich > ha > > scritto: > > > >> On 06.07.2023 18:08, Simone Ballarin wrote: > >>> Il giorno gio 6 lug 2023 alle ore 10:26 Jan Beulich > > >> ha > >>> scritto: > >>> > >>>> On 05.07.2023 17:26, Simone Ballarin wrote: > >>>>> --- a/xen/arch/x86/apic.c > >>>>> +++ b/xen/arch/x86/apic.c > >>>>> @@ -1211,7 +1211,7 @@ static void __init calibrate_APIC_clock(void) > >>>>> * Setup the APIC counter to maximum. There is no way the lapic > >>>>> * can underflow in the 100ms detection time frame. > >>>>> */ > >>>>> - __setup_APIC_LVTT(0xffffffff); > >>>>> + __setup_APIC_LVTT(0xffffffffU); > >>>> > >>>> While making the change less mechanical, we want to consider to switch > >>>> to ~0 in this and similar cases. > >>>> > >>> > >>> Changing ~0U is more than not mechanical: it is possibly dangerous. > >>> The resulting value could be different depending on the architecture, > >>> I prefer to not make such kind of changes in a MISRA-related patch. > >> > >> What do you mean by "depending on the architecture", when this is > >> x86-only code _and_ you can check what type parameter the called > >> function has? > > > > Ok, I will change these literals in ~0U in the next submission. > > Except that I specifically meant ~0, not ~0U. We mean "maximum value" > here, and at the call site it doesn't matter how wide the function > parameter's type is. If it was 64-bit, ~0U would not do what is wanted. > > Jan > ~0 is not a MISRA-compliant solution since bitwise operations on signed integers have implementation-defined behavior. This solution definitively violates Rule 10.1. As you said ~0 is different than ~0U, 0xffffffffU, and 0xffffffff, so using ~0 means changing the semantics of the code: this is not the aim of the patch. -- Simone Ballarin, M.Sc. Field Application Engineer, BUGSENG (https://bugseng.com ) --000000000000762c8405ffe11458 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


Il giorno ven 7 lug 2023 alle or= e 09:04 Jan Beulich <jbeulich@suse.= com> ha scritto:
On 07.07.2023 08:50, Simone Ballarin wrote:
> Il giorno gio 6 lug 2023 alle ore 18:22 Jan Beulich <jbeulich@suse.com> ha
> scritto:
>
>> On 06.07.2023 18:08, Simone Ballarin wrote:
>>> Il giorno gio 6 lug 2023 alle ore 10:26 Jan Beulich <jbeulich@suse.com><= br> >> ha
>>> scritto:
>>>
>>>> On 05.07.2023 17:26, Simone Ballarin wrote:
>>>>> --- a/xen/arch/x86/apic.c
>>>>> +++ b/xen/arch/x86/apic.c
>>>>> @@ -1211,7 +1211,7 @@ static void __init calibrate_API= C_clock(void)
>>>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0* Setup the APIC counter to = maximum. There is no way the lapic
>>>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0* can underflow in the 100ms= detection time frame.
>>>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0*/
>>>>> -=C2=A0 =C2=A0 __setup_APIC_LVTT(0xffffffff);
>>>>> +=C2=A0 =C2=A0 __setup_APIC_LVTT(0xffffffffU);
>>>>
>>>> While making the change less mechanical, we want to consid= er to switch
>>>> to ~0 in this and similar cases.
>>>>
>>>
>>> Changing ~0U is more than not mechanical: it is possibly dange= rous.
>>> The resulting value could be different depending on the archit= ecture,
>>> I prefer to not make such kind of changes in a MISRA-related p= atch.
>>
>> What do you mean by "depending on the architecture", whe= n this is
>> x86-only code _and_ you can check what type parameter the called >> function has?
>
> Ok, I will change these literals in ~0U in the next submission.

Except that I specifically meant ~0, not ~0U. We mean "maximum value&q= uot;
here, and at the call site it doesn't matter how wide the function
parameter's type is. If it was 64-bit, ~0U would not do what is wanted.=

Jan

~0 is not a MISRA-compliant solu= tion since bitwise operations on signed integers have implementation-define= d behavior. This solution definitively violates Rule 10.1.
As you said = ~0 is different than ~0U, 0xffffffffU, and 0xffffffff, so using ~0 means ch= anging the semantics of the code: this is not the aim of the patch.
--
Simone Ballarin, M.Sc.<= br>
Field Application Engineer, BUGSENG (https://bugseng.com)
--000000000000762c8405ffe11458--