Re: [Qemu-devel] [PATCH] sparc32 boot mode flag fix
On 11/6/07, Paul Brook <[EMAIL PROTECTED]> wrote: > > > IIUC enabling/disabling boot mode is no different to and other VM change. > > > If the virtual->physical mapping happens to be the same then it's > > > perfectly ok to reuse the TB. > > > > Not in this case: in boot mode, physical and virtual address 0 > > generates TBs from PROM code. > > How is this different to using the MMU to map the PROM at virtual address > zero? In that case, the virtual address is zero, but physical address is 0xff000. Oh, now I see my error, it is the same in the boot mode case too! Sorry about the noise. > I see exactly one use of MMU_BM. It simply hard-wires a particular > virtual->physical address mapping. It derives from the Sparc specification, where the PC at reset is zero. I wonder why they didn't put the start of memory somewhere higher instead of this hack. On Sparc64 the reset vector is implementation dependent so it can be directly set to the PROM address.
Re: [Qemu-devel] [PATCH] sparc32 boot mode flag fix
This patch also performs a CPU reset after the CPU is registered rather than before. Why is this change needed? Reset should be doing CPU dependent stuff and the CPU dependent setup is performed when the CPU is registered.
Re: [Qemu-devel] [PATCH] sparc32 boot mode flag fix
> > IIUC enabling/disabling boot mode is no different to and other VM change. > > If the virtual->physical mapping happens to be the same then it's > > perfectly ok to reuse the TB. > > Not in this case: in boot mode, physical and virtual address 0 > generates TBs from PROM code. How is this different to using the MMU to map the PROM at virtual address zero? I see exactly one use of MMU_BM. It simply hard-wires a particular virtual->physical address mapping. Paul
Re: [Qemu-devel] [PATCH] sparc32 boot mode flag fix
On 11/6/07, Paul Brook <[EMAIL PROTECTED]> wrote: > > > This patch also removes the MMU flags from being saved in the > > > translation block code as a result of an off line discussion with Paul > > > Brook. > > > > I'd like to hear the reasoning behind that. The TBs generated while in > > boot mode and MMU disabled may contain translations generated from > > virtual to physical mappings that do not exist when the mode is > > changed. Boot mode and MMU disable are not used after boot and these > > bits don't affect translation, so those bits may be less important and > > not worth the few bits in TB flags. > > It think you're confusing the TB cache with the TLB. Each TB is already > indexed by both physical and virtual address (explicitly in tb_find_slow, and > implicitly in tb_find_fast because a tlb flush clears env->tb_jmp_cache). > > IIUC enabling/disabling boot mode is no different to and other VM change. If > the virtual->physical mapping happens to be the same then it's perfectly ok > to reuse the TB. Not in this case: in boot mode, physical and virtual address 0 generates TBs from PROM code. When control transfers to OS in normal MMU mode, virtual and physical address 0 generates TBs from code in RAM. > The TLB is already flushed whenever the MMU mode is changes. There is no need > to invalidate the TB. The TB is valid, but only when the boot mode bit is the same as recorded in the TB flags.
Re: [Qemu-devel] [PATCH] sparc32 boot mode flag fix
> > This patch also removes the MMU flags from being saved in the > > translation block code as a result of an off line discussion with Paul > > Brook. > > I'd like to hear the reasoning behind that. The TBs generated while in > boot mode and MMU disabled may contain translations generated from > virtual to physical mappings that do not exist when the mode is > changed. Boot mode and MMU disable are not used after boot and these > bits don't affect translation, so those bits may be less important and > not worth the few bits in TB flags. It think you're confusing the TB cache with the TLB. Each TB is already indexed by both physical and virtual address (explicitly in tb_find_slow, and implicitly in tb_find_fast because a tlb flush clears env->tb_jmp_cache). IIUC enabling/disabling boot mode is no different to and other VM change. If the virtual->physical mapping happens to be the same then it's perfectly ok to reuse the TB. The TLB is already flushed whenever the MMU mode is changes. There is no need to invalidate the TB. Paul
Re: [Qemu-devel] [PATCH] sparc32 boot mode flag fix
On 11/6/07, Robert Reif <[EMAIL PROTECTED]> wrote: > This patch adds CPU dependent boot mode flag support. > > Different CPUs use different bits for the boot mode flag. The constant > MMU_BM is replaced with a variable which is set for the selected CPU. Nice. I have a patch for OpenBIOS to fix setting of the boot flag in the exception handler. It's not used except for crashes. > This patch also removes the MMU flags from being saved in the > translation block code as a result of an off line discussion with Paul > Brook. I'd like to hear the reasoning behind that. The TBs generated while in boot mode and MMU disabled may contain translations generated from virtual to physical mappings that do not exist when the mode is changed. Boot mode and MMU disable are not used after boot and these bits don't affect translation, so those bits may be less important and not worth the few bits in TB flags. No-fault mode is used by Linux to store user register windows forcibly to user stack. This happens often. Looks like this part works, but I'm not sure this is a correct solution. I'd like to get rid of the TLB flushing when MMU flags change to support the hardware contexts better, this change would rely on that heavily. There is also a long standing problem: system_reset in a SMP system crashes and I suspect this area. > This patch also performs a CPU reset after the CPU is registered rather > than before. Why is this change needed? > This patch has successfully booted the debian installer and the initrd > kernel > in sparc-test successfully for both an ss5 and ss10. It also makes running > an ss10 openboot rom image behave a little better. It also passes my tests and the patch looks OK for me, except for the questions above.
[Qemu-devel] [PATCH] sparc32 boot mode flag fix
This patch adds CPU dependent boot mode flag support. Different CPUs use different bits for the boot mode flag. The constant MMU_BM is replaced with a variable which is set for the selected CPU. This patch also removes the MMU flags from being saved in the translation block code as a result of an off line discussion with Paul Brook. This patch also performs a CPU reset after the CPU is registered rather than before. This patch has successfully booted the debian installer and the initrd kernel in sparc-test successfully for both an ss5 and ss10. It also makes running an ss10 openboot rom image behave a little better. Index: cpu-exec.c === RCS file: /sources/qemu/qemu/cpu-exec.c,v retrieving revision 1.120 diff -p -u -r1.120 cpu-exec.c --- cpu-exec.c 14 Oct 2007 07:07:04 - 1.120 +++ cpu-exec.c 6 Nov 2007 00:12:56 - @@ -181,10 +181,8 @@ static inline TranslationBlock *tb_find_ flags = (((env->pstate & PS_PEF) >> 1) | ((env->fprs & FPRS_FEF) << 2)) | (env->pstate & PS_PRIV) | ((env->lsu & (DMMU_E | IMMU_E)) >> 2); #else -// FPU enable . MMU Boot . MMU enabled . MMU no-fault . Supervisor -flags = (env->psref << 4) | (((env->mmuregs[0] & MMU_BM) >> 14) << 3) -| ((env->mmuregs[0] & (MMU_E | MMU_NF)) << 1) -| env->psrs; +// FPU enable . Supervisor +flags = (env->psref << 4) | env->psrs; #endif cs_base = env->npc; pc = env->pc; Index: target-sparc/cpu.h === RCS file: /sources/qemu/qemu/target-sparc/cpu.h,v retrieving revision 1.56 diff -p -u -r1.56 cpu.h --- target-sparc/cpu.h 14 Oct 2007 17:07:21 - 1.56 +++ target-sparc/cpu.h 6 Nov 2007 00:13:00 - @@ -147,7 +147,6 @@ /* MMU */ #define MMU_E (1<<0) #define MMU_NF(1<<1) -#define MMU_BM(1<<14) #define PTE_ENTRYTYPE_MASK 3 #define PTE_ACCESS_MASK0x1c @@ -200,6 +199,7 @@ typedef struct CPUSPARCState { int interrupt_index; int interrupt_request; int halted; +uint32_t mmu_bm; /* NOTE: we allow 8 more registers to handle wrapping */ target_ulong regbase[NWINDOWS * 16 + 8]; Index: target-sparc/helper.c === RCS file: /sources/qemu/qemu/target-sparc/helper.c,v retrieving revision 1.28 diff -p -u -r1.28 helper.c --- target-sparc/helper.c 14 Oct 2007 07:07:08 - 1.28 +++ target-sparc/helper.c 6 Nov 2007 00:13:00 - @@ -114,7 +114,7 @@ int get_physical_address (CPUState *env, if ((env->mmuregs[0] & MMU_E) == 0) { /* MMU disabled */ // Boot mode: instruction fetches are taken from PROM -if (rw == 2 && (env->mmuregs[0] & MMU_BM)) { +if (rw == 2 && (env->mmuregs[0] & env->mmu_bm)) { *physical = 0xff000ULL | (address & 0x3ULL); *prot = PAGE_READ | PAGE_EXEC; return 0; Index: target-sparc/op_helper.c === RCS file: /sources/qemu/qemu/target-sparc/op_helper.c,v retrieving revision 1.50 diff -p -u -r1.50 op_helper.c --- target-sparc/op_helper.c29 Oct 2007 14:39:49 - 1.50 +++ target-sparc/op_helper.c6 Nov 2007 00:13:01 - @@ -493,8 +493,8 @@ void helper_st_asi(int asi, int size) oldreg = env->mmuregs[reg]; switch(reg) { case 0: -env->mmuregs[reg] &= ~(MMU_E | MMU_NF | MMU_BM); -env->mmuregs[reg] |= T1 & (MMU_E | MMU_NF | MMU_BM); +env->mmuregs[reg] &= ~(MMU_E | MMU_NF | env->mmu_bm); +env->mmuregs[reg] |= T1 & (MMU_E | MMU_NF | env->mmu_bm); // Mappings generated during no-fault mode or MMU // disabled mode are invalid in normal mode if (oldreg != env->mmuregs[reg]) Index: target-sparc/translate.c === RCS file: /sources/qemu/qemu/target-sparc/translate.c,v retrieving revision 1.78 diff -p -u -r1.78 translate.c --- target-sparc/translate.c17 Oct 2007 17:34:57 - 1.78 +++ target-sparc/translate.c6 Nov 2007 00:13:03 - @@ -59,6 +59,7 @@ struct sparc_def_t { target_ulong iu_version; uint32_t fpu_version; uint32_t mmu_version; +uint32_t mmu_bm; }; static uint16_t *gen_opc_ptr; @@ -3482,7 +3483,7 @@ void cpu_reset(CPUSPARCState *env) #else env->pc = 0; env->mmuregs[0] &= ~(MMU_E | MMU_NF); -env->mmuregs[0] |= MMU_BM; +env->mmuregs[0] |= env->mmu_bm; #endif env->npc = env->pc + 4; #endif @@ -3496,7 +3497,6 @@ CPUSPARCState *cpu_sparc_init(void) if (!env) return NULL; cpu_exec_init(env); -cpu_reset(env); return (env); } @@ -3515,30 +3515,35 @@ static const sparc_def_t sparc_defs[] = .iu_version = 0x04 << 24, /* Impl 0, ver 4 */ .fpu_version = 4