Re: [PATCH v3 05/40] arm64/sysreg: Add GICv5 GIC VDPEND and VDRCFG encodings

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing VM ID in GIC VDRCFG payload causes SPI state corruption for VMs > 0 or during restore.
--

commit af2100cc7ed8bea5d8db7c7e09a649c12e033d80
Author: Sascha Bischoff <Sascha.Bischoff@arm.com>

arm64/sysreg: Add GICv5 GIC VDPEND and VDRCFG encodings
    
Add the encodings for the GIC VDPEND and GIC VDRCFG system
instructions. These operate on the virtual interrupt domain, and are
used to make interrupts pending for a VM and to read back the
configuration of a VM's interrupts.

> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index 7aa08d59d4944..40ff7d25d37b0 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
[ ... ]
> @@ -1105,6 +1105,22 @@
>  #define GICV5_GICR_CDNMIA_TYPE_MASK	GENMASK_ULL(31, 29)
>  #define GICV5_GICR_CDNMIA_ID_MASK	GENMASK_ULL(23, 0)
>  
> +/*
> + * Definitions for GICv5 instructions for the Virtual Domain
> + */
> +#define GICV5_OP_GIC_VDPEND		sys_insn(1, 4, 12, 1, 4)
> +#define GICV5_OP_GIC_VDRCFG		sys_insn(1, 4, 12, 1, 5)
> +
> +/* Shift and mask definitions for GIC VDPEND */
> +#define GICV5_GIC_VDPEND_PENDING_MASK	BIT_ULL(63)
> +#define GICV5_GIC_VDPEND_VM_MASK	GENMASK_ULL(47, 32)
> +#define GICV5_GIC_VDPEND_TYPE_MASK	GENMASK_ULL(31, 29)
> +#define GICV5_GIC_VDPEND_ID_MASK	GENMASK_ULL(23, 0)
> +
> +/* Shift and mask definitions for GIC VDRCFG */
> +#define GICV5_GIC_VDRCFG_TYPE_MASK	GENMASK_ULL(31, 29)
> +#define GICV5_GIC_VDRCFG_ID_MASK	GENMASK_ULL(23, 0)

[Severity: High]
Does GIC VDRCFG also need a VM_MASK definition?

The mask definitions for the GIC VDRCFG payload omit a VM_MASK (e.g.,
GENMASK_ULL(47, 32)), which is present for VDPEND above. 

Could missing the VM ID in the payload cause the GIC to query the SPI state
for VM 0 instead of the target VM?

This regression might lead to incorrect functional behavior for VMs > 0.
For example, when kvm_vgic_sync_hwstate() calls __vgic_v5_vdrcfg(), an empty
VM ID field could cause the hypervisor to fold the SPI state of VM 0 into the
shadow state of VM > 0.

Alternatively, if the instruction architecturally relies on the resident VM in
ICH_CONTEXTR_EL2 rather than a payload field, would executing it during VM
restore when no vCPU is resident (such as in vgic_v5_restore_spi_ap_list())
fail to work correctly?

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