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@xxxxxxxxxx>
> ---
> 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,
