linux-kernel - Re: [PATCH v3 3/3] staging: vt6655: Replace VNSvInPortD with ioread32

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]

Date:   Wed, 27 Apr 2022 21:10:07 +0200
From:   Philipp Hortmann <philipp.g.hortmann@...il.com>
To:     David Laight <David.Laight@...LAB.COM>
Cc:     Forest Bond <forest@...ttletooquiet.net>,
        "linux-staging@...ts.linux.dev" <linux-staging@...ts.linux.dev>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        'Greg Kroah-Hartman' <gregkh@...uxfoundation.org>
Subject: Re: [PATCH v3 3/3] staging: vt6655: Replace VNSvInPortD with ioread32

On 4/27/22 10:01, David Laight wrote:
> Actually I suspect that 'iobase' should be an __iomem structure
> pointer, pqwCurrTSF a point of the same type and MAC_REG_xxxx
> structure members.
> 
> Then the code should be using readl() not ioread32().
> I very much doubt that 'iobase' is in PCI IO space.

Hi David,

here some infos and questions:

kernel@...rix-ESPRIMO-P710:~/Documents/git/kernels/staging$ sudo lspci 
-s 01:05.0 -vvv
01:05.0 Network controller: VIA Technologies, Inc. VT6655 WiFi Adapter, 
802.11a/b/g
	Subsystem: VIA Technologies, Inc. VT6655 WiFi Adapter, 802.11a/b/g
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
Stepping+ SERR- FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- 
<TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 32 (8000ns max), Cache Line Size: 64 bytes
	Interrupt: pin A routed to IRQ 19
	Region 0: Memory at f7c00000 (32-bit, non-prefetchable) [size=256]
	Region 1: I/O ports at e000 [size=256]
	Capabilities: [50] Power Management version 2
		Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=0mA PME(D0-,D1+,D2+,D3hot+,D3cold-)
		Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
	Kernel driver in use: vt6655
	Kernel modules: vt6655_stage


---- In file device_main.c line 1699
	priv->memaddr = pci_resource_start(pcid, 0);
	priv->ioaddr = pci_resource_start(pcid, 1);
	priv->port_offset = ioremap(priv->memaddr & PCI_BASE_ADDRESS_MEM_MASK, 
256);
	dev_info(&pcid->dev, "vt6655_probe priv->memaddr: %x priv->ioaddr: %x", 
priv->memaddr, priv->ioaddr);

----- Output:
[  +0.000018] vt6655 0000:01:05.0: vt6655_probe priv->memaddr: f7c00000 
priv->ioaddr: e000


So port_offset is derived from memaddr.


----- In file card.c line 742
bool CARDbGetCurrentTSF(struct vnt_private *priv, u64 *pqwCurrTSF)
{
	void __iomem *iobase = priv->port_offset;
...
	VNSvInPortD(iobase + MAC_REG_TSFCNTR, (u32 *)pqwCurrTSF);

Please tell me if you need further infos to see if it is PCI IO space.
I think it is memory-mapped.

So is ioread32 wrong, right or can it be used?

This article gives more info:
https://www.kernel.org/doc/html/latest/driver-api/device-io.html

Thanks for your support.

Bye Philipp