Jordan Niethe's on February 26, 2020 2:07 pm:
> From: Alistair Popple <alist...@popple.id.au>
>
> Prefix instructions have their own FSCR bit which needs to enabled via
> a CPU feature. The kernel will save the FSCR for problem state but it
> needs to be enabled initially.
>
> Signed-off-by: Alistair Popple <alist...@popple.id.au>
> ---
> arch/powerpc/include/asm/reg.h | 3 +++
> arch/powerpc/kernel/dt_cpu_ftrs.c | 23 +++++++++++++++++++++++
> 2 files changed, 26 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index 1aa46dff0957..c7758c2ccc5f 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
> @@ -397,6 +397,7 @@
> #define SPRN_RWMR 0x375 /* Region-Weighting Mode Register */
>
> /* HFSCR and FSCR bit numbers are the same */
> +#define FSCR_PREFIX_LG 13 /* Enable Prefix Instructions */
> #define FSCR_SCV_LG 12 /* Enable System Call Vectored */
> #define FSCR_MSGP_LG 10 /* Enable MSGP */
> #define FSCR_TAR_LG 8 /* Enable Target Address Register */
> @@ -408,11 +409,13 @@
> #define FSCR_VECVSX_LG 1 /* Enable VMX/VSX */
> #define FSCR_FP_LG 0 /* Enable Floating Point */
> #define SPRN_FSCR 0x099 /* Facility Status & Control Register */
> +#define FSCR_PREFIX __MASK(FSCR_PREFIX_LG)
When you add a new FSCR, there's a couple more things to do, check
out traps.c.
> #define FSCR_SCV __MASK(FSCR_SCV_LG)
> #define FSCR_TAR __MASK(FSCR_TAR_LG)
> #define FSCR_EBB __MASK(FSCR_EBB_LG)
> #define FSCR_DSCR __MASK(FSCR_DSCR_LG)
> #define SPRN_HFSCR 0xbe /* HV=1 Facility Status & Control Register */
> +#define HFSCR_PREFIX __MASK(FSCR_PREFIX_LG)
> #define HFSCR_MSGP __MASK(FSCR_MSGP_LG)
> #define HFSCR_TAR __MASK(FSCR_TAR_LG)
> #define HFSCR_EBB __MASK(FSCR_EBB_LG)
> diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c
> b/arch/powerpc/kernel/dt_cpu_ftrs.c
> index 182b4047c1ef..396f2c6c588e 100644
> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
> @@ -553,6 +553,28 @@ static int __init feat_enable_large_ci(struct
> dt_cpu_feature *f)
> return 1;
> }
>
> +static int __init feat_enable_prefix(struct dt_cpu_feature *f)
> +{
> + u64 fscr, hfscr;
> +
> + if (f->usable_privilege & USABLE_HV) {
> + hfscr = mfspr(SPRN_HFSCR);
> + hfscr |= HFSCR_PREFIX;
> + mtspr(SPRN_HFSCR, hfscr);
> + }
> +
> + if (f->usable_privilege & USABLE_OS) {
> + fscr = mfspr(SPRN_FSCR);
> + fscr |= FSCR_PREFIX;
> + mtspr(SPRN_FSCR, fscr);
> +
> + if (f->usable_privilege & USABLE_PR)
> + current->thread.fscr |= FSCR_PREFIX;
> + }
> +
> + return 1;
> +}
It would be good to be able to just use the default feature matching
for this, if possible? Do we not do the right thing with
init_thread.fscr?
> +
> struct dt_cpu_feature_match {
> const char *name;
> int (*enable)(struct dt_cpu_feature *f);
> @@ -626,6 +648,7 @@ static struct dt_cpu_feature_match __initdata
> {"vector-binary128", feat_enable, 0},
> {"vector-binary16", feat_enable, 0},
> {"wait-v3", feat_enable, 0},
> + {"prefix-instructions", feat_enable_prefix, 0},
That's reasonable to make that a feature, will it specify a minimum
base set of prefix instructions or just that prefix instructions
with the prefix/suffix arrangement exist?
You may not need "-instructions" on the end, none of the other
instructions do.
I would maybe just hold off upstreaming the dt_cpu_ftrs changes for
a bit. We have to do a pass over new CPU feature device tree, and
some compatibility questions have come up recently.
If you wouldn't mind just adding the new [H]FSCR bits and faults
upstream for now, that would be good.
Thanks,
Nick
