| Message ID | 1475316333-9776-10-git-send-email-atar4qemu@gmail.com |
|---|---|
| State | New |
| Headers | show |
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~
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.
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~
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 --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 {
Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com> --- target-sparc/cpu.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)