Re: [PATCH] powerpc/64: Simplify adaptation to new ISA v3.00 HPTE format

[Paul Mackerras <paulus@ozlabs.org>]

On Mon, Nov 14, 2016 at 11:26:00AM +1100, Balbir Singh wrote:
> 
> 
> On 11/11/16 16:55, Paul Mackerras wrote:
> > This changes the way that we support the new ISA v3.00 HPTE format.
> > Instead of adapting everything that uses HPTE values to handle either
> > the old format or the new format, depending on which CPU we are on,
> > we now convert explicitly between old and new formats if necessary
> > in the low-level routines that actually access HPTEs in memory.
> > This limits the amount of code that needs to know about the new
> > format and makes the conversions explicit.  This is OK because the
> > old format contains all the information that is in the new format.
...
> > +	if (cpu_has_feature(CPU_FTR_ARCH_300)) {
> > +		hpte_r = hpte_old_to_new_r(hpte_v, hpte_r);
> > +		hpte_v = hpte_old_to_new_v(hpte_v);
> 
> I don't think its called out, but it seems like we have a depedency where
> hpte_old_to_new_r MUST be called prior to hpte_old_to_new_v, since we need
> the v bit to be extracted and moved to the _r bit. I suspect one way to avoid
> that dependency is to pass the ssize_field or to do both conversions at once.

There is no dependency between the functions.  The functions are pure
functions.  If you are overwriting the input values with the output
values, then yes you need to call the functions with the inputs not
the outputs.  I would like to think that even programmers of average
skill can see that

	a = foo(a, b);
	b = bar(b);

computes something different from

	b = bar(b);
	a = foo(a, b);

If someone is going to be so careless as to overlook that difference,
then I don't believe they would bother to read a comment either.  (I
know for sure that people don't read comments, as witnessed by the KVM
bug I found recently.)

Paul.