From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marc Zyngier Subject: Re: [PATCH v2 10/25] KVM: arm64: vgic-v3: Add ICV_HPPIR1_EL1 handler Date: Tue, 6 Jun 2017 14:57:42 +0100 Message-ID: <88dd9993-79ec-31bd-e49d-4049656bc525@arm.com> References: <20170601102117.17750-1-marc.zyngier@arm.com> <20170601102117.17750-11-marc.zyngier@arm.com> <20170606115130.GM9464@cbox> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: Christoffer Dall , David Daney , Catalin Marinas , Mark Rutland , Robert Richter , Eric Auger , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org To: Christoffer Dall Return-path: Received: from foss.arm.com ([217.140.101.70]:46720 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751404AbdFFN5q (ORCPT ); Tue, 6 Jun 2017 09:57:46 -0400 In-Reply-To: <20170606115130.GM9464@cbox> Sender: kvm-owner@vger.kernel.org List-ID: On 06/06/17 12:51, Christoffer Dall wrote: > On Thu, Jun 01, 2017 at 11:21:02AM +0100, Marc Zyngier wrote: >> Add a handler for reading the guest's view of the ICV_HPPIR1_EL1 >> register. This is a simple parsing of the available LRs, extracting the >> highest available interrupt. >> >> Reviewed-by: Eric Auger >> Signed-off-by: Marc Zyngier >> --- >> arch/arm64/include/asm/sysreg.h | 1 + >> virt/kvm/arm/hyp/vgic-v3-sr.c | 23 +++++++++++++++++++++++ >> 2 files changed, 24 insertions(+) >> >> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h >> index aad46b8eea5e..bd000686194a 100644 >> --- a/arch/arm64/include/asm/sysreg.h >> +++ b/arch/arm64/include/asm/sysreg.h >> @@ -185,6 +185,7 @@ >> #define SYS_ICC_SGI1R_EL1 sys_reg(3, 0, 12, 11, 5) >> #define SYS_ICC_IAR1_EL1 sys_reg(3, 0, 12, 12, 0) >> #define SYS_ICC_EOIR1_EL1 sys_reg(3, 0, 12, 12, 1) >> +#define SYS_ICC_HPPIR1_EL1 sys_reg(3, 0, 12, 12, 2) >> #define SYS_ICC_BPR1_EL1 sys_reg(3, 0, 12, 12, 3) >> #define SYS_ICC_CTLR_EL1 sys_reg(3, 0, 12, 12, 4) >> #define SYS_ICC_SRE_EL1 sys_reg(3, 0, 12, 12, 5) >> diff --git a/virt/kvm/arm/hyp/vgic-v3-sr.c b/virt/kvm/arm/hyp/vgic-v3-sr.c >> index d4f07f84602d..f0bc711db258 100644 >> --- a/virt/kvm/arm/hyp/vgic-v3-sr.c >> +++ b/virt/kvm/arm/hyp/vgic-v3-sr.c >> @@ -772,6 +772,26 @@ static void __hyp_text __vgic_v3_write_apxr3(struct kvm_vcpu *vcpu, >> __vgic_v3_write_apxrn(vcpu, rt, 3); >> } >> >> +static void __hyp_text __vgic_v3_read_hppir(struct kvm_vcpu *vcpu, >> + u32 vmcr, int rt) >> +{ >> + u64 lr_val; >> + int lr, lr_grp, grp; >> + >> + grp = __vgic_v3_get_group(vcpu); >> + >> + lr = __vgic_v3_highest_priority_lr(vcpu, vmcr, &lr_val); >> + if (lr == -1) >> + goto spurious; >> + >> + lr_grp = !!(lr_val & ICH_LR_GROUP); >> + if (lr_grp != grp) >> + lr_val = ICC_IAR1_EL1_SPURIOUS; > > I don't get this. The spec says that the special INTID 1023 > "...is returned in response to an interrupt acknowledge, if there is > no pending interrupt with sufficient priority for it to be signaled to > the PE, or if the highest priority pending interrupt is not > appropriate for the: > * Current Security state. Interrupt group that is associated with > * the System register." > > So do we just take this to imply that it also covers the HPPIRx_EL1 > registers (despite them being mentioned explicitly for the other special > INTIDs) or is there some other piece of spec I'm missing here? Here's what the ICV_HPPIRx_EL1 INTID description says: "If the highest priority pending interrupt is not observable, this field contains a special INTID to indicate the reason. This special INTID can take the value 1023 only." Yes, the disparity between ICC_HPPIRx_EL1 and ICV_HPPIRx_EL1 is very confusing and makes me want to shout at people. Thanks, M. -- Jazz is not dead. It just smells funny...