[PATCH v3] x86/EPT: work around hardware erratum setting A bit

[PATCH v3] x86/EPT: work around hardware erratum setting A bit

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 6406 bytes --]

Since commit 191b3f3344ee ("p2m/ept: enable PML in p2m-ept for
log-dirty"), the A and D bits of EPT paging entries are set
unconditionally, regardless of whether PML is enabled or not. This
causes a regression in Xen 4.6 on some processors due to Intel Errata
AVR41 -- HVM guests get severe memory corruption when the A bit is set
due to incorrect TLB flushing on mov to cr3. The errata affects the Atom
C2000 family (Avoton).

To fix, do not set the A bit on this processor family.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>

Move feature suppression to feature detection code. Add command line
override.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -705,19 +705,28 @@ virtualization, to allow the L1 hypervis
 does not provide VM\_ENTRY\_LOAD\_GUEST\_PAT.
 
 ### ept (Intel)
-> `= List of ( pml<boolean> )`
+> `= List of ( pml | ad )`
+
+Controls EPT related features.
+
+> Sub-options:
+
+> `pml`
 
 > Default: `false`
 
-Controls EPT related features. Currently only Page Modification Logging (PML) is
-the controllable feature as boolean type.
+>> PML is a new hardware feature in Intel's Broadwell Server and further
+>> platforms which reduces hypervisor overhead of log-dirty mechanism by
+>> automatically recording GPAs (guest physical addresses) when guest memory
+>> gets dirty, and therefore significantly reducing number of EPT violation
+>> caused by write protection of guest memory, which is a necessity to
+>> implement log-dirty mechanism before PML.
+
+> `ad`
+
+> Default: Hardware dependent
 
-PML is a new hardware feature in Intel's Broadwell Server and further platforms
-which reduces hypervisor overhead of log-dirty mechanism by automatically
-recording GPAs (guest physical addresses) when guest memory gets dirty, and
-therefore significantly reducing number of EPT violation caused by write
-protection of guest memory, which is a necessity to implement log-dirty
-mechanism before PML.
+>> Have hardware keep accessed/dirty (A/D) bits updated.
 
 ### gdb
 > `= <baud>[/<clock_hz>][,DPS[,<io-base>[,<irq>[,<port-bdf>[,<bridge-bdf>]]]] | pci | amt ] `
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -64,12 +64,14 @@ static unsigned int __read_mostly ple_wi
 integer_param("ple_window", ple_window);
 
 static bool_t __read_mostly opt_pml_enabled = 0;
+static s8 __read_mostly opt_ept_ad = -1;
 
 /*
  * The 'ept' parameter controls functionalities that depend on, or impact the
  * EPT mechanism. Optional comma separated value may contain:
  *
  *  pml                 Enable PML
+ *  ad                  Use A/D bits
  */
 static void __init parse_ept_param(char *s)
 {
@@ -87,6 +89,8 @@ static void __init parse_ept_param(char 
 
         if ( !strcmp(s, "pml") )
             opt_pml_enabled = val;
+        else if ( !strcmp(s, "ad") )
+            opt_ept_ad = val;
 
         s = ss + 1;
     } while ( ss );
@@ -268,6 +272,13 @@ static int vmx_init_vmcs_config(void)
     {
         rdmsrl(MSR_IA32_VMX_EPT_VPID_CAP, _vmx_ept_vpid_cap);
 
+        if ( !opt_ept_ad )
+            _vmx_ept_vpid_cap &= ~VMX_EPT_AD_BIT;
+        else if ( /* Work around Erratum AVR41 on Avoton processors. */
+                  boot_cpu_data.x86 == 6 && boot_cpu_data.x86_model == 0x4d &&
+                  opt_ept_ad < 0 )
+            _vmx_ept_vpid_cap &= ~VMX_EPT_AD_BIT;
+
         /*
          * Additional sanity checking before using EPT:
          * 1) the CPU we are running on must support EPT WB, as we will set
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -130,14 +130,14 @@ static void ept_p2m_type_to_flags(struct
             break;
         case p2m_ram_rw:
             entry->r = entry->w = entry->x = 1;
-            entry->a = entry->d = 1;
+            entry->a = entry->d = !!cpu_has_vmx_ept_ad;
             break;
         case p2m_mmio_direct:
             entry->r = entry->x = 1;
             entry->w = !rangeset_contains_singleton(mmio_ro_ranges,
                                                     entry->mfn);
-            entry->a = 1;
-            entry->d = entry->w;
+            entry->a = !!cpu_has_vmx_ept_ad;
+            entry->d = entry->w && cpu_has_vmx_ept_ad;
             break;
         case p2m_ram_logdirty:
             entry->r = entry->x = 1;
@@ -152,7 +152,7 @@ static void ept_p2m_type_to_flags(struct
                 entry->w = 1;
             else
                 entry->w = 0;
-            entry->a = 1;
+            entry->a = !!cpu_has_vmx_ept_ad;
             /* For both PML or non-PML cases we clear D bit anyway */
             entry->d = 0;
             break;
@@ -160,20 +160,20 @@ static void ept_p2m_type_to_flags(struct
         case p2m_ram_shared:
             entry->r = entry->x = 1;
             entry->w = 0;
-            entry->a = 1;
+            entry->a = !!cpu_has_vmx_ept_ad;
             entry->d = 0;
             break;
         case p2m_grant_map_rw:
         case p2m_map_foreign:
             entry->r = entry->w = 1;
             entry->x = 0;
-            entry->a = entry->d = 1;
+            entry->a = entry->d = !!cpu_has_vmx_ept_ad;
             break;
         case p2m_grant_map_ro:
         case p2m_mmio_write_dm:
             entry->r = 1;
             entry->w = entry->x = 0;
-            entry->a = 1;
+            entry->a = !!cpu_has_vmx_ept_ad;
             entry->d = 0;
             break;
     }
@@ -233,7 +233,7 @@ static int ept_set_middle_entry(struct p
 
     ept_entry->r = ept_entry->w = ept_entry->x = 1;
     /* Manually set A bit to avoid overhead of MMU having to write it later. */
-    ept_entry->a = 1;
+    ept_entry->a = !!cpu_has_vmx_ept_ad;
 
     ept_entry->suppress_ve = 1;
 
--- a/xen/include/asm-x86/hvm/vmx/vmx.h
+++ b/xen/include/asm-x86/hvm/vmx/vmx.h
@@ -262,6 +262,7 @@ extern uint8_t posted_intr_vector;
     (vmx_ept_vpid_cap & VMX_EPT_SUPERPAGE_1GB)
 #define cpu_has_vmx_ept_2mb                     \
     (vmx_ept_vpid_cap & VMX_EPT_SUPERPAGE_2MB)
+#define cpu_has_vmx_ept_ad (vmx_ept_vpid_cap & VMX_EPT_AD_BIT)
 #define cpu_has_vmx_ept_invept_single_context   \
     (vmx_ept_vpid_cap & VMX_EPT_INVEPT_SINGLE_CONTEXT)
 



[-- Attachment #2: x86-EPT-Avoton-erratum-AVR41.patch --]
[-- Type: text/plain, Size: 6457 bytes --]

x86/EPT: work around hardware erratum setting A bit

Since commit 191b3f3344ee ("p2m/ept: enable PML in p2m-ept for
log-dirty"), the A and D bits of EPT paging entries are set
unconditionally, regardless of whether PML is enabled or not. This
causes a regression in Xen 4.6 on some processors due to Intel Errata
AVR41 -- HVM guests get severe memory corruption when the A bit is set
due to incorrect TLB flushing on mov to cr3. The errata affects the Atom
C2000 family (Avoton).

To fix, do not set the A bit on this processor family.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>

Move feature suppression to feature detection code. Add command line
override.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -705,19 +705,28 @@ virtualization, to allow the L1 hypervis
 does not provide VM\_ENTRY\_LOAD\_GUEST\_PAT.
 
 ### ept (Intel)
-> `= List of ( pml<boolean> )`
+> `= List of ( pml | ad )`
+
+Controls EPT related features.
+
+> Sub-options:
+
+> `pml`
 
 > Default: `false`
 
-Controls EPT related features. Currently only Page Modification Logging (PML) is
-the controllable feature as boolean type.
+>> PML is a new hardware feature in Intel's Broadwell Server and further
+>> platforms which reduces hypervisor overhead of log-dirty mechanism by
+>> automatically recording GPAs (guest physical addresses) when guest memory
+>> gets dirty, and therefore significantly reducing number of EPT violation
+>> caused by write protection of guest memory, which is a necessity to
+>> implement log-dirty mechanism before PML.
+
+> `ad`
+
+> Default: Hardware dependent
 
-PML is a new hardware feature in Intel's Broadwell Server and further platforms
-which reduces hypervisor overhead of log-dirty mechanism by automatically
-recording GPAs (guest physical addresses) when guest memory gets dirty, and
-therefore significantly reducing number of EPT violation caused by write
-protection of guest memory, which is a necessity to implement log-dirty
-mechanism before PML.
+>> Have hardware keep accessed/dirty (A/D) bits updated.
 
 ### gdb
 > `= <baud>[/<clock_hz>][,DPS[,<io-base>[,<irq>[,<port-bdf>[,<bridge-bdf>]]]] | pci | amt ] `
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -64,12 +64,14 @@ static unsigned int __read_mostly ple_wi
 integer_param("ple_window", ple_window);
 
 static bool_t __read_mostly opt_pml_enabled = 0;
+static s8 __read_mostly opt_ept_ad = -1;
 
 /*
  * The 'ept' parameter controls functionalities that depend on, or impact the
  * EPT mechanism. Optional comma separated value may contain:
  *
  *  pml                 Enable PML
+ *  ad                  Use A/D bits
  */
 static void __init parse_ept_param(char *s)
 {
@@ -87,6 +89,8 @@ static void __init parse_ept_param(char 
 
         if ( !strcmp(s, "pml") )
             opt_pml_enabled = val;
+        else if ( !strcmp(s, "ad") )
+            opt_ept_ad = val;
 
         s = ss + 1;
     } while ( ss );
@@ -268,6 +272,13 @@ static int vmx_init_vmcs_config(void)
     {
         rdmsrl(MSR_IA32_VMX_EPT_VPID_CAP, _vmx_ept_vpid_cap);
 
+        if ( !opt_ept_ad )
+            _vmx_ept_vpid_cap &= ~VMX_EPT_AD_BIT;
+        else if ( /* Work around Erratum AVR41 on Avoton processors. */
+                  boot_cpu_data.x86 == 6 && boot_cpu_data.x86_model == 0x4d &&
+                  opt_ept_ad < 0 )
+            _vmx_ept_vpid_cap &= ~VMX_EPT_AD_BIT;
+
         /*
          * Additional sanity checking before using EPT:
          * 1) the CPU we are running on must support EPT WB, as we will set
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -130,14 +130,14 @@ static void ept_p2m_type_to_flags(struct
             break;
         case p2m_ram_rw:
             entry->r = entry->w = entry->x = 1;
-            entry->a = entry->d = 1;
+            entry->a = entry->d = !!cpu_has_vmx_ept_ad;
             break;
         case p2m_mmio_direct:
             entry->r = entry->x = 1;
             entry->w = !rangeset_contains_singleton(mmio_ro_ranges,
                                                     entry->mfn);
-            entry->a = 1;
-            entry->d = entry->w;
+            entry->a = !!cpu_has_vmx_ept_ad;
+            entry->d = entry->w && cpu_has_vmx_ept_ad;
             break;
         case p2m_ram_logdirty:
             entry->r = entry->x = 1;
@@ -152,7 +152,7 @@ static void ept_p2m_type_to_flags(struct
                 entry->w = 1;
             else
                 entry->w = 0;
-            entry->a = 1;
+            entry->a = !!cpu_has_vmx_ept_ad;
             /* For both PML or non-PML cases we clear D bit anyway */
             entry->d = 0;
             break;
@@ -160,20 +160,20 @@ static void ept_p2m_type_to_flags(struct
         case p2m_ram_shared:
             entry->r = entry->x = 1;
             entry->w = 0;
-            entry->a = 1;
+            entry->a = !!cpu_has_vmx_ept_ad;
             entry->d = 0;
             break;
         case p2m_grant_map_rw:
         case p2m_map_foreign:
             entry->r = entry->w = 1;
             entry->x = 0;
-            entry->a = entry->d = 1;
+            entry->a = entry->d = !!cpu_has_vmx_ept_ad;
             break;
         case p2m_grant_map_ro:
         case p2m_mmio_write_dm:
             entry->r = 1;
             entry->w = entry->x = 0;
-            entry->a = 1;
+            entry->a = !!cpu_has_vmx_ept_ad;
             entry->d = 0;
             break;
     }
@@ -233,7 +233,7 @@ static int ept_set_middle_entry(struct p
 
     ept_entry->r = ept_entry->w = ept_entry->x = 1;
     /* Manually set A bit to avoid overhead of MMU having to write it later. */
-    ept_entry->a = 1;
+    ept_entry->a = !!cpu_has_vmx_ept_ad;
 
     ept_entry->suppress_ve = 1;
 
--- a/xen/include/asm-x86/hvm/vmx/vmx.h
+++ b/xen/include/asm-x86/hvm/vmx/vmx.h
@@ -262,6 +262,7 @@ extern uint8_t posted_intr_vector;
     (vmx_ept_vpid_cap & VMX_EPT_SUPERPAGE_1GB)
 #define cpu_has_vmx_ept_2mb                     \
     (vmx_ept_vpid_cap & VMX_EPT_SUPERPAGE_2MB)
+#define cpu_has_vmx_ept_ad (vmx_ept_vpid_cap & VMX_EPT_AD_BIT)
 #define cpu_has_vmx_ept_invept_single_context   \
     (vmx_ept_vpid_cap & VMX_EPT_INVEPT_SINGLE_CONTEXT)
 

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v3] x86/EPT: work around hardware erratum setting A bit

[Andrew Cooper]

On 30/09/15 12:36, Jan Beulich wrote:
> Since commit 191b3f3344ee ("p2m/ept: enable PML in p2m-ept for
> log-dirty"), the A and D bits of EPT paging entries are set
> unconditionally, regardless of whether PML is enabled or not. This
> causes a regression in Xen 4.6 on some processors due to Intel Errata
> AVR41 -- HVM guests get severe memory corruption when the A bit is set
> due to incorrect TLB flushing on mov to cr3. The errata affects the Atom
> C2000 family (Avoton).
>
> To fix, do not set the A bit on this processor family.
>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
>
> Move feature suppression to feature detection code. Add command line
> override.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -705,19 +705,28 @@ virtualization, to allow the L1 hypervis
>  does not provide VM\_ENTRY\_LOAD\_GUEST\_PAT.
>  
>  ### ept (Intel)
> -> `= List of ( pml<boolean> )`
> +> `= List of ( pml | ad )`

Please keep the type annotations.  Future sub-options might not be
boolean parameters.

Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

The setup of vmx features looks ripe for some future cleanup, allowing
quite a few bits of data to move from __read_mostly into __initdata. 
However, the patch does match the prevailing style so should be fixed in
this way, given the proximity to the 4.6 release.

~Andrew

Re: [PATCH v3] x86/EPT: work around hardware erratum setting A bit

[Jan Beulich]

>>> On 30.09.15 at 13:47, <andrew.cooper3@citrix.com> wrote:
> On 30/09/15 12:36, Jan Beulich wrote:
>> Since commit 191b3f3344ee ("p2m/ept: enable PML in p2m-ept for
>> log-dirty"), the A and D bits of EPT paging entries are set
>> unconditionally, regardless of whether PML is enabled or not. This
>> causes a regression in Xen 4.6 on some processors due to Intel Errata
>> AVR41 -- HVM guests get severe memory corruption when the A bit is set
>> due to incorrect TLB flushing on mov to cr3. The errata affects the Atom
>> C2000 family (Avoton).
>>
>> To fix, do not set the A bit on this processor family.
>>
>> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
>>
>> Move feature suppression to feature detection code. Add command line
>> override.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> --- a/docs/misc/xen-command-line.markdown
>> +++ b/docs/misc/xen-command-line.markdown
>> @@ -705,19 +705,28 @@ virtualization, to allow the L1 hypervis
>>  does not provide VM\_ENTRY\_LOAD\_GUEST\_PAT.
>>  
>>  ### ept (Intel)
>> -> `= List of ( pml<boolean> )`
>> +> `= List of ( pml | ad )`
> 
> Please keep the type annotations.  Future sub-options might not be
> boolean parameters.

None of the three other examples (efi, iommu, and pci) have such
annotations. If anything, I'd lean towards the way pci is being
described.

> Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Let me know.

Jan

Re: [PATCH v3] x86/EPT: work around hardware erratum setting A bit

[Andrew Cooper]

On 30/09/15 13:02, Jan Beulich wrote:
>>>> On 30.09.15 at 13:47, <andrew.cooper3@citrix.com> wrote:
>> On 30/09/15 12:36, Jan Beulich wrote:
>>> Since commit 191b3f3344ee ("p2m/ept: enable PML in p2m-ept for
>>> log-dirty"), the A and D bits of EPT paging entries are set
>>> unconditionally, regardless of whether PML is enabled or not. This
>>> causes a regression in Xen 4.6 on some processors due to Intel Errata
>>> AVR41 -- HVM guests get severe memory corruption when the A bit is set
>>> due to incorrect TLB flushing on mov to cr3. The errata affects the Atom
>>> C2000 family (Avoton).
>>>
>>> To fix, do not set the A bit on this processor family.
>>>
>>> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
>>>
>>> Move feature suppression to feature detection code. Add command line
>>> override.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>
>>> --- a/docs/misc/xen-command-line.markdown
>>> +++ b/docs/misc/xen-command-line.markdown
>>> @@ -705,19 +705,28 @@ virtualization, to allow the L1 hypervis
>>>  does not provide VM\_ENTRY\_LOAD\_GUEST\_PAT.
>>>  
>>>  ### ept (Intel)
>>> -> `= List of ( pml<boolean> )`
>>> +> `= List of ( pml | ad )`
>> Please keep the type annotations.  Future sub-options might not be
>> boolean parameters.
> None of the three other examples (efi, iommu, and pci) have such
> annotations. If anything, I'd lean towards the way pci is being
> described.

Oh - they are missing.  Going with the PCI route seems like a good way
forwards.

>
>> Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Let me know.

Given I was mistaken about the other tags, consider this an
unconditional review, although a modification towards the PCI route
would be nice.

~Andrew

Re: [PATCH v3] x86/EPT: work around hardware erratum setting A bit

[Wei Liu]

On Wed, Sep 30, 2015 at 05:36:22AM -0600, Jan Beulich wrote:
> Since commit 191b3f3344ee ("p2m/ept: enable PML in p2m-ept for
> log-dirty"), the A and D bits of EPT paging entries are set
> unconditionally, regardless of whether PML is enabled or not. This
> causes a regression in Xen 4.6 on some processors due to Intel Errata
> AVR41 -- HVM guests get severe memory corruption when the A bit is set
> due to incorrect TLB flushing on mov to cr3. The errata affects the Atom
> C2000 family (Avoton).
> 
> To fix, do not set the A bit on this processor family.
> 
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> 
> Move feature suppression to feature detection code. Add command line
> override.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 

Release-acked-by: Wei Liu <wei.liu2@citrix.com>

Thanks for handling this issue!

Re: [PATCH v3] x86/EPT: work around hardware erratum setting A bit

[Wei Liu]

On Wed, Sep 30, 2015 at 01:25:49PM +0100, Wei Liu wrote:
> On Wed, Sep 30, 2015 at 05:36:22AM -0600, Jan Beulich wrote:
> > Since commit 191b3f3344ee ("p2m/ept: enable PML in p2m-ept for
> > log-dirty"), the A and D bits of EPT paging entries are set
> > unconditionally, regardless of whether PML is enabled or not. This
> > causes a regression in Xen 4.6 on some processors due to Intel Errata
> > AVR41 -- HVM guests get severe memory corruption when the A bit is set
> > due to incorrect TLB flushing on mov to cr3. The errata affects the Atom
> > C2000 family (Avoton).
> > 
> > To fix, do not set the A bit on this processor family.
> > 
> > Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> > 
> > Move feature suppression to feature detection code. Add command line
> > override.
> > 
> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
> > 
> 
> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
> 
> Thanks for handling this issue!

In light of both the author and vmx maintainer are on vacation until
October 8, I think we might as well commit this today.

Kevin and Kai, when you're back, please have a look at this patch. And,
if you disagree with the approach, please provide a patch to be
backported to 4.6.1.

Wei.

Re: [PATCH v3] x86/EPT: work around hardware erratum setting A bit

[Andrew Cooper]

On 30/09/15 12:47, Andrew Cooper wrote:
> On 30/09/15 12:36, Jan Beulich wrote:
>> Since commit 191b3f3344ee ("p2m/ept: enable PML in p2m-ept for
>> log-dirty"), the A and D bits of EPT paging entries are set
>> unconditionally, regardless of whether PML is enabled or not. This
>> causes a regression in Xen 4.6 on some processors due to Intel Errata
>> AVR41 -- HVM guests get severe memory corruption when the A bit is set
>> due to incorrect TLB flushing on mov to cr3. The errata affects the Atom
>> C2000 family (Avoton).
>>
>> To fix, do not set the A bit on this processor family.
>>
>> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
>>
>> Move feature suppression to feature detection code. Add command line
>> override.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> --- a/docs/misc/xen-command-line.markdown
>> +++ b/docs/misc/xen-command-line.markdown
>> @@ -705,19 +705,28 @@ virtualization, to allow the L1 hypervis
>>  does not provide VM\_ENTRY\_LOAD\_GUEST\_PAT.
>>  
>>  ### ept (Intel)
>> -> `= List of ( pml<boolean> )`
>> +> `= List of ( pml | ad )`
> Please keep the type annotations.  Future sub-options might not be
> boolean parameters.
>
> Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> The setup of vmx features looks ripe for some future cleanup, allowing
> quite a few bits of data to move from __read_mostly into __initdata. 
> However, the patch does match the prevailing style so should be fixed in
> this way, given the proximity to the 4.6 release.

I can no longer reproduce the issue, given this patch.

Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>

Re: [PATCH v3] x86/EPT: work around hardware erratum setting A bit

[Wei Liu]

On Fri, Oct 02, 2015 at 11:29:54AM +0100, Andrew Cooper wrote:
> On 30/09/15 12:47, Andrew Cooper wrote:
> > On 30/09/15 12:36, Jan Beulich wrote:
> >> Since commit 191b3f3344ee ("p2m/ept: enable PML in p2m-ept for
> >> log-dirty"), the A and D bits of EPT paging entries are set
> >> unconditionally, regardless of whether PML is enabled or not. This
> >> causes a regression in Xen 4.6 on some processors due to Intel Errata
> >> AVR41 -- HVM guests get severe memory corruption when the A bit is set
> >> due to incorrect TLB flushing on mov to cr3. The errata affects the Atom
> >> C2000 family (Avoton).
> >>
> >> To fix, do not set the A bit on this processor family.
> >>
> >> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> >>
> >> Move feature suppression to feature detection code. Add command line
> >> override.
> >>
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >>
> >> --- a/docs/misc/xen-command-line.markdown
> >> +++ b/docs/misc/xen-command-line.markdown
> >> @@ -705,19 +705,28 @@ virtualization, to allow the L1 hypervis
> >>  does not provide VM\_ENTRY\_LOAD\_GUEST\_PAT.
> >>  
> >>  ### ept (Intel)
> >> -> `= List of ( pml<boolean> )`
> >> +> `= List of ( pml | ad )`
> > Please keep the type annotations.  Future sub-options might not be
> > boolean parameters.
> >
> > Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >
> > The setup of vmx features looks ripe for some future cleanup, allowing
> > quite a few bits of data to move from __read_mostly into __initdata. 
> > However, the patch does match the prevailing style so should be fixed in
> > this way, given the proximity to the 4.6 release.
> 
> I can no longer reproduce the issue, given this patch.
> 
> Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>

Great! Thanks for testing.

Re: [PATCH v3] x86/EPT: work around hardware erratum setting A bit

[Kai Huang]

On 10/02/2015 05:36 PM, Wei Liu wrote:
> On Wed, Sep 30, 2015 at 01:25:49PM +0100, Wei Liu wrote:
>> On Wed, Sep 30, 2015 at 05:36:22AM -0600, Jan Beulich wrote:
>>> Since commit 191b3f3344ee ("p2m/ept: enable PML in p2m-ept for
>>> log-dirty"), the A and D bits of EPT paging entries are set
>>> unconditionally, regardless of whether PML is enabled or not. This
>>> causes a regression in Xen 4.6 on some processors due to Intel Errata
>>> AVR41 -- HVM guests get severe memory corruption when the A bit is set
>>> due to incorrect TLB flushing on mov to cr3. The errata affects the Atom
>>> C2000 family (Avoton).
>>>
>>> To fix, do not set the A bit on this processor family.
>>>
>>> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
>>>
>>> Move feature suppression to feature detection code. Add command line
>>> override.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>
>> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
>>
>> Thanks for handling this issue!
> In light of both the author and vmx maintainer are on vacation until
> October 8, I think we might as well commit this today.
>
> Kevin and Kai, when you're back, please have a look at this patch. And,
> if you disagree with the approach, please provide a patch to be
> backported to 4.6.1.
Hi Wei,

Sorry for late response. Our QA (Xudong and Robert, also copied here) 
has conducted PML testing on Xen 4.6 release (which contains this patch) 
and looks everything works fine.

Thanks,
-Kai
>
> Wei.
>