Re: [Xen-devel] [PATCH v2 3/5] x86: Fix APIC MSR constant names

2018-09-10 Thread Roger Pau Monné
On Wed, Mar 07, 2018 at 06:58:34PM +, Andrew Cooper wrote:
> We currently have MSR_IA32_APICBASE and MSR_IA32_APICBASE_MSR which are
> synonymous from a naming point of view, but refer to very different things.
> 
> Rename the x2APIC MSRs to MSR_X2APIC_*, which are shorter constants and
> visually separate the register function from the generic APIC name.  For the
> case ranges, introduce MSR_X2APIC_LAST, rather than relying on the knowledge
> that there are 0x3ff MSRs architecturally reserved for x2APIC functionality.
> 
> For functionality relating to the APIC_BASE MSR, use MSR_APIC_BASE for the MSR
> itself, but drop the MSR prefix from the other constants to shorten the names.
> In all cases, the fact that we are dealing with the APIC_BASE MSR is obvious
> from the context.
> 
> No functional change (the combined binary is identical).
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Roger Pau Monné 

> diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
> index 2b4014c..07f2209 100644
> --- a/xen/include/asm-x86/msr-index.h
> +++ b/xen/include/asm-x86/msr-index.h
> @@ -312,18 +312,21 @@
>  
>  #define MSR_IA32_TSC_ADJUST  0x003b
>  
> -#define MSR_IA32_APICBASE0x001b
> -#define MSR_IA32_APICBASE_BSP(1<<8)
> -#define MSR_IA32_APICBASE_EXTD   (1<<10)
> -#define MSR_IA32_APICBASE_ENABLE (1<<11)
> -#define MSR_IA32_APICBASE_BASE   0x000ff000ul
> -#define MSR_IA32_APICBASE_MSR   0x800
> -#define MSR_IA32_APICTPR_MSR0x808
> -#define MSR_IA32_APICPPR_MSR0x80a
> -#define MSR_IA32_APICEOI_MSR0x80b
> -#define MSR_IA32_APICTMICT_MSR  0x838
> -#define MSR_IA32_APICTMCCT_MSR  0x839
> -#define MSR_IA32_APICSELF_MSR   0x83f
> +#define MSR_APIC_BASE   0x001b
> +#define APIC_BASE_BSP   (1<<8)
> +#define APIC_BASE_EXTD  (1<<10)
> +#define APIC_BASE_ENABLE(1<<11)
> +#define APIC_BASE_BASE  0x000ff000ul

Maybe those could be indented like:

#define MSR_FOO
#define  FOO_BAR

Thanks, Roger.

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

Re: [Xen-devel] [PATCH v2 3/5] x86: Fix APIC MSR constant names

2018-03-13 Thread Jan Beulich
>>> On 07.03.18 at 19:58,  wrote:
> --- a/xen/include/asm-x86/msr-index.h
> +++ b/xen/include/asm-x86/msr-index.h
> @@ -312,18 +312,21 @@
>  
>  #define MSR_IA32_TSC_ADJUST  0x003b
>  
> -#define MSR_IA32_APICBASE0x001b
> -#define MSR_IA32_APICBASE_BSP(1<<8)
> -#define MSR_IA32_APICBASE_EXTD   (1<<10)
> -#define MSR_IA32_APICBASE_ENABLE (1<<11)
> -#define MSR_IA32_APICBASE_BASE   0x000ff000ul
> -#define MSR_IA32_APICBASE_MSR   0x800
> -#define MSR_IA32_APICTPR_MSR0x808
> -#define MSR_IA32_APICPPR_MSR0x80a
> -#define MSR_IA32_APICEOI_MSR0x80b
> -#define MSR_IA32_APICTMICT_MSR  0x838
> -#define MSR_IA32_APICTMCCT_MSR  0x839
> -#define MSR_IA32_APICSELF_MSR   0x83f
> +#define MSR_APIC_BASE   0x001b
> +#define APIC_BASE_BSP   (1<<8)
> +#define APIC_BASE_EXTD  (1<<10)
> +#define APIC_BASE_ENABLE(1<<11)
> +#define APIC_BASE_BASE  0x000ff000ul

This sounds a little clumsy; how about APIC_BASE_ADDR_MASK?

> +#define MSR_X2APIC_BASE 0x800
> +#define MSR_X2APIC_LAST 0xbff

With "LAST", perhaps also MSR_X2APIC_FIRST (even further
separating it from MSR_APIC_BASE)?

> +#define MSR_X2APIC_TPR  0x808
> +#define MSR_X2APIC_PPR  0x80a
> +#define MSR_X2APIC_EOI  0x80b
> +#define MSR_X2APIC_TMICT0x838
> +#define MSR_X2APIC_TMCCT0x839
> +#define MSR_X2APIC_SELF 0x83f

All surrounding MSR indexes have leading zeros spelled out; would
you mind doing so for the x2APIC ones as well?

I won't insist on any of these though, so with or without any or all
of them
Acked-by: Jan Beulich 

Jan


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

Re: [Xen-devel] [PATCH v2 3/5] x86: Fix APIC MSR constant names

2018-03-07 Thread Tian, Kevin
> From: Andrew Cooper [mailto:andrew.coop...@citrix.com]
> Sent: Thursday, March 8, 2018 2:59 AM
> 
> We currently have MSR_IA32_APICBASE and MSR_IA32_APICBASE_MSR
> which are
> synonymous from a naming point of view, but refer to very different things.
> 
> Rename the x2APIC MSRs to MSR_X2APIC_*, which are shorter constants
> and
> visually separate the register function from the generic APIC name.  For the
> case ranges, introduce MSR_X2APIC_LAST, rather than relying on the
> knowledge
> that there are 0x3ff MSRs architecturally reserved for x2APIC functionality.
> 
> For functionality relating to the APIC_BASE MSR, use MSR_APIC_BASE for
> the MSR
> itself, but drop the MSR prefix from the other constants to shorten the
> names.
> In all cases, the fact that we are dealing with the APIC_BASE MSR is obvious
> from the context.
> 
> No functional change (the combined binary is identical).
> 
> Signed-off-by: Andrew Cooper 
> ---
> CC: Jan Beulich 
> CC: Roger Pau Monné 
> CC: Wei Liu 
> CC: Jun Nakajima 
> CC: Kevin Tian 
> 

Reviewed-by: Kevin Tian 
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2 3/5] x86: Fix APIC MSR constant names

2018-03-07 Thread Konrad Rzeszutek Wilk
On Wed, Mar 07, 2018 at 06:58:34PM +, Andrew Cooper wrote:
> We currently have MSR_IA32_APICBASE and MSR_IA32_APICBASE_MSR which are
> synonymous from a naming point of view, but refer to very different things.
> 
> Rename the x2APIC MSRs to MSR_X2APIC_*, which are shorter constants and
> visually separate the register function from the generic APIC name.  For the
> case ranges, introduce MSR_X2APIC_LAST, rather than relying on the knowledge
> that there are 0x3ff MSRs architecturally reserved for x2APIC functionality.
> 
> For functionality relating to the APIC_BASE MSR, use MSR_APIC_BASE for the MSR
> itself, but drop the MSR prefix from the other constants to shorten the names.
> In all cases, the fact that we are dealing with the APIC_BASE MSR is obvious
> from the context.
> 
> No functional change (the combined binary is identical).
> 
> Signed-off-by: Andrew Cooper 
Reviewed-by: Konrad Rzeszutek Wilk 

Thank you!

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