From: Tu Dinh <ngoc-tu.dinh@vates.tech>

Signed-off-by: Tu Dinh <ngoc-tu.dinh@vates.tech>
---
 xen/arch/x86/include/asm/msr-index.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/xen/arch/x86/include/asm/msr-index.h b/xen/arch/x86/include/asm/msr-index.h
index 9cdb5b2625..867deab3c6 100644
--- a/xen/arch/x86/include/asm/msr-index.h
+++ b/xen/arch/x86/include/asm/msr-index.h
@@ -304,6 +304,17 @@
 #define MSR_IA32_LASTINTFROMIP		0x000001dd
 #define MSR_IA32_LASTINTTOIP		0x000001de
 
+/* Architectural LBR state MSRs */
+#define MSR_IA32_LASTBRANCH_CTL		0x000014ce
+#define  LASTBRANCH_CTL_LBREN		(1<<0) /* Enable LBR recording */
+#define  LASTBRANCH_CTL_VALID		_AC(0x7f000f, ULL)
+#define MSR_IA32_LASTBRANCH_DEPTH	0x000014cf
+#define MSR_IA32_LER_INFO		0x000001e0
+#define MSR_IA32_LASTBRANCH_0_INFO	0x00001200
+#define MSR_IA32_LASTBRANCH_0_FROM_IP	0x00001500
+#define MSR_IA32_LASTBRANCH_0_TO_IP	0x00001600
+#define MAX_MSR_ARCH_LASTBRANCH_FROM_TO	64
+
 #define MSR_IA32_POWER_CTL		0x000001fc
 
 #define MSR_IA32_MTRR_PHYSBASE(n)   (0x00000200 + 2 * (n))
-- 
2.43.0



Ngoc Tu Dinh | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

On 18.11.2024 09:49, ngoc-tu.dinh@vates.tech wrote:
> From: Tu Dinh <ngoc-tu.dinh@vates.tech>
> 
> Signed-off-by: Tu Dinh <ngoc-tu.dinh@vates.tech>
> ---
>  xen/arch/x86/include/asm/msr-index.h | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/xen/arch/x86/include/asm/msr-index.h b/xen/arch/x86/include/asm/msr-index.h
> index 9cdb5b2625..867deab3c6 100644
> --- a/xen/arch/x86/include/asm/msr-index.h
> +++ b/xen/arch/x86/include/asm/msr-index.h
> @@ -304,6 +304,17 @@
>  #define MSR_IA32_LASTINTFROMIP		0x000001dd
>  #define MSR_IA32_LASTINTTOIP		0x000001de
>  
> +/* Architectural LBR state MSRs */
> +#define MSR_IA32_LASTBRANCH_CTL		0x000014ce

The SDM calls this IA32_LBR_CTL, so we want it to be MSR_LBR_CTL, ...

> +#define  LASTBRANCH_CTL_LBREN		(1<<0) /* Enable LBR recording */
> +#define  LASTBRANCH_CTL_VALID		_AC(0x7f000f, ULL)
> +#define MSR_IA32_LASTBRANCH_DEPTH	0x000014cf

... MSR_LBR_DEPTH, and ...

> +#define MSR_IA32_LER_INFO		0x000001e0
> +#define MSR_IA32_LASTBRANCH_0_INFO	0x00001200
> +#define MSR_IA32_LASTBRANCH_0_FROM_IP	0x00001500
> +#define MSR_IA32_LASTBRANCH_0_TO_IP	0x00001600

... similarly *_LBR_* here.

For these three, as they actually cover groups, in other similar cases we
use function like macros, to ease access to any of the MSRs in the group.
I.e.

#define MSR_LBR_INFO(n) (0x00001200 + (n))

> +#define MAX_MSR_ARCH_LASTBRANCH_FROM_TO	64

Where's this 64 coming from? I understand CPUID permits up to 64, but the
XSAVES area layout covers only up to 32. Is the SDM flawed here?

Jan

On 18.11.2024 09:49, ngoc-tu.dinh@vates.tech wrote:
> --- a/xen/arch/x86/include/asm/msr-index.h
> +++ b/xen/arch/x86/include/asm/msr-index.h
> @@ -304,6 +304,17 @@
>  #define MSR_IA32_LASTINTFROMIP		0x000001dd
>  #define MSR_IA32_LASTINTTOIP		0x000001de
>  
> +/* Architectural LBR state MSRs */
> +#define MSR_IA32_LASTBRANCH_CTL		0x000014ce
> +#define  LASTBRANCH_CTL_LBREN		(1<<0) /* Enable LBR recording */
> +#define  LASTBRANCH_CTL_VALID		_AC(0x7f000f, ULL)
> +#define MSR_IA32_LASTBRANCH_DEPTH	0x000014cf
> +#define MSR_IA32_LER_INFO		0x000001e0
> +#define MSR_IA32_LASTBRANCH_0_INFO	0x00001200
> +#define MSR_IA32_LASTBRANCH_0_FROM_IP	0x00001500
> +#define MSR_IA32_LASTBRANCH_0_TO_IP	0x00001600
> +#define MAX_MSR_ARCH_LASTBRANCH_FROM_TO	64

This is rather NUM than MAX; MAX would be 63.

Jan

On 18.11.2024 09:49, ngoc-tu.dinh@vates.tech wrote:
> --- a/xen/arch/x86/include/asm/msr-index.h
> +++ b/xen/arch/x86/include/asm/msr-index.h
> @@ -304,6 +304,17 @@

Please note this comment around line 250:

/*
 * Legacy MSR constants in need of cleanup.  No new MSRs below this comment.
 */

You want to ...

>  #define MSR_IA32_LASTINTFROMIP		0x000001dd
>  #define MSR_IA32_LASTINTTOIP		0x000001de
>  
> +/* Architectural LBR state MSRs */
> +#define MSR_IA32_LASTBRANCH_CTL		0x000014ce
> +#define  LASTBRANCH_CTL_LBREN		(1<<0) /* Enable LBR recording */
> +#define  LASTBRANCH_CTL_VALID		_AC(0x7f000f, ULL)
> +#define MSR_IA32_LASTBRANCH_DEPTH	0x000014cf
> +#define MSR_IA32_LER_INFO		0x000001e0
> +#define MSR_IA32_LASTBRANCH_0_INFO	0x00001200
> +#define MSR_IA32_LASTBRANCH_0_FROM_IP	0x00001500
> +#define MSR_IA32_LASTBRANCH_0_TO_IP	0x00001600
> +#define MAX_MSR_ARCH_LASTBRANCH_FROM_TO	64

... move your addition up, omit the IA32 infixes, format according to how
other entries there are formatted, and sort numerically (implying these
can't all stay together).

Jan