lkml.org 
[lkml]   [2009]   [Nov]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH -tip] x86: fix iommu=soft boot option
    FUJITA Tomonori wrote:
    > On Wed, 25 Nov 2009 01:45:10 -0800
    > Yinghai Lu <yinghai@kernel.org> wrote:
    >
    >> Ingo Molnar wrote:
    >>> * FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:
    >>>
    >>>> On Wed, 25 Nov 2009 00:54:34 -0800
    >>>> Yinghai Lu <yinghai@kernel.org> wrote:
    >>>>
    >>>>> only remember that SLES 9 SP3 (?) at some point has problem with AMD
    >>>>> 10h ( quad core version) when memory > 4 g (with USB controller ?)
    >>>>> because the gart code only check K8 device id, and didn't check 10h
    >>>>> device id. so gart iommu is not used. and happenly swiotlb code has
    >>>>> problem with that kernel version.
    >>>>>
    >>>>> thinking we should keep old behavior, until some day we can remove
    >>>>> them all.
    >>>> Why? We are talking about changing 2.6.33 behavior. swiotlb in 2.6.33
    >>>> should be the safe option.
    >>> If that behavior was relied on for suspected (or real) bugs in the
    >>> swiotlb code then i agree that we should do this change. (and fix any
    >>> bugs if they still occur.)
    >> after look at gart_iommu_hole_init() closely,
    >>
    >> should be
    >>
    >> for AMD 64bit, MEM > 4g, no AGP, iommu=soft
    >> 1. if BIOS have correct gart setting, Kernel will use swiotlb
    >> 2. if BIOS does not have correct gart setting, Kernel will use swiotlb
    >>
    >> and for the all the cases, the codes after that
    >> /* Fix up the north bridges */
    >> ...
    >>
    >> will disable the translation...
    >
    > What code are you talking about? GART translation is disabled at boot
    > time (if not, we are dead because some GART hardware are broken so we
    > need to fix things before enabling them).
    >
    >
    >> so even swiotlb=soft is used, gart_iommu_hole_init() still need to
    >> be called. to make sure aperture is disabled somehow.
    >
    > I don't think so.

    in aperture_64.c::gart_iommu_hole_init()
    printk(KERN_INFO "Checking aperture...\n");

    if (!fallback_aper_force)
    agp_aper_base = search_agp_bridge(&agp_aper_order, &valid_agp);

    fix = 0;
    node = 0;
    for (i = 0; i < ARRAY_SIZE(bus_dev_ranges); i++) {
    int bus;
    int dev_base, dev_limit;

    bus = bus_dev_ranges[i].bus;
    dev_base = bus_dev_ranges[i].dev_base;
    dev_limit = bus_dev_ranges[i].dev_limit;

    for (slot = dev_base; slot < dev_limit; slot++) {

    if (!early_is_k8_nb(read_pci_config(bus, slot, 3, 0x00)))
    continue;

    iommu_detected = 1;
    gart_iommu_aperture = 1;

    aper_order = (read_pci_config(bus, slot, 3, AMD64_GARTAPERTURECTL) >> 1) & 7;
    aper_size = (32 * 1024 * 1024) << aper_order;
    aper_base = read_pci_config(bus, slot, 3, AMD64_GARTAPERTUREBASE) & 0x7fff;
    aper_base <<= 25;

    printk(KERN_INFO "Node %d: aperture @ %Lx size %u MB\n",
    node, aper_base, aper_size >> 20);
    node++;

    if (!aperture_valid(aper_base, aper_size, 64<<20)) {
    if (valid_agp && agp_aper_base &&
    agp_aper_base == aper_base &&
    agp_aper_order == aper_order) {
    /* the same between two setting from NB and agp */
    if (!no_iommu &&
    max_pfn > MAX_DMA32_PFN &&
    !printed_gart_size_msg) {
    printk(KERN_ERR "you are using iommu with agp, but GART size is less than 64M\n");
    printk(KERN_ERR "please increase GART size in your BIOS setup\n");
    printk(KERN_ERR "if BIOS doesn't have that option, contact your HW vendor!\n");
    printed_gart_size_msg = 1;
    }
    } else {
    POINT A:
    fix = 1;
    goto out;
    }
    }

    if ((last_aper_order && aper_order != last_aper_order) ||
    (last_aper_base && aper_base != last_aper_base)) {
    fix = 1;
    goto out;
    }
    last_aper_order = aper_order;
    last_aper_base = aper_base;
    }
    }

    out:
    if (!fix && !fallback_aper_force) {
    if (last_aper_base) {
    unsigned long n = (32 * 1024 * 1024) << last_aper_order;

    insert_aperture_resource((u32)last_aper_base, n);
    }
    return;
    }

    if (!fallback_aper_force) {
    aper_alloc = agp_aper_base;
    aper_order = agp_aper_order;
    }

    if (aper_alloc) {
    /* Got the aperture from the AGP bridge */
    } else if (swiotlb && !valid_agp) {
    POINT: B
    /* Do nothing */
    } else if ((!no_iommu && max_pfn > MAX_DMA32_PFN) ||
    force_iommu ||
    valid_agp ||
    fallback_aper_force) {
    POINT: C
    printk(KERN_INFO
    "Your BIOS doesn't leave a aperture memory hole\n");
    printk(KERN_INFO
    "Please enable the IOMMU option in the BIOS setup\n");
    printk(KERN_INFO
    "This costs you %d MB of RAM\n",
    32 << fallback_aper_order);

    aper_order = fallback_aper_order;
    aper_alloc = allocate_aperture();
    if (!aper_alloc) {
    /*
    * Could disable AGP and IOMMU here, but it's
    * probably not worth it. But the later users
    * cannot deal with bad apertures and turning
    * on the aperture over memory causes very
    * strange problems, so it's better to panic
    * early.
    */
    panic("Not enough memory for aperture");
    }
    } else {
    return;
    }

    POINT X:

    /* Fix up the north bridges */
    for (i = 0; i < ARRAY_SIZE(bus_dev_ranges); i++) {
    int bus;
    int dev_base, dev_limit;

    bus = bus_dev_ranges[i].bus;
    dev_base = bus_dev_ranges[i].dev_base;
    dev_limit = bus_dev_ranges[i].dev_limit;
    for (slot = dev_base; slot < dev_limit; slot++) {
    if (!early_is_k8_nb(read_pci_config(bus, slot, 3, 0x00)))
    continue;

    /* Don't enable translation yet. That is done later.
    Assume this BIOS didn't initialise the GART so
    just overwrite all previous bits */
    write_pci_config(bus, slot, 3, AMD64_GARTAPERTURECTL, aper_order << 1);
    write_pci_config(bus, slot, 3, AMD64_GARTAPERTUREBASE, aper_alloc >> 25);
    }
    }

    when AMD64 mem > 4g, no AGP, BIOS set all gart iommu on all nodes size to 32M, and enable bit are set.
    1. iommu=soft, will go through POINT A and POINT B
    2. no "iommu=soft", will go through POINT A and POINT C
    and all will reach POINT X to make sure ENABLE bit is not set.

    please check

    [PATCH] x86: fix gart iommu using for amd 64 bit system -v3

    after
    |commit 75f1cdf1dda92cae037ec848ae63690d91913eac
    |Author: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
    |Date: Tue Nov 10 19:46:20 2009 +0900
    |
    | x86: Handle HW IOMMU initialization failure gracefully
    |
    | If HW IOMMU initialization fails (Intel VT-d often does this,
    | typically due to BIOS bugs), we fall back to nommu. It doesn't
    | work for the majority since nowadays we have more than 4GB
    | memory so we must use swiotlb instead of nommu.
    ...

    amd 64 systems that
    1. do not have AGP
    2. do not have IOMMU
    3. mem > 4g
    4. BIOS do not allocate correct gart in NB.
    will leave them to use SWIOTLB forcely.

    also in pci_iommu_alloc()
    pci_swiotlb_init is stealing the preallocate range that is for
    gart_iommu_hole workaround.

    so restore the sequence...

    will get back
    [ 0.000000] Your BIOS doesn't leave a aperture memory hole
    [ 0.000000] Please enable the IOMMU option in the BIOS setup
    [ 0.000000] This costs you 64 MB of RAM
    [ 0.000000] Mapping aperture over 65536 KB of RAM @ 20000000
    ...
    [ 12.702822] calling pci_iommu_init+0x0/0x31 @ 1
    [ 12.708728] PCI-DMA: Disabling AGP.
    [ 12.712812] PCI-DMA: aperture base @ 20000000 size 65536 KB
    [ 12.718384] PCI-DMA: using GART IOMMU.
    [ 12.722139] PCI-DMA: Reserving 64MB of IOMMU area in the AGP aperture
    [ 12.736944] initcall pci_iommu_init+0x0/0x31 returned 0 after 28802 usecs

    -v2: call shutdown when no agp is there
    -v3: add check_store to restore state for swiotlb
    separate pci_swiotlb_detect and pci_swiotlb_init from FUJITA

    Signed-off-by: Yinghai Lu <yinghai@kernel.org>

    ---
    arch/x86/include/asm/swiotlb.h | 8 ++++++--
    arch/x86/kernel/aperture_64.c | 17 ++++++++++++++---
    arch/x86/kernel/pci-dma.c | 11 +++++++++--
    arch/x86/kernel/pci-gart_64.c | 3 ++-
    arch/x86/kernel/pci-swiotlb.c | 11 +++++++----
    5 files changed, 38 insertions(+), 12 deletions(-)

    Index: linux-2.6/arch/x86/kernel/aperture_64.c
    ===================================================================
    --- linux-2.6.orig/arch/x86/kernel/aperture_64.c
    +++ linux-2.6/arch/x86/kernel/aperture_64.c
    @@ -375,6 +375,9 @@ void __init gart_iommu_hole_init(void)
    u64 aper_base, last_aper_base = 0;
    int fix, slot, valid_agp = 0;
    int i, node;
    + int iommu_detected_old = iommu_detected;
    + int gart_iommu_aperture_old = gart_iommu_aperture;
    + int (*iommu_init_old)(void) = x86_init.iommu.iommu_init;

    if (gart_iommu_aperture_disabled || !fix_aperture ||
    !early_pci_allowed())
    @@ -448,7 +451,7 @@ out:

    insert_aperture_resource((u32)last_aper_base, n);
    }
    - return;
    + goto check_restore;
    }

    if (!fallback_aper_force) {
    @@ -458,7 +461,7 @@ out:

    if (aper_alloc) {
    /* Got the aperture from the AGP bridge */
    - } else if (!valid_agp) {
    + } else if (swiotlb && !valid_agp) {
    /* Do nothing */
    } else if ((!no_iommu && max_pfn > MAX_DMA32_PFN) ||
    force_iommu ||
    @@ -486,7 +489,7 @@ out:
    panic("Not enough memory for aperture");
    }
    } else {
    - return;
    + goto check_restore;
    }

    /* Fix up the north bridges */
    @@ -510,4 +513,12 @@ out:
    }

    set_up_gart_resume(aper_order, aper_alloc);
    +
    +check_restore:
    + if (swiotlb) {
    + /* restore the setting */
    + iommu_detected = iommu_detected_old;
    + gart_iommu_aperture = gart_iommu_aperture_old;
    + x86_init.iommu.iommu_init = iommu_init_old;
    + }
    }
    Index: linux-2.6/arch/x86/kernel/pci-dma.c
    ===================================================================
    --- linux-2.6.orig/arch/x86/kernel/pci-dma.c
    +++ linux-2.6/arch/x86/kernel/pci-dma.c
    @@ -120,21 +120,28 @@ static void __init dma32_free_bootmem(vo

    void __init pci_iommu_alloc(void)
    {
    + int ret;
    +
    #ifdef CONFIG_X86_64
    /* free the range so iommu could get some range less than 4G */
    dma32_free_bootmem();
    #endif
    - if (pci_swiotlb_init())
    - return;
    + ret = pci_swiotlb_detect();

    gart_iommu_hole_init();

    + if (ret)
    + goto out;
    +
    detect_calgary();

    detect_intel_iommu();

    /* needs to be called after gart_iommu_hole_init */
    amd_iommu_detect();
    +
    +out:
    + pci_swiotlb_init();
    }

    void *dma_generic_alloc_coherent(struct device *dev, size_t size,
    Index: linux-2.6/arch/x86/kernel/pci-gart_64.c
    ===================================================================
    --- linux-2.6.orig/arch/x86/kernel/pci-gart_64.c
    +++ linux-2.6/arch/x86/kernel/pci-gart_64.c
    @@ -710,7 +710,8 @@ static void gart_iommu_shutdown(void)
    struct pci_dev *dev;
    int i;

    - if (no_agp)
    + /* don't shutdown it if there is AGP installed */
    + if (!no_agp)
    return;

    for (i = 0; i < num_k8_northbridges; i++) {
    Index: linux-2.6/arch/x86/include/asm/swiotlb.h
    ===================================================================
    --- linux-2.6.orig/arch/x86/include/asm/swiotlb.h
    +++ linux-2.6/arch/x86/include/asm/swiotlb.h
    @@ -5,13 +5,17 @@

    #ifdef CONFIG_SWIOTLB
    extern int swiotlb;
    -extern int pci_swiotlb_init(void);
    +int pci_swiotlb_detect(void);
    +void pci_swiotlb_init(void);
    #else
    #define swiotlb 0
    -static inline int pci_swiotlb_init(void)
    +static inline int pci_swiotlb_detect(void)
    {
    return 0;
    }
    +static inline void pci_swiotlb_init(void)
    +{
    +}
    #endif

    static inline void dma_mark_clean(void *addr, size_t size) {}
    Index: linux-2.6/arch/x86/kernel/pci-swiotlb.c
    ===================================================================
    --- linux-2.6.orig/arch/x86/kernel/pci-swiotlb.c
    +++ linux-2.6/arch/x86/kernel/pci-swiotlb.c
    @@ -43,12 +43,12 @@ static struct dma_map_ops swiotlb_dma_op
    };

    /*
    - * pci_swiotlb_init - initialize swiotlb if necessary
    + * pci_swiotlb_detect - initialize swiotlb if necessary
    *
    * This returns non-zero if we are forced to use swiotlb (by the boot
    * option).
    */
    -int __init pci_swiotlb_init(void)
    +int __init pci_swiotlb_detect(void)
    {
    int use_swiotlb = swiotlb | swiotlb_force;

    @@ -60,10 +60,13 @@ int __init pci_swiotlb_init(void)
    if (swiotlb_force)
    swiotlb = 1;

    + return use_swiotlb;
    +}
    +
    +void __init pci_swiotlb_init(void)
    +{
    if (swiotlb) {
    swiotlb_init(0);
    dma_ops = &swiotlb_dma_ops;
    }
    -
    - return use_swiotlb;
    }



    \
     
     \ /
      Last update: 2009-11-25 23:37    [W:0.056 / U:2.012 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site