| Message ID | 1432658961-48553-3-git-send-email-yongbok.kim@imgtec.com |
|---|---|
| State | New |
| Headers | show |
On 26 May 2015 at 17:49, Yongbok Kim <yongbok.kim@imgtec.com> wrote: > Add probe_write() forces a tlb_fill if the specified guest virtual > index isn't in the TCG softmmu TLB. Surely the point is not to fill the TLB but to raise an exception if the address is not writable? > +#if DATA_SIZE == 1 > +/* > + * Force a tlb_fill if the specified guest virtual index isn't in the TCG > + * softmmu TLB. > + */ > +void probe_write(CPUArchState *env, target_ulong addr, int mmu_idx, > + uintptr_t retaddr) > +{ > + int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); > + target_ulong tlb_addr = env->tlb_table[mmu_idx][index].addr_write; > + > + if ((addr & TARGET_PAGE_MASK) > + != (tlb_addr & (TARGET_PAGE_MASK | TLB_INVALID_MASK))) { > + /* TLB entry is for a different page */ > + if (!VICTIM_TLB_HIT(addr_write)) { > + tlb_fill(ENV_GET_CPU(env), addr, MMU_DATA_STORE, mmu_idx, retaddr); > + } > + } > +} > +#endif > #endif /* !defined(SOFTMMU_CODE_ACCESS) */ I think this code does what you want, but the comments are rather misleading... -- PMM
Am 26.05.2015 um 18:49 schrieb Yongbok Kim: > Add probe_write() forces a tlb_fill if the specified guest virtual > index isn't in the TCG softmmu TLB. > > Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com> > --- > include/exec/exec-all.h | 2 ++ > softmmu_template.h | 20 ++++++++++++++++++++ > 2 files changed, 22 insertions(+), 0 deletions(-) > > diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h > index b58cd47..af51203 100644 > --- a/include/exec/exec-all.h > +++ b/include/exec/exec-all.h > @@ -109,6 +109,8 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr, > hwaddr paddr, MemTxAttrs attrs, > int prot, int mmu_idx, target_ulong size); > void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr); > +void probe_write(CPUArchState *env, target_ulong addr, int mmu_idx, > + uintptr_t retaddr); > #else > static inline void tlb_flush_page(CPUState *cpu, target_ulong addr) > { > diff --git a/softmmu_template.h b/softmmu_template.h > index 9cb1659..1a1de4a 100644 > --- a/softmmu_template.h > +++ b/softmmu_template.h > @@ -548,6 +548,26 @@ glue(glue(helper_st, SUFFIX), MMUSUFFIX)(CPUArchState *env, target_ulong addr, > helper_te_st_name(env, addr, val, oi, GETRA()); > } > > +#if DATA_SIZE == 1 > +/* > + * Force a tlb_fill if the specified guest virtual index isn't in the TCG > + * softmmu TLB. > + */ > +void probe_write(CPUArchState *env, target_ulong addr, int mmu_idx, > + uintptr_t retaddr) > +{ > + int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); > + target_ulong tlb_addr = env->tlb_table[mmu_idx][index].addr_write; > + > + if ((addr & TARGET_PAGE_MASK) > + != (tlb_addr & (TARGET_PAGE_MASK | TLB_INVALID_MASK))) { > + /* TLB entry is for a different page */ > + if (!VICTIM_TLB_HIT(addr_write)) { > + tlb_fill(ENV_GET_CPU(env), addr, MMU_DATA_STORE, mmu_idx, retaddr); Isn't the use of ENV_GET_CPU() here contradicting Peter C.'s series? Regards, Andreas > + } > + } > +} > +#endif > #endif /* !defined(SOFTMMU_CODE_ACCESS) */ > > #undef READ_ACCESS_TYPE
On 26/05/2015 18:58, Andreas Färber wrote: >> > +#if DATA_SIZE == 1 >> > +/* >> > + * Force a tlb_fill if the specified guest virtual index isn't in the TCG >> > + * softmmu TLB. >> > + */ >> > +void probe_write(CPUArchState *env, target_ulong addr, int mmu_idx, >> > + uintptr_t retaddr) >> > +{ >> > + int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); >> > + target_ulong tlb_addr = env->tlb_table[mmu_idx][index].addr_write; >> > + >> > + if ((addr & TARGET_PAGE_MASK) >> > + != (tlb_addr & (TARGET_PAGE_MASK | TLB_INVALID_MASK))) { >> > + /* TLB entry is for a different page */ >> > + if (!VICTIM_TLB_HIT(addr_write)) { >> > + tlb_fill(ENV_GET_CPU(env), addr, MMU_DATA_STORE, mmu_idx, retaddr); > Isn't the use of ENV_GET_CPU() here contradicting Peter C.'s series? No, I don't think so. This function has a genuine need to access env's fields, so it is okay for it to accept CPUArchState*, especially because it is called from the TCG innards where everything is based on the CPUArchState* anyway. The functions touched by Peter's series (gdbserver_fork, tb_flush, tcg_cpu_exec, cpu_exec_init) don't, so they should accept CPUState*. In other words, it's okay to go from env to cpu, but you should do it as soon as possible. Paolo
On Tue, May 26, 2015 at 9:58 AM, Andreas Färber <afaerber@suse.de> wrote: > Am 26.05.2015 um 18:49 schrieb Yongbok Kim: >> Add probe_write() forces a tlb_fill if the specified guest virtual >> index isn't in the TCG softmmu TLB. >> >> Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com> >> --- >> include/exec/exec-all.h | 2 ++ >> softmmu_template.h | 20 ++++++++++++++++++++ >> 2 files changed, 22 insertions(+), 0 deletions(-) >> >> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h >> index b58cd47..af51203 100644 >> --- a/include/exec/exec-all.h >> +++ b/include/exec/exec-all.h >> @@ -109,6 +109,8 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr, >> hwaddr paddr, MemTxAttrs attrs, >> int prot, int mmu_idx, target_ulong size); >> void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr); >> +void probe_write(CPUArchState *env, target_ulong addr, int mmu_idx, >> + uintptr_t retaddr); >> #else >> static inline void tlb_flush_page(CPUState *cpu, target_ulong addr) >> { >> diff --git a/softmmu_template.h b/softmmu_template.h >> index 9cb1659..1a1de4a 100644 >> --- a/softmmu_template.h >> +++ b/softmmu_template.h >> @@ -548,6 +548,26 @@ glue(glue(helper_st, SUFFIX), MMUSUFFIX)(CPUArchState *env, target_ulong addr, >> helper_te_st_name(env, addr, val, oi, GETRA()); >> } >> >> +#if DATA_SIZE == 1 >> +/* >> + * Force a tlb_fill if the specified guest virtual index isn't in the TCG >> + * softmmu TLB. >> + */ >> +void probe_write(CPUArchState *env, target_ulong addr, int mmu_idx, >> + uintptr_t retaddr) >> +{ >> + int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); >> + target_ulong tlb_addr = env->tlb_table[mmu_idx][index].addr_write; >> + >> + if ((addr & TARGET_PAGE_MASK) >> + != (tlb_addr & (TARGET_PAGE_MASK | TLB_INVALID_MASK))) { >> + /* TLB entry is for a different page */ >> + if (!VICTIM_TLB_HIT(addr_write)) { >> + tlb_fill(ENV_GET_CPU(env), addr, MMU_DATA_STORE, mmu_idx, retaddr); > > Isn't the use of ENV_GET_CPU() here contradicting Peter C.'s series? > Actually no this one is OK. softmmu_template is in the "write off" bucket out outside my definition of core-code still. I am looking into paolos approach of cputlb, exec-all and translate-all being a multi-compiled template. Files inside the template are free to use ENV_GET_CPU. Regards, Peter > Regards, > Andreas > >> + } >> + } >> +} >> +#endif >> #endif /* !defined(SOFTMMU_CODE_ACCESS) */ >> >> #undef READ_ACCESS_TYPE > > -- > SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB > 21284 (AG Nürnberg) >
On 05/26/2015 09:53 AM, Peter Maydell wrote: > On 26 May 2015 at 17:49, Yongbok Kim <yongbok.kim@imgtec.com> wrote: >> Add probe_write() forces a tlb_fill if the specified guest virtual >> index isn't in the TCG softmmu TLB. > > Surely the point is not to fill the TLB but to raise an > exception if the address is not writable? > >> +#if DATA_SIZE == 1 >> +/* >> + * Force a tlb_fill if the specified guest virtual index isn't in the TCG >> + * softmmu TLB. >> + */ >> +void probe_write(CPUArchState *env, target_ulong addr, int mmu_idx, >> + uintptr_t retaddr) >> +{ >> + int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); >> + target_ulong tlb_addr = env->tlb_table[mmu_idx][index].addr_write; >> + >> + if ((addr & TARGET_PAGE_MASK) >> + != (tlb_addr & (TARGET_PAGE_MASK | TLB_INVALID_MASK))) { >> + /* TLB entry is for a different page */ >> + if (!VICTIM_TLB_HIT(addr_write)) { >> + tlb_fill(ENV_GET_CPU(env), addr, MMU_DATA_STORE, mmu_idx, retaddr); >> + } >> + } >> +} >> +#endif >> #endif /* !defined(SOFTMMU_CODE_ACCESS) */ > > I think this code does what you want, but the comments are > rather misleading... Indeed. The fact that the TLB gets loaded is merely a happy side-effect. r~
On 26/05/2015 18:33, Richard Henderson wrote: > On 05/26/2015 09:53 AM, Peter Maydell wrote: >> On 26 May 2015 at 17:49, Yongbok Kim <yongbok.kim@imgtec.com> wrote: >>> Add probe_write() forces a tlb_fill if the specified guest virtual >>> index isn't in the TCG softmmu TLB. >> >> Surely the point is not to fill the TLB but to raise an >> exception if the address is not writable? >> >>> +#if DATA_SIZE == 1 >>> +/* >>> + * Force a tlb_fill if the specified guest virtual index isn't in the TCG >>> + * softmmu TLB. >>> + */ >>> +void probe_write(CPUArchState *env, target_ulong addr, int mmu_idx, >>> + uintptr_t retaddr) >>> +{ >>> + int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); >>> + target_ulong tlb_addr = env->tlb_table[mmu_idx][index].addr_write; >>> + >>> + if ((addr & TARGET_PAGE_MASK) >>> + != (tlb_addr & (TARGET_PAGE_MASK | TLB_INVALID_MASK))) { >>> + /* TLB entry is for a different page */ >>> + if (!VICTIM_TLB_HIT(addr_write)) { >>> + tlb_fill(ENV_GET_CPU(env), addr, MMU_DATA_STORE, mmu_idx, retaddr); >>> + } >>> + } >>> +} >>> +#endif >>> #endif /* !defined(SOFTMMU_CODE_ACCESS) */ >> >> I think this code does what you want, but the comments are >> rather misleading... > > Indeed. The fact that the TLB gets loaded is merely a happy side-effect. > > > r~ > Agreed, I will change the description. Regards, Yongbok
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index b58cd47..af51203 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -109,6 +109,8 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr, hwaddr paddr, MemTxAttrs attrs, int prot, int mmu_idx, target_ulong size); void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr); +void probe_write(CPUArchState *env, target_ulong addr, int mmu_idx, + uintptr_t retaddr); #else static inline void tlb_flush_page(CPUState *cpu, target_ulong addr) { diff --git a/softmmu_template.h b/softmmu_template.h index 9cb1659..1a1de4a 100644 --- a/softmmu_template.h +++ b/softmmu_template.h @@ -548,6 +548,26 @@ glue(glue(helper_st, SUFFIX), MMUSUFFIX)(CPUArchState *env, target_ulong addr, helper_te_st_name(env, addr, val, oi, GETRA()); } +#if DATA_SIZE == 1 +/* + * Force a tlb_fill if the specified guest virtual index isn't in the TCG + * softmmu TLB. + */ +void probe_write(CPUArchState *env, target_ulong addr, int mmu_idx, + uintptr_t retaddr) +{ + int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); + target_ulong tlb_addr = env->tlb_table[mmu_idx][index].addr_write; + + if ((addr & TARGET_PAGE_MASK) + != (tlb_addr & (TARGET_PAGE_MASK | TLB_INVALID_MASK))) { + /* TLB entry is for a different page */ + if (!VICTIM_TLB_HIT(addr_write)) { + tlb_fill(ENV_GET_CPU(env), addr, MMU_DATA_STORE, mmu_idx, retaddr); + } + } +} +#endif #endif /* !defined(SOFTMMU_CODE_ACCESS) */ #undef READ_ACCESS_TYPE
Add probe_write() forces a tlb_fill if the specified guest virtual index isn't in the TCG softmmu TLB. Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com> --- include/exec/exec-all.h | 2 ++ softmmu_template.h | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 0 deletions(-)