From patchwork Thu Jan 9 16:34:00 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Julien Grall X-Patchwork-Id: 23062 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-ie0-f198.google.com (mail-ie0-f198.google.com [209.85.223.198]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id E8EAE202E2 for ; Thu, 9 Jan 2014 16:34:08 +0000 (UTC) Received: by mail-ie0-f198.google.com with SMTP id tp5sf13050546ieb.9 for ; Thu, 09 Jan 2014 08:34:08 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:delivered-to:from:to:cc:subject :date:message-id:x-original-sender:x-original-authentication-results :precedence:mailing-list:list-id:list-post:list-help:list-archive :list-unsubscribe; bh=F4U21s+i4XZQQyMyGi3WdHDh9HA1isj12LVdk5m9jdU=; b=bwuwNHu3kNY/ClfXu0t2sMREcNCRWE2lSvvyRd9pVdhGo/xtu6U/Feb0HJzcA4tap2 QL3a/yTFVyf7XImvM3rgpUXQ1HvBozlzOWOGVMgNeUnLtTpWJOJSzQkZPyrAvV5IRaR5 3lP20vtVr3tzmU1O6LsX8SmsDlLLQCcuHAG2MviiXKTWVshHtVsU8Nk4y9z9WMhaRgWa qT7FhKMjN5xjJMyJEbMwhQCWzRnE7Z9VSX78DUyEGH6U6alvyHJsLZhmnJ/OHY3r0x4k XCNh507l+Mj74ZvSiFoKKWSBABt3Psl/g+NN77adL43s+192ACk1wVrNeeaJ2gng7vbJ jM7A== X-Gm-Message-State: ALoCoQkQT0UqNJu4wSiTH2e9MvyVLqQTmb0BOJC/3OnuXB+vHhkBP4oxmh2uRCtsvLw20hGd9buU X-Received: by 10.182.112.231 with SMTP id it7mr1537760obb.22.1389285248205; Thu, 09 Jan 2014 08:34:08 -0800 (PST) MIME-Version: 1.0 X-BeenThere: patchwork-forward@linaro.org Received: by 10.49.121.98 with SMTP id lj2ls1199557qeb.45.gmail; Thu, 09 Jan 2014 08:34:08 -0800 (PST) X-Received: by 10.58.132.203 with SMTP id ow11mr3660769veb.1.1389285248039; Thu, 09 Jan 2014 08:34:08 -0800 (PST) Received: from mail-ve0-f173.google.com (mail-ve0-f173.google.com [209.85.128.173]) by mx.google.com with ESMTPS id gx6si3064607vdc.24.2014.01.09.08.34.08 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 09 Jan 2014 08:34:08 -0800 (PST) Received-SPF: neutral (google.com: 209.85.128.173 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=209.85.128.173; Received: by mail-ve0-f173.google.com with SMTP id oz11so2543699veb.4 for ; Thu, 09 Jan 2014 08:34:07 -0800 (PST) X-Received: by 10.58.243.37 with SMTP id wv5mr2394235vec.41.1389285247901; Thu, 09 Jan 2014 08:34:07 -0800 (PST) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patches@linaro.org Received: by 10.59.13.131 with SMTP id ey3csp24197ved; Thu, 9 Jan 2014 08:34:07 -0800 (PST) X-Received: by 10.14.109.5 with SMTP id r5mr4310703eeg.110.1389285246733; Thu, 09 Jan 2014 08:34:06 -0800 (PST) Received: from mail-ee0-f42.google.com (mail-ee0-f42.google.com [74.125.83.42]) by mx.google.com with ESMTPS id 5si4566900eei.228.2014.01.09.08.34.06 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 09 Jan 2014 08:34:06 -0800 (PST) Received-SPF: neutral (google.com: 74.125.83.42 is neither permitted nor denied by best guess record for domain of julien.grall@linaro.org) client-ip=74.125.83.42; Received: by mail-ee0-f42.google.com with SMTP id e53so1441811eek.1 for ; Thu, 09 Jan 2014 08:34:06 -0800 (PST) X-Received: by 10.14.88.134 with SMTP id a6mr4252672eef.5.1389285246091; Thu, 09 Jan 2014 08:34:06 -0800 (PST) Received: from belegaer.uk.xensource.com. ([185.25.64.249]) by mx.google.com with ESMTPSA id 1sm6940951eeg.4.2014.01.09.08.34.04 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 09 Jan 2014 08:34:04 -0800 (PST) From: Julien Grall To: xen-devel@lists.xenproject.org Cc: patches@linaro.org, ian.campbell@citrix.com, tim@xen.org, stefano.stabellini@citrix.com, Julien Grall Subject: [PATCH v2] xen/arm: p2m: Correctly flush TLB in create_p2m_entries Date: Thu, 9 Jan 2014 16:34:00 +0000 Message-Id: <1389285240-7116-1-git-send-email-julien.grall@linaro.org> X-Mailer: git-send-email 1.7.10.4 X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: julien.grall@linaro.org X-Original-Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.128.173 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Precedence: list Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org List-ID: X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , The p2m is shared between VCPUs for each domain. Currently Xen only flush TLB on the local PCPU. This could result to mismatch between the mapping in the p2m and TLBs. Flush TLB entries used by this domain on every PCPU. The flush can also be moved out of the loop because: - ALLOCATE: only called for dom0 RAM allocation, so the flush is never called - INSERT: if valid = 1 that would means with have replaced a page that already belongs to the domain. A VCPU can write on the wrong page. This can append for dom0 with the 1:1 mapping because the mapping is not removed from the p2m. - REMOVE: except for grant-table (replace_grant_host_mapping), each call to guest_physmap_remove_page are protected by the callers via a get_page -> .... -> guest_physmap_remove_page -> ... -> put_page. So the page can't be allocated for another domain until the last put_page. - RELINQUISH : the domain is not running anymore so we don't care... Signed-off-by: Julien Grall --- Changes in v2: - Switch to the domain for only flush its TLBs entries - Move the flush out of the loop This is a possible bug fix (found by reading the code) for Xen 4.4, I moved the flush out of the loop which should be safe (see why in the commit message). Without this patch, the guest can have stale TLB entries when the VCPU is moved to another PCPU. Except grant-table (I can't find {get,put}_page for grant-table code???), all the callers are protected by a get_page before removing the page. So if the another VCPU is trying to access to this page before the flush, it will just read/write the wrong page. The downside of this patch is Xen flushes less TLBs. Instead of flushing all TLBs on the current PCPU, Xen flushes TLBs for a specific VMID on every CPUs. This should be safe because create_p2m_entries only deal with a specific domain. I don't think I forget case in this function. Let me know if it's the case. --- xen/arch/arm/p2m.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index 11f4714..ad6f76e 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -238,7 +238,7 @@ static int create_p2m_entries(struct domain *d, int mattr, p2m_type_t t) { - int rc, flush; + int rc; struct p2m_domain *p2m = &d->arch.p2m; lpae_t *first = NULL, *second = NULL, *third = NULL; paddr_t addr; @@ -246,10 +246,14 @@ static int create_p2m_entries(struct domain *d, cur_first_offset = ~0, cur_second_offset = ~0; unsigned long count = 0; + unsigned int flush = 0; bool_t populate = (op == INSERT || op == ALLOCATE); spin_lock(&p2m->lock); + if ( d != current->domain ) + p2m_load_VTTBR(d); + addr = start_gpaddr; while ( addr < end_gpaddr ) { @@ -316,7 +320,7 @@ static int create_p2m_entries(struct domain *d, cur_second_offset = second_table_offset(addr); } - flush = third[third_table_offset(addr)].p2m.valid; + flush |= third[third_table_offset(addr)].p2m.valid; /* Allocate a new RAM page and attach */ switch (op) { @@ -373,9 +377,6 @@ static int create_p2m_entries(struct domain *d, break; } - if ( flush ) - flush_tlb_all_local(); - /* Preempt every 2MiB (mapped) or 32 MiB (unmapped) - arbitrary */ if ( op == RELINQUISH && count >= 0x2000 ) { @@ -392,6 +393,16 @@ static int create_p2m_entries(struct domain *d, addr += PAGE_SIZE; } + if ( flush ) + { + /* At the beginning of the function, Xen is updating VTTBR + * with the domain where the mappings are created. In this + * case it's only necessary to flush TLBs on every CPUs with + * the current VMID (our domain). + */ + flush_tlb(); + } + if ( op == ALLOCATE || op == INSERT ) { unsigned long sgfn = paddr_to_pfn(start_gpaddr); @@ -409,6 +420,9 @@ out: if (second) unmap_domain_page(second); if (first) unmap_domain_page(first); + if ( d != current->domain ) + p2m_load_VTTBR(current->domain); + spin_unlock(&p2m->lock); return rc;