[PATCH 2/3] KVM: arm64: Replace vgic_v3_uaccess_read_pending with vgic_uaccess_read_pending

[Eric Auger]

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 <maz at 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,