diff mbox

[09/29] target-sparc: hypervisor mode takes over nucleus mode

Message ID 1475316333-9776-10-git-send-email-atar4qemu@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Artyom Tarasenko Oct. 1, 2016, 10:05 a.m. UTC
Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
---
 target-sparc/cpu.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Richard Henderson Oct. 10, 2016, 9:41 p.m. UTC | #1
On 10/01/2016 05:05 AM, Artyom Tarasenko wrote:
> Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
> ---
>  target-sparc/cpu.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
> index 0b5c79f..fbeb8d7 100644
> --- a/target-sparc/cpu.h
> +++ b/target-sparc/cpu.h
> @@ -699,10 +699,10 @@ static inline int cpu_mmu_index(CPUSPARCState *env1, bool ifetch)
>  #elif !defined(TARGET_SPARC64)
>      return env1->psrs;
>  #else
> -    if (env1->tl > 0) {
> -        return MMU_NUCLEUS_IDX;
> -    } else if (cpu_hypervisor_mode(env1)) {
> +    if (cpu_hypervisor_mode(env1)) {
>          return MMU_HYPV_IDX;
> +    } else if (env1->tl > 0) {
> +        return MMU_NUCLEUS_IDX;
>      } else if (cpu_supervisor_mode(env1)) {
>          return MMU_KERNEL_IDX;
>      } else {
>

While playing with your patch set, I discovered that we also need a patch to 
get_asi for ASI_N et al to retain MMU_HYPV_IDX, and not decrease privilege. 
This happens *very* early in the prom boot, with the first casx (when casx is 
implemented inline).


r~
Artyom Tarasenko Oct. 12, 2016, 11:33 a.m. UTC | #2
On Mon, Oct 10, 2016 at 11:41 PM, Richard Henderson <rth@twiddle.net> wrote:
> On 10/01/2016 05:05 AM, Artyom Tarasenko wrote:
>>
>> Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
>> ---
>>  target-sparc/cpu.h | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
>> index 0b5c79f..fbeb8d7 100644
>> --- a/target-sparc/cpu.h
>> +++ b/target-sparc/cpu.h
>> @@ -699,10 +699,10 @@ static inline int cpu_mmu_index(CPUSPARCState *env1,
>> bool ifetch)
>>  #elif !defined(TARGET_SPARC64)
>>      return env1->psrs;
>>  #else
>> -    if (env1->tl > 0) {
>> -        return MMU_NUCLEUS_IDX;
>> -    } else if (cpu_hypervisor_mode(env1)) {
>> +    if (cpu_hypervisor_mode(env1)) {
>>          return MMU_HYPV_IDX;
>> +    } else if (env1->tl > 0) {
>> +        return MMU_NUCLEUS_IDX;
>>      } else if (cpu_supervisor_mode(env1)) {
>>          return MMU_KERNEL_IDX;
>>      } else {
>>
>
> While playing with your patch set, I discovered that we also need a patch to
> get_asi for ASI_N et al to retain MMU_HYPV_IDX, and not decrease privilege.
> This happens *very* early in the prom boot, with the first casx (when casx
> is implemented inline).

Why is the bug not visible with the current master? I wonder if we
have a symmetrical bug somewhere.
Richard Henderson Oct. 12, 2016, 1:29 p.m. UTC | #3
On 10/12/2016 06:33 AM, Artyom Tarasenko wrote:
> On Mon, Oct 10, 2016 at 11:41 PM, Richard Henderson <rth@twiddle.net> wrote:
>> On 10/01/2016 05:05 AM, Artyom Tarasenko wrote:
>>>
>>> Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
>>> ---
>>>  target-sparc/cpu.h | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
>>> index 0b5c79f..fbeb8d7 100644
>>> --- a/target-sparc/cpu.h
>>> +++ b/target-sparc/cpu.h
>>> @@ -699,10 +699,10 @@ static inline int cpu_mmu_index(CPUSPARCState *env1,
>>> bool ifetch)
>>>  #elif !defined(TARGET_SPARC64)
>>>      return env1->psrs;
>>>  #else
>>> -    if (env1->tl > 0) {
>>> -        return MMU_NUCLEUS_IDX;
>>> -    } else if (cpu_hypervisor_mode(env1)) {
>>> +    if (cpu_hypervisor_mode(env1)) {
>>>          return MMU_HYPV_IDX;
>>> +    } else if (env1->tl > 0) {
>>> +        return MMU_NUCLEUS_IDX;
>>>      } else if (cpu_supervisor_mode(env1)) {
>>>          return MMU_KERNEL_IDX;
>>>      } else {
>>>
>>
>> While playing with your patch set, I discovered that we also need a patch to
>> get_asi for ASI_N et al to retain MMU_HYPV_IDX, and not decrease privilege.
>> This happens *very* early in the prom boot, with the first casx (when casx
>> is implemented inline).
>
> Why is the bug not visible with the current master? I wonder if we
> have a symmetrical bug somewhere.

Hmm, I dunno.  I assume it has something to do with casx being implemented out 
of line, and using helper_ld_asi instead of tcg_gen_qemu_ld_tl directly.


r~
Artyom Tarasenko Nov. 1, 2016, 6:12 p.m. UTC | #4
On Wed, Oct 12, 2016 at 3:29 PM, Richard Henderson <rth@twiddle.net> wrote:
> On 10/12/2016 06:33 AM, Artyom Tarasenko wrote:
>>
>> On Mon, Oct 10, 2016 at 11:41 PM, Richard Henderson <rth@twiddle.net>
>> wrote:
>>>
>>> On 10/01/2016 05:05 AM, Artyom Tarasenko wrote:
>>>>
>>>>
>>>> Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
>>>> ---
>>>>  target-sparc/cpu.h | 6 +++---
>>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
>>>> index 0b5c79f..fbeb8d7 100644
>>>> --- a/target-sparc/cpu.h
>>>> +++ b/target-sparc/cpu.h
>>>> @@ -699,10 +699,10 @@ static inline int cpu_mmu_index(CPUSPARCState
>>>> *env1,
>>>> bool ifetch)
>>>>  #elif !defined(TARGET_SPARC64)
>>>>      return env1->psrs;
>>>>  #else
>>>> -    if (env1->tl > 0) {
>>>> -        return MMU_NUCLEUS_IDX;
>>>> -    } else if (cpu_hypervisor_mode(env1)) {
>>>> +    if (cpu_hypervisor_mode(env1)) {
>>>>          return MMU_HYPV_IDX;
>>>> +    } else if (env1->tl > 0) {
>>>> +        return MMU_NUCLEUS_IDX;
>>>>      } else if (cpu_supervisor_mode(env1)) {
>>>>          return MMU_KERNEL_IDX;
>>>>      } else {
>>>>
>>>
>>> While playing with your patch set, I discovered that we also need a patch
>>> to
>>> get_asi for ASI_N et al to retain MMU_HYPV_IDX, and not decrease
>>> privilege.
>>> This happens *very* early in the prom boot, with the first casx (when
>>> casx
>>> is implemented inline).
>>
>>
>> Why is the bug not visible with the current master? I wonder if we
>> have a symmetrical bug somewhere.
>
>
> Hmm, I dunno.  I assume it has something to do with casx being implemented
> out of line, and using helper_ld_asi instead of tcg_gen_qemu_ld_tl directly.
>

Actually I don't see where the  privilege is decreased: get_asi uses a
local mem_idx variable, the dc->mem_idx is retained.
What patch do you have in mind?
diff mbox

Patch

diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
index 0b5c79f..fbeb8d7 100644
--- a/target-sparc/cpu.h
+++ b/target-sparc/cpu.h
@@ -699,10 +699,10 @@  static inline int cpu_mmu_index(CPUSPARCState *env1, bool ifetch)
 #elif !defined(TARGET_SPARC64)
     return env1->psrs;
 #else
-    if (env1->tl > 0) {
-        return MMU_NUCLEUS_IDX;
-    } else if (cpu_hypervisor_mode(env1)) {
+    if (cpu_hypervisor_mode(env1)) {
         return MMU_HYPV_IDX;
+    } else if (env1->tl > 0) {
+        return MMU_NUCLEUS_IDX;
     } else if (cpu_supervisor_mode(env1)) {
         return MMU_KERNEL_IDX;
     } else {