On Tue, Jun 16, 2020 at 08:11:12AM +0200, Jan Beulich wrote:
> On 10.06.2020 16:29, Roger Pau Monne wrote:
> > @@ -186,9 +187,10 @@ void hvm_gsi_assert(struct domain *d, unsigned int gsi)
> > * to know if the GSI is pending or not.
> > */
> > spin_lock(&d->arch.hvm.irq_lock);
> > - if ( !hvm_irq->gsi_assert_count[gsi] )
> > + if ( trig == VIOAPIC_EDGE_TRIG || !hvm_irq->gsi_assert_count[gsi] )
> > {
> > - hvm_irq->gsi_assert_count[gsi] = 1;
> > + if ( trig == VIOAPIC_LEVEL_TRIG )
> > + hvm_irq->gsi_assert_count[gsi] = 1;
>
> Btw, along the lines of how you do things here, I think ...
>
> > @@ -196,11 +198,12 @@ void hvm_gsi_assert(struct domain *d, unsigned int
> > gsi)
> >
> > void hvm_gsi_deassert(struct domain *d, unsigned int gsi)
> > {
> > + int trig = vioapic_get_trigger_mode(d, gsi);
> > struct hvm_irq *hvm_irq = hvm_domain_irq(d);
> >
> > - if ( gsi >= hvm_irq->nr_gsis )
> > + if ( trig <= VIOAPIC_EDGE_TRIG || gsi >= hvm_irq->nr_gsis )
>
> ... this would better have been "trig != VIOAPIC_LEVEL_TRIG", to
> avoid the code being dependent upon the actual values of both
> VIOAPIC_*_TRIG constants.
Sure, let me send a follow up patch, it's trivial to fix.
Thanks, Roger.
