Hi Marc,
On 6/7/22 15:14, Marc Zyngier wrote:
> Now that GICv2 has a proper userspace accessor for the pending state,
> switch GICv3 over to it, dropping the local version, moving over the
> specific behaviours that CGIv3 requires (such as the distinction
> between pending latch and line level which were never enforced
> with GICv2).
>
> We also gain extra locking that isn't really necessary for userspace,
> but that's a small price to pay for getting rid of superfluous code.
>
> Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
> ---
> arch/arm64/kvm/vgic/vgic-mmio-v3.c | 40 ++----------------------------
> arch/arm64/kvm/vgic/vgic-mmio.c | 21 +++++++++++++++-
> 2 files changed, 22 insertions(+), 39 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.
> - */
> - 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;
> -
> - err = irq_get_irqchip_state(irq->host_irq,
> - IRQCHIP_STATE_PENDING,
> - &state);
> - WARN_ON(err);
> - }
> -
> - 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,
> diff --git a/arch/arm64/kvm/vgic/vgic-mmio.c b/arch/arm64/kvm/vgic/vgic-mmio.c
> index dc8c52487e47..997d0fce2088 100644
> --- a/arch/arm64/kvm/vgic/vgic-mmio.c
> +++ b/arch/arm64/kvm/vgic/vgic-mmio.c
> @@ -240,6 +240,15 @@ static unsigned long __read_pending(struct kvm_vcpu *vcpu,
> unsigned long flags;
> bool val;
>
> + /*
> + * When used from userspace with a GICv3 model:
> + *
> + * 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.
> + */
> raw_spin_lock_irqsave(&irq->irq_lock, flags);
> if (irq->hw && vgic_irq_is_sgi(irq->intid)) {
> int err;
> @@ -252,7 +261,17 @@ static unsigned long __read_pending(struct kvm_vcpu *vcpu,
> } else if (!is_user && vgic_irq_is_mapped_level(irq)) {
> val = vgic_get_phys_line_level(irq);
> } else {
> - val = irq_is_pending(irq);
> + switch (vcpu->kvm->arch.vgic.vgic_model) {
> + case KVM_DEV_TYPE_ARM_VGIC_V3:
> + if (is_user) {
> + val = irq->pending_latch;
> + break;
> + }
> + fallthrough;
> + default:
> + val = irq_is_pending(irq);
> + break;
> + }
> }
>
> value |= ((u32)val << i);
Reviewed-by: Eric Auger <eric.auger@xxxxxxxxxx>
Thanks!
Eric
