From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH 4/9] xen: arm: turn vtimer traps for cp32/64 and sysreg into #undef Date: Thu, 11 Sep 2014 09:43:18 +0100 Message-ID: <1410424998.6166.25.camel@kazak.uk.xensource.com> References: <1410279730.8217.238.camel@kazak.uk.xensource.com> <1410279788-27167-4-git-send-email-ian.campbell@citrix.com> <540F8DC3.3050305@linaro.org> <1410342387.8217.272.camel@kazak.uk.xensource.com> <54109E68.7020102@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <54109E68.7020102@linaro.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Julien Grall Cc: stefano.stabellini@eu.citrix.com, tim@xen.org, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On Wed, 2014-09-10 at 11:54 -0700, Julien Grall wrote: > Hi Ian, > > On 10/09/14 02:46, Ian Campbell wrote: > > On Tue, 2014-09-09 at 16:31 -0700, Julien Grall wrote: > >> Hi Ian, > >> > >> On 09/09/14 09:23, Ian Campbell wrote: > >>> We have allowed EL1 to access these registers directly for some time > >>> (at least since 4.3.0). They were only ever trapped to support very > >>> early models which had a buggy hypervisor timer, requiring us to use > >>> the phys timer for Xen itself. > >>> In the interests of minimising the patch for the security update just > >>> remove the call to vtimer_emulate and inject an #undef exception. In > >>> practice we will never see any of these traps. > >> > >> I disagree with the commit message, a guest may use the physical timer > >> rather than the virtual timer. It's the case when a guest doesn't have > >> the necessary code to use the virtual timer. > > > > I think you've misunderstood. The guest is allowed direct access to the > > physical timer ever since we removed the workaround for the buggy > > hypervisor timer on the models. Hence we are never trapping these > > registers anyway. Probably I should go further here and actually remove > > all the phys timer emulation support from vtimer.c. > > Are you sure? In init_interrupt_timer (xen/arch/arm/timer.c) we disable > the access to the physical timer to the guest. See > WRITE_SYSREG32(CNTHCTL_PA, CNTHCTL_EL2). Hrm, I mistakenly thought that was enabling them, but we do indeed need to set a second bit there, this only allows access to the counter, not the control registers. I'll take another look. > Hence, I don't see any save/restore for the physical timer in > xen/arch/arm/vtimer.c. I only see them for the virtual timer. > > Regards, >