On Wed, 21 Sep 2016, Matt Redfearn wrote:
> > > When reading the CP0_EBase register containing the WG (write gate) bit,
> > > the ebase variable should be set to the full value of the register, i.e.
> > > on a 64-bit kernel the full 64-bit width of the register via
> > > read_cp0_ebase_64(), and on a 32-bit kernel the full 32-bit width
> > > including bits 31:30 which may be writeable.
> > How about changing the definition of read/write_c0_ebase to
> >
> > #define read_c0_ebase() __read_ulong_c0_register($15, 1)
> > #define write_c0_ebase(val) __write_ulong_c0_register($15, 1, val)
>
> James added the {read,write}_c0_ebase_64 functions in
> 37fb60f8e3f011c25c120081a73886ad8dbc42fd, because performing a 64bit access to
> 32bit cp0 registers (like ebase on 32bit cpus) was an undefined operation
> pre-r6, so we can't always access them as longs.
Well, `long' is 32-bit with 32-bit processors, however in older (as in:
before 3.50) architecture revisions EBase was 32-bit even with 64-bit
processors, so I take it you meant "like ebase on 64bit cpus", right?
> > or using a new variant like
> >
> > #define read_c0_ebase_ulong() __read_ulong_c0_register($15, 1)
> > #define write_c0_ebase_ulong(val) __write_ulong_c0_register($15, 1, val)
> >
> > to avoid the ifdefery? This could also make this bit
> >
> > ebase = cpu_has_mips64r6 ? read_c0_ebase_64()
> > : (s32)read_c0_ebase();
>
> This relies on being able to determine a 64bit value for ebase, either by
> reading it in its entirety on a 64bit cpu (including on a 32bit kernel) or sign
> extending it from a 32bit read.
This does look wrong to me, as I noted above EBase is 64-bit with MIPS64
processors as from architecture revision 3.50. Also I don't think we want
to have EBase set to outside the 32-bit address space with 32-bit kernels.
So Ralf's proposal is actually close to being right, except for the
condition. I'd also move the condition to the macro definition itself so
that it doesn't have to be repeated inline, making the whole piece in
question look like:
#define read_c0_ebase (cpu_has_ebase_wg ?
__read_ulong_c0_register($15, 1) :
__read_32bit_c0_register($15, 1))
if (cpu_has_veic || cpu_has_vint) {
unsigned long size = 0x200 + VECTORSPACING*64;
ebase = (unsigned long)
__alloc_bootmem(size, 1 << fls(size), 0);
} else if (cpu_has_mips_r2_r6) {
ebase = read_c0_ebase() & ~0xfff;
} else {
ebase = CAC_BASE;
}
-- short and sweet, isn't it?
Maciej
