Re: [Xen-devel] [PATCH v2 1/2] x86/vpt: execute callbacks for masked interrupts

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 30.03.18 at 14:35, <roger.pau@xxxxxxxxxx> wrote:
> Execute periodic_time callbacks even if the interrupt is not actually
> injected because the IRQ is masked.
> 
> Current callbacks from emulated timer devices only update emulated
> registers, which from my reading of the specs should happen regardless
> of whether the interrupt has been injected or not.

While generally I agree, it also means extra work done. Looking
at the PIT case, for example, there's no strict need to do the
update when the IRQ is masked, as the value being updated is
only used to subtract from get_guest_time()'s return value.
Similarly for the LAPIC case.

In the RTC case your change actually looks risky, due to the
pt_dead_ticks logic. I can't help getting the impression that the
IRQ being off for 10 ticks would lead to no RTC interrupts at all
anymore for the guest (until something resets that counter),
which seems wrong to me.

> @@ -282,6 +305,12 @@ int pt_update_irq(struct vcpu *v)
>  
>      if ( earliest_pt == NULL )
>      {
> +        /*
> +         * NB: although the to_purge list is local, calls to
> +         * destroy_periodic_time can still remove items from the list, hence

pt_adjust_vcpu() as well as it looks.

> +         * the need to hold the lock while accessing it.
> +         */
> +        execute_callbacks(v, &to_purge);
>          spin_unlock(&v->arch.hvm_vcpu.tm_lock);
>          return -1;
>      }
> @@ -290,6 +319,8 @@ int pt_update_irq(struct vcpu *v)
>      irq = earliest_pt->irq;
>      is_lapic = (earliest_pt->source == PTSRC_lapic);
>  
> +    execute_callbacks(v, &to_purge);
> +
>      spin_unlock(&v->arch.hvm_vcpu.tm_lock);

It seems to me that with your addition some code restructuring
would actually be desirable, such that execute_callbacks() (and
the lock release) would occur just once. Perhaps the mid-function
return could be avoided altogether.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel