Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 3 Sep 2009 22:25:49 +0200
From:      Marko Zec <zec@icir.org>
To:        "Jung-uk Kim" <jkim@freebsd.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r196769 - in head/sys: amd64/amd64 i386/i386
Message-ID:  <200909032225.50210.zec@icir.org>
In-Reply-To: <200909021602.n82G2mpE000812@svn.freebsd.org>
References:  <200909021602.n82G2mpE000812@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wednesday 02 September 2009 18:02:48 Jung-uk Kim wrote:
> Author: jkim
> Date: Wed Sep  2 16:02:48 2009
> New Revision: 196769
> URL: http://svn.freebsd.org/changeset/base/196769
>
> Log:
>   - Work around ACPI mode transition problem for recent NVIDIA 9400M
> chipset based Intel Macs.  Since r189055, these platforms started freezing
> when ACPI is being initialized for unknown reason.  For these platforms, we
> just use the old PAT layout.  Note this change is not enough to boot fully
> on these platforms because of other problems but it makes debugging
> possible. Note MacBook5,2 may be affected as well but it was not added here
> because of lack of hardware to test.
>   - Initialize PAT MSR fully instead of reading and modifying it for
> safety.

Perhaps I'm missing something, but I fail to understand what is the purpose / 
use of variable pat_tested here?

Marko


>   Reported by:	rpaulo, hps, Eygene Ryabinkin (rea-fbsd at codelabs dot ru)
>   Reviewed by:	jhb
>
> Modified:
>   head/sys/amd64/amd64/pmap.c
>   head/sys/i386/i386/pmap.c
>
> Modified: head/sys/amd64/amd64/pmap.c
> ===========================================================================
>=== --- head/sys/amd64/amd64/pmap.c	Wed Sep  2 14:56:23 2009	(r196768) +++
> head/sys/amd64/amd64/pmap.c	Wed Sep  2 16:02:48 2009	(r196769) @@ -178,6
> +178,8 @@ static vm_paddr_t dmaplimit;
>  vm_offset_t kernel_vm_end = VM_MIN_KERNEL_ADDRESS;
>  pt_entry_t pg_nx;
>
> +static int pat_works = 0;		/* Is page attribute table sane? */
> +
>  SYSCTL_NODE(_vm, OID_AUTO, pmap, CTLFLAG_RD, 0, "VM/pmap parameters");
>
>  static int pg_ps_enabled = 1;
> @@ -590,20 +592,56 @@ void
>  pmap_init_pat(void)
>  {
>  	uint64_t pat_msr;
> +	char *sysenv;
> +	static int pat_tested = 0;
>
>  	/* Bail if this CPU doesn't implement PAT. */
>  	if (!(cpu_feature & CPUID_PAT))
>  		panic("no PAT??");
>
>  	/*
> -	 * Leave the indices 0-3 at the default of WB, WT, UC, and UC-.
> -	 * Program 4 and 5 as WP and WC.
> -	 * Leave 6 and 7 as UC and UC-.
> -	 */
> -	pat_msr = rdmsr(MSR_PAT);
> -	pat_msr &= ~(PAT_MASK(4) | PAT_MASK(5));
> -	pat_msr |= PAT_VALUE(4, PAT_WRITE_PROTECTED) |
> -	    PAT_VALUE(5, PAT_WRITE_COMBINING);
> +	 * Some Apple Macs based on nVidia chipsets cannot enter ACPI mode
> +	 * via SMI# when we use upper 4 PAT entries for unknown reason.
> +	 */
> +	if (!pat_tested) {
> +		pat_works = 1;
> +		sysenv = getenv("smbios.system.product");
> +		if (sysenv != NULL) {
> +			if (strncmp(sysenv, "MacBook5,1", 10) == 0 ||
> +			    strncmp(sysenv, "MacBookPro5,5", 13) == 0 ||
> +			    strncmp(sysenv, "Macmini3,1", 10) == 0)
> +				pat_works = 0;
> +			freeenv(sysenv);
> +		}
> +		pat_tested = 1;
> +	}
> +
> +	/* Initialize default PAT entries. */
> +	pat_msr = PAT_VALUE(0, PAT_WRITE_BACK) |
> +	    PAT_VALUE(1, PAT_WRITE_THROUGH) |
> +	    PAT_VALUE(2, PAT_UNCACHED) |
> +	    PAT_VALUE(3, PAT_UNCACHEABLE) |
> +	    PAT_VALUE(4, PAT_WRITE_BACK) |
> +	    PAT_VALUE(5, PAT_WRITE_THROUGH) |
> +	    PAT_VALUE(6, PAT_UNCACHED) |
> +	    PAT_VALUE(7, PAT_UNCACHEABLE);
> +
> +	if (pat_works) {
> +		/*
> +		 * Leave the indices 0-3 at the default of WB, WT, UC, and UC-.
> +		 * Program 4 and 5 as WP and WC.
> +		 * Leave 6 and 7 as UC and UC-.
> +		 */
> +		pat_msr &= ~(PAT_MASK(4) | PAT_MASK(5));
> +		pat_msr |= PAT_VALUE(4, PAT_WRITE_PROTECTED) |
> +		    PAT_VALUE(5, PAT_WRITE_COMBINING);
> +	} else {
> +		/*
> +		 * Just replace PAT Index 2 with WC instead of UC-.
> +		 */
> +		pat_msr &= ~PAT_MASK(2);
> +		pat_msr |= PAT_VALUE(2, PAT_WRITE_COMBINING);
> +	}
>  	wrmsr(MSR_PAT, pat_msr);
>  }
>
> @@ -754,27 +792,48 @@ pmap_cache_bits(int mode, boolean_t is_p
>  	pat_flag = is_pde ? PG_PDE_PAT : PG_PTE_PAT;
>
>  	/* Map the caching mode to a PAT index. */
> -	switch (mode) {
> -	case PAT_UNCACHEABLE:
> -		pat_index = 3;
> -		break;
> -	case PAT_WRITE_THROUGH:
> -		pat_index = 1;
> -		break;
> -	case PAT_WRITE_BACK:
> -		pat_index = 0;
> -		break;
> -	case PAT_UNCACHED:
> -		pat_index = 2;
> -		break;
> -	case PAT_WRITE_COMBINING:
> -		pat_index = 5;
> -		break;
> -	case PAT_WRITE_PROTECTED:
> -		pat_index = 4;
> -		break;
> -	default:
> -		panic("Unknown caching mode %d\n", mode);
> +	if (pat_works) {
> +		switch (mode) {
> +		case PAT_UNCACHEABLE:
> +			pat_index = 3;
> +			break;
> +		case PAT_WRITE_THROUGH:
> +			pat_index = 1;
> +			break;
> +		case PAT_WRITE_BACK:
> +			pat_index = 0;
> +			break;
> +		case PAT_UNCACHED:
> +			pat_index = 2;
> +			break;
> +		case PAT_WRITE_COMBINING:
> +			pat_index = 5;
> +			break;
> +		case PAT_WRITE_PROTECTED:
> +			pat_index = 4;
> +			break;
> +		default:
> +			panic("Unknown caching mode %d\n", mode);
> +		}
> +	} else {
> +		switch (mode) {
> +		case PAT_UNCACHED:
> +		case PAT_UNCACHEABLE:
> +		case PAT_WRITE_PROTECTED:
> +			pat_index = 3;
> +			break;
> +		case PAT_WRITE_THROUGH:
> +			pat_index = 1;
> +			break;
> +		case PAT_WRITE_BACK:
> +			pat_index = 0;
> +			break;
> +		case PAT_WRITE_COMBINING:
> +			pat_index = 2;
> +			break;
> +		default:
> +			panic("Unknown caching mode %d\n", mode);
> +		}
>  	}
>
>  	/* Map the 3-bit index value into the PAT, PCD, and PWT bits. */
>
> Modified: head/sys/i386/i386/pmap.c
> ===========================================================================
>=== --- head/sys/i386/i386/pmap.c	Wed Sep  2 14:56:23 2009	(r196768)
> +++ head/sys/i386/i386/pmap.c	Wed Sep  2 16:02:48 2009	(r196769)
> @@ -212,7 +212,7 @@ pt_entry_t pg_nx;
>  static uma_zone_t pdptzone;
>  #endif
>
> -static int pat_works;			/* Is page attribute table sane? */
> +static int pat_works = 0;		/* Is page attribute table sane? */
>
>  SYSCTL_NODE(_vm, OID_AUTO, pmap, CTLFLAG_RD, 0, "VM/pmap parameters");
>
> @@ -478,40 +478,69 @@ void
>  pmap_init_pat(void)
>  {
>  	uint64_t pat_msr;
> +	char *sysenv;
> +	static int pat_tested = 0;
>
>  	/* Bail if this CPU doesn't implement PAT. */
>  	if (!(cpu_feature & CPUID_PAT))
>  		return;
>
> -	if (cpu_vendor_id != CPU_VENDOR_INTEL ||
> -	    (I386_CPU_FAMILY(cpu_id) == 6 && I386_CPU_MODEL(cpu_id) >= 0xe)) {
> +	/*
> +	 * Due to some Intel errata, we can only safely use the lower 4
> +	 * PAT entries.
> +	 *
> +	 *   Intel Pentium III Processor Specification Update
> +	 * Errata E.27 (Upper Four PAT Entries Not Usable With Mode B
> +	 * or Mode C Paging)
> +	 *
> +	 *   Intel Pentium IV  Processor Specification Update
> +	 * Errata N46 (PAT Index MSB May Be Calculated Incorrectly)
> +	 *
> +	 * Some Apple Macs based on nVidia chipsets cannot enter ACPI mode
> +	 * via SMI# when we use upper 4 PAT entries for unknown reason.
> +	 */
> +	if (!pat_tested) {
> +		if (cpu_vendor_id != CPU_VENDOR_INTEL ||
> +		    (I386_CPU_FAMILY(cpu_id) == 6 &&
> +		    I386_CPU_MODEL(cpu_id) >= 0xe)) {
> +			pat_works = 1;
> +			sysenv = getenv("smbios.system.product");
> +			if (sysenv != NULL) {
> +				if (strncmp(sysenv, "MacBook5,1", 10) == 0 ||
> +				    strncmp(sysenv, "MacBookPro5,5", 13) == 0 ||
> +				    strncmp(sysenv, "Macmini3,1", 10) == 0)
> +					pat_works = 0;
> +				freeenv(sysenv);
> +			}
> +		}
> +		pat_tested = 1;
> +	}
> +
> +	/* Initialize default PAT entries. */
> +	pat_msr = PAT_VALUE(0, PAT_WRITE_BACK) |
> +	    PAT_VALUE(1, PAT_WRITE_THROUGH) |
> +	    PAT_VALUE(2, PAT_UNCACHED) |
> +	    PAT_VALUE(3, PAT_UNCACHEABLE) |
> +	    PAT_VALUE(4, PAT_WRITE_BACK) |
> +	    PAT_VALUE(5, PAT_WRITE_THROUGH) |
> +	    PAT_VALUE(6, PAT_UNCACHED) |
> +	    PAT_VALUE(7, PAT_UNCACHEABLE);
> +
> +	if (pat_works) {
>  		/*
>  		 * Leave the indices 0-3 at the default of WB, WT, UC, and UC-.
>  		 * Program 4 and 5 as WP and WC.
>  		 * Leave 6 and 7 as UC and UC-.
>  		 */
> -		pat_msr = rdmsr(MSR_PAT);
>  		pat_msr &= ~(PAT_MASK(4) | PAT_MASK(5));
>  		pat_msr |= PAT_VALUE(4, PAT_WRITE_PROTECTED) |
>  		    PAT_VALUE(5, PAT_WRITE_COMBINING);
> -		pat_works = 1;
>  	} else {
>  		/*
> -		 * Due to some Intel errata, we can only safely use the lower 4
> -		 * PAT entries.  Thus, just replace PAT Index 2 with WC instead
> -		 * of UC-.
> -		 *
> -		 *   Intel Pentium III Processor Specification Update
> -		 * Errata E.27 (Upper Four PAT Entries Not Usable With Mode B
> -		 * or Mode C Paging)
> -		 *
> -		 *   Intel Pentium IV  Processor Specification Update
> -		 * Errata N46 (PAT Index MSB May Be Calculated Incorrectly)
> +		 * Just replace PAT Index 2 with WC instead of UC-.
>  		 */
> -		pat_msr = rdmsr(MSR_PAT);
>  		pat_msr &= ~PAT_MASK(2);
>  		pat_msr |= PAT_VALUE(2, PAT_WRITE_COMBINING);
> -		pat_works = 0;
>  	}
>  	wrmsr(MSR_PAT, pat_msr);
>  }





Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200909032225.50210.zec>