netdev - Re: [PATCH v4 1/2] r8169: Dereference MMIO address immediately before use

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: <1519935703.10722.375.camel@linux.intel.com>
Date:   Thu, 01 Mar 2018 22:21:43 +0200
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Heiner Kallweit <hkallweit1@...il.com>, nic_swsd@...ltek.com,
        "David S . Miller" <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH v4 1/2] r8169: Dereference MMIO address immediately
 before use

On Thu, 2018-03-01 at 21:01 +0100, Heiner Kallweit wrote:
> Am 01.03.2018 um 12:27 schrieb Andy Shevchenko:

> > Next step might be a conversion of RTL_Wxx() / RTL_Rxx() macros
> > to inline functions for sake of type checking.

^^^^^

> >  /* write/read MMIO register */
> > -#define RTL_W8(reg, val8)	writeb ((val8), ioaddr + (reg))
> > -#define RTL_W16(reg, val16)	writew ((val16), ioaddr + (reg))
> > -#define RTL_W32(reg, val32)	writel ((val32), ioaddr + (reg))
> > -#define RTL_R8(reg)		readb (ioaddr + (reg))
> > -#define RTL_R16(reg)		readw (ioaddr + (reg))
> > -#define RTL_R32(reg)		readl (ioaddr + (reg))
> > +#define RTL_W8(tp, reg, val8)	writeb((val8), tp->mmio_addr +
> > (reg))
> > +#define RTL_W16(tp, reg, val16)	writew((val16), tp-
> > >mmio_addr + (reg))
> > +#define RTL_W32(tp, reg, val32)	writel((val32), tp-
> > >mmio_addr + (reg))
> > +#define RTL_R8(tp, reg)		readb(tp->mmio_addr + (reg))
> > +#define RTL_R16(tp, reg)		readw(tp->mmio_addr +
> > (reg))
> > +#define RTL_R32(tp, reg)		readl(tp->mmio_addr +
> > (reg))
 
> I like te idea because I also found the frequent definition of
> variable ioaddr
> w/o explicit use quite ugly. Just instead of using macro's we could
> define
> RTL_R32 et al as inline functions.

I left a last paragraph in my commit message. I wouldn't really to do a
functional change right now. It may be easily done as simple followup,
plain to test, etc.

> In parallel to the readl API there's the ioread32() API. Even though I
> read
> somewhere that the readl API is a legacy API, I don't have an idea
> which one
> to prefer and why.

You don't need ioread*().
Or do you have an example of hardware that uses I/O instead of MMIO?

Rather some _relaxed variants might be useful.

-- 
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Intel Finland Oy