From patchwork Wed Feb 22 17:13:21 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Alex_Benn=C3=A9e?= X-Patchwork-Id: 94357 Delivered-To: patch@linaro.org Received: by 10.140.20.99 with SMTP id 90csp1030096qgi; Wed, 22 Feb 2017 09:58:11 -0800 (PST) X-Received: by 10.237.58.41 with SMTP id n38mr33672958qte.134.1487786291622; Wed, 22 Feb 2017 09:58:11 -0800 (PST) Return-Path: Received: from lists.gnu.org (lists.gnu.org. [2001:4830:134:3::11]) by mx.google.com with ESMTPS id y13si1418977qkb.283.2017.02.22.09.58.11 for (version=TLS1 cipher=AES128-SHA bits=128/128); Wed, 22 Feb 2017 09:58:11 -0800 (PST) Received-SPF: pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) client-ip=2001:4830:134:3::11; Authentication-Results: mx.google.com; dkim=fail header.i=@linaro.org; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) smtp.mailfrom=qemu-devel-bounces+patch=linaro.org@nongnu.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from localhost ([::1]:54462 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cgbB3-0006Pw-0m for patch@linaro.org; Wed, 22 Feb 2017 12:58:09 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38007) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cgabq-0005lo-Rz for qemu-devel@nongnu.org; Wed, 22 Feb 2017 12:21:51 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cgabp-00033c-4i for qemu-devel@nongnu.org; Wed, 22 Feb 2017 12:21:46 -0500 Received: from mail-wr0-x229.google.com ([2a00:1450:400c:c0c::229]:36322) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1cgabo-00032n-SD for qemu-devel@nongnu.org; Wed, 22 Feb 2017 12:21:45 -0500 Received: by mail-wr0-x229.google.com with SMTP id 89so6733014wrr.3 for ; Wed, 22 Feb 2017 09:21:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=0tVFYz/nzL0By9w3sLfPvjvA6MQu3IYvxpDN14+1TYM=; b=bwyp5DMc+5rOYV7pML+Z6pv5XBBLt7R/+KWnbwjpSWLJgz+YO79Ys8X/weEzxs1SGK yyCQ0D03DhTPD2XI+uqNHy7E7H0I5yW6N+en40uBTYMiHybLYBljPeFMg2XIKWjgAcjr DHT3mbYz5SE6aV/0EoGFSlB6X9GghVQEqIxG0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=0tVFYz/nzL0By9w3sLfPvjvA6MQu3IYvxpDN14+1TYM=; b=ogrNnyWQMepSbSwqphNIpov4wXJydmyltprN/+Ni95USm2pFbSNOOlYDlCc9nJRHAj O+cvWb8Ni1/A4phobyKszIFiVj4eON1KUPiiNLr8wwB7DDE82TPnae20gyfayLJ2Sp3S 0NzJENawg71IrC+1R7u1UiNz+ibZafn2uBzgrYO1mbSod3tbxVqKh88RuDxWN3YI8WXU q3PXcs0j4UbQK68Y9aq3oOyPA3zzKwWdq0F413Qtk16W/kIxVQNY/WPfIZ8Oy+vn9D75 t2/lBAV0Zn+3m5YqP23va58XBZUtUraK+PgfqtQK52JXjAsWmbAJ3Uiv6iaCqnmGLpGb zsow== X-Gm-Message-State: AMke39kpqvn5k/Vgpl+aNqqSefyQr+sWgDLWb2a+bKV4nN3BCwaMONwQ8+HB/avd3kYhcbRd X-Received: by 10.223.142.240 with SMTP id q103mr25102181wrb.180.1487784103744; Wed, 22 Feb 2017 09:21:43 -0800 (PST) Received: from zen.linaro.local ([81.128.185.34]) by smtp.gmail.com with ESMTPSA id l7sm2481361wrl.59.2017.02.22.09.21.39 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 22 Feb 2017 09:21:39 -0800 (PST) Received: from zen.linaroharston (localhost [127.0.0.1]) by zen.linaro.local (Postfix) with ESMTP id 4B81D3E0520; Wed, 22 Feb 2017 17:13:29 +0000 (GMT) From: =?utf-8?q?Alex_Benn=C3=A9e?= To: rth@twiddle.net, peter.maydell@linaro.org Date: Wed, 22 Feb 2017 17:13:21 +0000 Message-Id: <20170222171327.26624-19-alex.bennee@linaro.org> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20170222171327.26624-1-alex.bennee@linaro.org> References: <20170222171327.26624-1-alex.bennee@linaro.org> MIME-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2a00:1450:400c:c0c::229 Subject: [Qemu-devel] [PATCH v13 18/24] cputlb: atomically update tlb fields used by tlb_reset_dirty X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: mttcg@listserver.greensocs.com, nikunj@linux.vnet.ibm.com, Peter Crosthwaite , jan.kiszka@siemens.com, mark.burton@greensocs.com, a.rigo@virtualopensystems.com, qemu-devel@nongnu.org, cota@braap.org, serge.fdrv@gmail.com, pbonzini@redhat.com, bobby.prani@gmail.com, =?utf-8?q?Alex_Benn=C3=A9e?= , bamvor.zhangjian@linaro.org, fred.konrad@greensocs.com Errors-To: qemu-devel-bounces+patch=linaro.org@nongnu.org Sender: "Qemu-devel" The main use case for tlb_reset_dirty is to set the TLB_NOTDIRTY flags in TLB entries to force the slow-path on writes. This is used to mark page ranges containing code which has been translated so it can be invalidated if written to. To do this safely we need to ensure the TLB entries in question for all vCPUs are updated before we attempt to run the code otherwise a race could be introduced. To achieve this we atomically set the flag in tlb_reset_dirty_range and take care when setting it when the TLB entry is filled. On 32 bit systems attempting to emulate 64 bit guests we don't even bother as we might not have the atomic primitives available. MTTCG is disabled in this case and can't be forced on. The copy_tlb_helper function helps keep the atomic semantics in one place to avoid confusion. The dirty helper function is made static as it isn't used outside of cputlb. Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson --- v6 - use TARGET_PAGE_BITS_MIN - use run_on_cpu helpers v7 - fix tlb_debug fmt for 32bit build - un-merged the mmuidx async work which got mashed in last round - introduced copy_tlb_helper function and made TCG_OVERSIZED_GUEST aware --- cputlb.c | 120 +++++++++++++++++++++++++++++++++++++++----------- include/exec/cputlb.h | 2 - 2 files changed, 95 insertions(+), 27 deletions(-) -- 2.11.0 diff --git a/cputlb.c b/cputlb.c index c50254be26..65003350e3 100644 --- a/cputlb.c +++ b/cputlb.c @@ -342,32 +342,90 @@ void tlb_unprotect_code(ram_addr_t ram_addr) cpu_physical_memory_set_dirty_flag(ram_addr, DIRTY_MEMORY_CODE); } -static bool tlb_is_dirty_ram(CPUTLBEntry *tlbe) -{ - return (tlbe->addr_write & (TLB_INVALID_MASK|TLB_MMIO|TLB_NOTDIRTY)) == 0; -} -void tlb_reset_dirty_range(CPUTLBEntry *tlb_entry, uintptr_t start, +/* + * Dirty write flag handling + * + * When the TCG code writes to a location it looks up the address in + * the TLB and uses that data to compute the final address. If any of + * the lower bits of the address are set then the slow path is forced. + * There are a number of reasons to do this but for normal RAM the + * most usual is detecting writes to code regions which may invalidate + * generated code. + * + * Because we want other vCPUs to respond to changes straight away we + * update the te->addr_write field atomically. If the TLB entry has + * been changed by the vCPU in the mean time we skip the update. + * + * As this function uses atomic accesses we also need to ensure + * updates to tlb_entries follow the same access rules. We don't need + * to worry about this for oversized guests as MTTCG is disabled for + * them. + */ + +static void tlb_reset_dirty_range(CPUTLBEntry *tlb_entry, uintptr_t start, uintptr_t length) { - uintptr_t addr; +#if TCG_OVERSIZED_GUEST + uintptr_t addr = tlb_entry->addr_write; - if (tlb_is_dirty_ram(tlb_entry)) { - addr = (tlb_entry->addr_write & TARGET_PAGE_MASK) + tlb_entry->addend; + if ((addr & (TLB_INVALID_MASK | TLB_MMIO | TLB_NOTDIRTY)) == 0) { + addr &= TARGET_PAGE_MASK; + addr += tlb_entry->addend; if ((addr - start) < length) { tlb_entry->addr_write |= TLB_NOTDIRTY; } } +#else + /* paired with atomic_mb_set in tlb_set_page_with_attrs */ + uintptr_t orig_addr = atomic_mb_read(&tlb_entry->addr_write); + uintptr_t addr = orig_addr; + + if ((addr & (TLB_INVALID_MASK | TLB_MMIO | TLB_NOTDIRTY)) == 0) { + addr &= TARGET_PAGE_MASK; + addr += atomic_read(&tlb_entry->addend); + if ((addr - start) < length) { + uintptr_t notdirty_addr = orig_addr | TLB_NOTDIRTY; + atomic_cmpxchg(&tlb_entry->addr_write, orig_addr, notdirty_addr); + } + } +#endif +} + +/* For atomic correctness when running MTTCG we need to use the right + * primitives when copying entries */ +static inline void copy_tlb_helper(CPUTLBEntry *d, CPUTLBEntry *s, + bool atomic_set) +{ +#if TCG_OVERSIZED_GUEST + *d = *s; +#else + if (atomic_set) { + d->addr_read = s->addr_read; + d->addr_code = s->addr_code; + atomic_set(&d->addend, atomic_read(&s->addend)); + /* Pairs with flag setting in tlb_reset_dirty_range */ + atomic_mb_set(&d->addr_write, atomic_read(&s->addr_write)); + } else { + d->addr_read = s->addr_read; + d->addr_write = atomic_read(&s->addr_write); + d->addr_code = s->addr_code; + d->addend = atomic_read(&s->addend); + } +#endif } +/* This is a cross vCPU call (i.e. another vCPU resetting the flags of + * the target vCPU). As such care needs to be taken that we don't + * dangerously race with another vCPU update. The only thing actually + * updated is the target TLB entry ->addr_write flags. + */ void tlb_reset_dirty(CPUState *cpu, ram_addr_t start1, ram_addr_t length) { CPUArchState *env; int mmu_idx; - assert_cpu_is_self(cpu); - env = cpu->env_ptr; for (mmu_idx = 0; mmu_idx < NB_MMU_MODES; mmu_idx++) { unsigned int i; @@ -455,7 +513,7 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr, target_ulong address; target_ulong code_address; uintptr_t addend; - CPUTLBEntry *te; + CPUTLBEntry *te, *tv, tn; hwaddr iotlb, xlat, sz; unsigned vidx = env->vtlb_index++ % CPU_VTLB_SIZE; int asidx = cpu_asidx_from_attrs(cpu, attrs); @@ -490,41 +548,50 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr, index = (vaddr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); te = &env->tlb_table[mmu_idx][index]; - /* do not discard the translation in te, evict it into a victim tlb */ - env->tlb_v_table[mmu_idx][vidx] = *te; + tv = &env->tlb_v_table[mmu_idx][vidx]; + + /* addr_write can race with tlb_reset_dirty_range */ + copy_tlb_helper(tv, te, true); + env->iotlb_v[mmu_idx][vidx] = env->iotlb[mmu_idx][index]; /* refill the tlb */ env->iotlb[mmu_idx][index].addr = iotlb - vaddr; env->iotlb[mmu_idx][index].attrs = attrs; - te->addend = addend - vaddr; + + /* Now calculate the new entry */ + tn.addend = addend - vaddr; if (prot & PAGE_READ) { - te->addr_read = address; + tn.addr_read = address; } else { - te->addr_read = -1; + tn.addr_read = -1; } if (prot & PAGE_EXEC) { - te->addr_code = code_address; + tn.addr_code = code_address; } else { - te->addr_code = -1; + tn.addr_code = -1; } + + tn.addr_write = -1; if (prot & PAGE_WRITE) { 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; + tn.addr_write = address | TLB_MMIO; } else if (memory_region_is_ram(section->mr) && cpu_physical_memory_is_clean( memory_region_get_ram_addr(section->mr) + xlat)) { - te->addr_write = address | TLB_NOTDIRTY; + tn.addr_write = address | TLB_NOTDIRTY; } else { - te->addr_write = address; + tn.addr_write = address; } - } else { - te->addr_write = -1; } + + /* Pairs with flag setting in tlb_reset_dirty_range */ + copy_tlb_helper(te, &tn, true); + /* atomic_mb_set(&te->addr_write, write_address); */ } /* Add a new TLB entry, but without specifying the memory @@ -687,10 +754,13 @@ static bool victim_tlb_hit(CPUArchState *env, size_t mmu_idx, size_t index, if (cmp == page) { /* Found entry in victim tlb, swap tlb and iotlb. */ CPUTLBEntry tmptlb, *tlb = &env->tlb_table[mmu_idx][index]; + + copy_tlb_helper(&tmptlb, tlb, false); + copy_tlb_helper(tlb, vtlb, true); + copy_tlb_helper(vtlb, &tmptlb, true); + CPUIOTLBEntry tmpio, *io = &env->iotlb[mmu_idx][index]; CPUIOTLBEntry *vio = &env->iotlb_v[mmu_idx][vidx]; - - tmptlb = *tlb; *tlb = *vtlb; *vtlb = tmptlb; tmpio = *io; *io = *vio; *vio = tmpio; return true; } diff --git a/include/exec/cputlb.h b/include/exec/cputlb.h index d454c005b7..3f941783c5 100644 --- a/include/exec/cputlb.h +++ b/include/exec/cputlb.h @@ -23,8 +23,6 @@ /* cputlb.c */ void tlb_protect_code(ram_addr_t ram_addr); void tlb_unprotect_code(ram_addr_t ram_addr); -void tlb_reset_dirty_range(CPUTLBEntry *tlb_entry, uintptr_t start, - uintptr_t length); extern int tlb_flush_count; #endif