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.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
