Re: [PATCH v5 2/7] x86/msi: Extend per-domain/device warning mechanism

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.03.2024 16:16, Marek Marczykowski-Górecki wrote:
> The arch_msix struct had a single "warned" field with a domid for which
> warning was issued. Upcoming patch will need similar mechanism for few
> more warnings, so change it to save a bit field of issued warnings.
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> ---
> Should I add also some helper for the boilerplate the handling requires
> now? I tried some macro that also sets the bit field, but I couldn't get
> it working. I guess I could make it working with a bitmask in a single
> uint8_t - would that be preferred?

Without you providing some hint as to where the problem was, it's hard to
see why something like this couldn't work:

#define MSIX_CHECK_WARN(msix, domid, which) ({ \
    if ( (msix)->warned_domid != (domid) ) \
    { \
        (msix)->warned_domid = (domid); \
        (msix)->warned_kind.all = 0; \
    } \
    (msix)->warned_kind.which ? false : ((msix)->warned_kind.which = true); \
})

> --- a/xen/arch/x86/include/asm/msi.h
> +++ b/xen/arch/x86/include/asm/msi.h
> @@ -217,7 +217,13 @@ struct arch_msix {
>      int table_idx[MAX_MSIX_TABLE_PAGES];
>      spinlock_t table_lock;
>      bool host_maskall, guest_maskall;
> -    domid_t warned;
> +    domid_t warned_domid;
> +    union {
> +        uint8_t all;
> +        struct {
> +            bool maskall                   : 1;
> +        } u;

No need for giving this struct a name, I don't think.

Jan