From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38883) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aVjlD-0000C8-DL for qemu-devel@nongnu.org; Tue, 16 Feb 2016 12:50:04 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aVjl9-0000jQ-4q for qemu-devel@nongnu.org; Tue, 16 Feb 2016 12:50:03 -0500 Received: from mail-wm0-x22f.google.com ([2a00:1450:400c:c09::22f]:35825) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aVjl8-0000jL-OI for qemu-devel@nongnu.org; Tue, 16 Feb 2016 12:49:59 -0500 Received: by mail-wm0-x22f.google.com with SMTP id c200so174436007wme.0 for ; Tue, 16 Feb 2016 09:49:58 -0800 (PST) References: <1454059965-23402-1-git-send-email-a.rigo@virtualopensystems.com> <1454059965-23402-10-git-send-email-a.rigo@virtualopensystems.com> From: Alex =?utf-8?Q?Benn=C3=A9e?= In-reply-to: <1454059965-23402-10-git-send-email-a.rigo@virtualopensystems.com> Date: Tue, 16 Feb 2016 17:49:56 +0000 Message-ID: <878u2ky27v.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [RFC v7 09/16] softmmu: Include MMIO/invalid exclusive accesses List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alvise Rigo Cc: mttcg@listserver.greensocs.com, claudio.fontana@huawei.com, qemu-devel@nongnu.org, pbonzini@redhat.com, jani.kokkonen@huawei.com, tech@virtualopensystems.com, rth@twiddle.net Alvise Rigo writes: > Enable exclusive accesses when the MMIO/invalid flag is set in the TLB > entry. > > In case a LL access is done to MMIO memory, we treat it differently from > a RAM access in that we do not rely on the EXCL bitmap to flag the page > as exclusive. In fact, we don't even need the TLB_EXCL flag to force the > slow path, since it is always forced anyway. > > This commit does not take care of invalidating an MMIO exclusive range from > other non-exclusive accesses i.e. CPU1 LoadLink to MMIO address X and > CPU2 writes to X. This will be addressed in the following commit. > > Suggested-by: Jani Kokkonen > Suggested-by: Claudio Fontana > Signed-off-by: Alvise Rigo > --- > cputlb.c | 7 +++---- > softmmu_template.h | 26 ++++++++++++++++++++------ > 2 files changed, 23 insertions(+), 10 deletions(-) > > diff --git a/cputlb.c b/cputlb.c > index aa9cc17..87d09c8 100644 > --- a/cputlb.c > +++ b/cputlb.c > @@ -424,7 +424,7 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr, > if ((memory_region_is_ram(section->mr) && section->readonly) > || memory_region_is_romd(section->mr)) { > /* Write access calls the I/O callback. */ > - te->addr_write = address | TLB_MMIO; > + address |= TLB_MMIO; > } else if (memory_region_is_ram(section->mr) > && cpu_physical_memory_is_clean(section->mr->ram_addr > + xlat)) { > @@ -437,11 +437,10 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr, > if (cpu_physical_memory_is_excl(section->mr->ram_addr + xlat)) { > /* There is at least one vCPU that has flagged the address as > * exclusive. */ > - te->addr_write = address | TLB_EXCL; > - } else { > - te->addr_write = address; > + address |= TLB_EXCL; > } > } > + te->addr_write = address; As mentioned before I think this bit belongs in the earlier patch. > } else { > te->addr_write = -1; > } > diff --git a/softmmu_template.h b/softmmu_template.h > index 267c52a..c54bdc9 100644 > --- a/softmmu_template.h > +++ b/softmmu_template.h > @@ -476,7 +476,7 @@ void helper_le_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val, > > /* Handle an IO access or exclusive access. */ > if (unlikely(tlb_addr & ~TARGET_PAGE_MASK)) { > - if ((tlb_addr & ~TARGET_PAGE_MASK) == TLB_EXCL) { > + if (tlb_addr & TLB_EXCL) { > CPUIOTLBEntry *iotlbentry = &env->iotlb[mmu_idx][index]; > CPUState *cpu = ENV_GET_CPU(env); > CPUClass *cc = CPU_GET_CLASS(cpu); > @@ -500,8 +500,15 @@ void helper_le_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val, > } > } > > - glue(helper_le_st_name, _do_ram_access)(env, val, addr, oi, > - mmu_idx, index, retaddr); > + if (tlb_addr & ~(TARGET_PAGE_MASK | TLB_EXCL)) { /* MMIO > access */ What about the other flags? Shouldn't this be tlb_addr & TLB_MMIO? > + glue(helper_le_st_name, _do_mmio_access)(env, val, addr, oi, > + mmu_idx, index, > + retaddr); > + } else { > + glue(helper_le_st_name, _do_ram_access)(env, val, addr, oi, > + mmu_idx, index, > + retaddr); > + } > > lookup_and_reset_cpus_ll_addr(hw_addr, DATA_SIZE); > > @@ -620,7 +627,7 @@ void helper_be_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val, > > /* Handle an IO access or exclusive access. */ > if (unlikely(tlb_addr & ~TARGET_PAGE_MASK)) { > - if ((tlb_addr & ~TARGET_PAGE_MASK) == TLB_EXCL) { > + if (tlb_addr & TLB_EXCL) { > CPUIOTLBEntry *iotlbentry = &env->iotlb[mmu_idx][index]; > CPUState *cpu = ENV_GET_CPU(env); > CPUClass *cc = CPU_GET_CLASS(cpu); > @@ -644,8 +651,15 @@ void helper_be_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val, > } > } > > - glue(helper_be_st_name, _do_ram_access)(env, val, addr, oi, > - mmu_idx, index, retaddr); > + if (tlb_addr & ~(TARGET_PAGE_MASK | TLB_EXCL)) { /* MMIO access */ > + glue(helper_be_st_name, _do_mmio_access)(env, val, addr, oi, > + mmu_idx, index, > + retaddr); > + } else { > + glue(helper_be_st_name, _do_ram_access)(env, val, addr, oi, > + mmu_idx, index, > + retaddr); > + } > > lookup_and_reset_cpus_ll_addr(hw_addr, DATA_SIZE); -- Alex Bennée