On 8/27/25 02:51, Stefano Stabellini wrote:
> On Tue, 26 Aug 2025, Nicola Vetrini wrote:
>> On 2025-08-26 20:07, Dmytro Prokopchuk1 wrote:
>>> On 8/25/25 13:07, Jan Beulich wrote:
>>>> On 24.08.2025 16:56, Dmytro Prokopchuk1 wrote:
>>>>> --- a/docs/misra/deviations.rst
>>>>> +++ b/docs/misra/deviations.rst
>>>>> @@ -97,6 +97,19 @@ Deviations related to MISRA C:2012 Rules:
>>>>> Xen expects developers to ensure code remains safe and reliable
>>>>> in builds,
>>>>> even when debug-only assertions like `ASSERT_UNREACHABLE() are
>>>>> removed.
>>>>>
>>>>> + * - R2.1
>>>>> + - The 'BUG()' macro is intentionally used in the 'prepare_acpi()'
>>>>> function
>>>>> + in specific build configuration (when the config CONFIG_ACPI is
>>>>> not
>>>>> + defined) to trigger an error if ACPI-related features are used
>>>>> incorrectly.
>>>>> + - Tagged as `deliberate` for ECLAIR.
>>>>
>>>> With
>>>>
>>>> #define acpi_disabled true
>>>>
>>>> in xen/acpi.h I don't see why we even have that inline stub. When it's
>>>> dropped
>>>> and the declaration left in place without #ifdef CONFIG_ACPI around it,
>>>> the
>>>> compiler will DCE the code (much like we arrange for in many other
>>>> places). No
>>>> deviation needed then.
>>>>
>>>> If such a deviation was to be added, it would need disambiguating. A
>>>> function
>>>> of the given name could appear in x86 as well. That wouldn't be covered by
>>>> the
>>>> Eclair config then, but it would be covered by the text here.
>>>>
>>>>> + * - R2.1
>>>>> + - The 'BUG()' macro is intentionally used in 'gicv3_do_LPI'() and
>>>>> + 'gicv3_its_setup_collection()' functions in specific build
>>>>> configuration
>>>>> + (when the config CONFIG_HAS_ITS is not defined) to catch and
>>>>> prevent any
>>>>> + unintended execution of code that should only run when ITS is
>>>>> available.
>>>>> + - Tagged as `deliberate` for ECLAIR.
>>>>
>>>> I didn't look at this, but I would very much hope that something similar
>>>> could
>>>> be done there as well.
>>>>
>>>> Jan
>>>
>>> After small changes related to prepare_acpi() function, Misra R2.1
>>> violation has gone. The compiler really does DCE:
>>>
>>> if ( acpi_disabled <<< this is TRUE )
>>> {
>>> rc = prepare_dtb_hwdom(d, kinfo);
>>> if ( rc < 0 )
>>> return rc;
>>> #ifdef CONFIG_HAS_PCI
>>> rc = pci_host_bridge_mappings(d);
>>> #endif
>>> }
>>> else
>>> rc = prepare_acpi(d, kinfo); <<< DCE
>>>
>>> I will publish it as separate patch.
>>> Thanks to Jan, I really appreciate his help.
>>>
>>>
>>> The situation with functions gicv3_do_LPI(),
>>> gicv3_its_setup_collection() and config CONFIG_HAS_ITS is little bit
>>> different.
>>> The compiler can do DCE in case when config CONFIG_HAS_ITS is "y", and
>>> Misra R2.1 violation related to these functions also can be resolved.
>>> Actually, no changes in source code need for that.
>>> But Eclair detects these violations because config CONFIG_HAS_ITS is
>>> "n", and source code is really compiled with inline stub functions (with
>>> BUG() macro).
>>> This is because config CONFIG_HAS_ITS is "experimental/unsupported"
>>>
>>> config HAS_ITS
>>> bool "GICv3 ITS MSI controller support (UNSUPPORTED)" if
>>> UNSUPPORTED
>>> depends on GICV3 && !NEW_VGIC && !ARM_32
>>>
>>> and to enable it need to set additional config: "CONFIG_UNSUPPORTED=y".
>>>
>>> I tried to test it (added "CONFIG_UNSUPPORTED=y" into
>>> automation/gitlab-ci/analyze.yaml file). You can see my CI pipeline:
>>> https://eclair-analysis-logs.xenproject.org/fs/var/local/eclair/xen-project.ecdf/xen-project/people/dimaprkp4k/xen/ECLAIR_normal/rule_2.1_gicv3_its_host_has_its_v2/ARM64/11144854092/PROJECT.ecd;/by_service.html#service&kind
>>>
>>> Unfortunately, I observed +6 new violations with that additional config
>>> "CONFIG_UNSUPPORTED=y".
>>>
>>> I don't know how and why these EXTRA_XEN_CONFIG were selected in the
>>> file 'automation/gitlab-ci/analyze.yaml'. And are we able to add new
>>> configs here ?....
>>>
>>
>> You'll have to ask Stefano about that, but I doubt at this stage. Those set
>> of
>> configs for Arm and X86 has been selected ~2 years ago.
>
> We have migrated to a new faster ECLAIR runner.
>
> I do not think we should change the existing configuration. If anything,
> I would remove options from it rather than add more.
>
> However, we can add additional configurations by creating more ECLAIR
> jobs, and thanks to the new runner we should be able to keep up.
I just realized, that the compiler should do DCE for
gicv3_its_setup_collection() in case of config CONFIG_HAS_ITS is "n".
if ( gicv3_its_host_has_its() <<< this is FALSE )
{
if ( !gicv3_enable_lpis() )
return -EBUSY;
ret = gicv3_its_setup_collection(smp_processor_id());
if ( ret )
return ret;
}
I'll take a look again.
