Re: [PATCH] x86, aperture: Check for GART before accessing GART registers

[Ingo Molnar <mingo@kernel.org>]

* Borislav Petkov <bp@suse.de> wrote:

> On Thu, Apr 02, 2015 at 12:04:21PM -0500, Aravind Gopalakrishnan wrote:
> > >No need - I can amend the local copy I have here.
> 
> Here's what I did:
> 
> ---
> From: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
> Date: Wed, 1 Apr 2015 09:32:08 -0500
> Subject: [PATCH] x86/gart: Check for GART support before accessing GART
>  registers
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> GART registers are not present in newer AMD processors (Fam15h,
> Model 10h and later). So, avoid accesses to GART registers in PCI
> config space by returning early in early_gart_iommu_check() and
> gart_iommu_hole_init() if GART is not available.
> 
> Current code doesn't break on existing processors but there are some
> side effects:
> 
> We get bogus AGP aperture messages which are simply noise on
> GART-less processors:
> 
>   AGP: Node 0: aperture [bus addr 0x00000000-0x01ffffff] (32MB)
>   AGP: Your BIOS doesn't leave a aperture memory hole

hah, someone should fix the typo in that message:

	s/a a/a

>   AGP: Please enable the IOMMU option in the BIOS setup
>   AGP: This costs you 64MB of RAM
>   AGP: Mapping aperture over RAM [mem 0xd4000000-0xd7ffffff]
> 
> We can avoid calling allocate_aperture() and would not have to
> wastefully reserve 64MB of RAM with memblock_reserve(). Also, we can
> avoid having to loop through all PCI buses and devices twice, searching
> for a non-existent AGP bridge if we bail out early.
> 
> Refactor the family check used in amd_nb into an inline function so we

s/amd_nb/amd_nb.c

>  
> +static inline bool amd_gart_present(void)
> +{
> +	/* GART present only on Fam15h upto model 0fh */

s/h u/h, u

> +	if (amd_gart_present())
>  		amd_northbridges.flags |= AMD_NB_GART;
>  
>  	/*
> diff --git a/arch/x86/kernel/aperture_64.c b/arch/x86/kernel/aperture_64.c
> index 76164e173a24..1cb170b06853 100644
> --- a/arch/x86/kernel/aperture_64.c
> +++ b/arch/x86/kernel/aperture_64.c
> @@ -262,7 +262,7 @@ void __init early_gart_iommu_check(void)
>  	u64 aper_base = 0, last_aper_base = 0;
>  	int aper_enabled = 0, last_aper_enabled = 0, last_valid = 0;
>  
> -	if (!early_pci_allowed())
> +	if (!early_pci_allowed() || !amd_gart_present())
>  		return;
>  
>  	/* This is mostly duplicate of iommu_hole_init */
> @@ -356,7 +356,7 @@ int __init gart_iommu_hole_init(void)
>  	int i, node;
>  
>  	if (gart_iommu_aperture_disabled || !fix_aperture ||
> -	    !early_pci_allowed())
> +	    !early_pci_allowed() || !amd_gart_present())
>  		return -ENODEV;

So what happens if !early_pci_allowed() but the GART is present? We'll 
set amd_northbridges.flags |= AMD_NB_GART, but won't run any of the 
setup code in aperture_64.c, right? Is that a valid setup?

Thanks,

	Ingo