[Qemu-devel] sparc64's get_physical_address_code should not fault on TTE's with V=0

[Qemu-devel] sparc64's get_physical_address_code should not fault on TTE's with V=0

[Jakub Jermar]

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?

Thanks,
Jakub

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

[Blue Swirl]

On 1/12/09, Jakub Jermar <jakub@jermar.eu> 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.

[Qemu-devel] Re: 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 <jakub@jermar.eu> 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