From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH for-4.5 v8 15/19] xen/arm: Data abort exception (R/W) mem_events. Date: Wed, 24 Sep 2014 21:52:51 +0100 Message-ID: <54232F23.50205@linaro.org> References: <1411478070-13836-1-git-send-email-tklengyel@sec.in.tum.de> <1411478070-13836-16-git-send-email-tklengyel@sec.in.tum.de> <5422E4AA.4060600@linaro.org> <5422F695.3000809@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Tamas K Lengyel Cc: Ian Campbell , Tim Deegan , Ian Jackson , "xen-devel@lists.xen.org" , Stefano Stabellini , Andres Lagar-Cavilla , Jan Beulich , Daniel De Graaf , Tamas K Lengyel List-Id: xen-devel@lists.xenproject.org Hello Tamas, On 24/09/2014 18:13, Tamas K Lengyel wrote: > > > On Wed, Sep 24, 2014 at 6:51 PM, Julien Grall > wrote: > > > > > + && hypercall_preempt_check() ) > > > + { > > > + rc = progress; > > > + goto out; > > > > Jumping directly to the label "out" will skip flushing the TLB for the > > domain. While it wasn't critical until now, partial redo during > > insertion/allocation or hypercall preemption only for relinquish, the > > guest may use the wrong permission because the TLB hasn't been flushed. > > > > At the same time, it looks like you never request to flush for the > > MEMACCESS operation (see *flush = true). Does memaccess does a TLB flush > > somewhere else? > > > > > > Yes, at the end of p2m_set_mem_access once all PTEs are updated > > successfully. I guess we could flush the TLB as we are progressing as > > well, it wouldn't hurt. > > We should flush the TLB as we are progressing because the guest may > technically continue to run... > > > Hm, I think the guest is always paused while mem_access is being set via > memop but sure, it can't hurt. I didn't find any domain pause call neither in the hypervisor nor in xen-access. Unless the pause is done by the hypervisor via the same hypercall, it's safer to flush the TLB if it's necessary. > This case made me also think about another possible issue. Permission > are checked in raw_copy_{from,to}_guest_helper during virtual address > translation to a physical address. > > As you modified the attribute in the P2M, the copy may failed because of > the lake of permission. > > > I'm not entire sure what you mean. Can you elaborate? Xen has a bunch of functions raw_copy_{from,to}_guest helpers which copy data from/to the guest. Since XSA-98, Xen checks that the guest has effectively the right to read/write (depending of the helpers) a specific mapping before copying the data. If the guest page doesn't have the good right, the helper will fail and therefore so do the hypercall. When memaccess is used, a RAM page may have its permission lower down. When the helpers are called, the code to check memory access during permission violation will never be called... and the guest will receive an hypercall failure. This is not the right behavior, the hypercall should only fail if the permissions are effectively wrong after check mem access has been called. Regards, -- Julien Grall