Re: [PATCH v2 1/4] xen/arm: optimize the size of struct vcpu

[Julien Grall <julien@xxxxxxx>]

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