From mboxrd@z Thu Jan  1 00:00:00 1970
From: Julien Grall <julien.grall@linaro.org>
Subject: Re: [PATCH for-4.5 v11 4/9] xen/arm: Data abort
 exception (R/W) mem_events.
Date: Mon, 29 Sep 2014 13:53:27 +0100
Message-ID: <54295647.5020207@linaro.org>
References: <1411990609-22374-1-git-send-email-tklengyel@sec.in.tum.de>	<1411990609-22374-5-git-send-email-tklengyel@sec.in.tum.de>	<54295214.6030902@linaro.org>
	<CAErYnsg-z8=tw9BTb3Mrfhv8AjA_MJyQc5M-73ZMW+zi8zyZhw@mail.gmail.com>
	<5429561B.3000803@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: <5429561B.3000803@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: Tamas K Lengyel <tamas.lengyel@zentific.com>
Cc: wei.liu2@citrix.com, Ian Campbell <ian.campbell@citrix.com>, Tim Deegan <tim@xen.org>, "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>, Stefano Stabellini <stefano.stabellini@citrix.com>, Daniel De Graaf <dgdegra@tycho.nsa.gov>, Tamas K Lengyel <tklengyel@sec.in.tum.de>
List-Id: xen-devel@lists.xenproject.org

On 09/29/2014 01:52 PM, Julien Grall wrote:
> On 09/29/2014 01:47 PM, Tamas K Lengyel wrote:
>>     > +/* Set access type for a region of pfns.
>>     > + * If start_pfn == -1ul, sets the default access type */
>>     > +long p2m_set_mem_access(struct domain *d, unsigned long pfn, uint32_t nr,
>>     > +                        uint32_t start, uint32_t mask, xenmem_access_t access)
>>     > +{
>>
>>
>>     [..]
>>
>>     > +
>>     > +    rc = apply_p2m_changes(d, MEMACCESS,
>>     > +                           pfn_to_paddr(pfn+start), pfn_to_paddr(pfn+nr),
>>     > +                           0, MATTR_MEM, mask, 0, a);
>>     > +
>>     > +    flush_tlb_domain(d);
>>     > +    iommu_iotlb_flush(d, pfn+start, nr-start);
>>
>>     With your solution, when rc == 0 (i.e the call memaccess has been fully
>>     applied), you will have one more TLB flush: one in apply_p2m_changes,
>>     the other one here...
>>
>>
>> No, I don't flush it in apply_p2m_changes, *flush is not set to true in
>> MEMACCESS in this version.
> 
> Oh right, sorry I haven't noticed it.
> 
>>     As this code already exists in apply_p2m_changes but in the wrong place,
>>     why didn't you move it later?
>>
>>
>> The problem is with the preemption case that just goes to out. I found
>> it cleaner to just flush the tlb here for both cases instead of having
>> the preemption case going to a flush: label then to out. If that's
>> preferred, I'm OK with that approach too.
> 
> What is the issue to do?
> 
> out:
>    if ( flush )
>    {
>       TLB flush
>    }
> 
>    if ( rc < 0 && ( op == INSERT ....
> 
>    ...
> 
>    return rc;
> 
> I would prefer if you have to duplicate the flush here and at the same

I meant avoid of course.

> time you will fix other flush issue in the different path in case of error.
> 
> Regards,
> 


-- 
Julien Grall