Re: [PATCH v3 4/7] xen: add bitmap to indicate per-domain state changes

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.12.2024 17:24, Juergen Gross wrote:
> @@ -138,6 +139,60 @@ bool __read_mostly vmtrace_available;
>  
>  bool __read_mostly vpmu_is_available;
>  
> +static DEFINE_SPINLOCK(dom_state_changed_lock);
> +static unsigned long *dom_state_changed;

__read_mostly?

> +int domain_init_states(void)
> +{
> +    const struct domain *d;
> +    int rc = -ENOMEM;
> +
> +    spin_lock(&dom_state_changed_lock);
> +
> +    if ( dom_state_changed )
> +        bitmap_zero(dom_state_changed, DOMID_FIRST_RESERVED);
> +    else
> +    {
> +        dom_state_changed = xvzalloc_array(unsigned long,
> +                                           
> BITS_TO_LONGS(DOMID_FIRST_RESERVED));
> +        if ( !dom_state_changed )
> +            goto unlock;
> +    }
> +
> +    rcu_read_lock(&domlist_read_lock);
> +
> +    for_each_domain ( d )
> +        set_bit(d->domain_id, dom_state_changed);

Use the cheaper __set_bit() here?

> +static void domain_changed_state(const struct domain *d)
> +{
> +    spin_lock(&dom_state_changed_lock);
> +
> +    if ( dom_state_changed )
> +        set_bit(d->domain_id, dom_state_changed);

And perhaps even here, considering everything's under lock now?

> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -485,6 +485,13 @@ int evtchn_bind_virq(evtchn_bind_virq_t *bind, 
> evtchn_port_t port)
>      if ( (v = domain_vcpu(d, vcpu)) == NULL )
>          return -ENOENT;
>  
> +    if ( virq == VIRQ_DOM_EXC )
> +    {
> +        rc = domain_init_states();
> +        if ( rc )
> +            goto deinit;
> +    }

This is tied to VIRQ_DOM_EXC. How come ...

> @@ -527,6 +534,10 @@ int evtchn_bind_virq(evtchn_bind_virq_t *bind, 
> evtchn_port_t port)
>   out:
>      write_unlock(&d->event_lock);
>  
> + deinit:
> +    if ( rc )
> +        domain_deinit_states();
> +
>      return rc;
>  }

... de-init happens upon any error, regardless of vIRQ? Even checking
the virq isn't sufficient, as we also need to gracefully deal with the
-EEXIST path.

Jan