On Tuesday 05 September 2017 09:02 PM, Jan Kiszka wrote:
> On 2017-09-05 16:32, Lokesh Vutla wrote:
>> For simplicity pass cluster id derived from mpidr instead of
>> passing affinity levels separately.
>>
>> Signed-off-by: Lokesh Vutla <[email protected]>
>> ---
>> hypervisor/arch/arm-common/include/asm/irqchip.h | 7 ++-----
>> hypervisor/arch/arm-common/irqchip.c | 4 +---
>> hypervisor/arch/arm/gic-v3.c | 24
>> +++++++++++++++++-------
>> 3 files changed, 20 insertions(+), 15 deletions(-)
>>
>> diff --git a/hypervisor/arch/arm-common/include/asm/irqchip.h
>> b/hypervisor/arch/arm-common/include/asm/irqchip.h
>> index 8058733..f140521 100644
>> --- a/hypervisor/arch/arm-common/include/asm/irqchip.h
>> +++ b/hypervisor/arch/arm-common/include/asm/irqchip.h
>> @@ -30,11 +30,8 @@ struct sgi {
>> * 2: only this CPU
>> */
>> u8 routing_mode;
>> - /* GICv2 only uses 8bit in targets, and no affinity routing */
>> - u8 aff1;
>> - u8 aff2;
>> - /* Only available on 64-bit, when CTLR.A3V is 1 */
>> - u8 aff3;
>> + /* cluster_id: mpidr & MPIDR_CLUSTERID_MASK */
>> + u64 cluster_id;
>> u16 targets;
>> u16 id;
>> };
>> diff --git a/hypervisor/arch/arm-common/irqchip.c
>> b/hypervisor/arch/arm-common/irqchip.c
>> index 12bf1e0..933df0d 100644
>> --- a/hypervisor/arch/arm-common/irqchip.c
>> +++ b/hypervisor/arch/arm-common/irqchip.c
>> @@ -118,9 +118,7 @@ static enum mmio_result handle_sgir_access(struct
>> mmio_access *mmio)
>>
>> sgi.targets = (val >> 16) & 0xff;
>> sgi.routing_mode = (val >> 24) & 0x3;
>> - sgi.aff1 = 0;
>> - sgi.aff2 = 0;
>> - sgi.aff3 = 0;
>> + sgi.cluster_id = 0;
>> sgi.id = val & 0xf;
>>
>> gic_handle_sgir_write(&sgi, false);
>> diff --git a/hypervisor/arch/arm/gic-v3.c b/hypervisor/arch/arm/gic-v3.c
>> index 75e337e..50a7c00 100644
>> --- a/hypervisor/arch/arm/gic-v3.c
>> +++ b/hypervisor/arch/arm/gic-v3.c
>> @@ -250,6 +250,10 @@ static int gic_cell_init(struct cell *cell)
>> return 0;
>> }
>>
>> +#define MPIDR_TO_SGI_AFFINITY(cluster_id, level) \
>> + (MPIDR_AFFINITY_LEVEL((cluster_id), (level)) \
>> + << ICC_SGIR_AFF## level ##_SHIFT)
>
> Shouldn't we call this "MPIDR_TO_SGIR_AFFINITY" as we are targeting
> ICC_SGIR? Same for the reverse makros.
okay
>
>> +
>> static int gic_send_sgi(struct sgi *sgi)
>> {
>> u64 val;
>> @@ -261,11 +265,11 @@ static int gic_send_sgi(struct sgi *sgi)
>> if (sgi->routing_mode == 2)
>> targets = 1 << phys_processor_id();
>>
>> - val = (u64)sgi->aff3 << ICC_SGIR_AFF3_SHIFT
>> - | (u64)sgi->aff2 << ICC_SGIR_AFF2_SHIFT
>> - | sgi->aff1 << ICC_SGIR_AFF1_SHIFT
>> + val = (MPIDR_TO_SGI_AFFINITY(sgi->cluster_id, 3)
>> + | MPIDR_TO_SGI_AFFINITY(sgi->cluster_id, 2)
>> + | MPIDR_TO_SGI_AFFINITY(sgi->cluster_id, 1)
>> | (targets & ICC_SGIR_TARGET_MASK)
>> - | (sgi->id & 0xf) << ICC_SGIR_IRQN_SHIFT;
>> + | (sgi->id & 0xf) << ICC_SGIR_IRQN_SHIFT);
>>
>> if (sgi->routing_mode == 1)
>> val |= ICC_SGIR_ROUTING_BIT;
>> @@ -282,6 +286,12 @@ static int gic_send_sgi(struct sgi *sgi)
>> return 0;
>> }
>>
>> +#define SGI_TO_AFFINITY(sgir, level) \
>> + ((sgir) >> ICC_SGIR_AFF## level ##_SHIFT & 0xff)
>> +
>> +#define SGI_TO_MPIDR_AFFINITY(sgir, level) \
>> + (SGI_TO_AFFINITY(sgir, level) << MPIDR_LEVEL_SHIFT(level))
>> +
>> void gicv3_handle_sgir_write(u64 sgir)
>> {
>> struct sgi sgi;
>> @@ -290,9 +300,9 @@ void gicv3_handle_sgir_write(u64 sgir)
>> /* FIXME: clusters are not supported yet. */
>> sgi.targets = sgir & ICC_SGIR_TARGET_MASK;
>> sgi.routing_mode = routing_mode;
>> - sgi.aff1 = sgir >> ICC_SGIR_AFF1_SHIFT & 0xff;
>> - sgi.aff2 = sgir >> ICC_SGIR_AFF2_SHIFT & 0xff;
>> - sgi.aff3 = sgir >> ICC_SGIR_AFF3_SHIFT & 0xff;
>> + sgi.cluster_id = (SGI_TO_MPIDR_AFFINITY(sgir, 3)
>> + | SGI_TO_MPIDR_AFFINITY(sgir, 2)
>> + | SGI_TO_MPIDR_AFFINITY(sgir, 1));
>> sgi.id = sgir >> ICC_SGIR_IRQN_SHIFT & 0xf;
>>
>> gic_handle_sgir_write(&sgi, true);
>>
>
> This obsoletes ICC_SGIR_AFF*_SHIFT macros, right? If there are no more
> users, let's remove them.
Nope. SGI_TO_AFFINITY() macro still uses it.
Thanks and regards,
Lokesh
>
> The patch looks clean. I was hoping the single cluster_id could simplify
> things, but the conversions between MPIDR and ICC_SGIR kills this,
> unfortunately.>
> Jan
>
--
You received this message because you are subscribed to the Google Groups
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
