Re: [PATCH V3 08/23] xen/ioreq: Move x86's ioreq_server to struct domain

[Jan Beulich <jbeulich@xxxxxxxx>]

On 30.11.2020 11:31, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> 
> The IOREQ is a common feature now and this struct will be used
> on Arm as is. Move it to common struct domain. This also
> significantly reduces the layering violation in the common code
> (*arch.hvm* usage).
> 
> We don't move ioreq_gfn since it is not used in the common code
> (the "legacy" mechanism is x86 specific).
> 
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>

Applicable parts
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
yet with a question, but maybe more to Paul than to you:

> --- a/xen/include/asm-x86/hvm/domain.h
> +++ b/xen/include/asm-x86/hvm/domain.h
> @@ -63,8 +63,6 @@ struct hvm_pi_ops {
>      void (*vcpu_block)(struct vcpu *);
>  };
>  
> -#define MAX_NR_IOREQ_SERVERS 8
> -
>  struct hvm_domain {
>      /* Guest page range used for non-default ioreq servers */
>      struct {
> @@ -73,12 +71,6 @@ struct hvm_domain {
>          unsigned long legacy_mask; /* indexed by HVM param number */
>      } ioreq_gfn;
>  
> -    /* Lock protects all other values in the sub-struct and the default */
> -    struct {
> -        spinlock_t              lock;
> -        struct ioreq_server *server[MAX_NR_IOREQ_SERVERS];
> -    } ioreq_server;
> -
>      /* Cached CF8 for guest PCI config cycles */
>      uint32_t                pci_cf8;
>  
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -316,6 +316,8 @@ struct sched_unit {
>  
>  struct evtchn_port_ops;
>  
> +#define MAX_NR_IOREQ_SERVERS 8
> +
>  struct domain
>  {
>      domid_t          domain_id;
> @@ -523,6 +525,14 @@ struct domain
>      /* Argo interdomain communication support */
>      struct argo_domain *argo;
>  #endif
> +
> +#ifdef CONFIG_IOREQ_SERVER
> +    /* Lock protects all other values in the sub-struct and the default */
> +    struct {
> +        spinlock_t              lock;
> +        struct ioreq_server     *server[MAX_NR_IOREQ_SERVERS];
> +    } ioreq_server;
> +#endif

The comment gets merely moved, but what "default" does it talk about?
Is this a stale part which would better be dropped at this occasion?

Jan