Message ID | 20220926130213.28274-1-andrew.cooper3@citrix.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [for-4.17] CI: Force CONFIG_XEN_IBT in the buster-gcc-ibt test | expand |
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 > >
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
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
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 >
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
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.
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
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(+)