On Fri, 04 Jul 2025 13:01:05 +0100,
Peter Maydell <peter.mayd...@linaro.org> wrote:
>
> On Wed, 2 Jul 2025 at 17:31, Eric Auger <eric.au...@redhat.com> wrote:
> >
> > From: Haibo Xu <haibo...@linaro.org>
> >
> > Allow virt arm machine to set the interrupt ID for the KVM
> > GIC maintenance interrupt.
> >
> > This setting must be done before the KVM_DEV_ARM_VGIC_CTRL_INIT
> > hence the choice to perform the setting in the GICv3 realize
> > instead of proceeding the same way as kvm_arm_pmu_set_irq().
> >
> > Signed-off-by: Haibo Xu <haibo...@linaro.org>
> > Signed-off-by: Miguel Luis <miguel.l...@oracle.com>
> > Signed-off-by: Eric Auger <eric.au...@redhat.com>
>
> > @@ -231,6 +231,7 @@ struct GICv3State {
> > uint32_t num_cpu;
> > uint32_t num_irq;
> > uint32_t revision;
> > + uint32_t maint_irq;
> > bool lpi_enable;
> > bool nmi_support;
> > bool security_extn;
>
> > + if (s->maint_irq) {
> > + int ret;
> > +
> > + ret = kvm_device_check_attr(s->dev_fd,
> > + KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ, 0);
> > + if (!ret) {
> > + error_setg_errno(errp, errno,
> > + "VGICv3 setting maintenance IRQ is not "
> > + "supported by this host kernel");
> > + return;
> > + }
> > +
> > + ret = kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ,
> > 0,
> > + &s->maint_irq, true, errp);
>
> This doesn't seem to line up with what the kernel documents
> for KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ:
>
> https://www.kernel.org/doc/Documentation/virt/kvm/devices/arm-vgic-v3.rst
> says
>
> # KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ
> # Attributes:
> #
> # The attr field of kvm_device_attr encodes the following values:
> #
> # bits: | 31 .... 5 | 4 .... 0 |
> # values: | RES0 | vINTID |
> #
> # The vINTID specifies which interrupt is generated when the vGIC
> # must generate a maintenance interrupt. This must be a PPI.
>
> but this code sets kvmattr.addr to 0 and kvmaddr.addr to
> the address of a uint32_t with the vINTID in it.
>
> Looking at the kernel code in vgic_v3_attr_regs_access() it
> looks like maybe the kernel docs are wrong, but I'm not sure.
I think this is a pretty bad case of one person implementing
something, and another (me) hoping (but not checking) it would be
implemented in a particular way. Oh well.
Would the following match your expectations (and the code)?
diff --git a/Documentation/virt/kvm/devices/arm-vgic-v3.rst
b/Documentation/virt/kvm/devices/arm-vgic-v3.rst
index e860498b1e359..4843c91052786 100644
--- a/Documentation/virt/kvm/devices/arm-vgic-v3.rst
+++ b/Documentation/virt/kvm/devices/arm-vgic-v3.rst
@@ -299,7 +299,7 @@ Groups:
KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ
Attributes:
- The attr field of kvm_device_attr encodes the following values:
+ The attribute data pointed to by kvm_device_attr.addr is a __u32 value::
bits: | 31 .... 5 | 4 .... 0 |
values: | RES0 | vINTID |
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
