linux-kernel - Re: [PATCH] x86: Add PCI extended config space access for AMD Barcelona

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: <20080602141928.GH23679@erda.amd.com>
Date:	Mon, 2 Jun 2008 16:19:29 +0200
From:	Robert Richter <robert.richter@....com>
To:	Arjan van de Ven <arjan@...radead.org>
CC:	Yinghai Lu <yhlu.kernel@...il.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...e.hu>,
	LKML <linux-kernel@...r.kernel.org>,
	Andi Kleen <andi@...stfloor.org>
Subject: Re: [PATCH] x86: Add PCI extended config space access for AMD
	Barcelona

Arjan,

On 28.05.08 12:02:53, Arjan van de Ven wrote:
> Comment 1:
> Can we make the 256/4096 thing conditional on actually having the
> feature somehow? (while not making the code TOO ugly)

In the first version I had 2 functions also. The patch have had lots
of duplicate code or inline functions. Since the conditional check is
already in raw_pci_* I decided to not implement an additional check
and use only one function.

int raw_pci_read(unsigned int domain, unsigned int bus, unsigned int devfn,
                                                int reg, int len, u32 *val)
{
	if (reg < 256 && raw_pci_ops)
	   return raw_pci_ops->read(domain, bus, devfn, reg, len, val);
	   if (raw_pci_ext_ops)
	      return raw_pci_ext_ops->read(domain, bus, devfn, reg, len, val);
	      return -EINVAL;
}

That leaves as a difference to the basic access is the shift left of
bits 8:11 in the PCI_CONF1_ADDRESS macro. Functional the new macro is
the same and the overhead for this is small. So I see keeping all code
in one function as the best solution.

> Comment 2: 
> The cpu_has_XXX is a bit dubious; while it's dependent on your cpu
> model right now, I'm a bit hesitant to consider a PCI feature something
> that belongs in the cpu_has_XXX namespace. (Yes I know PCI is moving
> into the cpu package, but on a logical level it seems just the wrong
> place).
> Do we need a platform_has_XXX namespace for things like this?

An alternative implementation would be here to use a check something
like pci_probe & PCI_HAS_EXT_CFG. If needed, I will send an updated
patch.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: robert.richter@....com

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