[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC 2/5] Add new TLB_EXCL flag
From: |
Richard Henderson |
Subject: |
Re: [Qemu-devel] [RFC 2/5] Add new TLB_EXCL flag |
Date: |
Thu, 07 May 2015 10:25:21 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 |
On 05/06/2015 08:38 AM, Alvise Rigo wrote:
> Add a new flag for the TLB entries to force all the accesses made to a
> page to follow the slow-path.
>
> Mark the accessed page as dirty to invalidate any pending operation of
> LL/SC.
>
> Suggested-by: Jani Kokkonen <address@hidden>
> Suggested-by: Claudio Fontana <address@hidden>
> Signed-off-by: Alvise Rigo <address@hidden>
> ---
> cputlb.c | 7 ++++++-
> include/exec/cpu-all.h | 1 +
> softmmu_template.h | 48 +++++++++++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 54 insertions(+), 2 deletions(-)
>
> diff --git a/cputlb.c b/cputlb.c
> index 38f2151..3e4ccba 100644
> --- a/cputlb.c
> +++ b/cputlb.c
> @@ -324,7 +324,12 @@ void tlb_set_page(CPUState *cpu, target_ulong vaddr,
> + xlat)) {
> te->addr_write = address | TLB_NOTDIRTY;
> } else {
> - te->addr_write = address;
> + if (!cpu_physical_memory_excl_is_dirty(section->mr->ram_addr
> + + xlat)) {
> + te->addr_write = address | TLB_EXCL;
> + } else {
> + te->addr_write = address;
> + }
I don't see that you initialize this new bitmap to all ones? That would
generate exclusive accesses on all of memory until each unit (page?) is touched.
Perhaps you should invert the sense of your use of the dirty bitmap, such that
a set bit means that some vcpu is monitoring the unit, and no vcpu has written
to the unit?
Your nesting above would seem to fail when TLB_NOTDIRTY needs to be set as well.
> @@ -262,7 +263,7 @@ WORD_TYPE helper_be_ld_name(CPUArchState *env,
> target_ulong addr, int mmu_idx,
> uintptr_t haddr;
> DATA_TYPE res;
>
> - /* Adjust the given return address. */
> + /* Adjust the given return address. */
> retaddr -= GETPC_ADJ;
Careful...
> @@ -387,6 +388,7 @@ void helper_le_st_name(CPUArchState *env, target_ulong
> addr, DATA_TYPE val,
> int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
> target_ulong tlb_addr = env->tlb_table[mmu_idx][index].addr_write;
> uintptr_t haddr;
> + bool to_excl_mem = false;
>
> /* Adjust the given return address. */
> retaddr -= GETPC_ADJ;
> @@ -406,6 +408,14 @@ void helper_le_st_name(CPUArchState *env, target_ulong
> addr, DATA_TYPE val,
> tlb_addr = env->tlb_table[mmu_idx][index].addr_write;
> }
>
> + if (unlikely(tlb_addr & TLB_EXCL &&
> + !(tlb_addr & ~(TARGET_PAGE_MASK | TLB_EXCL)))) {
This is (tlb_addr & ~TARGET_PAGE_MASK) == TLB_EXCL, but...
> + /* The slow-path has been forced since we are reading a page used
> for a
> + * load-link operation. */
> + to_excl_mem = true;
> + goto skip_io;
I'm not fond of either the boolean or the goto.
> + }
> +
> /* Handle an IO access. */
> if (unlikely(tlb_addr & ~TARGET_PAGE_MASK)) {
Nesting your unlikely code under one unlikely test seems better. And would be
more likely to work in the case you need to handle both TLB_NOTDIRTY and
TLB_EXCL.
r~
- [Qemu-devel] [RFC 0/5] Slow-path for atomic instruction translation, Alvise Rigo, 2015/05/06
- [Qemu-devel] [RFC 1/5] exec: Add new exclusive bitmap to ram_list, Alvise Rigo, 2015/05/06
- [Qemu-devel] [RFC 2/5] Add new TLB_EXCL flag, Alvise Rigo, 2015/05/06
- Re: [Qemu-devel] [RFC 2/5] Add new TLB_EXCL flag,
Richard Henderson <=
- [Qemu-devel] [RFC 4/5] tcg-op: create new TCG qemu_ldlink and qemu_stcond instructions, Alvise Rigo, 2015/05/06
- [Qemu-devel] [RFC 3/5] softmmu: Add helpers for a new slow-path, Alvise Rigo, 2015/05/06
- [Qemu-devel] [RFC 5/5] target-arm: translate: implement qemu_ldlink and qemu_stcond ops, Alvise Rigo, 2015/05/06
- Re: [Qemu-devel] [RFC 0/5] Slow-path for atomic instruction translation, Paolo Bonzini, 2015/05/06