[PATCH for-4.17] CI: Force CONFIG_XEN_IBT in the buster-gcc-ibt test

[PATCH for-4.17] CI: Force CONFIG_XEN_IBT in the buster-gcc-ibt test

[Andrew Cooper]

buster-gcc-ibt is a dedicated test to run a not-yet-upstreamed compiler patch
which is relevant to CONFIG_XEN_IBT in 4.17 and later.

Force it on, rather than having 50% of the jobs not testing what they're
supposed to be testing.

Fixes: 5d59421815d5 ("x86: Use control flow typechecking where possible")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Doug Goldstein <cardoe@cardoe.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Henry Wang <Henry.Wang@arm.com>

For 4.17: This is bugfix to CI only, to avoid it producing a false negative.
Currently, the test intermittently fails to spot the error it was intended to
identify.  It is very low risk as far as the 4.17 release goes.

https://gitlab.com/xen-project/people/andyhhp/xen/-/jobs/3084774561#L373 for
proof that CONFIG_XEN_IBT=y is being fed into allrandom.config
---
 automation/gitlab-ci/build.yaml | 2 ++
 automation/scripts/build        | 5 +++++
 2 files changed, 7 insertions(+)

diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
index 720ce6e07ba0..65e06c858ef3 100644
--- a/automation/gitlab-ci/build.yaml
+++ b/automation/gitlab-ci/build.yaml
@@ -299,6 +299,8 @@ debian-buster-gcc-ibt:
   variables:
     CONTAINER: debian:buster-gcc-ibt
     RANDCONFIG: y
+    EXTRA_FIXED_RANDCONFIG: |
+      CONFIG_XEN_IBT=y
 
 debian-unstable-clang:
   extends: .clang-x86-64-build
diff --git a/automation/scripts/build b/automation/scripts/build
index 2f15ab3198e6..2d9dd86df904 100755
--- a/automation/scripts/build
+++ b/automation/scripts/build
@@ -12,6 +12,11 @@ cc-ver()
 
 # random config or default config
 if [[ "${RANDCONFIG}" == "y" ]]; then
+
+    # Append job-specific fixed configuration
+    [[ -n "${EXTRA_FIXED_RANDCONFIG}" ]] &&
+        echo "${EXTRA_FIXED_RANDCONFIG}" >> xen/tools/kconfig/allrandom.config
+
     make -j$(nproc) -C xen KCONFIG_ALLCONFIG=tools/kconfig/allrandom.config randconfig
     hypervisor_only="y"
 else
-- 
2.11.0

Re: [PATCH for-4.17] CI: Force CONFIG_XEN_IBT in the buster-gcc-ibt test

[Marek Marczykowski-Górecki]

[-- Attachment #1: Type: text/plain, Size: 2348 bytes --]

On Mon, Sep 26, 2022 at 02:02:13PM +0100, Andrew Cooper wrote:
> buster-gcc-ibt is a dedicated test to run a not-yet-upstreamed compiler patch
> which is relevant to CONFIG_XEN_IBT in 4.17 and later.
> 
> Force it on, rather than having 50% of the jobs not testing what they're
> supposed to be testing.

Shouldn't this job be with a static (or rather: all yes) config?

> Fixes: 5d59421815d5 ("x86: Use control flow typechecking where possible")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Doug Goldstein <cardoe@cardoe.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Henry Wang <Henry.Wang@arm.com>
> 
> For 4.17: This is bugfix to CI only, to avoid it producing a false negative.
> Currently, the test intermittently fails to spot the error it was intended to
> identify.  It is very low risk as far as the 4.17 release goes.
> 
> https://gitlab.com/xen-project/people/andyhhp/xen/-/jobs/3084774561#L373 for
> proof that CONFIG_XEN_IBT=y is being fed into allrandom.config
> ---
>  automation/gitlab-ci/build.yaml | 2 ++
>  automation/scripts/build        | 5 +++++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
> index 720ce6e07ba0..65e06c858ef3 100644
> --- a/automation/gitlab-ci/build.yaml
> +++ b/automation/gitlab-ci/build.yaml
> @@ -299,6 +299,8 @@ debian-buster-gcc-ibt:
>    variables:
>      CONTAINER: debian:buster-gcc-ibt
>      RANDCONFIG: y
> +    EXTRA_FIXED_RANDCONFIG: |
> +      CONFIG_XEN_IBT=y
>  
>  debian-unstable-clang:
>    extends: .clang-x86-64-build
> diff --git a/automation/scripts/build b/automation/scripts/build
> index 2f15ab3198e6..2d9dd86df904 100755
> --- a/automation/scripts/build
> +++ b/automation/scripts/build
> @@ -12,6 +12,11 @@ cc-ver()
>  
>  # random config or default config
>  if [[ "${RANDCONFIG}" == "y" ]]; then
> +
> +    # Append job-specific fixed configuration
> +    [[ -n "${EXTRA_FIXED_RANDCONFIG}" ]] &&
> +        echo "${EXTRA_FIXED_RANDCONFIG}" >> xen/tools/kconfig/allrandom.config
> +
>      make -j$(nproc) -C xen KCONFIG_ALLCONFIG=tools/kconfig/allrandom.config randconfig
>      hypervisor_only="y"
>  else
> -- 
> 2.11.0
> 
> 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH for-4.17] CI: Force CONFIG_XEN_IBT in the buster-gcc-ibt test

[Andrew Cooper]

On 26/09/2022 14:19, Marek Marczykowski-Górecki wrote:
> On Mon, Sep 26, 2022 at 02:02:13PM +0100, Andrew Cooper wrote:
>> buster-gcc-ibt is a dedicated test to run a not-yet-upstreamed compiler patch
>> which is relevant to CONFIG_XEN_IBT in 4.17 and later.
>>
>> Force it on, rather than having 50% of the jobs not testing what they're
>> supposed to be testing.
> Shouldn't this job be with a static (or rather: all yes) config?

That's a separate thing needing a reversion... Currently make
allyesconfig disabled CONFIG_HVM.

But more generally, we have a pile of cases where different config
options produces differences in which and/or whether a function pointer
gets used, so a single largely-static case doesn't find any of the
interesting corner cases.

~Andrew

RE: [PATCH for-4.17] CI: Force CONFIG_XEN_IBT in the buster-gcc-ibt test

[Henry Wang]

Hi Andrew,

> -----Original Message-----
> From: Andrew Cooper <andrew.cooper3@citrix.com>
> Subject: [PATCH for-4.17] CI: Force CONFIG_XEN_IBT in the buster-gcc-ibt test
> 
> buster-gcc-ibt is a dedicated test to run a not-yet-upstreamed compiler patch
> which is relevant to CONFIG_XEN_IBT in 4.17 and later.
> 
> Force it on, rather than having 50% of the jobs not testing what they're
> supposed to be testing.
> 
> Fixes: 5d59421815d5 ("x86: Use control flow typechecking where possible")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Doug Goldstein <cardoe@cardoe.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Henry Wang <Henry.Wang@arm.com>
> 
> For 4.17: This is bugfix to CI only, to avoid it producing a false negative.
> Currently, the test intermittently fails to spot the error it was intended to
> identify.  It is very low risk as far as the 4.17 release goes.
> 
> https://gitlab.com/xen-project/people/andyhhp/xen/-
> /jobs/3084774561#L373 for
> proof that CONFIG_XEN_IBT=y is being fed into allrandom.config

Thanks for sending this patch!

I agree that considering this patch to 4.17 is low risk, and as long as
this patch is properly reviewed by CI maintainers, you can have my:

Release-acked-by: Henry Wang <Henry.Wang@arm.com>

Kind regards,
Henry

Re: [PATCH for-4.17] CI: Force CONFIG_XEN_IBT in the buster-gcc-ibt test

[Stefano Stabellini]

On Mon, 26 Sep 2022, Andrew Cooper wrote:
> buster-gcc-ibt is a dedicated test to run a not-yet-upstreamed compiler patch
> which is relevant to CONFIG_XEN_IBT in 4.17 and later.
> 
> Force it on, rather than having 50% of the jobs not testing what they're
> supposed to be testing.
> 
> Fixes: 5d59421815d5 ("x86: Use control flow typechecking where possible")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Doug Goldstein <cardoe@cardoe.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Henry Wang <Henry.Wang@arm.com>
> 
> For 4.17: This is bugfix to CI only, to avoid it producing a false negative.
> Currently, the test intermittently fails to spot the error it was intended to
> identify.  It is very low risk as far as the 4.17 release goes.

Thanks for the patch! Very recently Michal has added a similar
EXTRA_XEN_CONFIG option. If you are OK with it, I'll rename
EXTRA_FIXED_RANDCONFIG to EXTRA_XEN_CONFIG in this patch for
consistency. I can do it on commit.


> https://gitlab.com/xen-project/people/andyhhp/xen/-/jobs/3084774561#L373 for
> proof that CONFIG_XEN_IBT=y is being fed into allrandom.config
> ---
>  automation/gitlab-ci/build.yaml | 2 ++
>  automation/scripts/build        | 5 +++++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
> index 720ce6e07ba0..65e06c858ef3 100644
> --- a/automation/gitlab-ci/build.yaml
> +++ b/automation/gitlab-ci/build.yaml
> @@ -299,6 +299,8 @@ debian-buster-gcc-ibt:
>    variables:
>      CONTAINER: debian:buster-gcc-ibt
>      RANDCONFIG: y
> +    EXTRA_FIXED_RANDCONFIG: |
> +      CONFIG_XEN_IBT=y
>  
>  debian-unstable-clang:
>    extends: .clang-x86-64-build
> diff --git a/automation/scripts/build b/automation/scripts/build
> index 2f15ab3198e6..2d9dd86df904 100755
> --- a/automation/scripts/build
> +++ b/automation/scripts/build
> @@ -12,6 +12,11 @@ cc-ver()
>  
>  # random config or default config
>  if [[ "${RANDCONFIG}" == "y" ]]; then
> +
> +    # Append job-specific fixed configuration
> +    [[ -n "${EXTRA_FIXED_RANDCONFIG}" ]] &&
> +        echo "${EXTRA_FIXED_RANDCONFIG}" >> xen/tools/kconfig/allrandom.config
> +
>      make -j$(nproc) -C xen KCONFIG_ALLCONFIG=tools/kconfig/allrandom.config randconfig
>      hypervisor_only="y"
>  else
> -- 
> 2.11.0
> 

Re: [PATCH for-4.17] CI: Force CONFIG_XEN_IBT in the buster-gcc-ibt test

[Andrew Cooper]

On 27/09/2022 23:47, Stefano Stabellini wrote:
> On Mon, 26 Sep 2022, Andrew Cooper wrote:
>> buster-gcc-ibt is a dedicated test to run a not-yet-upstreamed compiler patch
>> which is relevant to CONFIG_XEN_IBT in 4.17 and later.
>>
>> Force it on, rather than having 50% of the jobs not testing what they're
>> supposed to be testing.
>>
>> Fixes: 5d59421815d5 ("x86: Use control flow typechecking where possible")
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Doug Goldstein <cardoe@cardoe.com>
>> CC: Stefano Stabellini <sstabellini@kernel.org>
>> CC: Henry Wang <Henry.Wang@arm.com>
>>
>> For 4.17: This is bugfix to CI only, to avoid it producing a false negative.
>> Currently, the test intermittently fails to spot the error it was intended to
>> identify.  It is very low risk as far as the 4.17 release goes.
> Thanks for the patch! Very recently Michal has added a similar
> EXTRA_XEN_CONFIG option.

I know - that's where I sto^W borrowed the idea from.

> If you are OK with it, I'll rename
> EXTRA_FIXED_RANDCONFIG to EXTRA_XEN_CONFIG in this patch for
> consistency. I can do it on commit.

No, that will break the fix.

These are not options to be inserted into a regular .config.

These are options passed to `make randconfig` via a sidedoor (the
KCONFIG_ALLCONFIG= variable) causing them to be handled specially while
the regular .config file is has it's contents randomised.

~Andrew

Re: [PATCH for-4.17] CI: Force CONFIG_XEN_IBT in the buster-gcc-ibt test

[Stefano Stabellini]

On Tue, 27 Sep 2022, Andrew Cooper wrote:
> On 27/09/2022 23:47, Stefano Stabellini wrote:
> > On Mon, 26 Sep 2022, Andrew Cooper wrote:
> >> buster-gcc-ibt is a dedicated test to run a not-yet-upstreamed compiler patch
> >> which is relevant to CONFIG_XEN_IBT in 4.17 and later.
> >>
> >> Force it on, rather than having 50% of the jobs not testing what they're
> >> supposed to be testing.
> >>
> >> Fixes: 5d59421815d5 ("x86: Use control flow typechecking where possible")
> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >> ---
> >> CC: Doug Goldstein <cardoe@cardoe.com>
> >> CC: Stefano Stabellini <sstabellini@kernel.org>
> >> CC: Henry Wang <Henry.Wang@arm.com>
> >>
> >> For 4.17: This is bugfix to CI only, to avoid it producing a false negative.
> >> Currently, the test intermittently fails to spot the error it was intended to
> >> identify.  It is very low risk as far as the 4.17 release goes.
> > Thanks for the patch! Very recently Michal has added a similar
> > EXTRA_XEN_CONFIG option.
> 
> I know - that's where I sto^W borrowed the idea from.
> 
> > If you are OK with it, I'll rename
> > EXTRA_FIXED_RANDCONFIG to EXTRA_XEN_CONFIG in this patch for
> > consistency. I can do it on commit.
> 
> No, that will break the fix.
> 
> These are not options to be inserted into a regular .config.
> 
> These are options passed to `make randconfig` via a sidedoor (the
> KCONFIG_ALLCONFIG= variable) causing them to be handled specially while
> the regular .config file is has it's contents randomised.

OK. I committed your original plus a minor code style fix.