From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43613) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aWRPy-0007nJ-Du for qemu-devel@nongnu.org; Thu, 18 Feb 2016 11:27:04 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aWRPv-0001Q5-6p for qemu-devel@nongnu.org; Thu, 18 Feb 2016 11:27:02 -0500 Received: from mail-wm0-x22b.google.com ([2a00:1450:400c:c09::22b]:35102) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aWRPu-0001Pi-RI for qemu-devel@nongnu.org; Thu, 18 Feb 2016 11:26:59 -0500 Received: by mail-wm0-x22b.google.com with SMTP id c200so36216256wme.0 for ; Thu, 18 Feb 2016 08:26:58 -0800 (PST) References: <1454059965-23402-1-git-send-email-a.rigo@virtualopensystems.com> <1454059965-23402-10-git-send-email-a.rigo@virtualopensystems.com> <878u2ky27v.fsf@linaro.org> From: Alex =?utf-8?Q?Benn=C3=A9e?= In-reply-to: Date: Thu, 18 Feb 2016 16:26:56 +0000 Message-ID: <8737sqdlwv.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 Devel , Claudio Fontana , QEMU Developers , Paolo Bonzini , Jani Kokkonen , VirtualOpenSystems Technical Team , Richard Henderson alvise rigo writes: > On Tue, Feb 16, 2016 at 6:49 PM, Alex Bennée wrote: >> >> 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? > > The upstream QEMU's condition to follow the IO access path is: > if (unlikely(tlb_addr & ~TARGET_PAGE_MASK)) > Now, we split this in: > if (tlb_addr & TLB_EXCL) > for RAM exclusive accesses and > if (tlb_addr & ~(TARGET_PAGE_MASK | TLB_EXCL)) > for IO accesses. In this last case, we handle also the IO exclusive > accesses. OK I see that now. Thanks. > >> >>> + 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 -- Alex Bennée