From mboxrd@z Thu Jan  1 00:00:00 1970
From: marc.zyngier@arm.com (Marc Zyngier)
Date: Wed, 3 May 2017 16:58:47 +0100
Subject: [PATCH 07/31] KVM: arm/arm64: vgic-v3: Add accessors for the
 ICH_APxRn_EL2 registers
In-Reply-To: <20170503153201.GC4951@leverpostej>
References: <20170503104606.19342-1-marc.zyngier@arm.com>
 <20170503104606.19342-8-marc.zyngier@arm.com>
 <20170503153201.GC4951@leverpostej>
Message-ID: <3b9ed468-71ac-786c-b587-27968c3f3af9@arm.com>
To: linux-arm-kernel@lists.infradead.org
List-Id: linux-arm-kernel.lists.infradead.org

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@arm.com>
>> ---
>>  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...