RE: [PATCH 2/2] x86/vmx: Support for CPUs without model-specific LBR

2023-01-11 Thread Tian, Kevin
> From: Andrew Cooper 
> Sent: Monday, January 9, 2023 8:08 PM
> 
> Ice Lake (server at least) has both Arch LBR and model-specific LBR.  Sapphire
> Rapids does not have model-specific LBR at all.  I.e. On SPR and later,
> model_specific_lbr will always be NULL, so we must make changes to avoid
> reliably hitting the domain_crash().
> 
> The Arch LBR spec states that CPUs without model-specific LBR implement
> MSR_DBG_CTL.LBR by discarding writes and always returning 0.
> 
> Do this for any CPU for which we lack model-specific LBR information.
> 
> Adjust the now-stale comment, now that the Arch LBR spec has created a
> way to
> signal "no model specific LBR" to guests.
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Kevin Tian 


Re: [PATCH 2/2] x86/vmx: Support for CPUs without model-specific LBR

2023-01-10 Thread Jan Beulich
On 09.01.2023 13:08, Andrew Cooper wrote:
> Ice Lake (server at least) has both Arch LBR and model-specific LBR.  Sapphire
> Rapids does not have model-specific LBR at all.  I.e. On SPR and later,
> model_specific_lbr will always be NULL, so we must make changes to avoid
> reliably hitting the domain_crash().
> 
> The Arch LBR spec states that CPUs without model-specific LBR implement
> MSR_DBG_CTL.LBR by discarding writes and always returning 0.
> 
> Do this for any CPU for which we lack model-specific LBR information.
> 
> Adjust the now-stale comment, now that the Arch LBR spec has created a way to
> signal "no model specific LBR" to guests.
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Jan Beulich 




[PATCH 2/2] x86/vmx: Support for CPUs without model-specific LBR

2023-01-09 Thread Andrew Cooper
Ice Lake (server at least) has both Arch LBR and model-specific LBR.  Sapphire
Rapids does not have model-specific LBR at all.  I.e. On SPR and later,
model_specific_lbr will always be NULL, so we must make changes to avoid
reliably hitting the domain_crash().

The Arch LBR spec states that CPUs without model-specific LBR implement
MSR_DBG_CTL.LBR by discarding writes and always returning 0.

Do this for any CPU for which we lack model-specific LBR information.

Adjust the now-stale comment, now that the Arch LBR spec has created a way to
signal "no model specific LBR" to guests.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Roger Pau Monné 
CC: Wei Liu 
CC: Jun Nakajima 
CC: Kevin Tian 
---
 xen/arch/x86/hvm/vmx/vmx.c | 31 ---
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 17320f9fb267..c76b09391c76 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3555,17 +3555,25 @@ static int cf_check vmx_msr_write_intercept(
 goto gp_fault;
 
 /*
+ * The Arch LBR spec (new in Ice Lake) states that CPUs with no
+ * model-specific LBRs implement MSR_DBG_CTL.LBR by discarding writes
+ * and always returning 0.
+ *
+ * Use this property in all cases where we don't know any
+ * model-specific LBR information, as it matches real hardware
+ * behaviour on post-Ice Lake systems.
+ */
+if ( !model_specific_lbr )
+msr_content &= ~IA32_DEBUGCTLMSR_LBR;
+
+/*
  * When a guest first enables LBR, arrange to save and restore the LBR
  * MSRs and allow the guest direct access.
  *
- * MSR_DEBUGCTL and LBR has existed almost as long as MSRs have
- * existed, and there is no architectural way to hide the feature, or
- * fail the attempt to enable LBR.
- *
- * Unknown host LBR MSRs or hitting -ENOSPC with the guest load/save
- * list are definitely hypervisor bugs, whereas -ENOMEM for allocating
- * the load/save list is simply unlucky (and shouldn't occur with
- * sensible management by the toolstack).
+ * Hitting -ENOSPC with the guest load/save list is definitely a
+ * hypervisor bug, whereas -ENOMEM for allocating the load/save list
+ * is simply unlucky (and shouldn't occur with sensible management by
+ * the toolstack).
  *
  * Either way, there is nothing we can do right now to recover, and
  * the guest won't execute correctly either.  Simply crash the domain
@@ -3576,13 +3584,6 @@ static int cf_check vmx_msr_write_intercept(
 {
 const struct lbr_info *lbr = model_specific_lbr;
 
-if ( unlikely(!lbr) )
-{
-gprintk(XENLOG_ERR, "Unknown Host LBR MSRs\n");
-domain_crash(v->domain);
-return X86EMUL_OKAY;
-}
-
 for ( ; lbr->count; lbr++ )
 {
 unsigned int i;
-- 
2.11.0