Current Eclair analysis

Current Eclair analysis

[Andrew Cooper]

Hello,

The Eclair step is now the dominating aspect of wallclock time.  While
the recent changes were a step in the right direction, we need some
adjustments.

First, we have *-testing running in all cases, but my understanding was
that that was supposed to be for deploying a new version of Eclair.  Can
we make this be generally off?

Next, jobs are scheduled in the order they appear in the yaml file,
which means the general ARM one goes ahead of the safety target.  Just
something to bear in mind as changes are being made.

While the x86 runs are non-fatal, having them fail is still gets in the
way of trivially telling that the pipeline is green.

The names, -safety and no suffix are a little problematic, seeing as
everything here is for safety use.


Overall, what I think we want is something more like this:

Jobs named as *-all and *-amd.  After all, it's AMD's safety target
specifically, not necessarily someone elses.

The *-all targets want everything possible enabling. Ideally we want
something like Linux's COMPILE_TEST, but in the short term we can just
adjust the input Kconfig.

Like we had with the common configuration and the per-arch
configuration, I think we want to express the clean rules as common,
with a wider (a.k.a stricter) set used for the *-amd target.

The longterm goal is to get the *-all targets as strict as the *-amd
targets, but right now because there are no blocking clean rules, it's
easier for regressions to slip in.

This brings us back to the debate about the excluded files from external
sources.  They still need fixing one way or another.  Do we see about
including them for analysis in the *-all targets, or leave them excluded
knowing that whomever need to unpack that can of worms needs to do a lot
of fixing anyway?

Does this sound sensible?

Thanks,

~Andrew

Re: Current Eclair analysis

[Nicola Vetrini]

Hi Andrew,

thanks for the feedback, it's very appreciated.

On 2025-12-10 19:14, Andrew Cooper wrote:
> Hello,
> 
> The Eclair step is now the dominating aspect of wallclock time.  While
> the recent changes were a step in the right direction, we need some
> adjustments.
> 
> First, we have *-testing running in all cases, but my understanding was
> that that was supposed to be for deploying a new version of Eclair.  
> Can
> we make this be generally off?
> 

Definitely; it was an oversight on my part when testing the patch, 
because I used a repo/tree that was supposed to run those *-testing 
jobs. As soon as I can find some free time to work on it, I'll 
investigate and send a patch, unless someone beats me to it.

> Next, jobs are scheduled in the order they appear in the yaml file,
> which means the general ARM one goes ahead of the safety target.  Just
> something to bear in mind as changes are being made.
> 

Well, but the general one should be allocated to the larger runner that 
runs both safety and non-safety jobs, so in my opinion this is fine. 
When the *-safety jobs start, they'll be picked up by the less powerful 
eclair-safety-runner one at a time.

> While the x86 runs are non-fatal, having them fail is still gets in the
> way of trivially telling that the pipeline is green.
> 

See below for the consideration about clean rules, but the idea is that 
we can get rid of most of those fairly quickly. I did glance at most of 
those, but the time I have for preparing patches is quite scarce. I see 
that Michal has done some work already on Arm; I did share with him a 
few half-done patches that I have in a branch of mine [1], and I will 
also take a look at the series you just sent.

[1] 
https://gitlab.com/xen-project/people/bugseng/xen/-/tree/eclair_pipeline?ref_type=heads

> The names, -safety and no suffix are a little problematic, seeing as
> everything here is for safety use.
> 
> 
> Overall, what I think we want is something more like this:
> 
> Jobs named as *-all and *-amd.  After all, it's AMD's safety target
> specifically, not necessarily someone elses.
> 

Well, depends on how you look at that: the *-safety jobs have a fixed 
config, while the configuration for the general Arm and x86 jobs may 
vary as Xen changes. That being said, I don't mind changing names 
personally; I just went with what seemed more natural at the time.

> The *-all targets want everything possible enabling. Ideally we want
> something like Linux's COMPILE_TEST, but in the short term we can just
> adjust the input Kconfig.
> 

Ack

> Like we had with the common configuration and the per-arch
> configuration, I think we want to express the clean rules as common,
> with a wider (a.k.a stricter) set used for the *-amd target.
> 
> The longterm goal is to get the *-all targets as strict as the *-amd
> targets, but right now because there are no blocking clean rules, it's
> easier for regressions to slip in.
> 

Ack. I tried to start simpler and then iterate based on feedback. Should 
be rather easy to craft a configuration doing that.

> This brings us back to the debate about the excluded files from 
> external
> sources.  They still need fixing one way or another.  Do we see about
> including them for analysis in the *-all targets, or leave them 
> excluded
> knowing that whomever need to unpack that can of worms needs to do a 
> lot
> of fixing anyway?
> 

I had debated addressing this, but in the end I opted to prioritize 
fixes to the violations originating from Xen code. Who wants to qualify 
Xen in the end needs to pick features/configurations, so my take is that 
everything that is not truly kept in sync with the external source 
(e.g., recent discussions about libelf w.r.t XSA-55) should be made 
compliant eventually, and then it is on the downstreams to decide on 
what to do with respect to external source dependencies in their 
usecase. Stricly speaking, they would be subjected to MISRA compliance, 
but the point is moot if they are not actually used.

> Does this sound sensible?
> 
> Thanks,
> 
> ~Andrew

-- 
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253

Re: Current Eclair analysis

[Jan Beulich]

On 10.12.2025 19:14, Andrew Cooper wrote:
> Overall, what I think we want is something more like this:
> 
> Jobs named as *-all and *-amd.  After all, it's AMD's safety target
> specifically, not necessarily someone elses.

+1

> The *-all targets want everything possible enabling. Ideally we want
> something like Linux's COMPILE_TEST, but in the short term we can just
> adjust the input Kconfig.

Assuming the PV_SHIM_EXCLUSIVE negative dependencies get sorted, what's
wrong with simply using allyesconfig there?

> Like we had with the common configuration and the per-arch
> configuration, I think we want to express the clean rules as common,
> with a wider (a.k.a stricter) set used for the *-amd target.

+1

Jan

Re: Current Eclair analysis

[Jan Beulich]

On 10.12.2025 19:14, Andrew Cooper wrote:
> The Eclair step is now the dominating aspect of wallclock time.  While
> the recent changes were a step in the right direction, we need some
> adjustments.

One other question, related to the "dominating" aspect, but not to any
of the points raised so far. Can scan results possibly be recorded
somehow, somewhere, such that upon re-scanning the same tree (pre-push
test followed by post-push test) the identical re-scan can be avoided?
And perhaps even incrementally - if only .c files change within a (set
of) commit(s), only re-scan those, rather than everything? Could be
extended to .h files if dependencies were properly taken into account.

Jan

Re: Current Eclair analysis

[Nicola Vetrini]

On 2025-12-11 11:00, Jan Beulich wrote:
> On 10.12.2025 19:14, Andrew Cooper wrote:
>> The Eclair step is now the dominating aspect of wallclock time.  While
>> the recent changes were a step in the right direction, we need some
>> adjustments.
> 
> One other question, related to the "dominating" aspect, but not to any
> of the points raised so far. Can scan results possibly be recorded
> somehow, somewhere, such that upon re-scanning the same tree (pre-push
> test followed by post-push test) the identical re-scan can be avoided?
> And perhaps even incrementally - if only .c files change within a (set
> of) commit(s), only re-scan those, rather than everything? Could be
> extended to .h files if dependencies were properly taken into account.
> 
> Jan

We support incremental analysis, but it requires non-trivial changes to 
the current Xen integration. If someone wants to invest time and/or 
resources in it, I can support the effort, but it requires a fair deal 
of rearrangements of scripting in order to avoid losing information

-- 
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253

Re: Current Eclair analysis

[Jan Beulich]

On 11.12.2025 11:08, Nicola Vetrini wrote:
> On 2025-12-11 11:00, Jan Beulich wrote:
>> On 10.12.2025 19:14, Andrew Cooper wrote:
>>> The Eclair step is now the dominating aspect of wallclock time.  While
>>> the recent changes were a step in the right direction, we need some
>>> adjustments.
>>
>> One other question, related to the "dominating" aspect, but not to any
>> of the points raised so far. Can scan results possibly be recorded
>> somehow, somewhere, such that upon re-scanning the same tree (pre-push
>> test followed by post-push test) the identical re-scan can be avoided?
>> And perhaps even incrementally - if only .c files change within a (set
>> of) commit(s), only re-scan those, rather than everything? Could be
>> extended to .h files if dependencies were properly taken into account.
> 
> We support incremental analysis, but it requires non-trivial changes to 
> the current Xen integration. If someone wants to invest time and/or 
> resources in it, I can support the effort, but it requires a fair deal 
> of rearrangements of scripting in order to avoid losing information

What about the initial part of the question, scanning the exact same tree
a 2nd time? Is that merely a special case of "incremental", and hence would
require the same amount of effort?

Jan

Re: Current Eclair analysis

[Nicola Vetrini]

On 2025-12-11 11:50, Jan Beulich wrote:
> On 11.12.2025 11:08, Nicola Vetrini wrote:
>> On 2025-12-11 11:00, Jan Beulich wrote:
>>> On 10.12.2025 19:14, Andrew Cooper wrote:
>>>> The Eclair step is now the dominating aspect of wallclock time.  
>>>> While
>>>> the recent changes were a step in the right direction, we need some
>>>> adjustments.
>>> 
>>> One other question, related to the "dominating" aspect, but not to 
>>> any
>>> of the points raised so far. Can scan results possibly be recorded
>>> somehow, somewhere, such that upon re-scanning the same tree 
>>> (pre-push
>>> test followed by post-push test) the identical re-scan can be 
>>> avoided?
>>> And perhaps even incrementally - if only .c files change within a 
>>> (set
>>> of) commit(s), only re-scan those, rather than everything? Could be
>>> extended to .h files if dependencies were properly taken into 
>>> account.
>> 
>> We support incremental analysis, but it requires non-trivial changes 
>> to
>> the current Xen integration. If someone wants to invest time and/or
>> resources in it, I can support the effort, but it requires a fair deal
>> of rearrangements of scripting in order to avoid losing information
> 
> What about the initial part of the question, scanning the exact same 
> tree
> a 2nd time? Is that merely a special case of "incremental", and hence 
> would
> require the same amount of effort?
> 
> Jan

I originally misinterpreted your question, and in fact I had an internal 
discussion on this. What you are asking is not directly supported by 
eclair because ECLAIR acts based on compiler invocations triggered by 
the build system, so two different builds are entirely independent of 
each other, unless a correspondence between analyzed artifacts (analysis 
frames in ECLAIR jargon) is estabilished. Overall, in our opinion this 
is best done not internally by ECLAIR, but at a higher (or lower, 
depending on how you represent it) level, which is the build system 
level. The idea is basically as follows:

- set up ccache with a remote storage backend (one per machine where 
runners are hosted is probably the best approximation);
- let ccache be used transparently across multiple runs; it will trigger 
re-compilation only when the file is not already in its cache, and as a 
consequence trigger the analysis of that TU or program. Due to the 
dockerization of builds and how gitlab is set up this may require some 
fumbling with env vars to point to the right compilation cache, but 
should be doable without too much effort;
- For each run:
   - fetch the result from the base run (for some meaning of "base")
   - run an eclair incremental analysis using option +project. This runs 
the Xen build as usual, but using ccache (I think other projects such as 
Zephyr already do something similar in their CI);
   - load new results in the old database, substituting evidences related 
to the same TU (done with a specific eclair_report flag).

In essence this covers both usecases, but it requires a nontrivial 
amount of effort.

N.B. ECLAIR does not use ccache internally: its concept of incremental 
analysis is completely orthogonal to the one that ccache uses.


-- 
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253

Re: Current Eclair analysis

[Andrew Cooper]

On 11/12/2025 9:04 am, Jan Beulich wrote:
> On 10.12.2025 19:14, Andrew Cooper wrote:
>> The *-all targets want everything possible enabling. Ideally we want
>> something like Linux's COMPILE_TEST, but in the short term we can just
>> adjust the input Kconfig.
> Assuming the PV_SHIM_EXCLUSIVE negative dependencies get sorted, what's
> wrong with simply using allyesconfig there?

Off the top of my head, ARM have two different GICv2 implementations
which are currently mutually exclusive.

Linux uses COMPILE_TEST to include drivers which would otherwise be
excluded by base architecture or platform.  We've not got much of that,
but IOMMUs are one example we do have.

But more generally, "I want to compile all the code for CI" doesn't
necessary mean "with coverage, and sanitisers, and LTO, and everything
else we've not gotten to yet".

~Andrew