From patchwork Tue Jul 26 14:03:33 2016 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: 72814 Delivered-To: patch@linaro.org Received: by 10.140.29.52 with SMTP id a49csp1705518qga; Tue, 26 Jul 2016 07:04:40 -0700 (PDT) X-Received: by 10.55.163.133 with SMTP id m127mr30032752qke.71.1469541880730; Tue, 26 Jul 2016 07:04:40 -0700 (PDT) Return-Path: Received: from lists.gnu.org (lists.gnu.org. [2001:4830:134:3::11]) by mx.google.com with ESMTPS id y34si575180qta.16.2016.07.26.07.04.40 for (version=TLS1 cipher=AES128-SHA bits=128/128); Tue, 26 Jul 2016 07:04:40 -0700 (PDT) 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 dis=NONE) header.from=linaro.org Received: from localhost ([::1]:40110 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bS2yO-0006oS-Bc for patch@linaro.org; Tue, 26 Jul 2016 10:04:40 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57049) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bS2xI-0006VV-Su for qemu-devel@nongnu.org; Tue, 26 Jul 2016 10:03:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bS2xE-0001Tj-Qb for qemu-devel@nongnu.org; Tue, 26 Jul 2016 10:03:32 -0400 Received: from mail-wm0-x230.google.com ([2a00:1450:400c:c09::230]:35331) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bS2xD-0001TU-Cy for qemu-devel@nongnu.org; Tue, 26 Jul 2016 10:03:28 -0400 Received: by mail-wm0-x230.google.com with SMTP id f65so175482538wmi.0 for ; Tue, 26 Jul 2016 07:03:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=references:user-agent:from:to:subject:in-reply-to:date:message-id :mime-version:content-transfer-encoding; bh=8vhVZl9USz4Sq0SZBGun3TOuoFsHWIFw1caRuJlyPMg=; b=F218XITXKXxs01Av87K6DUnuAryKvDOaOPW/uK8XXc2/tV11+hk8yOOnC/gPNTCfR5 FCj285JRZzPYRGfv0zyG7xFgrQe6S3xuEK5oGKqgOc39fvumXsV3YNtoIBscFPflTRGm QYf8v13xNbKEpbsxifj9NWQCu8QJtrvjWdmhE= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:references:user-agent:from:to:subject :in-reply-to:date:message-id:mime-version:content-transfer-encoding; bh=8vhVZl9USz4Sq0SZBGun3TOuoFsHWIFw1caRuJlyPMg=; b=QDAeOemp3mUrjPiQN5IGwp0iwVSiHI8vtKfRtmmZeDhGl4hHyF0awaqND5YmrOFNzs Hi81ubwtSbY/4qG1N3gGWrFlP5BSvwVGrkzslnVRrkj3HxqmYwrtsf9/Z8DeLurdr1s3 FkutasEZ0LPxFFdYzwZBr3RIr/B2GujwqreWQ+orqwBedMyar82XPqjztAzYA2Gr55Gb 5hRMhuTrvuZPhK/LXHDZWImO6hEaLEaCtxPb/U2MVqLH/faejsoAo3dCr4G4pKUW0brB pg1E5TOYX7FpjC0t70bkmrhXfAHiwKxh+eOe93aMeWep8YTSQ5qU9+q1oscSGCtUSa6R 3SMQ== X-Gm-Message-State: AEkoousgDYNt8OC8wvkmYZoHXXW56VdrcX/a/HFpubjbFlJaRgL8xO8HrwIFXXZkZWbB+Mi5 X-Received: by 10.194.248.33 with SMTP id yj1mr13011507wjc.158.1469541806341; Tue, 26 Jul 2016 07:03:26 -0700 (PDT) Received: from zen.linaro.local ([81.128.185.34]) by smtp.gmail.com with ESMTPSA id x133sm1890410wmf.16.2016.07.26.07.03.24 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 26 Jul 2016 07:03:24 -0700 (PDT) Received: from zen (localhost [127.0.0.1]) by zen.linaro.local (Postfix) with ESMTPS id D21583E03F0; Tue, 26 Jul 2016 15:03:33 +0100 (BST) References: <87wpk8k3dn.fsf@linaro.org> User-agent: mu4e 0.9.17; emacs 25.1.1 From: Alex =?utf-8?Q?Benn=C3=A9e?= To: MTTCG Devel , QEMU Developers , Paolo Bonzini , Richard Henderson , Sergey Fedorov , "Emilio G. Cota" , Frederic Konrad , a.rigo@virtualopensystems.com In-reply-to: <87wpk8k3dn.fsf@linaro.org> Date: Tue, 26 Jul 2016 15:03:33 +0100 Message-ID: <87vazsjy2y.fsf@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:c09::230 Subject: [Qemu-devel] [PATCH] cputlb: make tlb_reset_dirty safe for MTTCG 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: , 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. The helper function is made static as it isn't used outside of cputlb. Signed-off-by: Alex Bennée --- cputlb.c | 57 ++++++++++++++++++++++++++++++++++++--------------- include/exec/cputlb.h | 2 -- 2 files changed, 40 insertions(+), 19 deletions(-) -- 2.7.4 diff --git a/cputlb.c b/cputlb.c index e0d5bdd..e7b6a08 100644 --- a/cputlb.c +++ b/cputlb.c @@ -275,32 +275,52 @@ 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. + */ + +static void tlb_reset_dirty_range(CPUTLBEntry *tlb_entry, uintptr_t start, uintptr_t length) { - uintptr_t addr; + /* 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 (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 += atomic_read(&tlb_entry->addend); if ((addr - start) < length) { - tlb_entry->addr_write |= TLB_NOTDIRTY; + uintptr_t notdirty_addr = orig_addr | TLB_NOTDIRTY; + if (!atomic_bool_cmpxchg(&tlb_entry->addr_write, orig_addr, notdirty_addr)) { + fprintf(stderr,"%s: raced setting the flag\n", __func__); + } } } } +/* 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; @@ -386,7 +406,7 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr, MemoryRegionSection *section; unsigned int index; target_ulong address; - target_ulong code_address; + target_ulong code_address, write_address; uintptr_t addend; CPUTLBEntry *te; hwaddr iotlb, xlat, sz; @@ -443,21 +463,24 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr, } else { te->addr_code = -1; } + + write_address = -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; + write_address = 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; + write_address = address | TLB_NOTDIRTY; } else { - te->addr_write = address; + write_address = address; } - } else { - te->addr_write = -1; } + + /* Pairs with flag setting in tlb_reset_dirty_range */ + atomic_mb_set(&te->addr_write, write_address); } /* Add a new TLB entry, but without specifying the memory diff --git a/include/exec/cputlb.h b/include/exec/cputlb.h index d454c00..3f94178 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