Re: [OpenBIOS] sparc64's get_physical_address_code should not fault on TTE's with V=0

[Jakub Jermar]

Blue Swirl wrote:
> On 1/12/09, Jakub Jermar  wrote:
> > Hi,
> >
> > when I make the following change to helper.c, get_physical_address_code:
> >    // ctx match, vaddr match?
> >    // ctx match, vaddr match, valid?
> >   if (env->dmmuregs[1] == (env->itlb_tag[i] & 0x1fff) &&
> >        (address & mask) == (env->itlb_tag[i] & ~0x1fffULL)) {
> >        // valid, access ok?
> >        if ((env->itlb_tte[i] & 0x8000000000000000ULL) == 0 ||
> >            ((env->itlb_tte[i] & 0x4) && is_user)) {
> >        (address & mask) == (env->itlb_tag[i] & ~0x1fffULL) &&
> >        env->itlb_tte[i] & 0x8000000000000000ULL) {
> >        // access ok?
> >        if ((env->itlb_tte[i] & 0x4) && is_user) {
> >
> > HelenOS gets a little bit further than without this change and seems
> >  to panic more or less gracefully later.
> >
> > In short, non-valid entries should be skipped by the loop in
> >  get_physical_address_code instead of causing TFAULT, because
> >  the ITLB can contain a valid entry for the same address. The
> >  same is true for DTLB.
> >
> > Can you confirm this?
>
> Yes, this looks correct. I'll apply a fix.

Thanks!

With this fix in place, HelenOS most of the time panics on:

Kernel panic: Cannot find property 'screen'.

I.e. it searches property 'screen' in /aliases and doesn't
find any. If it is not there already, could you please add
it (together with keyboard) to openbios?

Thanks,
Jakub

sparc64's get_physical_address_code should not fault on TTE's with V=0

[Blue Swirl]

On 1/13/09, Jakub Jermar  wrote:
> Blue Swirl wrote:
> > On 1/12/09, Jakub Jermar  wrote:
> > > Hi,
> > >
> > > when I make the following change to helper.c, get_physical_address_code:
> > >    // ctx match, vaddr match?
> > >    // ctx match, vaddr match, valid?
> > >   if (env->dmmuregs[1] == (env->itlb_tag[i] & 0x1fff) &&
> > >        (address & mask) == (env->itlb_tag[i] & ~0x1fffULL)) {
> > >        // valid, access ok?
> > >        if ((env->itlb_tte[i] & 0x8000000000000000ULL) == 0 ||
> > >            ((env->itlb_tte[i] & 0x4) && is_user)) {
> > >        (address & mask) == (env->itlb_tag[i] & ~0x1fffULL) &&
> > >        env->itlb_tte[i] & 0x8000000000000000ULL) {
> > >        // access ok?
> > >        if ((env->itlb_tte[i] & 0x4) && is_user) {
> > >
> > > HelenOS gets a little bit further than without this change and seems
> > >  to panic more or less gracefully later.
> > >
> > > In short, non-valid entries should be skipped by the loop in
> > >  get_physical_address_code instead of causing TFAULT, because
> > >  the ITLB can contain a valid entry for the same address. The
> > >  same is true for DTLB.
> > >
> > > Can you confirm this?
> >
> > Yes, this looks correct. I'll apply a fix.
>
> Thanks!
>
> With this fix in place, HelenOS most of the time panics on:
>
> Kernel panic: Cannot find property 'screen'.
>
> I.e. it searches property 'screen' in /aliases and doesn't
>  find any. If it is not there already, could you please add
>  it (together with keyboard) to openbios?

Recent OpenBIOS (including the version in Qemu) should have both
"screen" and "keyboard" aliases. In /chosen there is "display" (?) and
"keyboard".

sparc64's get_physical_address_code should not fault on TTE's with V=0

[Jakub Jermar]

Blue Swirl wrote:
> > I.e. it searches property 'screen' in /aliases and doesn't
> >  find any. If it is not there already, could you please add
> >  it (together with keyboard) to openbios?
>
> Recent OpenBIOS (including the version in Qemu) should have both
> "screen" and "keyboard" aliases. In /chosen there is "display" (?) and
> "keyboard".

That's true, but somehow the following cycle used to detect number
of node's properties returns 1 even if there are more properties:

name[0] = '\0';
    while (ofw_next_property(current, name, name) == 1)
        current_node->properties++;

The code above can be found in HelenOS's boot loader and so far
has worked on other machines. The ofw_next_property is a wrapper
for "nextprop" cif method.  I am not that good in Forth, so
I can't verify the implementation of "nextprop". Can you please
have a look if there is anything which could be causing my
wrapper to return incorrect number of properties?

Thanks,
Jakub

sparc64's get_physical_address_code should not fault on TTE's with V=0

[Jakub Jermar]

Jakub Jermar wrote:
> Blue Swirl wrote:
> > > I.e. it searches property 'screen' in /aliases and doesn't
> > >  find any. If it is not there already, could you please add
> > >  it (together with keyboard) to openbios?
> >
> > Recent OpenBIOS (including the version in Qemu) should have both
> > "screen" and "keyboard" aliases. In /chosen there is "display" (?) and
> > "keyboard".
>
> That's true, but somehow the following cycle used to detect number
> of node's properties returns 1 even if there are more properties:
> name[0] = '\0';
> while (ofw_next_property(current, name, name) == 1)
>     current_node->properties++;

It turns out that one char array is not enough. When I use two
arrays and copy the result from the destination to the source
one, the algorithm works.

Jakub