From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751952AbdA3R0g (ORCPT ); Mon, 30 Jan 2017 12:26:36 -0500 Received: from mail-vk0-f50.google.com ([209.85.213.50]:34372 "EHLO mail-vk0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751078AbdA3R0e (ORCPT ); Mon, 30 Jan 2017 12:26:34 -0500 MIME-Version: 1.0 In-Reply-To: References: <1485479100-4966-1-git-send-email-jintack@cs.columbia.edu> <1485479100-4966-11-git-send-email-jintack@cs.columbia.edu> <86lgtt98gs.fsf@arm.com> From: Peter Maydell Date: Mon, 30 Jan 2017 17:26:04 +0000 Message-ID: Subject: Re: [RFC v2 10/10] KVM: arm/arm64: Emulate the EL1 phys timer register access To: Jintack Lim Cc: Marc Zyngier , KVM General , Catalin Marinas , Will Deacon , linux@armlinux.org.uk, lkml - Kernel Mailing List , arm-mail-list , Andre Przywara , Paolo Bonzini , "kvmarm@lists.cs.columbia.edu" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 30 January 2017 at 17:08, Jintack Lim wrote: > On Sun, Jan 29, 2017 at 10:44 AM, Marc Zyngier wrote: >> Shouldn't we take the ENABLE bit into account? The ARMv8 ARM version I >> have at hand (version h) seems to indicate that we should, but we should >> check with the latest and greatest... > > Thanks! I was not clear about this. I have ARM ARM version k, and it > says that 'When the value of the ENABLE bit is 0, the ISTATUS field is > UNKNOWN.' So I thought the istatus value doesn't matter if ENABLE is > 0, and just set istatus bit regardless of ENABLE bit. If this is not > what the manual meant, then I'm happy to fix this. It looks like the spec has been relaxed between the doc version that Marc was looking at and the current one. So it's OK for an implementation to either (a) set ISTATUS to 0 if ENABLE is 0, or (b) do what you've done and set ISTATUS according to the timer comparison whether ENABLE is clear or not (or even (c) set ISTATUS to a random value if ENABLE is clear, and other less likely choices). I think we should add a comment to note that it's architecturally UNKNOWN and we've made a choice for our implementation convenience. thanks -- PMM From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Maydell Subject: Re: [RFC v2 10/10] KVM: arm/arm64: Emulate the EL1 phys timer register access Date: Mon, 30 Jan 2017 17:26:04 +0000 Message-ID: References: <1485479100-4966-1-git-send-email-jintack@cs.columbia.edu> <1485479100-4966-11-git-send-email-jintack@cs.columbia.edu> <86lgtt98gs.fsf@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: KVM General , Marc Zyngier , Catalin Marinas , Will Deacon , linux@armlinux.org.uk, lkml - Kernel Mailing List , Andre Przywara , Paolo Bonzini , "kvmarm@lists.cs.columbia.edu" , arm-mail-list To: Jintack Lim Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu List-Id: kvm.vger.kernel.org On 30 January 2017 at 17:08, Jintack Lim wrote: > On Sun, Jan 29, 2017 at 10:44 AM, Marc Zyngier wrote: >> Shouldn't we take the ENABLE bit into account? The ARMv8 ARM version I >> have at hand (version h) seems to indicate that we should, but we should >> check with the latest and greatest... > > Thanks! I was not clear about this. I have ARM ARM version k, and it > says that 'When the value of the ENABLE bit is 0, the ISTATUS field is > UNKNOWN.' So I thought the istatus value doesn't matter if ENABLE is > 0, and just set istatus bit regardless of ENABLE bit. If this is not > what the manual meant, then I'm happy to fix this. It looks like the spec has been relaxed between the doc version that Marc was looking at and the current one. So it's OK for an implementation to either (a) set ISTATUS to 0 if ENABLE is 0, or (b) do what you've done and set ISTATUS according to the timer comparison whether ENABLE is clear or not (or even (c) set ISTATUS to a random value if ENABLE is clear, and other less likely choices). I think we should add a comment to note that it's architecturally UNKNOWN and we've made a choice for our implementation convenience. thanks -- PMM From mboxrd@z Thu Jan 1 00:00:00 1970 From: peter.maydell@linaro.org (Peter Maydell) Date: Mon, 30 Jan 2017 17:26:04 +0000 Subject: [RFC v2 10/10] KVM: arm/arm64: Emulate the EL1 phys timer register access In-Reply-To: References: <1485479100-4966-1-git-send-email-jintack@cs.columbia.edu> <1485479100-4966-11-git-send-email-jintack@cs.columbia.edu> <86lgtt98gs.fsf@arm.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 30 January 2017 at 17:08, Jintack Lim wrote: > On Sun, Jan 29, 2017 at 10:44 AM, Marc Zyngier wrote: >> Shouldn't we take the ENABLE bit into account? The ARMv8 ARM version I >> have at hand (version h) seems to indicate that we should, but we should >> check with the latest and greatest... > > Thanks! I was not clear about this. I have ARM ARM version k, and it > says that 'When the value of the ENABLE bit is 0, the ISTATUS field is > UNKNOWN.' So I thought the istatus value doesn't matter if ENABLE is > 0, and just set istatus bit regardless of ENABLE bit. If this is not > what the manual meant, then I'm happy to fix this. It looks like the spec has been relaxed between the doc version that Marc was looking at and the current one. So it's OK for an implementation to either (a) set ISTATUS to 0 if ENABLE is 0, or (b) do what you've done and set ISTATUS according to the timer comparison whether ENABLE is clear or not (or even (c) set ISTATUS to a random value if ENABLE is clear, and other less likely choices). I think we should add a comment to note that it's architecturally UNKNOWN and we've made a choice for our implementation convenience. thanks -- PMM