Re: [Xen-devel] [PATCH 42/57] ARM: new VGIC: Add TARGET registers handlers

[Julien Grall <julien.grall@xxxxxxx>]

Hi Andre,

On 05/03/18 16:04, Andre Przywara wrote:
> The target register handlers are v2 emulation specific, so their
> implementation lives entirely in vgic-mmio-v2.c.
> We copy the old VGIC behaviour of assigning an IRQ to the first VCPU
> set in the target mask instead of making it possibly pending on
> multiple VCPUs.
> We update the physical affinity of a hardware mapped vIRQ on the way.
>
> This is based on Linux commit 2c234d6f1826, written by Andre Przywara.
>
> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxxxxx>
> ---
> Changelog RFC ... v1:
> - propagate affinity changes to hardware mapped IRQs
>
>   xen/arch/arm/vgic/vgic-mmio-v2.c | 64 +++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 63 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/vgic/vgic-mmio-v2.c b/xen/arch/arm/vgic/vgic-mmio-v2.c
> index d19ddd3f1e..01c6a7198c 100644
> --- a/xen/arch/arm/vgic/vgic-mmio-v2.c
> +++ b/xen/arch/arm/vgic/vgic-mmio-v2.c
> @@ -72,6 +72,68 @@ static void vgic_mmio_write_v2_misc(struct vcpu *vcpu,
>       }
>   }
>   
> +static unsigned long vgic_mmio_read_target(struct vcpu *vcpu,
> +                                           paddr_t addr, unsigned int len)
> +{
> +    uint32_t intid = VGIC_ADDR_TO_INTID(addr, 8);
> +    uint32_t val = 0;
> +    unsigned int i;
> +
> +    for ( i = 0; i < len; i++ )
> +    {
> +        struct vgic_irq *irq = vgic_get_irq(vcpu->domain, vcpu, intid + i);
> +
> +        val |= (uint32_t)irq->targets << (i * 8);
> +
> +        vgic_put_irq(vcpu->domain, irq);
> +    }
> +
> +    return val;
> +}
> +
> +static void vgic_mmio_write_target(struct vcpu *vcpu,
> +                                   paddr_t addr, unsigned int len,
> +                                   unsigned long val)
> +{
> +    uint32_t intid = VGIC_ADDR_TO_INTID(addr, 8);
> +    uint8_t cpu_mask = GENMASK(vcpu->domain->max_vcpus - 1, 0);
> +    unsigned int i;
> +    unsigned long flags;
> +
> +    /* GICD_ITARGETSR[0-7] are read-only */
> +    if ( intid < VGIC_NR_PRIVATE_IRQS )
> +        return;
> +
> +    for ( i = 0; i < len; i++ )
> +    {
> +        struct vgic_irq *irq = vgic_get_irq(vcpu->domain, NULL, intid + i);
> +        struct irq_desc *desc;
> +
> +        spin_lock_irqsave(&irq->irq_lock, flags);
> +
> +        irq->targets = (val >> (i * 8)) & cpu_mask;
> +        if ( irq->targets )
> +        {
> +            irq->target_vcpu = vcpu->domain->vcpu[ffs(irq->targets) - 1];
> +            if ( irq->hw )
> +                desc = irq_to_desc(irq->hwintid);
> +            else
> +                desc = NULL;
If you initialized desc to NULL during the declaration, then the else is 
not necessary.
> +        }
> +        else {
> +            irq->target_vcpu = NULL;
> +            desc = NULL;
> +        }
> +
> +        spin_unlock_irqrestore(&irq->irq_lock, flags);
> +
> +        if ( desc )
> +            vgic_update_hardware_irq(desc, irq);
I can't find this function in the tree.

> +
> +        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,
> @@ -101,7 +163,7 @@ static const struct vgic_register_region 
> vgic_v2_dist_registers[] = {
>           vgic_mmio_read_priority, vgic_mmio_write_priority, 8,
>           VGIC_ACCESS_32bit | VGIC_ACCESS_8bit),
>       REGISTER_DESC_WITH_BITS_PER_IRQ(GICD_ITARGETSR,
> -        vgic_mmio_read_raz, vgic_mmio_write_wi, 8,
> +        vgic_mmio_read_target, vgic_mmio_write_target, 8,
>           VGIC_ACCESS_32bit | VGIC_ACCESS_8bit),
>       REGISTER_DESC_WITH_BITS_PER_IRQ(GICD_ICFGR,
>           vgic_mmio_read_config, vgic_mmio_write_config, 2,
Cheers,

--
Julien Grall

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