Re: [PATCH v3 06/11] x86/hvm: allowing registering EOI callbacks for GSIs

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Apr 07, 2021 at 07:08:06PM +0200, Roger Pau Monné wrote:
> 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_execute_callbacks(unsigned int gsi)
> > > +{
> > > +    struct hvm_irq *hvm_irq = hvm_domain_irq(current->domain);
> > > +    struct hvm_gsi_eoi_callback *cb;
> > > +
> > > +    read_lock(&hvm_irq->gsi_callbacks_lock);
> > > +    list_for_each_entry ( cb, &hvm_irq->gsi_callbacks[gsi], list )
> > > +        cb->callback(gsi, cb->data);
> > > +    read_unlock(&hvm_irq->gsi_callbacks_lock);
> > > +}
> > 
> > Just as an observation (for now at least) - holding the lock here
> > means the callbacks cannot re-register themselves.
> 
> Well, re-registering would be weird, as the callback is not
> unregistered after execution. What is likely more relevant is that the
> callback cannot unregister itself. I haven't found a need for this so
> far, so I think it's fine.

I'm afraid I was wrong here - rtc_pf_callback could attempt to
unregister the timer, and thus end up calling
hvm_gsi_unregister_callback inside of a callback.

I need to figure a way to solve this. We already run the RTC in no ack
mode (which is correct because of the flag we expose in the WAET ACPI
table), and hence I wonder if we still need to keep the code for the
strict_mode around, since it's not used at all. Would you be OK with
me removing the mode_strict related code?

Thanks, Roger.