Hi Andrew,
On 18/12/2025 18:15, Andrew Cooper wrote:
diff --git a/xen/arch/arm/vgic/vgic-init.c b/xen/arch/arm/vgic/vgic-init.c
index f8d7d3a226..67f297797f 100644
--- a/xen/arch/arm/vgic/vgic-init.c
+++ b/xen/arch/arm/vgic/vgic-init.c
@@ -241,10 +245,12 @@ void domain_vgic_free(struct domain *d)
int vcpu_vgic_free(struct vcpu *v)
{
- struct vgic_cpu *vgic_cpu = &v->arch.vgic;
+ struct vgic_cpu *vgic_cpu = v->arch.vgic;
INIT_LIST_HEAD(&vgic_cpu->ap_list_head);
+ xfree(vgic_cpu);
+
return 0;
}
Not in your part of the change, but this is bogus. It's not remotely
safe to init the list head like that.
The list is either already empty, in which case it's a no-op, or it
corrupts the list. It appears that the list mixes entries from other
vCPUs, and from the domain.
I think this is further proof that NEW_VGIC should be deleted
wholesale. It's clearly not in a good state, and I get the impression
that a badly timed evtchn sent to a domain in the middle of being
cleaned up will cause Xen to trip over the corrupted list.
I am probably missing something. What other issues are in the NEW_VGIC?
But note, if we delete the NEW_VGIC then we are back to a situation
where the vGIC implementation we have is not even remotely spec
compliant (no level support, not pending/active clear/set support) and
from previous discussion it was not possible to rework the default vGIC
implementation to make it compliant.
I was going to say that the NEW_VGIC is meant to be experimental. But I
see, it can selected without EXPERT :(. Thankfully it is marked as not
security supported at least in the Kconfig doc.
Cheers,
--
Julien Grall
