Re: [Xen-devel] [RFC PATCH 37/49] ARM: new VGIC: Add SGIPENDR register handlers

[Andre Przywara <andre.przywara@xxxxxxxxxx>]

Hi,

On 19/02/18 12:02, Julien Grall wrote:
> Hi,
> 
> On 09/02/18 14:39, Andre Przywara wrote:
>> As this register is v2 specific, its implementation lives entirely
>> in vgic-mmio-v2.c.
>> This register allows setting the source mask of an IPI.
>>
>> This is based on Linux commit ed40213ef9b0, written by Andre Przywara.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxxxxx>
>> ---
>>   xen/arch/arm/vgic/vgic-mmio-v2.c | 77
>> ++++++++++++++++++++++++++++++++++++++--
>>   1 file changed, 75 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/arch/arm/vgic/vgic-mmio-v2.c
>> b/xen/arch/arm/vgic/vgic-mmio-v2.c
>> index 3f67b4659a..c62307c3aa 100644
>> --- a/xen/arch/arm/vgic/vgic-mmio-v2.c
>> +++ b/xen/arch/arm/vgic/vgic-mmio-v2.c
>> @@ -161,6 +161,79 @@ static void vgic_mmio_write_target(struct vcpu
>> *vcpu,
>>       }
>>   }
>>   +static unsigned long vgic_mmio_read_sgipend(struct vcpu *vcpu,
>> +                        paddr_t addr, unsigned int len)
> 
> Indentation
> 
>> +{
>> +    u32 intid = addr & 0x0f;
> 
> uint32_t also why not using VGIC_ADDR_TO_INTID here?

Yeah, slight overkill, but probably better.
(Hopefully) fixed all the rest.

Cheers,
Andre.

> 
>> +    int i;
> 
> unsigned int.
> 
>> +    u64 val = 0;
> 
> Why 64-bit?
> 
>> +
>> +    for ( i = 0; i < len; i++ )
>> +    {
>> +        struct vgic_irq *irq = vgic_get_irq(vcpu->domain,
>> +                            vcpu, intid + i);
> 
> Indentation.
> 
>> +
>> +        val |= (u64)irq->source << (i * 8);
> 
> See above for u64.
> 
>> +
>> +        vgic_put_irq(vcpu->domain, irq);
>> +    }
>> +    return val;
>> +}
>> +
>> +static void vgic_mmio_write_sgipendc(struct vcpu *vcpu,
>> +                     paddr_t addr, unsigned int len,
>> +                     unsigned long val)
> 
> Same remarks as above for this function.
> 
>> +{ > +    u32 intid = addr & 0x0f;> +    int i;
>> +    unsigned long flags;
>> +
>> +    for ( i = 0; i < len; i++ )
>> +    {
>> +        struct vgic_irq *irq = vgic_get_irq(vcpu->domain,
>> +                            vcpu, intid + i);
>> +
>> +        spin_lock_irqsave(&irq->irq_lock, flags);
>> +
>> +        irq->source &= ~((val >> (i * 8)) & 0xff);
>> +        if ( !irq->source )
>> +            irq->pending_latch = false;
>> +
>> +        spin_unlock_irqrestore(&irq->irq_lock, flags);
>> +        vgic_put_irq(vcpu->domain, irq);
>> +    }
>> +}
>> +
>> +static void vgic_mmio_write_sgipends(struct vcpu *vcpu,
>> +                     paddr_t addr, unsigned int len,
>> +                     unsigned long val)
> 
> Ditto.
> 
>> +{
>> +    u32 intid = addr & 0x0f;
>> +    int i;
>> +    unsigned long flags;
>> +
>> +    for ( i = 0; i < len; i++ )
>> +    {
>> +        struct vgic_irq *irq = vgic_get_irq(vcpu->domain,
>> +                            vcpu, intid + i);
>> +
>> +        spin_lock_irqsave(&irq->irq_lock, flags);
>> +
>> +        irq->source |= (val >> (i * 8)) & 0xff;
>> +
>> +        if ( irq->source )
>> +        {
>> +            irq->pending_latch = true;
>> +            vgic_queue_irq_unlock(vcpu->domain, irq, flags);
>> +        }
>> +        else
>> +        {
>> +            spin_unlock_irqrestore(&irq->irq_lock, flags);
>> +        }
>> +        vgic_put_irq(vcpu->domain, irq);
>> +    }
>> +}
>> +
>>   static const struct vgic_register_region vgic_v2_dist_registers[] = {
>>       REGISTER_DESC_WITH_LENGTH(GICD_CTLR,
>>           vgic_mmio_read_v2_misc, vgic_mmio_write_v2_misc, 12,
>> @@ -199,10 +272,10 @@ static const struct vgic_register_region
>> vgic_v2_dist_registers[] = {
>>           vgic_mmio_read_raz, vgic_mmio_write_sgir, 4,
>>           VGIC_ACCESS_32bit),
>>       REGISTER_DESC_WITH_LENGTH(GICD_CPENDSGIR,
>> -        vgic_mmio_read_raz, vgic_mmio_write_wi, 16,
>> +        vgic_mmio_read_sgipend, vgic_mmio_write_sgipendc, 16,
>>           VGIC_ACCESS_32bit | VGIC_ACCESS_8bit),
>>       REGISTER_DESC_WITH_LENGTH(GICD_SPENDSGIR,
>> -        vgic_mmio_read_raz, vgic_mmio_write_wi, 16,
>> +        vgic_mmio_read_sgipend, vgic_mmio_write_sgipends, 16,
>>           VGIC_ACCESS_32bit | VGIC_ACCESS_8bit),
>>   };
>>  
> 
> Cheers,
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel