Re: [PATCH v2 2/5] MIPS: Add defs & probing of extended CP0_EBase

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, May 12, 2016 at 02:10:41AM +0100, Maciej W. Rozycki wrote:
> Hi James,
> 
> > diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
> > index a6ce1db191aa..c4795568c1f2 100644
> > --- a/arch/mips/kernel/cpu-probe.c
> > +++ b/arch/mips/kernel/cpu-probe.c
> > @@ -858,6 +858,41 @@ static void decode_configs(struct cpuinfo_mips *c)
> >  	if (ok)
> >  		ok = decode_config5(c);
> >  
> > +	/* Probe the EBase.WG bit */
> > +	if (cpu_has_mips_r2_r6) {
> > +		u64 ebase;
> > +		unsigned int status;
> > +
> > +		/* {read,write}_c0_ebase_64() may be UNDEFINED prior to r6 */
> > +		ebase = cpu_has_mips64r6 ? read_c0_ebase_64()
> > +					 : (s32)read_c0_ebase();
> > +		if (ebase & MIPS_EBASE_WG) {
> > +			/* WG bit already set, we can avoid the clumsy probe */
> > +			c->options |= MIPS_CPU_EBASE_WG;
> 
>  You may additionally check for bits 31:30 != 0b10 as a satisfactory 
> condition for WG's presence, under the assumption that 0b10 is not very 
> likely if a truly 64-bit exception base has been loaded.  E.g.:
> 
> #define MIPS_EBASE_SEG_MASK (3 << 30)
> 		s32 mask;
> 
> 		/* Avoid the clumsy probe if contents indicate 64 bits.  */
> 		mask = MIPS_EBASE_SEG_MASK | MIPS_CPU_EBASE_WG;
> 		if ((ebase & mask) != CKSEG0) {
> 			c->options |= MIPS_CPU_EBASE_WG;
> 
> or so.

Yep, good idea (I was originally working under the mistaken assumption
MIPS32 wouldn't have WG :-) ).

> 
>  NB I find the current description of EBase questionable to say the least.  
> This statement:
> 
> "The addition of the base address and the exception offset is performed 
> inhibiting a carry between bits 29 and 30 of the final exception address." 
> 
> is repeated twice as if a leftover from the days before WG support.  I 
> think this needs to be clarified in the case of bits 31:30 != 0b10.  Also 
> I think the effect on the Cache Error exception vector in this case has to 
> be better specified.  Can you please raise it with the architecture 
> documentation maintainers?

I agree, and ISTR its also stated that it must be set such that the
vectored interrupt spacing doesn't make it cross that boundary either,
in which case the inhibited carry is redundant. How it is supposed to
behave in the presence of cache error exceptions is indeed also
confusing, can it even ensure it gets handled from uncached memory? I
did raise this a while back, but will chase it up.

> 
>  Also the description of DMFC0 is inconsistent with the corresponding 
> pseudo code.  As from r6.04 of the instruction set document the pseudo 
> code has been updated to take into account the R6 semantics for 32-bit 
> registers you rely on here, however the description still claims such 
> operation is UNDEFINED.  Can you please raise it with the architecture 
> documentation maintainers as well?

Yes, I'll chase this up too.

Thanks
James

Attachment: signature.asc
Description: Digital signature


[Index of Archives]     [Linux MIPS Home]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Linux]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux