On Tue, Jun 06, 2017 at 02:57:42PM +0100, Marc Zyngier wrote:
> 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 <eric.auger@xxxxxxxxxx>
> >> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
> >> ---
> >> 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.
>
ok, shitty spec.
Thanks,
-Christoffer
