From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ian Campbell <ian.campbell@citrix.com>
Subject: Re: [PATCH V13 4/7] xen/arm: Data abort exception (R/W)
 mem_events.
Date: Thu, 12 Mar 2015 15:46:43 +0000
Message-ID: <1426175203.32572.21.camel@citrix.com>
References: <1425677073-13729-1-git-send-email-tklengyel@sec.in.tum.de>
	<1425677073-13729-5-git-send-email-tklengyel@sec.in.tum.de>
	<5501AD07.6050609@linaro.org>
	<CABfawhm5eMXOpx-G=oPHznOB7heyROw3-D0SrNe_O1NkwfaFdQ@mail.gmail.com>
	<5501B2C6.2040301@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: <5501B2C6.2040301@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: wei.liu2@citrix.com, Stefano Stabellini <stefano.stabellini@eu.citrix.com>, Tim Deegan <tim@xen.org>, Ian Jackson <ian.jackson@eu.citrix.com>, xen-devel@lists.xen.org, stefano.stabellini@citrix.com, Jan Beulich <jbeulich@suse.com>, Keir Fraser <keir@xen.org>, Tamas K Lengyel <tklengyel@sec.in.tum.de>
List-Id: xen-devel@lists.xenproject.org

On Thu, 2015-03-12 at 15:37 +0000, Julien Grall wrote:
> On 12/03/15 15:26, Tamas K Lengyel wrote:
> > 
> > 
> > On Thu, Mar 12, 2015 at 4:13 PM, Julien Grall <julien.grall@linaro.org
> > <mailto:julien.grall@linaro.org>> wrote:
> > 
> >     Hi Tamas,
> > 
> >     On 06/03/15 21:24, Tamas K Lengyel wrote:
> >     > +        /*
> >     > +         * Preempt setting mem_access permissions as required by XSA-89,
> >     > +         * if it's not the last iteration.
> >     > +         */
> >     > +        if ( op == MEMACCESS && count )
> >     > +        {
> >     > +            uint32_t progress = paddr_to_pfn(addr) - sgfn + 1;
> >     > +
> >     > +            if ( (egfn - sgfn) > progress && !(progress & mask)
> >     > +                 && hypercall_preempt_check() )
> >     > +            {
> >     > +                rc = progress;
> >     > +                goto out;
> >     > +            }
> >     > +        }
> >     > +
> > 
> >     Would it be possible to merge the memaccess prempt check with the
> >     relinquish one?
> > 
> >     That may require some change in the relinquish version but I think it
> >     would be cleaner.
> > 
> > 
> > Well, I don't really see an obvious way how the two could be combined.
> 
> The preemption for relinquish has been chosen arbitrarily so it would be
> possible to change it.
> 
> > 
> >     [..]
> > 
> >     > +bool_t p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const struct npfec npfec)
> >     > +{
> >     > +    int rc;
> >     > +    bool_t violation;
> >     > +    xenmem_access_t xma;
> >     > +    mem_event_request_t *req;
> >     > +    struct vcpu *v = current;
> >     > +    struct p2m_domain *p2m = p2m_get_hostp2m(v->domain);
> >     > +
> >     > +    /* Mem_access is not in use. */
> >     > +    if ( !p2m->mem_access_enabled )
> >     > +        return true;
> > 
> >     true should be used with bool not boot_t.
> > 
> >     > +
> >     > +    rc = p2m_get_mem_access(v->domain, paddr_to_pfn(gpa), &xma);
> >     > +    if ( rc )
> >     > +        return true;
> > 
> >     Ditto (I won't repeat for the few other place below)
> > 
> > 
> > There was just a discussion how there is no difference between 0/1 and
> > false/true other than maybe style. If one is preferred over the other,
> > I'm fine with either. Is this really an issue?
> 
> You are mixing 2 different types. bool/false/true are part of stdbool.h
> rather than bool_t is part of asm/types.h

I don't think this matters one jot.

        bool foo = true;
        bool_t foo = true;

are both perfectly fine, work as expected (since both are equivalent to
= !0), and are easier to read.

Why do you think this is so important?

Ian.