Re: [PATCH v5 17/36] KVM: arm64: gic-v5: Finalize GICv5 PPIs and generate mask

[Marc Zyngier <maz@kernel.org>]

On Thu, 26 Feb 2026 15:59:48 +0000,
Sascha Bischoff <Sascha.Bischoff@arm.com> wrote:
> 
> We only want to expose a subset of the PPIs to a guest. If a PPI does
> not have an owner, it is not being actively driven by a device. The
> SW_PPI is a special case, as it is likely for userspace to wish to
> inject that.
> 
> Therefore, just prior to running the guest for the first time, we need
> to finalize the PPIs. A mask is generated which, when combined with
> trapping a guest's PPI accesses, allows for the guest's view of the
> PPI to be filtered. This mask is global to the VM as all VCPUs PPI
> configurations must match.
> 
> In addition, the PPI HMR is calculated.
> 
> Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> ---
>  arch/arm64/kvm/arm.c               |  4 +++
>  arch/arm64/kvm/vgic/vgic-v5.c      | 46 ++++++++++++++++++++++++++++++
>  include/kvm/arm_vgic.h             |  9 ++++++
>  include/linux/irqchip/arm-gic-v5.h | 17 +++++++++++
>  4 files changed, 76 insertions(+)
> 
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index eb2ca65dc7297..8290c5df0616e 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -935,6 +935,10 @@ int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
>  			return ret;
>  	}
>  
> +	ret = vgic_v5_finalize_ppi_state(kvm);
> +	if (ret)
> +		return ret;
> +
>  	if (is_protected_kvm_enabled()) {
>  		ret = pkvm_create_hyp_vm(kvm);
>  		if (ret)
> diff --git a/arch/arm64/kvm/vgic/vgic-v5.c b/arch/arm64/kvm/vgic/vgic-v5.c
> index f5cd9decfc26e..db2225aefb130 100644
> --- a/arch/arm64/kvm/vgic/vgic-v5.c
> +++ b/arch/arm64/kvm/vgic/vgic-v5.c
> @@ -86,6 +86,52 @@ int vgic_v5_probe(const struct gic_kvm_info *info)
>  	return 0;
>  }
>  
> +int vgic_v5_finalize_ppi_state(struct kvm *kvm)
> +{
> +	struct kvm_vcpu *vcpu;
> +
> +	if (!vgic_is_v5(kvm))
> +		return 0;
> +
> +	/* The PPI state for all VCPUs should be the same. Pick the first. */
> +	vcpu = kvm_get_vcpu(kvm, 0);
> +
> +	vcpu->kvm->arch.vgic.gicv5_vm.vgic_ppi_mask[0] = 0;
> +	vcpu->kvm->arch.vgic.gicv5_vm.vgic_ppi_mask[1] = 0;
> +	vcpu->kvm->arch.vgic.gicv5_vm.vgic_ppi_hmr[0] = 0;
> +	vcpu->kvm->arch.vgic.gicv5_vm.vgic_ppi_hmr[1] = 0;

vcpu->kvm == kvm. You don't need the indirection (same in most of the
function).

> +
> +	for (int i = 0; i < VGIC_V5_NR_PRIVATE_IRQS; i++) {
> +		int reg = i / 64;
> +		u64 bit = BIT_ULL(i % 64);
> +		struct vgic_irq *irq = &vcpu->arch.vgic_cpu.private_irqs[i];

vgic_get_vcpu_irq()?

> +
> +		guard(raw_spinlock_irqsave)(&irq->irq_lock);
> +
> +		/*
> +		 * We only expose PPIs with an owner or the SW_PPI to the
> +		 * guest.
> +		 */
> +		if (!irq->owner &&
> +		    FIELD_GET(GICV5_HWIRQ_ID, irq->intid) != GICV5_ARCH_PPI_SW_PPI)
> +			continue;

This sort of construct is rather cumbersome, and I see it replicated
in quite a few places. How about introducing a couple of basic
accessors:

#define vgic_v5_get_hwirq_id(x) FIELD_GET(GICV5_HWIRQ_ID, (x))
#define vgic_v5_set_hwirq_id(x) FIELD_PREP(GICV5_HWIRQ_ID, (x))

which is a bit easier on the eye?

> +
> +		/*
> +		 * If the PPI isn't implemented, we can't pass it through to a
> +		 * guest anyhow.
> +		 */
> +		if (!(ppi_caps.impl_ppi_mask[reg] & bit))
> +			continue;
> +
> +		vcpu->kvm->arch.vgic.gicv5_vm.vgic_ppi_mask[reg] |= bit;
> +
> +		if (irq->config == VGIC_CONFIG_LEVEL)
> +			vcpu->kvm->arch.vgic.gicv5_vm.vgic_ppi_hmr[reg] |= bit;
> +	}
> +
> +	return 0;
> +}
> +
>  /*
>   * Sets/clears the corresponding bit in the ICH_PPI_DVIR register.
>   */
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index d828861f8298a..a4416afca5efc 100644
> --- a/include/kvm/arm_vgic.h
> +++ b/include/kvm/arm_vgic.h
> @@ -32,6 +32,8 @@
>  #define VGIC_MIN_LPI		8192
>  #define KVM_IRQCHIP_NUM_PINS	(1020 - 32)
>  
> +#define VGIC_V5_NR_PRIVATE_IRQS	128
> +
>  #define is_v5_type(t, i)	(FIELD_GET(GICV5_HWIRQ_TYPE, (i)) == (t))
>  
>  #define __irq_is_sgi(t, i)						\
> @@ -381,6 +383,11 @@ struct vgic_dist {
>  	 * else.
>  	 */
>  	struct its_vm		its_vm;
> +
> +	/*
> +	 * GICv5 per-VM data.
> +	 */
> +	struct gicv5_vm		gicv5_vm;

Depending how this grows, we may have to move that as part of a union
with the previous member (which is obviously v4 specific).

>  };
>  
>  struct vgic_v2_cpu_if {
> @@ -567,6 +574,8 @@ int vgic_v4_load(struct kvm_vcpu *vcpu);
>  void vgic_v4_commit(struct kvm_vcpu *vcpu);
>  int vgic_v4_put(struct kvm_vcpu *vcpu);
>  
> +int vgic_v5_finalize_ppi_state(struct kvm *kvm);
> +
>  bool vgic_state_is_nested(struct kvm_vcpu *vcpu);
>  
>  /* CPU HP callbacks */
> diff --git a/include/linux/irqchip/arm-gic-v5.h b/include/linux/irqchip/arm-gic-v5.h
> index 3e838a3058861..30a1b656daa35 100644
> --- a/include/linux/irqchip/arm-gic-v5.h
> +++ b/include/linux/irqchip/arm-gic-v5.h
> @@ -380,6 +380,23 @@ struct gicv5_vpe {
>  	bool			resident;
>  };
>  
> +struct gicv5_vm {
> +	/*
> +	 * We only expose a subset of PPIs to the guest. This subset
> +	 * is a combination of the PPIs that are actually implemented
> +	 * and what we actually choose to expose.
> +	 */
> +	u64			vgic_ppi_mask[2];
> +
> +	/*
> +	 * The HMR itself is handled by the hardware, but we still need to have
> +	 * a mask that we can use when merging in pending state (only the state
> +	 * of Edge PPIs is merged back in from the guest an the HMR provides a
> +	 * convenient way to do that).
> +	 */
> +	u64			vgic_ppi_hmr[2];
> +};
> +
>  struct gicv5_its_devtab_cfg {
>  	union {
>  		struct {

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.