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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Fri, Feb 23, 2018 at 01:27:41PM +0000, Roger Pau Monne 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.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> ---
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Cc: Stefan Bader <stefan.bader@xxxxxxxxxxxxx>
> ---
>  xen/arch/x86/hvm/vpt.c | 30 +++++++++++++++++++++++++++++-
>  1 file changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/x86/hvm/vpt.c b/xen/arch/x86/hvm/vpt.c
> index 181f4cb631..1a24fbaa44 100644
> --- a/xen/arch/x86/hvm/vpt.c
> +++ b/xen/arch/x86/hvm/vpt.c
> @@ -247,9 +247,31 @@ static void pt_timer_fn(void *data)
>      pt_unlock(pt);
>  }
>  
> +static void execute_callbacks(struct vcpu *v, struct list_head *tm)
> +{
> +    spin_lock(&v->arch.hvm_vcpu.tm_lock);
> +    while ( !list_empty(tm) )
> +    {
> +        struct periodic_time *pt = list_first_entry(tm, struct periodic_time,
> +                                                    list);
> +        time_cb *cb = pt->cb;
> +        void *cb_priv = pt->priv;
> +
> +        list_del(&pt->list);
> +        pt->on_list = 0;
> +        spin_unlock(&v->arch.hvm_vcpu.tm_lock);
> +
> +        cb(v, cb_priv);
> +
> +        spin_lock(&v->arch.hvm_vcpu.tm_lock);
> +    }
> +    spin_unlock(&v->arch.hvm_vcpu.tm_lock);
> +}
> +
>  int pt_update_irq(struct vcpu *v)
>  {
>      struct list_head *head = &v->arch.hvm_vcpu.tm_list;
> +    LIST_HEAD(purged);

to_purge?

>      struct periodic_time *pt, *temp, *earliest_pt;
>      uint64_t max_lag;
>      int irq, is_lapic, pt_vector;
> @@ -267,7 +289,10 @@ int pt_update_irq(struct vcpu *v)
>              {
>                  /* suspend timer emulation */
>                  list_del(&pt->list);
> -                pt->on_list = 0;
> +                if ( pt->cb )
> +                    list_add(&pt->list, &purged);
> +                else
> +                    pt->on_list = 0;
>              }
>              else
>              {
> @@ -283,6 +308,7 @@ int pt_update_irq(struct vcpu *v)
>      if ( earliest_pt == NULL )
>      {
>          spin_unlock(&v->arch.hvm_vcpu.tm_lock);
> +        execute_callbacks(v, &purged);

It would be better to check if the list is not empty before calling the
function to avoid the extra lock / unlock.

(Haven't checked if the basic premise of this patch is compatible with
the spec)

Wei.

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