On Wed, Apr 07, 2021 at 05:51:14PM +0200, Jan Beulich wrote:
> On 31.03.2021 12:32, Roger Pau Monne wrote:
> > --- a/xen/arch/x86/hvm/irq.c
> > +++ b/xen/arch/x86/hvm/irq.c
> > +void hvm_gsi_unregister_callback(struct domain *d, unsigned int gsi,
> > + struct hvm_gsi_eoi_callback *cb)
> > +{
> > + struct hvm_irq *hvm_irq = hvm_domain_irq(d);
> > + const struct list_head *tmp;
> > +
> > + if ( gsi >= hvm_irq->nr_gsis )
> > + {
> > + ASSERT_UNREACHABLE();
> > + return;
> > + }
> > +
> > + write_lock(&hvm_irq->gsi_callbacks_lock);
> > + list_for_each ( tmp, &hvm_irq->gsi_callbacks[gsi] )
> > + if ( tmp == &cb->list )
> > + {
> > + list_del(&cb->list);
> > + break;
> > + }
> > + write_unlock(&hvm_irq->gsi_callbacks_lock);
> > +}
>
> Perhaps somehow flag, at least in debug builds, if the callback
> wasn#t found?
I've added a debug printf here to warn if the callback is not found,
but I see it triggering because hpet_set_timer will call
destroy_periodic_time and create_periodic_time and thus two calls will
be made to hvm_gsi_unregister_callback. This is fine, but adding a
message there gets too verbose, so I will drop it and leave the code
as-is.
I don't see a problem with calling destroy_periodic_time multiple
times even if the timer was not active, and that shouldn't result in a
message being printed.
Thanks, Roger.
