Re: [Xen-devel] [PATCH 2 of 3] xen/pat: After suspend re-write PAT if BIOS changed it

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 02.04.12 at 22:27, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> wrote:
> # HG changeset patch
> # User Simon Graham <simon.graham@xxxxxxxxxxxxxxxxxxx>
> # Date 1333398413 14400
> # Node ID caefa03c38366c3e70d937bc95740c5d33c0892a
> # Parent  f1da2ce71ed41d1b74ebe6916ff7710d6579438e
> xen/pat: After suspend re-write PAT if BIOS changed it.
> 
> Certain AMD machines (this was a MSI or GigaBYTE BIOS) after resume
> would reset the PAT MSR causing rather weird issues - where
> the pages would (say they would be set to WC) would end up with the
> wrong type (as they would use the BIOS PAT instead of the one set by
> the hypervisor). And in some cases the PAT was stuck and needed
> a couple of WRMSRL to actually take.

But you still fail to explain why the existing PAT resume handling is
insufficient, and without understanding that we shouldn't be applying
a patch like this.

Jan

> Signed-off-by: Simon Graham <simon.graham@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> 
> diff -r f1da2ce71ed4 -r caefa03c3836 xen/arch/x86/acpi/power.c
> --- a/xen/arch/x86/acpi/power.c       Mon Apr 02 16:26:48 2012 -0400
> +++ b/xen/arch/x86/acpi/power.c       Mon Apr 02 16:26:53 2012 -0400
> @@ -41,8 +41,27 @@ static DEFINE_SPINLOCK(pm_lock);
>  
>  struct acpi_sleep_info acpi_sinfo;
>  
> +static void pat_resume(void);
>  void do_suspend_lowlevel(void);
>  
> +static void
> +pat_resume()
> +{
> +    u64 pat;
> +    if (!cpu_has_pat)
> +        return;
> +
> +    rdmsrl(MSR_IA32_CR_PAT, pat);
> +    if (pat != host_pat) {
> +     printk(KERN_INFO PREFIX "Found PAT MSR: 0x%"PRIx64"\n", pat);
> +     printk(KERN_INFO PREFIX "reseting to 0x%"PRIx64"\n", host_pat);
> +     wrmsrl(MSR_IA32_CR_PAT, host_pat);
> +     rdmsrl(MSR_IA32_CR_PAT, pat);
> +     if (pat != host_pat)
> +         printk(KERN_WARNING PREFIX "PAT MSR stuck on: 0x%"PRIx64"\n", pat);
> +    }
> +}
> +
>  static int device_power_down(void)
>  {
>      console_suspend();
> @@ -194,6 +213,7 @@ static int enter_state(u32 state)
>      if ( cpu_has_efer )
>          write_efer(read_efer());
>  
> +    pat_resume();
>      device_power_up();
>  
>      mcheck_init(&boot_cpu_data, 0);




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