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

Ingo Molnar
2015-04-02T18:19:38Z

* 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