On Wed, Mar 31, 2021 at 12:47:54PM +0100, Andrew Cooper wrote:
> On 31/03/2021 11:32, Roger Pau Monne wrote:
> > @@ -1620,9 +1666,22 @@ int vlapic_init(struct vcpu *v)
> >
> > clear_page(vlapic->regs);
> >
> > + vlapic->callbacks = xmalloc_array(typeof(*vlapic->callbacks),
> > + X86_NR_VECTORS - 16);
> > + if ( !vlapic->callbacks )
> > + {
> > + dprintk(XENLOG_ERR, "%pv: alloc vlapic callbacks error\n", v);
> > + unmap_domain_page_global(vlapic->regs);
> > + free_domheap_page(vlapic->regs_page);
> > + return -ENOMEM;
> > + }
> > + memset(vlapic->callbacks, 0,
> > + sizeof(*vlapic->callbacks) * (X86_NR_VECTORS - 16));
>
> xzalloc_array() instead of memset(). Also, we shouldn't be printing for
> -ENOMEM cases.
>
> As for the construction/teardown logic, vlapic_init()'s caller already
> vlapic_destory(). Therefore, the existing error path you've copied is
> buggy because it will cause a double-free if __map_domain_page_global()
> fails.
Right, let me adjust that path.
> I'll do a cleanup patch to fix the idempotency, which needs backporting too.
Ack, I don't mind doiing one myself either.
Thanks, Roger.
