Hi Eric,
On 30/05/17 09:02, Auger Eric wrote:
> Hi Marc,
>
> On 30/05/2017 09:48, Auger Eric wrote:
>> H Marc,
>>
>> On 03/05/2017 12:45, Marc Zyngier wrote:
>>> Add a handler for reading/writing the guest's view of the ICV_AP1Rn_EL1
>>> registers. We just map them to the corresponding ICH_AP1Rn_EL2 registers.
>>>
>>> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
>>> ---
>>> arch/arm64/include/asm/sysreg.h | 1 +
>>> virt/kvm/arm/hyp/vgic-v3-sr.c | 94 +++++++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 95 insertions(+)
>>>
>>> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
>>> index 15c142ce991c..aad46b8eea5e 100644
>>> --- a/arch/arm64/include/asm/sysreg.h
>>> +++ b/arch/arm64/include/asm/sysreg.h
>>> @@ -180,6 +180,7 @@
>>>
>>> #define SYS_VBAR_EL1 sys_reg(3, 0, 12, 0, 0)
>>>
>>> +#define SYS_ICC_AP1Rn_EL1(n) sys_reg(3, 0, 12, 9, n)
>>> #define SYS_ICC_DIR_EL1 sys_reg(3, 0, 12, 11, 1)
>>> #define SYS_ICC_SGI1R_EL1 sys_reg(3, 0, 12, 11, 5)
>>> #define SYS_ICC_IAR1_EL1 sys_reg(3, 0, 12, 12, 0)
>>> diff --git a/virt/kvm/arm/hyp/vgic-v3-sr.c b/virt/kvm/arm/hyp/vgic-v3-sr.c
>>> index a76351b3ad66..b6803989da1f 100644
>>> --- a/virt/kvm/arm/hyp/vgic-v3-sr.c
>>> +++ b/virt/kvm/arm/hyp/vgic-v3-sr.c
>>> @@ -684,6 +684,76 @@ static void __hyp_text __vgic_v3_write_bpr1(struct kvm_vcpu *vcpu, u32 vmcr, int
>>> __vgic_v3_write_vmcr(vmcr);
>>> }
>>>
>>> +static void __hyp_text __vgic_v3_read_apxrn(struct kvm_vcpu *vcpu, int rt, int n)
>> Shouldn't you test somewhere that n is correct given the number of
>> implemented priority bits
If n is invalid, then the corresponding system register access will have
UNDEFed at EL1, irrespective of the trapping (UNDEF has a higher
priority than trapping, unfortunately).
>>> +{
>>> + u32 val;
>>> +
>>> + if (!__vgic_v3_get_group(vcpu))
>> I don't really get how an access to AP1Rn can end up in AP0Rn. I am not
>> able to find any related description in the spec?
> Forget that one. This is simply a generic helper and an access from NS
> world will access the AP1Rn reg.
It is not really S vs NS here, since the guest is probably aware it is
running non-secure. The GICv3 architecture allows the guest to have its
own Group0 interrupts (reported as FIQ).
> so besides the comment on "n" check,
>
> Reviewed-by: Eric Auger <eric.auger@xxxxxxxxxx>
Thanks!
M.
--
Jazz is not dead. It just smells funny...
