agpgart: when telling user you'll corrupt his data, at least do it at KERN_CRIT - Kernel

This is a discussion on agpgart: when telling user you'll corrupt his data, at least do it at KERN_CRIT - Kernel ; IOMMU off means very bad stuff may happen, like data corruption on your hard drives. At least tells users this is serious... Plus, agp_amd64_init should go into .h file. Signed-off-by: Pavel Machek diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/pci-gart_64.c index faf3229..19e4f74 100644 --- ...

+ Reply to Thread
Results 1 to 12 of 12

Thread: agpgart: when telling user you'll corrupt his data, at least do it at KERN_CRIT

  1. agpgart: when telling user you'll corrupt his data, at least do it at KERN_CRIT


    IOMMU off means very bad stuff may happen, like data corruption on
    your hard drives. At least tells users this is serious...

    Plus, agp_amd64_init should go into .h file.

    Signed-off-by: Pavel Machek

    diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/pci-gart_64.c
    index faf3229..19e4f74 100644
    --- a/arch/x86/kernel/pci-gart_64.c
    +++ b/arch/x86/kernel/pci-gart_64.c
    @@ -615,13 +615,11 @@ static __init int init_k8_gatt(struct ag

    nommu:
    /* Should not happen anymore */
    - printk(KERN_ERR "PCI-DMA: More than 4GB of RAM and no IOMMU\n"
    - KERN_ERR "PCI-DMA: 32bit PCI IO may malfunction.\n");
    + printk(KERN_CRIT "PCI-DMA: More than 4GB of RAM and no IOMMU\n"
    + KERN_CRIT "PCI-DMA: 32bit PCI IO may malfunction.\n");
    return -1;
    }

    -extern int agp_amd64_init(void);
    -
    static const struct dma_mapping_ops gart_dma_ops = {
    .mapping_error = NULL,
    .map_single = gart_map_single,
    @@ -692,9 +690,9 @@ #endif
    !gart_iommu_aperture ||
    (no_agp && init_k8_gatt(&info) < 0)) {
    if (end_pfn > MAX_DMA32_PFN) {
    - printk(KERN_ERR "WARNING more than 4GB of memory "
    - "but GART IOMMU not available.\n"
    - KERN_ERR "WARNING 32bit PCI may malfunction.\n");
    + printk(KERN_CRIT "More than 4GB of memory "
    + "but GART IOMMU not available.\n"
    + KERN_CRIT "32bit PCI may malfunction.\n");
    }
    return;
    }
    diff --git a/include/asm-x86/agp.h b/include/asm-x86/agp.h
    index e4004a9..657c25b 100644
    --- a/include/asm-x86/agp.h
    +++ b/include/asm-x86/agp.h
    @@ -32,4 +32,7 @@ #define alloc_gatt_pages(order) \
    #define free_gatt_pages(table, order) \
    free_pages((unsigned long)(table), (order))

    +/* Only available in certain configs */
    +extern int agp_amd64_init(void);
    +
    #endif

    --
    (english) http://www.livejournal.com/~pavelmachek
    (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pav...rses/blog.html
    pomozte zachranit klanovicky les: http://www.ujezdskystrom.info/
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: agpgart: when telling user you'll corrupt his data, at least do it at KERN_CRIT

    On Fri, Mar 28, 2008 at 12:25:07PM +0100, Pavel Machek wrote:
    >
    > IOMMU off means very bad stuff may happen, like data corruption on


    Actually swiotlb takes over then in this case. The printk really
    predates the introduction of swiotlb and could be actually removed.
    That is why swiotlb is always SELECTed for gart too.

    > your hard drives. At least tells users this is serious...


    No there shouldn't be any data corruption with the default options.

    Also especially hard disk drivers are expected (and generally do)
    check the dma_map_sg() etc. return values and bail out if the IOMMU
    fails e.g. due to overflow.

    -Andi
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: agpgart: when telling user you'll corrupt his data, at least do it at KERN_CRIT


    * Pavel Machek wrote:

    > IOMMU off means very bad stuff may happen, like data corruption on
    > your hard drives. At least tells users this is serious...


    ouch. Please at minimum lets turn this into a panic(), but best would be
    to trim memory in this case, hm?

    Ingo
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: agpgart: when telling user you'll corrupt his data, at least do it at KERN_CRIT

    On Fri, 28 Mar 2008, Ingo Molnar wrote:

    > > IOMMU off means very bad stuff may happen, like data corruption on
    > > your hard drives. At least tells users this is serious...

    > ouch. Please at minimum lets turn this into a panic(), but best would be
    > to trim memory in this case, hm?


    Are you suggesting to panic on all 64bit machines having more than 4G RAM
    without IOMMU turned on? Why, if swiotlb can still work nicely in such
    situations?

    --
    Jiri Kosina
    SUSE Labs

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: agpgart: when telling user you'll corrupt his data, at least do it at KERN_CRIT


    * Jiri Kosina wrote:

    > On Fri, 28 Mar 2008, Ingo Molnar wrote:
    >
    > > > IOMMU off means very bad stuff may happen, like data corruption on
    > > > your hard drives. At least tells users this is serious...

    > > ouch. Please at minimum lets turn this into a panic(), but best would be
    > > to trim memory in this case, hm?

    >
    > Are you suggesting to panic on all 64bit machines having more than 4G
    > RAM without IOMMU turned on? Why, if swiotlb can still work nicely in
    > such situations?


    no, but we should avoid Pavel's disk corruption scenario ;-)

    Ingo
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: agpgart: when telling user you'll corrupt his data, at least do it at KERN_CRIT

    On Fri 2008-03-28 12:35:28, Ingo Molnar wrote:
    >
    > * Pavel Machek wrote:
    >
    > > IOMMU off means very bad stuff may happen, like data corruption on
    > > your hard drives. At least tells users this is serious...

    >
    > ouch. Please at minimum lets turn this into a panic(), but best would be
    > to trim memory in this case, hm?


    Andi tells me we already fallback to swiotlb, so messages are not on
    wrong loglevel; they contain confusing/obsolete text.

    Pavel
    --
    (english) http://www.livejournal.com/~pavelmachek
    (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pav...rses/blog.html
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  7. Re: agpgart: when telling user you'll corrupt his data, at least do it at KERN_CRIT


    * Pavel Machek wrote:

    > On Fri 2008-03-28 12:35:28, Ingo Molnar wrote:
    > >
    > > * Pavel Machek wrote:
    > >
    > > > IOMMU off means very bad stuff may happen, like data corruption on
    > > > your hard drives. At least tells users this is serious...

    > >
    > > ouch. Please at minimum lets turn this into a panic(), but best would be
    > > to trim memory in this case, hm?

    >
    > Andi tells me we already fallback to swiotlb, so messages are not on
    > wrong loglevel; they contain confusing/obsolete text.


    yes that is the theory - but how did your disk get corrupted in
    practice? ;-)

    Ingo
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  8. Re: agpgart: when telling user you'll corrupt his data, at least do it at KERN_CRIT

    > yes that is the theory - but how did your disk get corrupted in
    > practice? ;-)


    I suspect after wakeup due to the wrong ordering in resume (agp resume function
    called too late).

    The IOMMU code unfortunately doesn't know that the GART has gone away.

    The right fix for that is probably to move the GART resume code into
    some shared file and make it a sysdev.

    -Andi

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  9. Re: agpgart: when telling user you'll corrupt his data, at least do it at KERN_CRIT

    On Fri 2008-03-28 15:13:48, Ingo Molnar wrote:
    >
    > * Pavel Machek wrote:
    >
    > > On Fri 2008-03-28 12:35:28, Ingo Molnar wrote:
    > > >
    > > > * Pavel Machek wrote:
    > > >
    > > > > IOMMU off means very bad stuff may happen, like data corruption on
    > > > > your hard drives. At least tells users this is serious...
    > > >
    > > > ouch. Please at minimum lets turn this into a panic(), but best would be
    > > > to trim memory in this case, hm?

    > >
    > > Andi tells me we already fallback to swiotlb, so messages are not on
    > > wrong loglevel; they contain confusing/obsolete text.

    >
    > yes that is the theory - but how did your disk get corrupted in
    > practice? ;-)


    Well, if you try suspend/resume, without iommu suspend/resume support
    (which is not there in some cases), you get nasty data
    corruption. ("PCI DMA will does not work").

    Then, I saw [obsolete] printk(KERN_ERR) that told me that PCI DMA will
    not work, and happily continued to boot.

    I'll fix the messages.
    Pavel
    --
    (english) http://www.livejournal.com/~pavelmachek
    (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pav...rses/blog.html
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  10. agpgart: scary messages are fortunately obsolete


    Fix obsolete printks in aperture-64. We used not to handle missing
    agpgart, but we handle it okay now.

    Signed-off-by: Pavel Machek

    diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/pci-gart_64.c
    index faf3229..3939c6d 100644
    --- a/arch/x86/kernel/pci-gart_64.c
    +++ b/arch/x86/kernel/pci-gart_64.c
    @@ -615,13 +615,13 @@ static __init int init_k8_gatt(struct ag

    nommu:
    /* Should not happen anymore */
    - printk(KERN_ERR "PCI-DMA: More than 4GB of RAM and no IOMMU\n"
    - KERN_ERR "PCI-DMA: 32bit PCI IO may malfunction.\n");
    + printk(KERN_WARNING "PCI-DMA: More than 4GB of RAM and no IOMMU\n"
    + KERN_WARNING "falling back to iommu=soft.\n");
    return -1;
    }

    extern int agp_amd64_init(void);

    static const struct dma_mapping_ops gart_dma_ops = {
    .mapping_error = NULL,
    .map_single = gart_map_single,
    @@ -692,9 +690,9 @@ #endif
    !gart_iommu_aperture ||
    (no_agp && init_k8_gatt(&info) < 0)) {
    if (end_pfn > MAX_DMA32_PFN) {
    - printk(KERN_ERR "WARNING more than 4GB of memory "
    - "but GART IOMMU not available.\n"
    - KERN_ERR "WARNING 32bit PCI may malfunction.\n");
    + printk(KERN_WARNING "More than 4GB of memory "
    + "but GART IOMMU not available.\n"
    + KERN_WARNING "falling back to iommu=soft.\n");
    }
    return;
    }

    --
    (english) http://www.livejournal.com/~pavelmachek
    (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pav...rses/blog.html
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  11. Re: agpgart: scary messages are fortunately obsolete

    > /* Should not happen anymore */
    > - printk(KERN_ERR "PCI-DMA: More than 4GB of RAM and no IOMMU\n"
    > - KERN_ERR "PCI-DMA: 32bit PCI IO may malfunction.\n");
    > + printk(KERN_WARNING "PCI-DMA: More than 4GB of RAM and no IOMMU\n"
    > + KERN_WARNING "falling back to iommu=soft.\n");


    It might also fall back to another IOMMU (Calgary, Intel, ...)
    soft is just last resort.

    You can probably just remove it.

    -Andi
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  12. Re: agpgart: scary messages are fortunately obsolete


    * Pavel Machek wrote:

    > Fix obsolete printks in aperture-64. We used not to handle missing
    > agpgart, but we handle it okay now.


    thanks Pavel, applied.

    Ingo
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread