Re: [Xen-devel] [PATCH 36/57] ARM: new VGIC: Add CTLR, TYPER and IIDR handlers

[Julien Grall <julien.grall@xxxxxxx>]

Hi Andre,

On 03/05/2018 04:03 PM, Andre Przywara wrote:
> +static void vgic_mmio_write_v2_misc(struct vcpu *vcpu,
> +                                    paddr_t addr, unsigned int len,
> +                                    unsigned long val)
> +{
> +    struct vgic_dist *dist = &vcpu->domain->arch.vgic;
> +    bool enabled;
> +
> +    switch ( addr & 0x0c )      /* filter for the 4 registers handled here */
> +    {
> +    case GICD_CTLR:
> +        domain_lock(vcpu->domain);
> +        enabled = dist->enabled;
> +        dist->enabled = val & GICD_CTL_ENABLE;
> +        enabled = !enabled && dist->enabled;
> +        domain_unlock(vcpu->domain);
> +
> +        if (enabled)
if ( ... )

Also, it took me a bit of time to understand what is the behavior of 
enable here. If I am not mistake it will be true only when toggle from 
disable -> enable, right? If so, I can you please either rework the code 
to make it more obvious (maybe a new variable) or add more comment?
> +            vgic_kick_vcpus(vcpu->domain);
> +
> +        break;
> +    case GICD_TYPER:
> +    case GICD_IIDR:
> +        /* read-only, writes ignored */
> +        return;
> +    }
> +}
> +
Cheers,


--
Julien Grall

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