Re: [PATCH v3 01/14] powerpc: Enable Prefixed Instructions

From: Alistair Popple <alistair@popple.id.au>
To: Nicholas Piggin <npiggin@gmail.com>
Cc: Jordan Niethe <jniethe5@gmail.com>,
	Daniel Axtens <dja@axtens.net>,
	linuxppc-dev@lists.ozlabs.org,
	Balamuruhan S <bala24@linux.ibm.com>
Subject: Re: [PATCH v3 01/14] powerpc: Enable Prefixed Instructions
Date: Wed, 04 Mar 2020 10:20:23 +1100	[thread overview]
Message-ID: <3079586.NPOWFCaNNt@townsend> (raw)
In-Reply-To: <1582864874.q013n2u3v1.astroid@bobo.none>

> But hmm, the dt_cpu_ftrs parsing should be picking those up and setting
> them by default I would think (or maybe not because you don't expect
> FSCR bit if OS support is not required).

Right - the generic dt_cpu_ftrs didn't do the FSCR enablement which I assume 
is because if OS support is required anyway we can just add it in a similar 
way to the below patch. My thinking was that we could use it to disable prefix 
with a firmware if required, however outside of a lab environment there isn't 
much practical use for that so I'm ok with just doing something similar to the 
below.

> All the other FSCR bits are done similarly to this AFAIKS:
> 
>  https://patchwork.ozlabs.org/patch/1244476/
> 
> I would do that for now rather than digging into it too far, but we
> should look at cleaning that up and doing something more like what
> you have here.
>
> >> >  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?
> > 
> > This was just going to be that they exist.
> > 
> >> You may not need "-instructions" on the end, none of the other
> >> instructions do.
> > 
> > Good point.
> > 
> >> 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.
> > 
> > No problem.
> 
> Thanks,
> Nick