On 03/05/17 16:32, Mark Rutland wrote:
> On Wed, May 03, 2017 at 11:45:42AM +0100, Marc Zyngier wrote:
>> As we're about to access the Active Priority registers a lot more,
>> let's define accessors that take the register number as a parameter.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
>> ---
>> virt/kvm/arm/hyp/vgic-v3-sr.c | 116 ++++++++++++++++++++++++++++++++++++------
>> 1 file changed, 100 insertions(+), 16 deletions(-)
>>
>> diff --git a/virt/kvm/arm/hyp/vgic-v3-sr.c b/virt/kvm/arm/hyp/vgic-v3-sr.c
>> index 32c3295929b0..990d9d1e85d0 100644
>> --- a/virt/kvm/arm/hyp/vgic-v3-sr.c
>> +++ b/virt/kvm/arm/hyp/vgic-v3-sr.c
>> @@ -118,6 +118,90 @@ static void __hyp_text __gic_v3_set_lr(u64 val, int lr)
>> }
>> }
>>
>> +static void __hyp_text __vgic_v3_write_ap0rn(u32 val, int n)
>> +{
>> + switch (n) {
>> + case 0:
>> + write_gicreg(val, ICH_AP0R0_EL2);
>> + break;
>> + case 1:
>> + write_gicreg(val, ICH_AP0R1_EL2);
>> + break;
>> + case 2:
>> + write_gicreg(val, ICH_AP0R2_EL2);
>> + break;
>> + case 3:
>> + write_gicreg(val, ICH_AP0R3_EL2);
>> + break;
>> + }
>
> Is there any way we can get a build or runtime failure for an
> out-of-bounds n value?
I'd rather avoid runtime failure on this path, because that's pretty
terminal. Build-time is a possibility, to some extent.
>
>> +}
>
> Given this is used with a constant n, you could make this:
>
> #define __vgic_v3_write_ap0rn(v, n) \
> write_gicreg(v, ICH_AP0R##n##_EL2)
>
> ... which should also give you a warning for an out-of-bounds n.
>
> Similar could apply for the other helpers here.
>
> That would require some function -> macro conversion in later patches
> though, so I can understand if you're not keen on that.
I don't mind reworking this if that makes it safer. But the real problem
is that the register number and the group are not necessarily constants
(see how this is used in __vgic_v3_get_highest_active_priority).
I'll have a look at how I can make that look a bit better.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
