netdev - Re: via velocity trivial patch

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4A0AC537.70605@podgorny.cz>
Date:	Wed, 13 May 2009 15:03:51 +0200
From:	Radek Podgorny <radek@...gorny.cz>
To:	netdev@...r.kernel.org
Subject: Re: via velocity trivial patch

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hello,

so I've tried to study the source a bit more and don't really get some things.
Why is there a mac_write_isr() in the IRQ handling routine? Isn't ISR supposed
to be read-only? Even if not, why does it write the exact same value as it has
read just moments ago?

    ...
    isr_status = mac_read_isr(vptr->mac_regs);

    /* Not us ? */
    if (isr_status == 0) {
        spin_unlock(&vptr->lock);
        return IRQ_NONE;
    }

    mac_disable_int(vptr->mac_regs);

    /*
     *  Keep processing the ISR until we have completed
     *  processing and the isr_status becomes zero
     */

    while (isr_status != 0) {
        mac_write_isr(vptr->mac_regs, isr_status);
        ...
        isr_status = mac_read_isr(vptr->mac_regs);
    ...

Maybe the value is mangled on write. But even if it was, it should not affect
other devices, just the one in question, right?

The other way for ISR to show 0xffffffff is in device init but I experience
repetitive all-ones. :-( Will have to test it again to be really sure, though.

Do you have any links to datasheets or something so I can try to look up the specs?
Or, do you have any ideas what I should do next to debug (or help you debug)
the issue?

Thanks in advance
Radek Podgorny


Francois Romieu wrote:
> Radek Podgorny <radek@...gorny.cz> :
> [...]
>> Anyway, I've verified this is really happening on my hardware when
>> multiple via velocity devices share the same interrupt. The one the
>> interrupt is for reads the correct ISR value, the others sometimes read
>> 0 and sometimes 0xffffffff. I'm not educated enough to tell why.
> 
> Your quad port network card includes its own pci bus, right ?
> If so can you send a 'lspci -vt' of the host ?
> 
>> Reasons to include the fix:
>> 1) Someone smarter than me suggested it (Jeff Garzik).
>> 2) Fixed my problem.
>> 3) Does not introduce a new problem.
> 
> 3. probably stands: there is no reason for the driver to see a
> 0xffffffff interupt status register (hardware fault or unplug apart).
> 
> Well, almost:
> 
> $ grep mac.*isr drivers/net/via-velocity.h
> #define mac_read_isr(regs)              readl(&((regs)->ISR))
> #define mac_write_isr(regs, x)          writel((x),&((regs)->ISR))
> #define mac_clear_isr(regs)             writel(0xffffffffL,&((regs)->ISR))
> 
> I can figure that there is a window through which the 2nd, 3rd and 4th
> devices can read a 0xffffffff when they are brought up but it should
> not be noticeable after this stage (and it may happen on the 1st device
> as well if it shares its interrupt with some other device).
> 
> Does it match your experience ?
>> So I'm attaching the patch again. Now with the proper sign-off.
> 
> davem's life is probably simpler if you do not include anything
> after the patch.
> 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkoKxTcACgkQ7mej6pjlbYTLtgCaAvmxl3/FLK4mmrznEiq7+XYD
TzAAnA+EewHvGOdKvwIIMZtisH+AQbsr
=3g6c
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html