From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ian Campbell <ian.campbell@citrix.com>
Subject: Re: [PATCH v8 02/14] arch/arm: add consistency check to
 REMOVE p2m changes
Date: Thu, 5 Jun 2014 14:45:32 +0100
Message-ID: <1401975932.15729.85.camel@hastur.hellion.org.uk>
References: <1401015115-7610-1-git-send-email-avanzini.arianna@gmail.com>
	<1401015115-7610-3-git-send-email-avanzini.arianna@gmail.com>
	<5382112B.9030406@linaro.org>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <5382112B.9030406@linaro.org>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Julien Grall <julien.grall@linaro.org>
Cc: julien.grall@citrix.com, paolo.valente@unimore.it, keir@xen.org, stefano.stabellini@eu.citrix.com, tim@xen.org, dario.faggioli@citrix.com, Ian.Jackson@eu.citrix.com, xen-devel@lists.xen.org, Ian.Campbell@eu.citrix.com, etrudeau@broadcom.com, andrew.cooper3@citrix.com, JBeulich@suse.com, Arianna Avanzini <avanzini.arianna@gmail.com>, viktor.kleinik@globallogic.com
List-Id: xen-devel@lists.xenproject.org

On Sun, 2014-05-25 at 16:50 +0100, Julien Grall wrote:
> Hi Arianna,
> 
> On 25/05/14 11:51, Arianna Avanzini wrote:
> > +                    unsigned long mfn = pte.p2m.base;
> > +
> > +                    /*
> > +                     * Ensure that the guest address given as argument to
> > +                     * this function is actually mapped to the specified
> > +                     * machine address. maddr here is the machine address
> > +                     * given to the function, while mfn is the machine
> > +                     * frame number actually mapped to the guest address:
> > +                     * check if the two correspond.
> > +                     */
> > +                    if ( !pte.p2m.valid || maddr != pfn_to_paddr(mfn) )
> > +                    {
> > +                        printk("p2m_remove: nonexistent mapping: "
> > +                               "%"PRIx64" and %"PRIx64"\n",
> > +                               pfn_to_paddr(mfn), maddr);
> > +                        /*
> > +                         * If we have successfully removed other mappings,
> > +                         * overload flush local variable to store if we need
> > +                         * to flush TLBs.
> > +                         */
> > +                        if (count) flush = 1; else flush = 0;
> 
> Hrrm, why do you need this line? Flush is already correctly set 
> previously (see flush |= ... few lines above).

That is:
         flush |= pte.p2m.valid;

But this is within a !pte.p2m.valid if.

Now I'm not quite sure why we need to flush if this mapping is not
present, and I'm pretty certain that setting flush=0 isn't correct,
since if it was previously true nothing we do should cause us to decide
we don't need to flush any more (or if there is it needs describing very
clearly why this is the case).

Ian.