From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v2] xen/arm: p2m: Correctly flush TLB in create_p2m_entries Date: Tue, 14 Jan 2014 12:41:47 +0000 Message-ID: <52D5308B.6020000@linaro.org> References: <1389285240-7116-1-git-send-email-julien.grall@linaro.org> <1389633015.13654.109.camel@kazak.uk.xensource.com> <52D4246D.1050400@linaro.org> <1389635845.13654.117.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1W33JY-0004C0-LH for xen-devel@lists.xenproject.org; Tue, 14 Jan 2014 12:41:52 +0000 Received: by mail-wg0-f47.google.com with SMTP id m15so315372wgh.26 for ; Tue, 14 Jan 2014 04:41:49 -0800 (PST) In-Reply-To: <1389635845.13654.117.camel@kazak.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: xen-devel@lists.xenproject.org, patches@linaro.org, stefano.stabellini@citrix.com, tim@xen.org List-Id: xen-devel@lists.xenproject.org On 01/13/2014 05:57 PM, Ian Campbell wrote: > On Mon, 2014-01-13 at 17:37 +0000, Julien Grall wrote: >> On 01/13/2014 05:10 PM, Ian Campbell wrote: >>> Hrm, our TLB flush discipline is horribly confused isn't it... >>> >>> On Thu, 2014-01-09 at 16:34 +0000, Julien Grall wrote: >>>> 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 >>> >>> OK. >>> >>> An ASSERT(!third[third_table_offset(addr)].p2m.valid) might be >>> worthwhile if that is the case. >> >> Will add it. > > Thanks. > >>> (I'm not sure why ALLOCATE can't be replaced by allocation followed by >>> an INSERT, it's seems very special case) >>> >>>> - 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. >>> >>> "append"? Do you mean "happen"? >> >> I meant "happen". >> >>> >>> In the non-dom0 1:1 case eventually the page will be freed, I guess by a >>> subsequent put_page elsewhere -- do they all contain the correct >>> flushing? Or do we just leak? >> >> As for foreign mapping the INSERT function should be hardened. We don't > > Did you mean "handled"? I meant both :). Actually we don't have any check in this function as for REMOVE case. I don't think it's possible to do it for 4.4, we take a reference on the mapping every time a new entrie is added in the p2m. -- Julien Grall