Re: [PATCH 5/3] x86/vPMU: Harden indirect branches

[Jan Beulich <jbeulich@xxxxxxxx>]

On 30.11.2021 23:05, Andrew Cooper wrote:
> --- a/xen/arch/x86/cpu/vpmu_amd.c
> +++ b/xen/arch/x86/cpu/vpmu_amd.c
> @@ -518,7 +518,7 @@ static int svm_vpmu_initialise(struct vcpu *v)
>      return 0;
>  }
>  
> -static const struct arch_vpmu_ops __initconstrel amd_vpmu_ops = {
> +static struct arch_vpmu_ops __initdata_cf_clobber amd_vpmu_ops = {

This depends on an uncommitted patch (introducing the annotation)
and hence is a little difficult to review without a pointer to that
patch (which doesn't look to have "cf_" in its subject, and I didn't
recall anything else to search for in my mailbox). The main question
I see here is whether it's warranted to drop the const: I'd like to
retain it if at all possible, just to document that the structure
doesn't get modified at runtime (read: initialization time).

Later... I've spotted it in my to-be-reviewed folder; it's
"x86/altcall: Optimise away endbr64 instruction where possible".
There's no discussion there either about const. Iirc the reason we
need __initconstrel alongside __initconst is for older tool chains
complaining about section conflicts. If that's right, for CET-IBT
we need relatively new tool chains anyway. Hence the mixing of
const and non-const within a section may not be an issue. IOW with
newer tool chains all could go in a single section; we'd need two
separate annotations only for older tool chains (falling back to
__initdata or __initconstrel). Of course at that point it might be
easier to have both .init.data.cf_clobber and
.init.rodata.cf_clobber in the first place, grouped together by
the linker script.

Jan