Re: [PATCH v4 32/48] KVM: arm64: gic-v5: Handle userspace accesses to IRS MMIO region

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Userspace writes to `GICV5_IRS_SPI_CFGR` are silently ignored without validation, breaking restoration of SPI configurations across live migrations.
- [Medium] The 64-bit registers GICV5_IRS_IST_BASER and GICV5_IRS_SPI_VMR lack support for 32-bit guest MMIO accesses, violating the GIC architecture.
- [Medium] KVM_HAS_DEVICE_ATTR for KVM_DEV_ARM_VGIC_GRP_IRS_REGS incorrectly fails with -EINVAL if VCPU 0 is not yet created, breaking feature enumeration.
--

--- Patch [32]: [PATCH v4 32/48] KVM: arm64: gic-v5: Handle userspace accesses to IRS MMIO region ---
commit 9b79670b7941a4c70c01c7a66f84cbfecafe7755
Author: Sascha Bischoff <Sascha.Bischoff@arm.com>

KVM: arm64: gic-v5: Handle userspace accesses to IRS MMIO region

As part of saving and restoring state of a GICv5-based system, userspace is
required to save/restore the IRS MMIO registers. Provide accessors to read
and write the IRS MMIO state. Where possible, the existing access mechanisms
are used, but for some registers the access is handled a bit differently as
they have wider effects.

> diff --git a/arch/arm64/kvm/vgic/vgic-irs-v5.c b/arch/arm64/kvm/vgic/vgic-irs-v5.c
> --- a/arch/arm64/kvm/vgic/vgic-irs-v5.c
> +++ b/arch/arm64/kvm/vgic/vgic-irs-v5.c

[ ... ]

> @@ -455,84 +584,217 @@ static void vgic_v5_mmio_write_irs_ist(struct kvm_vcpu *vcpu, gpa_t addr,
>  	case GICV5_IRS_CR0:
>  		vgic->enabled = !!(val & GICV5_IRS_CR0_IRSEN);
>  		break;
> +	case GICV5_IRS_SPI_CFGR:
> +		break;

[Severity: High]
Does this silently ignore userspace writes to restore SPI configurations?

The commit message mentions blocking userspace from setting anything that
doesn't match what has been set already. However, the uaccess write handler
here just executes a break and returns 0 without validating the write or
applying any updates.

Could this cause SPI configurations (like edge vs level triggered) to be lost
during VM migration, permanently leaving restored SPIs in their default state?

> +	case GICV5_IRS_IIDR:
> +		fallthrough;
> +	case GICV5_IRS_AIDR:
> +		break;
>  	default:
> -		return;
> +		return -EINVAL;
>  	}
> +
> +	return 0;
>  }

[ ... ]

> @@ -494,35 +673,50 @@ static const struct vgic_register_region vgic_v5_irs_registers[] = {
> -	REGISTER_DESC_WITH_LENGTH(GICV5_IRS_IST_BASER, vgic_v5_mmio_read_irs_ist,
> -				  vgic_v5_mmio_write_irs_ist, 8,
> -				  VGIC_ACCESS_64bit),
> +	REGISTER_DESC_WITH_LENGTH_UACCESS(GICV5_IRS_IST_BASER,
> +					  vgic_v5_mmio_read_irs_ist,
> +					  vgic_v5_mmio_write_irs_ist, NULL,
> +					  vgic_v5_mmio_uaccess_write_irs, 8,
> +					  VGIC_ACCESS_64bit),

[Severity: Medium]
Should these 64-bit registers support 32-bit MMIO accesses?

The region descriptor specifies only VGIC_ACCESS_64bit. The GIC architecture
mandates that 64-bit registers must also support 32-bit accesses, so valid
32-bit guest reads or writes might be improperly rejected.

Additionally, if 32-bit accesses were permitted, would the write handler in
vgic_v5_mmio_write_irs_ist() improperly clobber the upper 32 bits? It
appears to lack the update_64bit_reg() wrapper used elsewhere in KVM to
handle partial writes safely.

[ ... ]

> diff --git a/arch/arm64/kvm/vgic/vgic-kvm-device.c b/arch/arm64/kvm/vgic/vgic-kvm-device.c
> --- a/arch/arm64/kvm/vgic/vgic-kvm-device.c
> +++ b/arch/arm64/kvm/vgic/vgic-kvm-device.c

[ ... ]

> @@ -786,6 +786,9 @@ int vgic_v5_parse_attr(struct kvm_device *dev, struct kvm_device_attr *attr,
>  		mpidr_reg = VGIC_TO_MPIDR(vgic_mpidr);
>  		reg_attr->vcpu = kvm_mpidr_to_vcpu(dev->kvm, mpidr_reg);
>  		break;
> +	case KVM_DEV_ARM_VGIC_GRP_IRS_REGS:
> +		reg_attr->vcpu = kvm_get_vcpu(dev->kvm, 0);
> +		break;
>  	default:
>  		return -EINVAL;
>  	}

[Severity: Medium]
Does this break KVM_HAS_DEVICE_ATTR feature enumeration before VCPUs are
created?

If a VMM queries KVM_HAS_DEVICE_ATTR for KVM_DEV_ARM_VGIC_GRP_IRS_REGS
immediately after creating the VGIC device, kvm_get_vcpu(dev->kvm, 0) will
return NULL.

Because vgic_v5_parse_attr() returns -EINVAL when reg_attr->vcpu is NULL,
this causes feature probing to yield false negatives. As IRS registers are
VM-wide features, can we avoid this dependency on VCPU 0 during enumeration?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724104819.1296803-1-sascha.bischoff@arm.com?part=32