Hi Marc,
On 6/2/22 10:30, Marc Zyngier wrote:
> Now that GICv2 has a proper userspace accessor for the pending state,
> switch GICv3 over to it, dropping the local version.
> 
> Signed-off-by: Marc Zyngier <m...@kernel.org>
> ---
>  arch/arm64/kvm/vgic/vgic-mmio-v3.c | 40 ++----------------------------
>  1 file changed, 2 insertions(+), 38 deletions(-)
> 
> diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c 
> b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> index f7aa7bcd6fb8..f15e29cc63ce 100644
> --- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> +++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> @@ -353,42 +353,6 @@ static unsigned long vgic_mmio_read_v3_idregs(struct 
> kvm_vcpu *vcpu,
>       return 0;
>  }
>  
> -static unsigned long vgic_v3_uaccess_read_pending(struct kvm_vcpu *vcpu,
> -                                               gpa_t addr, unsigned int len)
> -{
> -     u32 intid = VGIC_ADDR_TO_INTID(addr, 1);
> -     u32 value = 0;
> -     int i;

> -
> -     /*
> -      * pending state of interrupt is latched in pending_latch variable.
> -      * Userspace will save and restore pending state and line_level
> -      * separately.
> -      * Refer to Documentation/virt/kvm/devices/arm-vgic-v3.rst
> -      * for handling of ISPENDR and ICPENDR.
Don't know if you want a derivative of this comment in
vgic_uaccess_read_pending()?
> -      */
> -     for (i = 0; i < len * 8; i++) {
> -             struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
> -             bool state = irq->pending_latch;
> -
> -             if (irq->hw && vgic_irq_is_sgi(irq->intid)) {
> -                     int err;
> -
in __read_pending(), irq->irq_lock is hold which looks safer at 1st
sight. If potentially fixing something this can be documented in the
commit msg.
> -                     err = irq_get_irqchip_state(irq->host_irq,
> -                                                 IRQCHIP_STATE_PENDING,
> -                                                 &state);
> -                     WARN_ON(err);
> -             }
> -
in __read_pending(), irq_is_pending(irq) is used instead of
irq->pending_latch. for level sensitive IRQ this is not identical. This
may also deserve some comment. The nuance may be related to the above
comment.

Thanks

Eric
> -             if (state)
> -                     value |= (1U << i);
> -
> -             vgic_put_irq(vcpu->kvm, irq);
> -     }
> -
> -     return value;
> -}
> -
>  static int vgic_v3_uaccess_write_pending(struct kvm_vcpu *vcpu,
>                                        gpa_t addr, unsigned int len,
>                                        unsigned long val)
> @@ -666,7 +630,7 @@ static const struct vgic_register_region 
> vgic_v3_dist_registers[] = {
>               VGIC_ACCESS_32bit),
>       REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ISPENDR,
>               vgic_mmio_read_pending, vgic_mmio_write_spending,
> -             vgic_v3_uaccess_read_pending, vgic_v3_uaccess_write_pending, 1,
> +             vgic_uaccess_read_pending, vgic_v3_uaccess_write_pending, 1,
>               VGIC_ACCESS_32bit),
>       REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ICPENDR,
>               vgic_mmio_read_pending, vgic_mmio_write_cpending,
> @@ -750,7 +714,7 @@ static const struct vgic_register_region 
> vgic_v3_rd_registers[] = {
>               VGIC_ACCESS_32bit),
>       REGISTER_DESC_WITH_LENGTH_UACCESS(SZ_64K + GICR_ISPENDR0,
>               vgic_mmio_read_pending, vgic_mmio_write_spending,
> -             vgic_v3_uaccess_read_pending, vgic_v3_uaccess_write_pending, 4,
> +             vgic_uaccess_read_pending, vgic_v3_uaccess_write_pending, 4,
>               VGIC_ACCESS_32bit),
>       REGISTER_DESC_WITH_LENGTH_UACCESS(SZ_64K + GICR_ICPENDR0,
>               vgic_mmio_read_pending, vgic_mmio_write_cpending,

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Reply via email to