Re: [Xen-devel] [PATCH 3/3] xen/x86: Rename and simplify async_event_* infrastructure

[Jan Beulich <jbeulich@xxxxxxxx>]

On 17.02.2020 12:17, Andrew Cooper wrote:
> --- a/xen/arch/x86/pv/iret.c
> +++ b/xen/arch/x86/pv/iret.c
> @@ -27,15 +27,15 @@ static void async_exception_cleanup(struct vcpu *curr)
>  {
>      unsigned int trap;
>  
> -    if ( !curr->arch.async_exception_mask )
> +    if ( !curr->arch.async_event_mask )
>          return;
>  
> -    if ( !(curr->arch.async_exception_mask & 
> (curr->arch.async_exception_mask - 1)) )
> -        trap = __scanbit(curr->arch.async_exception_mask, VCPU_TRAP_NONE);
> +    if ( !(curr->arch.async_event_mask & (curr->arch.async_event_mask - 1)) )
> +        trap = __scanbit(curr->arch.async_event_mask, 0);

The transformation just by itself is clearly not "no functional
change"; it is together with the prior if(), but it took me a
little to convince myself. I don't recall why VCPU_TRAP_NONE was
used here originally (possibly just because of it being zero),
but I think the latest now it would be better to use
VCPU_TRAP_LAST + 1 instead, as 0 now has an actual meaning.

> @@ -557,12 +546,22 @@ struct arch_vcpu
>  
>      struct vpmu_struct vpmu;
>  
> -    struct {
> -        bool    pending;
> -        uint8_t old_mask;
> -    } async_exception_state[VCPU_TRAP_LAST];
> -#define async_exception_state(t) async_exception_state[(t)-1]
> -    uint8_t async_exception_mask;
> +    union {
> +#define VCPU_TRAP_NMI          0
> +#define VCPU_TRAP_MCE          1
> +#define VCPU_TRAP_LAST         VCPU_TRAP_MCE
> +        struct {
> +            bool    pending;
> +            uint8_t old_mask;
> +        } async_event[VCPU_TRAP_LAST + 1];
> +        struct {
> +            bool    nmi_pending;
> +            uint8_t nmi_old_mask;
> +            bool    mce_pending;
> +            uint8_t mce_old_mask;
> +        };
> +    };

How about

    union {
#define VCPU_TRAP_NMI          0
#define VCPU_TRAP_MCE          1
#define VCPU_TRAP_LAST         VCPU_TRAP_MCE
        struct async_event_state {
            bool    pending;
            uint8_t old_mask;
        } async_event[VCPU_TRAP_LAST + 1];
        struct {
            struct async_event_state nmi;
            struct async_event_state mce;
        };
    };

(structure tag subject to improvement)?

Jan

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