* linux-next: manual merge of the char-misc tree with the powerpc tree
From: Stephen Rothwell @ 2020-10-06 7:35 UTC (permalink / raw)
To: Greg KH, Arnd Bergmann, Michael Ellerman, PowerPC
Cc: Frederic Barrat, Necip Fazil Yildiran, Linux Next Mailing List,
Linux Kernel Mailing List
[-- Attachment #1: Type: text/plain, Size: 1313 bytes --]
Hi all,
Today's linux-next merge of the char-misc tree got a conflict in:
drivers/misc/ocxl/Kconfig
between commit:
dde6f18a8779 ("ocxl: Don't return trigger page when allocating an interrupt")
from the powerpc tree and commit:
4b53a3c72116 ("ocxl: fix kconfig dependency warning for OCXL")
from the char-misc tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc drivers/misc/ocxl/Kconfig
index 0d815b2a40b3,947294f6d7f4..000000000000
--- a/drivers/misc/ocxl/Kconfig
+++ b/drivers/misc/ocxl/Kconfig
@@@ -9,9 -9,8 +9,9 @@@ config OCXL_BAS
config OCXL
tristate "OpenCAPI coherent accelerator support"
- depends on PPC_POWERNV && PCI && EEH && HOTPLUG_PCI_POWERNV
+ depends on PPC_POWERNV && PCI && EEH && PPC_XIVE_NATIVE
++ depends on HOTPLUG_PCI_POWERNV
select OCXL_BASE
- select HOTPLUG_PCI_POWERNV
default m
help
Select this option to enable the ocxl driver for Open
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH v4] powernv/elog: Fix the race while processing OPAL error log event.
From: Mahesh Salgaonkar @ 2020-10-06 7:32 UTC (permalink / raw)
To: linuxppc-dev
Cc: Aneesh Kumar K.V, Oliver O'Halloran, stable, Vasant Hegde
Every error log reported by OPAL is exported to userspace through a sysfs
interface and notified using kobject_uevent(). The userspace daemon
(opal_errd) then reads the error log and acknowledges it error log is saved
safely to disk. Once acknowledged the kernel removes the respective sysfs
file entry causing respective resources getting released including kobject.
However there are chances where user daemon may already be scanning elog
entries while new sysfs elog entry is being created by kernel. User daemon
may read this new entry and ack it even before kernel can notify userspace
about it through kobject_uevent() call. If that happens then we have a
potential race between elog_ack_store->kobject_put() and kobject_uevent
which can lead to use-after-free issue of a kernfs object resulting into a
kernel crash. This patch fixes this race by protecting a sysfs file
creation/notification by holding a reference count on kobject until we
safely send kobject_uevent().
The function create_elog_obj() returns the elog object which if used by
caller function will end up in use-after-free problem again. However, the
return value of create_elog_obj() function isn't being used today and there
is need as well. Hence change it to return void to make this fix complete.
Fixes: 774fea1a38c6 ("powerpc/powernv: Read OPAL error log and export it through sysfs")
Cc: <stable@vger.kernel.org> # v3.15+
Reported-by: Oliver O'Halloran <oohall@gmail.com>
Signed-off-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Reviewed-by: Oliver O'Halloran <oohall@gmail.com>
Reviewed-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
---
Chnage in v4:
- Re-worded comments. No code change.
Change in v3:
- Change create_elog_obj function signature to return void.
Change in v2:
- Instead of mutex and use extra reference count on kobject to avoid the
race.
---
arch/powerpc/platforms/powernv/opal-elog.c | 34 ++++++++++++++++++++++++----
1 file changed, 29 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/opal-elog.c b/arch/powerpc/platforms/powernv/opal-elog.c
index 62ef7ad995da..adf4ff8d0bea 100644
--- a/arch/powerpc/platforms/powernv/opal-elog.c
+++ b/arch/powerpc/platforms/powernv/opal-elog.c
@@ -179,14 +179,14 @@ static ssize_t raw_attr_read(struct file *filep, struct kobject *kobj,
return count;
}
-static struct elog_obj *create_elog_obj(uint64_t id, size_t size, uint64_t type)
+static void create_elog_obj(uint64_t id, size_t size, uint64_t type)
{
struct elog_obj *elog;
int rc;
elog = kzalloc(sizeof(*elog), GFP_KERNEL);
if (!elog)
- return NULL;
+ return;
elog->kobj.kset = elog_kset;
@@ -219,18 +219,42 @@ static struct elog_obj *create_elog_obj(uint64_t id, size_t size, uint64_t type)
rc = kobject_add(&elog->kobj, NULL, "0x%llx", id);
if (rc) {
kobject_put(&elog->kobj);
- return NULL;
+ return;
}
+ /*
+ * As soon as sysfs file for this elog is created/activated there is
+ * chance opal_errd daemon might read and acknowledge this elog before
+ * kobject_uevent() is called. If that happens then we have a potential
+ * race between elog_ack_store->kobject_put() and kobject_uevent which
+ * leads to use-after-free issue of a kernfs object resulting into
+ * kernel crash.
+ *
+ * We already have one reference count on kobject and is been used for
+ * sysfs_create_bin_file() function. This initial one reference count
+ * is valid until it is dropped by elog_ack_store() function.
+ *
+ * However if userspace acknowledges the elog before this code reaches
+ * to kobject_uevent(), the reference count on kobject drops to zero
+ * and no longer stay valid for kobject_uevent() invocation. To avoid
+ * this race take reference count on kobject for bin file creation and
+ * drop it after kobject_uevent() is sent.
+ */
+
+ kobject_get(&elog->kobj); /* take a reference for the bin file. */
rc = sysfs_create_bin_file(&elog->kobj, &elog->raw_attr);
if (rc) {
kobject_put(&elog->kobj);
- return NULL;
+ /* Drop reference count taken for bin file. */
+ kobject_put(&elog->kobj);
+ return;
}
kobject_uevent(&elog->kobj, KOBJ_ADD);
+ /* Drop reference count taken for bin file. */
+ kobject_put(&elog->kobj);
- return elog;
+ return;
}
static irqreturn_t elog_event(int irq, void *data)
^ permalink raw reply related
* Re: [PATCH v2] powernv/elog: Fix the race while processing OPAL error log event.
From: Michael Ellerman @ 2020-10-06 5:40 UTC (permalink / raw)
To: Mahesh Salgaonkar, linuxppc-dev
Cc: Aneesh Kumar K.V, Oliver O'Halloran, Vasant Hegde
In-Reply-To: <160187115555.1589942.2124270585910076829.stgit@jupiter>
Mahesh Salgaonkar <mahesh@linux.ibm.com> writes:
> Every error log reported by OPAL is exported to userspace through a sysfs
> interface and notified using kobject_uevent(). The userspace daemon
> (opal_errd) then reads the error log and acknowledges it error log is saved
> safely to disk. Once acknowledged the kernel removes the respective sysfs
> file entry causing respective resources getting released including kobject.
>
> However there are chances where user daemon may already be scanning elog
> entries while new sysfs elog entry is being created by kernel. User daemon
> may read this new entry and ack it even before kernel can notify userspace
> about it through kobject_uevent() call. If that happens then we have a
> potential race between elog_ack_store->kobject_put() and kobject_uevent
> which can lead to use-after-free issue of a kernfs object resulting into a
> kernel crash. This patch fixes this race by protecting a sysfs file
> creation/notification by holding an additional reference count on kobject
> until we safely send kobject_uevent().
>
> Reported-by: Oliver O'Halloran <oohall@gmail.com>
> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> Change in v2:
> - Instead of mutex and use extra reference count on kobject to avoid the
> race.
> ---
> arch/powerpc/platforms/powernv/opal-elog.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/arch/powerpc/platforms/powernv/opal-elog.c b/arch/powerpc/platforms/powernv/opal-elog.c
> index 62ef7ad995da..230f102e87c0 100644
> --- a/arch/powerpc/platforms/powernv/opal-elog.c
> +++ b/arch/powerpc/platforms/powernv/opal-elog.c
> @@ -222,13 +222,28 @@ static struct elog_obj *create_elog_obj(uint64_t id, size_t size, uint64_t type)
> return NULL;
> }
>
> + /*
> + * As soon as sysfs file for this elog is created/activated there is
> + * chance opal_errd daemon might read and acknowledge this elog before
> + * kobject_uevent() is called. If that happens then we have a potential
> + * race between elog_ack_store->kobject_put() and kobject_uevent which
> + * leads to use-after-free issue of a kernfs object resulting into
> + * kernel crash. To avoid this race take an additional reference count
> + * on kobject until we safely send kobject_uevent().
> + */
> +
> + kobject_get(&elog->kobj); /* extra reference count */
It's not really an "extra" reference.
The way the code is currently written there's one reference and it's
given to (moved into) sysfs_create_bin_file(). (Because elog_ack_store()
drops that reference).
So after that call this function no longer has a valid reference to
kobj.
If this function wants to continue to use kobj (which it does) it needs
its own reference.
Or the other way to think about it is that this code owns the initial
reference, and it needs to take a reference on behalf of the bin file
before creating the bin file.
So the patch is not wrong, but I think the comments are a bit
misleading.
> rc = sysfs_create_bin_file(&elog->kobj, &elog->raw_attr);
> if (rc) {
> + kobject_put(&elog->kobj);
> + /* Drop the extra reference count */
> kobject_put(&elog->kobj);
> return NULL;
> }
>
> kobject_uevent(&elog->kobj, KOBJ_ADD);
> + /* Drop the extra reference count */
> + kobject_put(&elog->kobj);
>
> return elog;
And it is bogus that we return elog here, because we no longer have a
valid reference to it, so it may already be freed.
> }
So please send a v3 with updated comments and the return dropped.
cheers
^ permalink raw reply
* [PATCH v3] powernv/elog: Fix the race while processing OPAL error log event.
From: Mahesh Salgaonkar @ 2020-10-06 5:24 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Aneesh Kumar K.V, Oliver O'Halloran, Vasant Hegde
Every error log reported by OPAL is exported to userspace through a sysfs
interface and notified using kobject_uevent(). The userspace daemon
(opal_errd) then reads the error log and acknowledges it error log is saved
safely to disk. Once acknowledged the kernel removes the respective sysfs
file entry causing respective resources getting released including kobject.
However there are chances where user daemon may already be scanning elog
entries while new sysfs elog entry is being created by kernel. User daemon
may read this new entry and ack it even before kernel can notify userspace
about it through kobject_uevent() call. If that happens then we have a
potential race between elog_ack_store->kobject_put() and kobject_uevent
which can lead to use-after-free issue of a kernfs object resulting into a
kernel crash. This patch fixes this race by protecting a sysfs file
creation/notification by holding an additional reference count on kobject
until we safely send kobject_uevent().
The function create_elog_obj() returns the elog object which if used by
caller function will end up in use-after-free problem again. However, the
return value of create_elog_obj() function isn't being used today and there
is need as well. Hence change it to return void to make this fix complete.
Fixes: 774fea1a38c6 ("powerpc/powernv: Read OPAL error log and export it through sysfs")
Cc: <stable@vger.kernel.org> # v3.15+
Reported-by: Oliver O'Halloran <oohall@gmail.com>
Signed-off-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Reviewed-by: Oliver O'Halloran <oohall@gmail.com>
Reviewed-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
---
Change in v3:
- Change create_elog_obj function signature to return void.
Change in v2:
- Instead of mutex and use extra reference count on kobject to avoid the
race.
---
arch/powerpc/platforms/powernv/opal-elog.c | 25 ++++++++++++++++++++-----
1 file changed, 20 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/opal-elog.c b/arch/powerpc/platforms/powernv/opal-elog.c
index 62ef7ad995da..e61cbf08e17e 100644
--- a/arch/powerpc/platforms/powernv/opal-elog.c
+++ b/arch/powerpc/platforms/powernv/opal-elog.c
@@ -179,14 +179,14 @@ static ssize_t raw_attr_read(struct file *filep, struct kobject *kobj,
return count;
}
-static struct elog_obj *create_elog_obj(uint64_t id, size_t size, uint64_t type)
+static void create_elog_obj(uint64_t id, size_t size, uint64_t type)
{
struct elog_obj *elog;
int rc;
elog = kzalloc(sizeof(*elog), GFP_KERNEL);
if (!elog)
- return NULL;
+ return;
elog->kobj.kset = elog_kset;
@@ -219,18 +219,33 @@ static struct elog_obj *create_elog_obj(uint64_t id, size_t size, uint64_t type)
rc = kobject_add(&elog->kobj, NULL, "0x%llx", id);
if (rc) {
kobject_put(&elog->kobj);
- return NULL;
+ return;
}
+ /*
+ * As soon as sysfs file for this elog is created/activated there is
+ * chance opal_errd daemon might read and acknowledge this elog before
+ * kobject_uevent() is called. If that happens then we have a potential
+ * race between elog_ack_store->kobject_put() and kobject_uevent which
+ * leads to use-after-free issue of a kernfs object resulting into
+ * kernel crash. To avoid this race take an additional reference count
+ * on kobject until we safely send kobject_uevent().
+ */
+
+ kobject_get(&elog->kobj); /* extra reference count */
rc = sysfs_create_bin_file(&elog->kobj, &elog->raw_attr);
if (rc) {
kobject_put(&elog->kobj);
- return NULL;
+ /* Drop the extra reference count */
+ kobject_put(&elog->kobj);
+ return;
}
kobject_uevent(&elog->kobj, KOBJ_ADD);
+ /* Drop the extra reference count */
+ kobject_put(&elog->kobj);
- return elog;
+ return;
}
static irqreturn_t elog_event(int irq, void *data)
^ permalink raw reply related
* Re: [PATCH v2] powernv/elog: Fix the race while processing OPAL error log event.
From: Mahesh Jagannath Salgaonkar @ 2020-10-06 5:11 UTC (permalink / raw)
To: ananth, linuxppc-dev
In-Reply-To: <df7cebd0-bec3-d716-5514-61c4043a6d30@linux.ibm.com>
On 10/5/20 4:17 PM, Ananth N Mavinakayanahalli wrote:
> On 10/5/20 9:42 AM, Mahesh Salgaonkar wrote:
>> Every error log reported by OPAL is exported to userspace through a sysfs
>> interface and notified using kobject_uevent(). The userspace daemon
>> (opal_errd) then reads the error log and acknowledges it error log is
>> saved
>> safely to disk. Once acknowledged the kernel removes the respective sysfs
>> file entry causing respective resources getting released including
>> kobject.
>>
>> However there are chances where user daemon may already be scanning elog
>> entries while new sysfs elog entry is being created by kernel. User
>> daemon
>> may read this new entry and ack it even before kernel can notify
>> userspace
>> about it through kobject_uevent() call. If that happens then we have a
>> potential race between elog_ack_store->kobject_put() and kobject_uevent
>> which can lead to use-after-free issue of a kernfs object resulting
>> into a
>> kernel crash. This patch fixes this race by protecting a sysfs file
>> creation/notification by holding an additional reference count on kobject
>> until we safely send kobject_uevent().
>>
>> Reported-by: Oliver O'Halloran <oohall@gmail.com>
>> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>
> cc stable?
>
Will add it in v3.
Thanks,
-Mahesh.
^ permalink raw reply
* Re: [PATCH v2] powernv/elog: Fix the race while processing OPAL error log event.
From: Mahesh Jagannath Salgaonkar @ 2020-10-06 4:48 UTC (permalink / raw)
To: Oliver O'Halloran; +Cc: linuxppc-dev, Vasant Hegde, Aneesh Kumar K.V
In-Reply-To: <CAOSf1CEx_vSrMNYCRrL7q168hXr+iEAG4jxhrjkXzqEMH5CkQA@mail.gmail.com>
On 10/6/20 5:55 AM, Oliver O'Halloran wrote:
> On Mon, Oct 5, 2020 at 3:12 PM Mahesh Salgaonkar <mahesh@linux.ibm.com> wrote:
>>
>> Every error log reported by OPAL is exported to userspace through a sysfs
>> interface and notified using kobject_uevent(). The userspace daemon
>> (opal_errd) then reads the error log and acknowledges it error log is saved
>> safely to disk. Once acknowledged the kernel removes the respective sysfs
>> file entry causing respective resources getting released including kobject.
>>
>> However there are chances where user daemon may already be scanning elog
>> entries while new sysfs elog entry is being created by kernel. User daemon
>> may read this new entry and ack it even before kernel can notify userspace
>> about it through kobject_uevent() call. If that happens then we have a
>> potential race between elog_ack_store->kobject_put() and kobject_uevent
>> which can lead to use-after-free issue of a kernfs object resulting into a
>> kernel crash. This patch fixes this race by protecting a sysfs file
>> creation/notification by holding an additional reference count on kobject
>> until we safely send kobject_uevent().
>>
>> Reported-by: Oliver O'Halloran <oohall@gmail.com>
>> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> ---
>> Change in v2:
>> - Instead of mutex and use extra reference count on kobject to avoid the
>> race.
>> ---
>> arch/powerpc/platforms/powernv/opal-elog.c | 15 +++++++++++++++
>> 1 file changed, 15 insertions(+)
>>
>> diff --git a/arch/powerpc/platforms/powernv/opal-elog.c b/arch/powerpc/platforms/powernv/opal-elog.c
>> index 62ef7ad995da..230f102e87c0 100644
>> --- a/arch/powerpc/platforms/powernv/opal-elog.c
>> +++ b/arch/powerpc/platforms/powernv/opal-elog.c
>> @@ -222,13 +222,28 @@ static struct elog_obj *create_elog_obj(uint64_t id, size_t size, uint64_t type)
>> return NULL;
>> }
>>
>> + /*
>> + * As soon as sysfs file for this elog is created/activated there is
>> + * chance opal_errd daemon might read and acknowledge this elog before
>> + * kobject_uevent() is called. If that happens then we have a potential
>> + * race between elog_ack_store->kobject_put() and kobject_uevent which
>> + * leads to use-after-free issue of a kernfs object resulting into
>> + * kernel crash. To avoid this race take an additional reference count
>> + * on kobject until we safely send kobject_uevent().
>> + */
>> +
>> + kobject_get(&elog->kobj); /* extra reference count */
>> rc = sysfs_create_bin_file(&elog->kobj, &elog->raw_attr);
>> if (rc) {
>> + kobject_put(&elog->kobj);
>> + /* Drop the extra reference count */
>> kobject_put(&elog->kobj);
>> return NULL;
>> }
>>
>> kobject_uevent(&elog->kobj, KOBJ_ADD);
>> + /* Drop the extra reference count */
>> + kobject_put(&elog->kobj);
>
> Makes sense,
>
> Reviewed-by: Oliver O'Halloran <oohall@gmail.com>
>
>>
>> return elog;
>
> Does the returned value actually get used anywhere? We'd have a
> similar use-after-free problem if it does. This should probably return
> an error code rather than the object itself.
>
Nope. It isn't being used. I can make it function as void and send v3.
Thanks,
-Mahesh.
^ permalink raw reply
* Re: [RFC PATCH next-20200930] treewide: Convert macro and uses of __section(foo) to __section("foo")
From: Joe Perches @ 2020-10-06 3:22 UTC (permalink / raw)
To: Joel Stanley, Michael Ellerman
Cc: Kees Cook, Paul E . McKenney, Clang-Built-Linux ML,
Nick Desaulniers, Lai Jiangshan, Josh Triplett, Steven Rostedt,
rcu, Miguel Ojeda, Mathieu Desnoyers, Sedat Dilek, Paul Mackerras,
linuxppc-dev, LKML
In-Reply-To: <CACPK8XdwX=1T8WrsVYurL+JedEsb1ZTyrWtJXDLXycu-qu4UTg@mail.gmail.com>
On Tue, 2020-10-06 at 00:34 +0000, Joel Stanley wrote:
> arch/powerpc/boot is the powerpc wrapper, and it's not built with the
> same includes or flags as the rest of the kernel. It doesn't include
> any of the headers in the top level include/ directory for hysterical
> raisins.
>
> The straightforward fix would be to exclude this directory from your script.
I agree and that's why I submitted another script
that does just that.
https://lore.kernel.org/lkml/75393e5ddc272dc7403de74d645e6c6e0f4e70eb.camel@perches.com/
^ permalink raw reply
* Re: [PATCH v2] powernv/elog: Fix the race while processing OPAL error log event.
From: Vasant Hegde @ 2020-10-06 2:26 UTC (permalink / raw)
To: Oliver O'Halloran, Mahesh Salgaonkar
Cc: linuxppc-dev, Vasant Hegde, Aneesh Kumar K.V
In-Reply-To: <CAOSf1CEx_vSrMNYCRrL7q168hXr+iEAG4jxhrjkXzqEMH5CkQA@mail.gmail.com>
On 10/6/20 5:55 AM, Oliver O'Halloran wrote:
> On Mon, Oct 5, 2020 at 3:12 PM Mahesh Salgaonkar <mahesh@linux.ibm.com> wrote:
>>
>> Every error log reported by OPAL is exported to userspace through a sysfs
>> interface and notified using kobject_uevent(). The userspace daemon
>> (opal_errd) then reads the error log and acknowledges it error log is saved
>> safely to disk. Once acknowledged the kernel removes the respective sysfs
>> file entry causing respective resources getting released including kobject.
>>
>> However there are chances where user daemon may already be scanning elog
>> entries while new sysfs elog entry is being created by kernel. User daemon
>> may read this new entry and ack it even before kernel can notify userspace
>> about it through kobject_uevent() call. If that happens then we have a
>> potential race between elog_ack_store->kobject_put() and kobject_uevent
>> which can lead to use-after-free issue of a kernfs object resulting into a
>> kernel crash. This patch fixes this race by protecting a sysfs file
>> creation/notification by holding an additional reference count on kobject
>> until we safely send kobject_uevent().
>>
>> Reported-by: Oliver O'Halloran <oohall@gmail.com>
>> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> ---
>> Change in v2:
>> - Instead of mutex and use extra reference count on kobject to avoid the
>> race.
>> ---
>> arch/powerpc/platforms/powernv/opal-elog.c | 15 +++++++++++++++
>> 1 file changed, 15 insertions(+)
>>
>> diff --git a/arch/powerpc/platforms/powernv/opal-elog.c b/arch/powerpc/platforms/powernv/opal-elog.c
>> index 62ef7ad995da..230f102e87c0 100644
>> --- a/arch/powerpc/platforms/powernv/opal-elog.c
>> +++ b/arch/powerpc/platforms/powernv/opal-elog.c
>> @@ -222,13 +222,28 @@ static struct elog_obj *create_elog_obj(uint64_t id, size_t size, uint64_t type)
>> return NULL;
>> }
>>
>> + /*
>> + * As soon as sysfs file for this elog is created/activated there is
>> + * chance opal_errd daemon might read and acknowledge this elog before
>> + * kobject_uevent() is called. If that happens then we have a potential
>> + * race between elog_ack_store->kobject_put() and kobject_uevent which
>> + * leads to use-after-free issue of a kernfs object resulting into
>> + * kernel crash. To avoid this race take an additional reference count
>> + * on kobject until we safely send kobject_uevent().
>> + */
>> +
>> + kobject_get(&elog->kobj); /* extra reference count */
>> rc = sysfs_create_bin_file(&elog->kobj, &elog->raw_attr);
>> if (rc) {
>> + kobject_put(&elog->kobj);
>> + /* Drop the extra reference count */
>> kobject_put(&elog->kobj);
>> return NULL;
>> }
>>
>> kobject_uevent(&elog->kobj, KOBJ_ADD);
>> + /* Drop the extra reference count */
>> + kobject_put(&elog->kobj);
>
> Makes sense,
>
> Reviewed-by: Oliver O'Halloran <oohall@gmail.com>
Reviewed-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
>
>>
>> return elog;
>
> Does the returned value actually get used anywhere? We'd have a
> similar use-after-free problem if it does. This should probably return
> an error code rather than the object itself.
No one is using it today. May be we should just make it void function.
-Vasant
^ permalink raw reply
* Re: [RFC PATCH next-20200930] treewide: Convert macro and uses of __section(foo) to __section("foo")
From: Joel Stanley @ 2020-10-06 0:34 UTC (permalink / raw)
To: Joe Perches, Michael Ellerman
Cc: Kees Cook, Paul E . McKenney, Clang-Built-Linux ML,
Nick Desaulniers, Lai Jiangshan, Josh Triplett, Steven Rostedt,
rcu, Miguel Ojeda, Mathieu Desnoyers, Sedat Dilek, Paul Mackerras,
linuxppc-dev, LKML
In-Reply-To: <61445711991c2d6eb7c8fb05bed2814458e2593b.camel@perches.com>
On Thu, 1 Oct 2020 at 20:19, Joe Perches <joe@perches.com> wrote:
>
> On Thu, 2020-10-01 at 14:39 -0500, Segher Boessenkool wrch/ote:
> > Hi!
> >
> > On Thu, Oct 01, 2020 at 12:15:39PM +0200, Miguel Ojeda wrote:
> > > > So it looks like the best option is to exclude these
> > > > 2 files from conversion.
> > >
> > > Agreed. Nevertheless, is there any reason arch/powerpc/* should not be
> > > compiling cleanly with compiler.h? (CC'ing the rest of the PowerPC
> > > reviewers and ML).
> >
> > You need to #include compiler_types.h to get this #define?
>
> Actually no, you need to add
>
> #include <linux/compiler_attributes.h>
>
> to both files and then it builds properly.
>
> Ideally though nothing should include this file directly.
arch/powerpc/boot is the powerpc wrapper, and it's not built with the
same includes or flags as the rest of the kernel. It doesn't include
any of the headers in the top level include/ directory for hysterical
raisins.
The straightforward fix would be to exclude this directory from your script.
Cheers,
Joel
^ permalink raw reply
* Re: [PATCH v2] powernv/elog: Fix the race while processing OPAL error log event.
From: Oliver O'Halloran @ 2020-10-06 0:25 UTC (permalink / raw)
To: Mahesh Salgaonkar; +Cc: linuxppc-dev, Vasant Hegde, Aneesh Kumar K.V
In-Reply-To: <160187115555.1589942.2124270585910076829.stgit@jupiter>
On Mon, Oct 5, 2020 at 3:12 PM Mahesh Salgaonkar <mahesh@linux.ibm.com> wrote:
>
> Every error log reported by OPAL is exported to userspace through a sysfs
> interface and notified using kobject_uevent(). The userspace daemon
> (opal_errd) then reads the error log and acknowledges it error log is saved
> safely to disk. Once acknowledged the kernel removes the respective sysfs
> file entry causing respective resources getting released including kobject.
>
> However there are chances where user daemon may already be scanning elog
> entries while new sysfs elog entry is being created by kernel. User daemon
> may read this new entry and ack it even before kernel can notify userspace
> about it through kobject_uevent() call. If that happens then we have a
> potential race between elog_ack_store->kobject_put() and kobject_uevent
> which can lead to use-after-free issue of a kernfs object resulting into a
> kernel crash. This patch fixes this race by protecting a sysfs file
> creation/notification by holding an additional reference count on kobject
> until we safely send kobject_uevent().
>
> Reported-by: Oliver O'Halloran <oohall@gmail.com>
> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> Change in v2:
> - Instead of mutex and use extra reference count on kobject to avoid the
> race.
> ---
> arch/powerpc/platforms/powernv/opal-elog.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/arch/powerpc/platforms/powernv/opal-elog.c b/arch/powerpc/platforms/powernv/opal-elog.c
> index 62ef7ad995da..230f102e87c0 100644
> --- a/arch/powerpc/platforms/powernv/opal-elog.c
> +++ b/arch/powerpc/platforms/powernv/opal-elog.c
> @@ -222,13 +222,28 @@ static struct elog_obj *create_elog_obj(uint64_t id, size_t size, uint64_t type)
> return NULL;
> }
>
> + /*
> + * As soon as sysfs file for this elog is created/activated there is
> + * chance opal_errd daemon might read and acknowledge this elog before
> + * kobject_uevent() is called. If that happens then we have a potential
> + * race between elog_ack_store->kobject_put() and kobject_uevent which
> + * leads to use-after-free issue of a kernfs object resulting into
> + * kernel crash. To avoid this race take an additional reference count
> + * on kobject until we safely send kobject_uevent().
> + */
> +
> + kobject_get(&elog->kobj); /* extra reference count */
> rc = sysfs_create_bin_file(&elog->kobj, &elog->raw_attr);
> if (rc) {
> + kobject_put(&elog->kobj);
> + /* Drop the extra reference count */
> kobject_put(&elog->kobj);
> return NULL;
> }
>
> kobject_uevent(&elog->kobj, KOBJ_ADD);
> + /* Drop the extra reference count */
> + kobject_put(&elog->kobj);
Makes sense,
Reviewed-by: Oliver O'Halloran <oohall@gmail.com>
>
> return elog;
Does the returned value actually get used anywhere? We'd have a
similar use-after-free problem if it does. This should probably return
an error code rather than the object itself.
^ permalink raw reply
* Re: [PATCH v2] powernv/elog: Fix the race while processing OPAL error log event.
From: Oliver O'Halloran @ 2020-10-06 0:22 UTC (permalink / raw)
To: Ananth N Mavinakayanahalli; +Cc: linuxppc-dev
In-Reply-To: <df7cebd0-bec3-d716-5514-61c4043a6d30@linux.ibm.com>
On Mon, Oct 5, 2020 at 11:07 PM Ananth N Mavinakayanahalli
<ananth@linux.ibm.com> wrote:
>
> On 10/5/20 9:42 AM, Mahesh Salgaonkar wrote:
> > Every error log reported by OPAL is exported to userspace through a sysfs
> > interface and notified using kobject_uevent(). The userspace daemon
> > (opal_errd) then reads the error log and acknowledges it error log is saved
> > safely to disk. Once acknowledged the kernel removes the respective sysfs
> > file entry causing respective resources getting released including kobject.
> >
> > However there are chances where user daemon may already be scanning elog
> > entries while new sysfs elog entry is being created by kernel. User daemon
> > may read this new entry and ack it even before kernel can notify userspace
> > about it through kobject_uevent() call. If that happens then we have a
> > potential race between elog_ack_store->kobject_put() and kobject_uevent
> > which can lead to use-after-free issue of a kernfs object resulting into a
> > kernel crash. This patch fixes this race by protecting a sysfs file
> > creation/notification by holding an additional reference count on kobject
> > until we safely send kobject_uevent().
> >
> > Reported-by: Oliver O'Halloran <oohall@gmail.com>
> > Signed-off-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
> > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>
> cc stable?
+1
^ permalink raw reply
* Re: [RFC PATCH next-20200930] treewide: Convert macro and uses of __section(foo) to __section("foo")
From: Joe Perches @ 2020-10-05 18:46 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Kees Cook, Paul E . McKenney, Clang-Built-Linux ML, LKML,
Lai Jiangshan, Josh Triplett, Steven Rostedt, rcu, Miguel Ojeda,
Mathieu Desnoyers, Sedat Dilek, Paul Mackerras, linuxppc-dev
In-Reply-To: <CAKwvOdmW4ZSo0yz9ZUjFhjzzDkNAghKYk_hxn9tvrKLBgCXx-A@mail.gmail.com>
On Mon, 2020-10-05 at 11:36 -0700, Nick Desaulniers wrote:
> I don't think there's anything wrong with manually including it and adding `-I
> <path>` (capital i) if needed.
All of this is secondary to the actual change to use
quoted __section("foo") rather than __section(foo)
I'd rather get that done first and then figure out if
additional changes could be done later.
^ permalink raw reply
* Re: [RFC PATCH next-20200930] treewide: Convert macro and uses of __section(foo) to __section("foo")
From: Nick Desaulniers @ 2020-10-05 18:36 UTC (permalink / raw)
To: Joe Perches
Cc: Kees Cook, Paul E . McKenney, Clang-Built-Linux ML, LKML,
Lai Jiangshan, Josh Triplett, Steven Rostedt, rcu, Miguel Ojeda,
Mathieu Desnoyers, Sedat Dilek, Paul Mackerras, linuxppc-dev
In-Reply-To: <61445711991c2d6eb7c8fb05bed2814458e2593b.camel@perches.com>
On Thu, Oct 1, 2020 at 1:19 PM Joe Perches <joe@perches.com> wrote:
>
> On Thu, 2020-10-01 at 14:39 -0500, Segher Boessenkool wrch/ote:
> > Hi!
> >
> > On Thu, Oct 01, 2020 at 12:15:39PM +0200, Miguel Ojeda wrote:
> > > > So it looks like the best option is to exclude these
> > > > 2 files from conversion.
> > >
> > > Agreed. Nevertheless, is there any reason arch/powerpc/* should not be
> > > compiling cleanly with compiler.h? (CC'ing the rest of the PowerPC
> > > reviewers and ML).
> >
> > You need to #include compiler_types.h to get this #define?
>
> Actually no, you need to add
>
> #include <linux/compiler_attributes.h>
>
> to both files and then it builds properly.
>
> Ideally though nothing should include this file directly.
That's because Kbuild injects it via command line flag `-include`.
(Well, it injects compiler_types.h which includes this). If part of
the tree reset KBUILD_CFLAGS, that `-include` gets dropped. I don't
think there's anything wrong with manually including it and adding `-I
<path>` (capital i) if needed.
>
> > (The twice-defined thing is a warning, not an error. It should be fixed
> > of course, but it is less important; although it may be pointing to a
> > deeper problem.)
> >
> >
> > Segher
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply
* Re: [PATCH v2] powernv/elog: Fix the race while processing OPAL error log event.
From: Ananth N Mavinakayanahalli @ 2020-10-05 10:47 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <160187115555.1589942.2124270585910076829.stgit@jupiter>
On 10/5/20 9:42 AM, Mahesh Salgaonkar wrote:
> Every error log reported by OPAL is exported to userspace through a sysfs
> interface and notified using kobject_uevent(). The userspace daemon
> (opal_errd) then reads the error log and acknowledges it error log is saved
> safely to disk. Once acknowledged the kernel removes the respective sysfs
> file entry causing respective resources getting released including kobject.
>
> However there are chances where user daemon may already be scanning elog
> entries while new sysfs elog entry is being created by kernel. User daemon
> may read this new entry and ack it even before kernel can notify userspace
> about it through kobject_uevent() call. If that happens then we have a
> potential race between elog_ack_store->kobject_put() and kobject_uevent
> which can lead to use-after-free issue of a kernfs object resulting into a
> kernel crash. This patch fixes this race by protecting a sysfs file
> creation/notification by holding an additional reference count on kobject
> until we safely send kobject_uevent().
>
> Reported-by: Oliver O'Halloran <oohall@gmail.com>
> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
cc stable?
--
Ananth
^ permalink raw reply
* [PATCH v2] powernv/elog: Fix the race while processing OPAL error log event.
From: Mahesh Salgaonkar @ 2020-10-05 4:12 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Aneesh Kumar K.V, Oliver O'Halloran, Vasant Hegde
Every error log reported by OPAL is exported to userspace through a sysfs
interface and notified using kobject_uevent(). The userspace daemon
(opal_errd) then reads the error log and acknowledges it error log is saved
safely to disk. Once acknowledged the kernel removes the respective sysfs
file entry causing respective resources getting released including kobject.
However there are chances where user daemon may already be scanning elog
entries while new sysfs elog entry is being created by kernel. User daemon
may read this new entry and ack it even before kernel can notify userspace
about it through kobject_uevent() call. If that happens then we have a
potential race between elog_ack_store->kobject_put() and kobject_uevent
which can lead to use-after-free issue of a kernfs object resulting into a
kernel crash. This patch fixes this race by protecting a sysfs file
creation/notification by holding an additional reference count on kobject
until we safely send kobject_uevent().
Reported-by: Oliver O'Halloran <oohall@gmail.com>
Signed-off-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
Change in v2:
- Instead of mutex and use extra reference count on kobject to avoid the
race.
---
arch/powerpc/platforms/powernv/opal-elog.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/arch/powerpc/platforms/powernv/opal-elog.c b/arch/powerpc/platforms/powernv/opal-elog.c
index 62ef7ad995da..230f102e87c0 100644
--- a/arch/powerpc/platforms/powernv/opal-elog.c
+++ b/arch/powerpc/platforms/powernv/opal-elog.c
@@ -222,13 +222,28 @@ static struct elog_obj *create_elog_obj(uint64_t id, size_t size, uint64_t type)
return NULL;
}
+ /*
+ * As soon as sysfs file for this elog is created/activated there is
+ * chance opal_errd daemon might read and acknowledge this elog before
+ * kobject_uevent() is called. If that happens then we have a potential
+ * race between elog_ack_store->kobject_put() and kobject_uevent which
+ * leads to use-after-free issue of a kernfs object resulting into
+ * kernel crash. To avoid this race take an additional reference count
+ * on kobject until we safely send kobject_uevent().
+ */
+
+ kobject_get(&elog->kobj); /* extra reference count */
rc = sysfs_create_bin_file(&elog->kobj, &elog->raw_attr);
if (rc) {
+ kobject_put(&elog->kobj);
+ /* Drop the extra reference count */
kobject_put(&elog->kobj);
return NULL;
}
kobject_uevent(&elog->kobj, KOBJ_ADD);
+ /* Drop the extra reference count */
+ kobject_put(&elog->kobj);
return elog;
}
^ permalink raw reply related
* Re: [PATCH v4] PCI: Unify ECAM constants in native PCI Express drivers
From: Florian Fainelli @ 2020-10-05 2:53 UTC (permalink / raw)
To: Krzysztof Wilczyński, Bjorn Helgaas
Cc: Heiko Stuebner, Shawn Lin, Paul Mackerras, Thomas Petazzoni,
Jonathan Chocron, Toan Le, Will Deacon, Rob Herring,
Lorenzo Pieralisi, Michal Simek, linux-rockchip,
bcm-kernel-feedback-list, Jonathan Derrick, linux-pci, Ray Jui,
linux-rpi-kernel, Jonathan Cameron, linux-arm-kernel,
Scott Branden, Zhou Wang, Robert Richter, linuxppc-dev,
Nicolas Saenz Julienne
In-Reply-To: <20201005003805.465057-1-kw@linux.com>
On 10/4/2020 5:38 PM, Krzysztof Wilczyński wrote:
> Unify ECAM-related constants into a single set of standard constants
> defining memory address shift values for the byte-level address that can
> be used when accessing the PCI Express Configuration Space, and then
> move native PCI Express controller drivers to use newly introduced
> definitions retiring any driver-specific ones.
>
> The ECAM ("Enhanced Configuration Access Mechanism") is defined by the
> PCI Express specification (see PCI Express Base Specification, Revision
> 5.0, Version 1.0, Section 7.2.2, p. 676), thus most hardware should
> implement it the same way. Most of the native PCI Express controller
> drivers define their ECAM-related constants, many of these could be
> shared, or use open-coded values when setting the .bus_shift field of
> the struct pci_ecam_ops.
>
> All of the newly added constants should remove ambiguity and reduce the
> number of open-coded values, and also correlate more strongly with the
> descriptions in the aforementioned specification (see Table 7-1
> "Enhanced Configuration Address Mapping", p. 677).
>
> There is no change to functionality.
>
> Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
> ---
[snip]
>
> -/* Configuration space read/write support */
> -static inline int brcm_pcie_cfg_index(int busnr, int devfn, int reg)
> -{
> - return ((PCI_SLOT(devfn) & 0x1f) << PCIE_EXT_SLOT_SHIFT)
> - | ((PCI_FUNC(devfn) & 0x07) << PCIE_EXT_FUNC_SHIFT)
> - | (busnr << PCIE_EXT_BUSNUM_SHIFT)
> - | (reg & ~3);
> -}
> -
> static void __iomem *brcm_pcie_map_conf(struct pci_bus *bus, unsigned int devfn,
> int where)
> {
> @@ -590,7 +578,7 @@ static void __iomem *brcm_pcie_map_conf(struct pci_bus *bus, unsigned int devfn,
> return PCI_SLOT(devfn) ? NULL : base + where;
>
> /* For devices, write to the config space index register */
> - idx = brcm_pcie_cfg_index(bus->number, devfn, 0);
> + idx = PCIE_ECAM_BUS(bus->number) | PCIE_ECAM_DEVFN(devfn);
This appears to be correct, so:
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
however, I would have defined a couple of additional helper macros and do:
idx = PCIE_ECAM_BUS(bus->number) | PCIE_ECAM_DEV(devfn) |
PCIE_ECAM_FUN(devfn);
for clarity.
[snip]
> +/*
> + * Memory address shift values for the byte-level address that
> + * can be used when accessing the PCI Express Configuration Space.
> + */
> +
> +/*
> + * Enhanced Configuration Access Mechanism (ECAM)
> + *
> + * See PCI Express Base Specification, Revision 5.0, Version 1.0,
> + * Section 7.2.2, Table 7-1, p. 677.
> + */
> +#define PCIE_ECAM_BUS_SHIFT 20 /* Bus Number */
> +#define PCIE_ECAM_DEV_SHIFT 15 /* Device Number */
> +#define PCIE_ECAM_FUN_SHIFT 12 /* Function Number */
> +
> +#define PCIE_ECAM_BUS(x) (((x) & 0xff) << PCIE_ECAM_BUS_SHIFT)
> +#define PCIE_ECAM_DEVFN(x) (((x) & 0xff) << PCIE_ECAM_FUN_SHIFT)
For instance, adding these two:
#define PCIE_ECAM_DEV(x) (((x) & 0x1f) << PCIE_ECAM_DEV_SHIFT)
#define PCIE_ECAM_FUN(x) (((x) & 0x7) << PCIE_ECAM_FUN_SHIFT)
may be clearer for use in drivers like pcie-brcmstb.c that used to treat
the device function in terms of device and function (though it was
called slot there).
--
Florian
^ permalink raw reply
* [PATCH v4] PCI: Unify ECAM constants in native PCI Express drivers
From: Krzysztof Wilczyński @ 2020-10-05 0:38 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Heiko Stuebner, Shawn Lin, Paul Mackerras, Thomas Petazzoni,
Jonathan Chocron, Toan Le, Will Deacon, Rob Herring,
Lorenzo Pieralisi, Michal Simek, linux-rockchip,
bcm-kernel-feedback-list, Jonathan Derrick, linux-pci, Ray Jui,
Florian Fainelli, linux-rpi-kernel, Jonathan Cameron,
linux-arm-kernel, Scott Branden, Zhou Wang, Robert Richter,
linuxppc-dev, Nicolas Saenz Julienne
Unify ECAM-related constants into a single set of standard constants
defining memory address shift values for the byte-level address that can
be used when accessing the PCI Express Configuration Space, and then
move native PCI Express controller drivers to use newly introduced
definitions retiring any driver-specific ones.
The ECAM ("Enhanced Configuration Access Mechanism") is defined by the
PCI Express specification (see PCI Express Base Specification, Revision
5.0, Version 1.0, Section 7.2.2, p. 676), thus most hardware should
implement it the same way. Most of the native PCI Express controller
drivers define their ECAM-related constants, many of these could be
shared, or use open-coded values when setting the .bus_shift field of
the struct pci_ecam_ops.
All of the newly added constants should remove ambiguity and reduce the
number of open-coded values, and also correlate more strongly with the
descriptions in the aforementioned specification (see Table 7-1
"Enhanced Configuration Address Mapping", p. 677).
There is no change to functionality.
Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
---
Changed in v4:
Removed constants related to "CAM".
Added more platforms and devices that can use new ECAM macros and
constants.
Removed unused ".bus_shift" initialisers from pci-xgene.c as
xgene_pcie_map_bus() did not use these.
Changes in v3:
Updated commit message wording.
Updated regarding custom ECAM bus shift values and concerning PCI base
configuration space access for Type 1 access.
Refactored rockchip_pcie_rd_other_conf() and rockchip_pcie_wr_other_conf()
and removed the "busdev" variable.
Removed surplus "relbus" variable from nwl_pcie_map_bus() and
xilinx_pcie_map_bus().
Renamed the PCIE_ECAM_ADDR() macro to PCIE_ECAM_OFFSET().
Changes in v2:
Use PCIE_ECAM_ADDR macro when computing ECAM address offset, but drop
PCI_SLOT and PCI_FUNC macros from the PCIE_ECAM_ADDR macro in favour
of using a single value for the device/function.
arch/powerpc/platforms/4xx/pci.c | 7 +++--
drivers/pci/controller/dwc/pcie-al.c | 8 +++---
drivers/pci/controller/dwc/pcie-hisi.c | 4 +--
drivers/pci/controller/pci-aardvark.c | 13 +++------
drivers/pci/controller/pci-host-generic.c | 2 +-
drivers/pci/controller/pci-thunder-ecam.c | 2 +-
drivers/pci/controller/pci-thunder-pem.c | 13 +++++++--
drivers/pci/controller/pci-xgene.c | 2 --
drivers/pci/controller/pcie-brcmstb.c | 16 ++----------
drivers/pci/controller/pcie-iproc.c | 29 ++++++---------------
drivers/pci/controller/pcie-rockchip-host.c | 27 +++++++++----------
drivers/pci/controller/pcie-rockchip.h | 8 +-----
drivers/pci/controller/pcie-tango.c | 2 +-
drivers/pci/controller/pcie-xilinx-nwl.c | 9 ++-----
drivers/pci/controller/pcie-xilinx.c | 11 ++------
drivers/pci/controller/vmd.c | 5 ++--
drivers/pci/ecam.c | 4 +--
include/linux/pci-ecam.h | 24 +++++++++++++++++
18 files changed, 82 insertions(+), 104 deletions(-)
diff --git a/arch/powerpc/platforms/4xx/pci.c b/arch/powerpc/platforms/4xx/pci.c
index c13d64c3b019..cee40e0b061c 100644
--- a/arch/powerpc/platforms/4xx/pci.c
+++ b/arch/powerpc/platforms/4xx/pci.c
@@ -20,6 +20,7 @@
#include <linux/kernel.h>
#include <linux/pci.h>
+#include <linux/pci-ecam.h>
#include <linux/init.h>
#include <linux/of.h>
#include <linux/delay.h>
@@ -1585,17 +1586,15 @@ static void __iomem *ppc4xx_pciex_get_config_base(struct ppc4xx_pciex_port *port
struct pci_bus *bus,
unsigned int devfn)
{
- int relbus;
-
/* Remove the casts when we finally remove the stupid volatile
* in struct pci_controller
*/
if (bus->number == port->hose->first_busno)
return (void __iomem *)port->hose->cfg_addr;
- relbus = bus->number - (port->hose->first_busno + 1);
return (void __iomem *)port->hose->cfg_data +
- ((relbus << 20) | (devfn << 12));
+ PCIE_ECAM_BUS(bus->number - (port->hose->first_busno + 1)) |
+ PCIE_ECAM_DEVFN(devfn);
}
static int ppc4xx_pciex_read_config(struct pci_bus *bus, unsigned int devfn,
diff --git a/drivers/pci/controller/dwc/pcie-al.c b/drivers/pci/controller/dwc/pcie-al.c
index d57d4ee15848..7c2aa049113c 100644
--- a/drivers/pci/controller/dwc/pcie-al.c
+++ b/drivers/pci/controller/dwc/pcie-al.c
@@ -76,7 +76,7 @@ static int al_pcie_init(struct pci_config_window *cfg)
}
const struct pci_ecam_ops al_pcie_ops = {
- .bus_shift = 20,
+ .bus_shift = PCIE_ECAM_BUS_SHIFT,
.init = al_pcie_init,
.pci_ops = {
.map_bus = al_pcie_map_bus,
@@ -138,8 +138,6 @@ struct al_pcie {
struct al_pcie_target_bus_cfg target_bus_cfg;
};
-#define PCIE_ECAM_DEVFN(x) (((x) & 0xff) << 12)
-
#define to_al_pcie(x) dev_get_drvdata((x)->dev)
static inline u32 al_pcie_controller_readl(struct al_pcie *pcie, u32 offset)
@@ -228,7 +226,7 @@ static void __iomem *al_pcie_conf_addr_map(struct al_pcie *pcie,
void __iomem *pci_base_addr;
pci_base_addr = (void __iomem *)((uintptr_t)pp->va_cfg0_base +
- (busnr_ecam << 20) +
+ PCIE_ECAM_BUS(busnr_ecam) +
PCIE_ECAM_DEVFN(devfn));
if (busnr_reg != target_bus_cfg->reg_val) {
@@ -300,7 +298,7 @@ static void al_pcie_config_prepare(struct al_pcie *pcie)
target_bus_cfg = &pcie->target_bus_cfg;
- ecam_bus_mask = (pcie->ecam_size >> 20) - 1;
+ ecam_bus_mask = (pcie->ecam_size >> PCIE_ECAM_BUS_SHIFT) - 1;
if (ecam_bus_mask > 255) {
dev_warn(pcie->dev, "ECAM window size is larger than 256MB. Cutting off at 256\n");
ecam_bus_mask = 255;
diff --git a/drivers/pci/controller/dwc/pcie-hisi.c b/drivers/pci/controller/dwc/pcie-hisi.c
index 5ca86796d43a..b7afbf1d4bd9 100644
--- a/drivers/pci/controller/dwc/pcie-hisi.c
+++ b/drivers/pci/controller/dwc/pcie-hisi.c
@@ -100,7 +100,7 @@ static int hisi_pcie_init(struct pci_config_window *cfg)
}
const struct pci_ecam_ops hisi_pcie_ops = {
- .bus_shift = 20,
+ .bus_shift = PCIE_ECAM_BUS_SHIFT,
.init = hisi_pcie_init,
.pci_ops = {
.map_bus = hisi_pcie_map_bus,
@@ -135,7 +135,7 @@ static int hisi_pcie_platform_init(struct pci_config_window *cfg)
}
static const struct pci_ecam_ops hisi_pcie_platform_ops = {
- .bus_shift = 20,
+ .bus_shift = PCIE_ECAM_BUS_SHIFT,
.init = hisi_pcie_platform_init,
.pci_ops = {
.map_bus = hisi_pcie_map_bus,
diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
index 1559f79e63b6..200ad07e4747 100644
--- a/drivers/pci/controller/pci-aardvark.c
+++ b/drivers/pci/controller/pci-aardvark.c
@@ -15,6 +15,7 @@
#include <linux/irqdomain.h>
#include <linux/kernel.h>
#include <linux/pci.h>
+#include <linux/pci-ecam.h>
#include <linux/init.h>
#include <linux/phy/phy.h>
#include <linux/platform_device.h>
@@ -163,14 +164,6 @@
#define PCIE_CONFIG_WR_TYPE0 0xa
#define PCIE_CONFIG_WR_TYPE1 0xb
-#define PCIE_CONF_BUS(bus) (((bus) & 0xff) << 20)
-#define PCIE_CONF_DEV(dev) (((dev) & 0x1f) << 15)
-#define PCIE_CONF_FUNC(fun) (((fun) & 0x7) << 12)
-#define PCIE_CONF_REG(reg) ((reg) & 0xffc)
-#define PCIE_CONF_ADDR(bus, devfn, where) \
- (PCIE_CONF_BUS(bus) | PCIE_CONF_DEV(PCI_SLOT(devfn)) | \
- PCIE_CONF_FUNC(PCI_FUNC(devfn)) | PCIE_CONF_REG(where))
-
#define PIO_RETRY_CNT 500
#define PIO_RETRY_DELAY 2 /* 2 us*/
@@ -683,7 +676,7 @@ static int advk_pcie_rd_conf(struct pci_bus *bus, u32 devfn,
advk_writel(pcie, reg, PIO_CTRL);
/* Program the address registers */
- reg = PCIE_CONF_ADDR(bus->number, devfn, where);
+ reg = PCIE_ECAM_OFFSET(bus, devfn, (where & 0xffc));
advk_writel(pcie, reg, PIO_ADDR_LS);
advk_writel(pcie, 0, PIO_ADDR_MS);
@@ -744,7 +737,7 @@ static int advk_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
advk_writel(pcie, reg, PIO_CTRL);
/* Program the address registers */
- reg = PCIE_CONF_ADDR(bus->number, devfn, where);
+ reg = PCIE_ECAM_OFFSET(bus, devfn, (where & 0xffc));
advk_writel(pcie, reg, PIO_ADDR_LS);
advk_writel(pcie, 0, PIO_ADDR_MS);
diff --git a/drivers/pci/controller/pci-host-generic.c b/drivers/pci/controller/pci-host-generic.c
index b51977abfdf1..c1c69b11615f 100644
--- a/drivers/pci/controller/pci-host-generic.c
+++ b/drivers/pci/controller/pci-host-generic.c
@@ -49,7 +49,7 @@ static void __iomem *pci_dw_ecam_map_bus(struct pci_bus *bus,
}
static const struct pci_ecam_ops pci_dw_ecam_bus_ops = {
- .bus_shift = 20,
+ .bus_shift = PCIE_ECAM_BUS_SHIFT,
.pci_ops = {
.map_bus = pci_dw_ecam_map_bus,
.read = pci_generic_config_read,
diff --git a/drivers/pci/controller/pci-thunder-ecam.c b/drivers/pci/controller/pci-thunder-ecam.c
index 7e8835fee5f7..22ed7e995b39 100644
--- a/drivers/pci/controller/pci-thunder-ecam.c
+++ b/drivers/pci/controller/pci-thunder-ecam.c
@@ -346,7 +346,7 @@ static int thunder_ecam_config_write(struct pci_bus *bus, unsigned int devfn,
}
const struct pci_ecam_ops pci_thunder_ecam_ops = {
- .bus_shift = 20,
+ .bus_shift = PCIE_ECAM_BUS_SHIFT,
.pci_ops = {
.map_bus = pci_ecam_map_bus,
.read = thunder_ecam_config_read,
diff --git a/drivers/pci/controller/pci-thunder-pem.c b/drivers/pci/controller/pci-thunder-pem.c
index 3f847969143e..1a3f70ac61fc 100644
--- a/drivers/pci/controller/pci-thunder-pem.c
+++ b/drivers/pci/controller/pci-thunder-pem.c
@@ -19,6 +19,15 @@
#define PEM_CFG_WR 0x28
#define PEM_CFG_RD 0x30
+/*
+ * Enhanced Configuration Access Mechanism (ECAM)
+ *
+ * N.B. This is a non-standard platform-specific ECAM bus shift value. For
+ * standard values defined in the PCI Express Base Specification see
+ * include/linux/pci-ecam.h.
+ */
+#define THUNDER_PCIE_ECAM_BUS_SHIFT 24
+
struct thunder_pem_pci {
u32 ea_entry[3];
void __iomem *pem_reg_base;
@@ -404,7 +413,7 @@ static int thunder_pem_acpi_init(struct pci_config_window *cfg)
}
const struct pci_ecam_ops thunder_pem_ecam_ops = {
- .bus_shift = 24,
+ .bus_shift = THUNDER_PCIE_ECAM_BUS_SHIFT,
.init = thunder_pem_acpi_init,
.pci_ops = {
.map_bus = pci_ecam_map_bus,
@@ -441,7 +450,7 @@ static int thunder_pem_platform_init(struct pci_config_window *cfg)
}
static const struct pci_ecam_ops pci_thunder_pem_ops = {
- .bus_shift = 24,
+ .bus_shift = THUNDER_PCIE_ECAM_BUS_SHIFT,
.init = thunder_pem_platform_init,
.pci_ops = {
.map_bus = pci_ecam_map_bus,
diff --git a/drivers/pci/controller/pci-xgene.c b/drivers/pci/controller/pci-xgene.c
index 8e0db84f089d..85e7c98265e8 100644
--- a/drivers/pci/controller/pci-xgene.c
+++ b/drivers/pci/controller/pci-xgene.c
@@ -257,7 +257,6 @@ static int xgene_v1_pcie_ecam_init(struct pci_config_window *cfg)
}
const struct pci_ecam_ops xgene_v1_pcie_ecam_ops = {
- .bus_shift = 16,
.init = xgene_v1_pcie_ecam_init,
.pci_ops = {
.map_bus = xgene_pcie_map_bus,
@@ -272,7 +271,6 @@ static int xgene_v2_pcie_ecam_init(struct pci_config_window *cfg)
}
const struct pci_ecam_ops xgene_v2_pcie_ecam_ops = {
- .bus_shift = 16,
.init = xgene_v2_pcie_ecam_init,
.pci_ops = {
.map_bus = xgene_pcie_map_bus,
diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 85fa7d54f11f..5d1e64550bba 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -22,6 +22,7 @@
#include <linux/of_pci.h>
#include <linux/of_platform.h>
#include <linux/pci.h>
+#include <linux/pci-ecam.h>
#include <linux/printk.h>
#include <linux/sizes.h>
#include <linux/slab.h>
@@ -116,11 +117,7 @@
#define PCIE_MSI_INTR2_MASK_CLR 0x4514
#define PCIE_EXT_CFG_DATA 0x8000
-
#define PCIE_EXT_CFG_INDEX 0x9000
-#define PCIE_EXT_BUSNUM_SHIFT 20
-#define PCIE_EXT_SLOT_SHIFT 15
-#define PCIE_EXT_FUNC_SHIFT 12
#define PCIE_RGR1_SW_INIT_1 0x9210
#define PCIE_RGR1_SW_INIT_1_PERST_MASK 0x1
@@ -569,15 +566,6 @@ static bool brcm_pcie_link_up(struct brcm_pcie *pcie)
return dla && plu;
}
-/* Configuration space read/write support */
-static inline int brcm_pcie_cfg_index(int busnr, int devfn, int reg)
-{
- return ((PCI_SLOT(devfn) & 0x1f) << PCIE_EXT_SLOT_SHIFT)
- | ((PCI_FUNC(devfn) & 0x07) << PCIE_EXT_FUNC_SHIFT)
- | (busnr << PCIE_EXT_BUSNUM_SHIFT)
- | (reg & ~3);
-}
-
static void __iomem *brcm_pcie_map_conf(struct pci_bus *bus, unsigned int devfn,
int where)
{
@@ -590,7 +578,7 @@ static void __iomem *brcm_pcie_map_conf(struct pci_bus *bus, unsigned int devfn,
return PCI_SLOT(devfn) ? NULL : base + where;
/* For devices, write to the config space index register */
- idx = brcm_pcie_cfg_index(bus->number, devfn, 0);
+ idx = PCIE_ECAM_BUS(bus->number) | PCIE_ECAM_DEVFN(devfn);
writel(idx, pcie->base + PCIE_EXT_CFG_INDEX);
return base + PCIE_EXT_CFG_DATA + where;
}
diff --git a/drivers/pci/controller/pcie-iproc.c b/drivers/pci/controller/pcie-iproc.c
index 905e93808243..30abe4b7be70 100644
--- a/drivers/pci/controller/pcie-iproc.c
+++ b/drivers/pci/controller/pcie-iproc.c
@@ -6,6 +6,7 @@
#include <linux/kernel.h>
#include <linux/pci.h>
+#include <linux/pci-ecam.h>
#include <linux/msi.h>
#include <linux/clk.h>
#include <linux/module.h>
@@ -39,15 +40,7 @@
#define CFG_IND_ADDR_MASK 0x00001ffc
-#define CFG_ADDR_BUS_NUM_SHIFT 20
-#define CFG_ADDR_BUS_NUM_MASK 0x0ff00000
-#define CFG_ADDR_DEV_NUM_SHIFT 15
-#define CFG_ADDR_DEV_NUM_MASK 0x000f8000
-#define CFG_ADDR_FUNC_NUM_SHIFT 12
-#define CFG_ADDR_FUNC_NUM_MASK 0x00007000
-#define CFG_ADDR_REG_NUM_SHIFT 2
#define CFG_ADDR_REG_NUM_MASK 0x00000ffc
-#define CFG_ADDR_CFG_TYPE_SHIFT 0
#define CFG_ADDR_CFG_TYPE_MASK 0x00000003
#define SYS_RC_INTX_MASK 0xf
@@ -459,18 +452,16 @@ static inline void iproc_pcie_apb_err_disable(struct pci_bus *bus,
static void __iomem *iproc_pcie_map_ep_cfg_reg(struct iproc_pcie *pcie,
unsigned int busno,
- unsigned int slot,
- unsigned int fn,
+ unsigned int devfn,
int where)
{
u16 offset;
u32 val;
/* EP device access */
- val = (busno << CFG_ADDR_BUS_NUM_SHIFT) |
- (slot << CFG_ADDR_DEV_NUM_SHIFT) |
- (fn << CFG_ADDR_FUNC_NUM_SHIFT) |
- (where & CFG_ADDR_REG_NUM_MASK) |
+ val = PCIE_ECAM_BUS(busno) |
+ PCIE_ECAM_DEVFN(devfn) |
+ PCIE_ECAM_REG(where & CFG_ADDR_REG_NUM_MASK) |
(1 & CFG_ADDR_CFG_TYPE_MASK);
iproc_pcie_write_reg(pcie, IPROC_PCIE_CFG_ADDR, val);
@@ -574,8 +565,6 @@ static int iproc_pcie_config_read(struct pci_bus *bus, unsigned int devfn,
int where, int size, u32 *val)
{
struct iproc_pcie *pcie = iproc_data(bus);
- unsigned int slot = PCI_SLOT(devfn);
- unsigned int fn = PCI_FUNC(devfn);
unsigned int busno = bus->number;
void __iomem *cfg_data_p;
unsigned int data;
@@ -590,7 +579,7 @@ static int iproc_pcie_config_read(struct pci_bus *bus, unsigned int devfn,
return ret;
}
- cfg_data_p = iproc_pcie_map_ep_cfg_reg(pcie, busno, slot, fn, where);
+ cfg_data_p = iproc_pcie_map_ep_cfg_reg(pcie, busno, devfn, where);
if (!cfg_data_p)
return PCIBIOS_DEVICE_NOT_FOUND;
@@ -631,13 +620,11 @@ static void __iomem *iproc_pcie_map_cfg_bus(struct iproc_pcie *pcie,
int busno, unsigned int devfn,
int where)
{
- unsigned slot = PCI_SLOT(devfn);
- unsigned fn = PCI_FUNC(devfn);
u16 offset;
/* root complex access */
if (busno == 0) {
- if (slot > 0 || fn > 0)
+ if (PCIE_ECAM_DEVFN(devfn) > 0)
return NULL;
iproc_pcie_write_reg(pcie, IPROC_PCIE_CFG_IND_ADDR,
@@ -649,7 +636,7 @@ static void __iomem *iproc_pcie_map_cfg_bus(struct iproc_pcie *pcie,
return (pcie->base + offset);
}
- return iproc_pcie_map_ep_cfg_reg(pcie, busno, slot, fn, where);
+ return iproc_pcie_map_ep_cfg_reg(pcie, busno, devfn, where);
}
static void __iomem *iproc_pcie_bus_map_cfg_bus(struct pci_bus *bus,
diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
index 0bb2fb3e8a0b..4c069f8fa420 100644
--- a/drivers/pci/controller/pcie-rockchip-host.c
+++ b/drivers/pci/controller/pcie-rockchip-host.c
@@ -160,12 +160,11 @@ static int rockchip_pcie_rd_other_conf(struct rockchip_pcie *rockchip,
struct pci_bus *bus, u32 devfn,
int where, int size, u32 *val)
{
- u32 busdev;
+ void __iomem *addr;
- busdev = PCIE_ECAM_ADDR(bus->number, PCI_SLOT(devfn),
- PCI_FUNC(devfn), where);
+ addr = rockchip->reg_base + PCIE_ECAM_OFFSET(bus, devfn, where);
- if (!IS_ALIGNED(busdev, size)) {
+ if (!IS_ALIGNED((uintptr_t)addr, size)) {
*val = 0;
return PCIBIOS_BAD_REGISTER_NUMBER;
}
@@ -178,11 +177,11 @@ static int rockchip_pcie_rd_other_conf(struct rockchip_pcie *rockchip,
AXI_WRAPPER_TYPE1_CFG);
if (size == 4) {
- *val = readl(rockchip->reg_base + busdev);
+ *val = readl(addr);
} else if (size == 2) {
- *val = readw(rockchip->reg_base + busdev);
+ *val = readw(addr);
} else if (size == 1) {
- *val = readb(rockchip->reg_base + busdev);
+ *val = readb(addr);
} else {
*val = 0;
return PCIBIOS_BAD_REGISTER_NUMBER;
@@ -194,11 +193,11 @@ static int rockchip_pcie_wr_other_conf(struct rockchip_pcie *rockchip,
struct pci_bus *bus, u32 devfn,
int where, int size, u32 val)
{
- u32 busdev;
+ void __iomem *addr;
- busdev = PCIE_ECAM_ADDR(bus->number, PCI_SLOT(devfn),
- PCI_FUNC(devfn), where);
- if (!IS_ALIGNED(busdev, size))
+ addr = rockchip->reg_base + PCIE_ECAM_OFFSET(bus, devfn, where);
+
+ if (!IS_ALIGNED((uintptr_t)addr, size))
return PCIBIOS_BAD_REGISTER_NUMBER;
if (pci_is_root_bus(bus->parent))
@@ -209,11 +208,11 @@ static int rockchip_pcie_wr_other_conf(struct rockchip_pcie *rockchip,
AXI_WRAPPER_TYPE1_CFG);
if (size == 4)
- writel(val, rockchip->reg_base + busdev);
+ writel(val, addr);
else if (size == 2)
- writew(val, rockchip->reg_base + busdev);
+ writew(val, addr);
else if (size == 1)
- writeb(val, rockchip->reg_base + busdev);
+ writeb(val, addr);
else
return PCIBIOS_BAD_REGISTER_NUMBER;
diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
index c7d0178fc8c2..1650a5087450 100644
--- a/drivers/pci/controller/pcie-rockchip.h
+++ b/drivers/pci/controller/pcie-rockchip.h
@@ -13,6 +13,7 @@
#include <linux/kernel.h>
#include <linux/pci.h>
+#include <linux/pci-ecam.h>
/*
* The upper 16 bits of PCIE_CLIENT_CONFIG are a write mask for the lower 16
@@ -178,13 +179,6 @@
#define MIN_AXI_ADDR_BITS_PASSED 8
#define PCIE_RC_SEND_PME_OFF 0x11960
#define ROCKCHIP_VENDOR_ID 0x1d87
-#define PCIE_ECAM_BUS(x) (((x) & 0xff) << 20)
-#define PCIE_ECAM_DEV(x) (((x) & 0x1f) << 15)
-#define PCIE_ECAM_FUNC(x) (((x) & 0x7) << 12)
-#define PCIE_ECAM_REG(x) (((x) & 0xfff) << 0)
-#define PCIE_ECAM_ADDR(bus, dev, func, reg) \
- (PCIE_ECAM_BUS(bus) | PCIE_ECAM_DEV(dev) | \
- PCIE_ECAM_FUNC(func) | PCIE_ECAM_REG(reg))
#define PCIE_LINK_IS_L2(x) \
(((x) & PCIE_CLIENT_DEBUG_LTSSM_MASK) == PCIE_CLIENT_DEBUG_LTSSM_L2)
#define PCIE_LINK_UP(x) \
diff --git a/drivers/pci/controller/pcie-tango.c b/drivers/pci/controller/pcie-tango.c
index d093a8ce4bb1..8f0d695afbde 100644
--- a/drivers/pci/controller/pcie-tango.c
+++ b/drivers/pci/controller/pcie-tango.c
@@ -208,7 +208,7 @@ static int smp8759_config_write(struct pci_bus *bus, unsigned int devfn,
}
static const struct pci_ecam_ops smp8759_ecam_ops = {
- .bus_shift = 20,
+ .bus_shift = PCIE_ECAM_BUS_SHIFT,
.pci_ops = {
.map_bus = pci_ecam_map_bus,
.read = smp8759_config_read,
diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
index f3cf7d61924f..cfd12b75bacb 100644
--- a/drivers/pci/controller/pcie-xilinx-nwl.c
+++ b/drivers/pci/controller/pcie-xilinx-nwl.c
@@ -18,6 +18,7 @@
#include <linux/of_platform.h>
#include <linux/of_irq.h>
#include <linux/pci.h>
+#include <linux/pci-ecam.h>
#include <linux/platform_device.h>
#include <linux/irqchip/chained_irq.h>
@@ -124,8 +125,6 @@
#define E_ECAM_CR_ENABLE BIT(0)
#define E_ECAM_SIZE_LOC GENMASK(20, 16)
#define E_ECAM_SIZE_SHIFT 16
-#define ECAM_BUS_LOC_SHIFT 20
-#define ECAM_DEV_LOC_SHIFT 12
#define NWL_ECAM_VALUE_DEFAULT 12
#define CFG_DMA_REG_BAR GENMASK(2, 0)
@@ -240,15 +239,11 @@ static void __iomem *nwl_pcie_map_bus(struct pci_bus *bus, unsigned int devfn,
int where)
{
struct nwl_pcie *pcie = bus->sysdata;
- int relbus;
if (!nwl_pcie_valid_device(bus, devfn))
return NULL;
- relbus = (bus->number << ECAM_BUS_LOC_SHIFT) |
- (devfn << ECAM_DEV_LOC_SHIFT);
-
- return pcie->ecam_base + relbus + where;
+ return pcie->ecam_base + PCIE_ECAM_OFFSET(bus, devfn, where);
}
/* PCIe operations */
diff --git a/drivers/pci/controller/pcie-xilinx.c b/drivers/pci/controller/pcie-xilinx.c
index 8523be61bba5..49bde5266aa2 100644
--- a/drivers/pci/controller/pcie-xilinx.c
+++ b/drivers/pci/controller/pcie-xilinx.c
@@ -21,6 +21,7 @@
#include <linux/of_platform.h>
#include <linux/of_irq.h>
#include <linux/pci.h>
+#include <linux/pci-ecam.h>
#include <linux/platform_device.h>
#include "../pci.h"
@@ -86,10 +87,6 @@
/* Phy Status/Control Register definitions */
#define XILINX_PCIE_REG_PSCR_LNKUP BIT(11)
-/* ECAM definitions */
-#define ECAM_BUS_NUM_SHIFT 20
-#define ECAM_DEV_NUM_SHIFT 12
-
/* Number of MSI IRQs */
#define XILINX_NUM_MSI_IRQS 128
@@ -183,15 +180,11 @@ static void __iomem *xilinx_pcie_map_bus(struct pci_bus *bus,
unsigned int devfn, int where)
{
struct xilinx_pcie_port *port = bus->sysdata;
- int relbus;
if (!xilinx_pcie_valid_device(bus, devfn))
return NULL;
- relbus = (bus->number << ECAM_BUS_NUM_SHIFT) |
- (devfn << ECAM_DEV_NUM_SHIFT);
-
- return port->reg_base + relbus + where;
+ return port->reg_base + PCIE_ECAM_OFFSET(bus, devfn, where);
}
/* PCIe operations */
diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index f69ef8c89f72..b14751845263 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -11,6 +11,7 @@
#include <linux/module.h>
#include <linux/msi.h>
#include <linux/pci.h>
+#include <linux/pci-ecam.h>
#include <linux/srcu.h>
#include <linux/rculist.h>
#include <linux/rcupdate.h>
@@ -302,8 +303,8 @@ static char __iomem *vmd_cfg_addr(struct vmd_dev *vmd, struct pci_bus *bus,
unsigned int devfn, int reg, int len)
{
char __iomem *addr = vmd->cfgbar +
- ((bus->number - vmd->busn_start) << 20) +
- (devfn << 12) + reg;
+ PCIE_ECAM_BUS(bus->number - vmd->busn_start) +
+ PCIE_ECAM_DEVFN(devfn) + PCIE_ECAM_REG(reg);
if ((addr - vmd->cfgbar) + len >=
resource_size(&vmd->dev->resource[VMD_CFGBAR]))
diff --git a/drivers/pci/ecam.c b/drivers/pci/ecam.c
index 8f065a42fc1a..ffd010290084 100644
--- a/drivers/pci/ecam.c
+++ b/drivers/pci/ecam.c
@@ -149,7 +149,7 @@ EXPORT_SYMBOL_GPL(pci_ecam_map_bus);
/* ECAM ops */
const struct pci_ecam_ops pci_generic_ecam_ops = {
- .bus_shift = 20,
+ .bus_shift = PCIE_ECAM_BUS_SHIFT,
.pci_ops = {
.map_bus = pci_ecam_map_bus,
.read = pci_generic_config_read,
@@ -161,7 +161,7 @@ EXPORT_SYMBOL_GPL(pci_generic_ecam_ops);
#if defined(CONFIG_ACPI) && defined(CONFIG_PCI_QUIRKS)
/* ECAM ops for 32-bit access only (non-compliant) */
const struct pci_ecam_ops pci_32b_ops = {
- .bus_shift = 20,
+ .bus_shift = PCIE_ECAM_BUS_SHIFT,
.pci_ops = {
.map_bus = pci_ecam_map_bus,
.read = pci_generic_config_read32,
diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
index 1af5cb02ef7f..3ca5674fdf5e 100644
--- a/include/linux/pci-ecam.h
+++ b/include/linux/pci-ecam.h
@@ -9,6 +9,30 @@
#include <linux/kernel.h>
#include <linux/platform_device.h>
+/*
+ * Memory address shift values for the byte-level address that
+ * can be used when accessing the PCI Express Configuration Space.
+ */
+
+/*
+ * Enhanced Configuration Access Mechanism (ECAM)
+ *
+ * See PCI Express Base Specification, Revision 5.0, Version 1.0,
+ * Section 7.2.2, Table 7-1, p. 677.
+ */
+#define PCIE_ECAM_BUS_SHIFT 20 /* Bus Number */
+#define PCIE_ECAM_DEV_SHIFT 15 /* Device Number */
+#define PCIE_ECAM_FUN_SHIFT 12 /* Function Number */
+
+#define PCIE_ECAM_BUS(x) (((x) & 0xff) << PCIE_ECAM_BUS_SHIFT)
+#define PCIE_ECAM_DEVFN(x) (((x) & 0xff) << PCIE_ECAM_FUN_SHIFT)
+#define PCIE_ECAM_REG(x) ((x) & 0xfff)
+
+#define PCIE_ECAM_OFFSET(bus, devfn, where) \
+ (PCIE_ECAM_BUS(bus->number) | \
+ PCIE_ECAM_DEVFN(devfn) | \
+ PCIE_ECAM_REG(where))
+
/*
* struct to hold pci ops and bus shift of the config window
* for a PCI controller.
--
2.28.0
^ permalink raw reply related
* [PATCH] powernv/elog: Fix the race while processing OPAL error log event.
From: Mahesh Salgaonkar @ 2020-10-04 7:04 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Aneesh Kumar K.V, Oliver O'Halloran, Vasant Hegde
Every error log reported by OPAL is exported to userspace through a sysfs
interface and notified using kobject_uevent(). The userspace daemon
(opal_errd) then reads the error log and acknowledges it error log is saved
safely to disk. Once acknowledged the kernel removes the respective sysfs
file entry causing respective resources getting released including kobject.
However there are chances where user daemon may already be scanning elog
entries while new sysfs elog entry is being created by kernel. User daemon
may read this new entry and ack it even before kernel can notify userspace
about it through kobject_uevent() call. If that happens then we have a
potential race between elog_ack_store->kobject_put() and kobject_uevent
which can lead to use-after-free issue of a kernfs object resulting into a
kernel crash. This patch fixes this race by protecting a sysfs file
creation/notification using mutex lock per elog record.
Reported-by: Oliver O'Halloran <oohall@gmail.com>
Signed-off-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
---
arch/powerpc/platforms/powernv/opal-elog.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/arch/powerpc/platforms/powernv/opal-elog.c b/arch/powerpc/platforms/powernv/opal-elog.c
index 62ef7ad995da..4bba8f968ced 100644
--- a/arch/powerpc/platforms/powernv/opal-elog.c
+++ b/arch/powerpc/platforms/powernv/opal-elog.c
@@ -24,6 +24,7 @@ struct elog_obj {
uint64_t type;
size_t size;
char *buffer;
+ struct mutex elog_mutex;
};
#define to_elog_obj(x) container_of(x, struct elog_obj, kobj)
@@ -73,7 +74,11 @@ static ssize_t elog_ack_store(struct elog_obj *elog_obj,
size_t count)
{
opal_send_ack_elog(elog_obj->id);
+
+ /* Wait until kobject_uevent() is called */
+ mutex_lock(&elog_obj->elog_mutex);
sysfs_remove_file_self(&elog_obj->kobj, &attr->attr);
+ mutex_unlock(&elog_obj->elog_mutex);
kobject_put(&elog_obj->kobj);
return count;
}
@@ -193,6 +198,7 @@ static struct elog_obj *create_elog_obj(uint64_t id, size_t size, uint64_t type)
kobject_init(&elog->kobj, &elog_ktype);
sysfs_bin_attr_init(&elog->raw_attr);
+ mutex_init(&elog->elog_mutex);
elog->raw_attr.attr.name = "raw";
elog->raw_attr.attr.mode = 0400;
@@ -222,13 +228,24 @@ static struct elog_obj *create_elog_obj(uint64_t id, size_t size, uint64_t type)
return NULL;
}
+ /*
+ * As soon as sysfs file for this elog is created/activated there is
+ * chance opal_errd daemon might read and acknowledge this elog before
+ * kobject_uevent() is called. If that happens then we have a potential
+ * race between elog_ack_store->kobject_put() and kobject_uevent which
+ * leads to use-after-free issue of a kernfs object resulting into
+ * kernel crash. Take a mutex lock to avoid this race.
+ */
+ mutex_lock(&elog->elog_mutex);
rc = sysfs_create_bin_file(&elog->kobj, &elog->raw_attr);
if (rc) {
kobject_put(&elog->kobj);
+ mutex_unlock(&elog->elog_mutex);
return NULL;
}
kobject_uevent(&elog->kobj, KOBJ_ADD);
+ mutex_unlock(&elog->elog_mutex);
return elog;
}
^ permalink raw reply related
* Re: Where is the declaration of buffer used in kernel_param_ops .get functions?
From: Joe Perches @ 2020-10-04 2:11 UTC (permalink / raw)
To: Matthew Wilcox
Cc: kvm, Greg KH, linux-kernel, kvm-ppc, rcu, linux-mm, linuxppc-dev
In-Reply-To: <20201004013626.GE20115@casper.infradead.org>
On Sun, 2020-10-04 at 02:36 +0100, Matthew Wilcox wrote:
> On Sat, Oct 03, 2020 at 06:19:18PM -0700, Joe Perches wrote:
> > These patches came up because I was looking for
> > the location of the declaration of the buffer used
> > in kernel/params.c struct kernel_param_ops .get
> > functions.
> >
> > I didn't find it.
> >
> > I want to see if it's appropriate to convert the
> > sprintf family of functions used in these .get
> > functions to sysfs_emit.
> >
> > Patches submitted here:
> > https://lore.kernel.org/lkml/5d606519698ce4c8f1203a2b35797d8254c6050a.1600285923.git.joe@perches.com/T/
> >
> > Anyone know if it's appropriate to change the
> > sprintf-like uses in these functions to sysfs_emit
> > and/or sysfs_emit_at?
>
> There's a lot of preprocessor magic to wade through.
>
> I'm pretty sure this comes through include/linux/moduleparam.h
> and kernel/module.c.
Dunno, looked there, still can't find it.
btw:
The __module_param_call macro looks very dodgy
as it uses both __used and __attribute__((unused))
and likely one of them should be removed (unused?)
It looks like the comes from varying definitions of
__attribute_used__ eventually converted to __used
for old gcc versions 2, 3, and 4.
1da177e4c3f4:include/linux/compiler-gcc2.h:#define __attribute_used__ __attribute__((__unused__))
1da177e4c3f4:include/linux/compiler-gcc3.h:# define __attribute_used__ __attribute__((__used__))
1da177e4c3f4:include/linux/compiler-gcc3.h:# define __attribute_used__ __attribute__((__unused__))
1da177e4c3f4:include/linux/compiler-gcc4.h:#define __attribute_used__ __attribute__((__used__))
Maybe:
---
include/linux/moduleparam.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 47879fc7f75e..fc820b27fb00 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -288,10 +288,10 @@ struct kparam_array
/* Default value instead of permissions? */ \
static const char __param_str_##name[] = prefix #name; \
static struct kernel_param __moduleparam_const __param_##name \
- __used \
- __attribute__ ((unused,__section__ ("__param"),aligned(sizeof(void *)))) \
- = { __param_str_##name, THIS_MODULE, ops, \
- VERIFY_OCTAL_PERMISSIONS(perm), level, flags, { arg } }
+ __used __section("__param") __aligned(sizeof(void *)) = { \
+ __param_str_##name, THIS_MODULE, ops, \
+ VERIFY_OCTAL_PERMISSIONS(perm), level, flags, { arg } \
+ }
/* Obsolete - use module_param_cb() */
#define module_param_call(name, _set, _get, arg, perm) \
^ permalink raw reply related
* Re: Where is the declaration of buffer used in kernel_param_ops .get functions?
From: Matthew Wilcox @ 2020-10-04 1:36 UTC (permalink / raw)
To: Joe Perches
Cc: kvm, Greg KH, linux-kernel, kvm-ppc, rcu, linux-mm, linuxppc-dev
In-Reply-To: <250919192de237dadf36218ee6b4dabf1bd4cbde.camel@perches.com>
On Sat, Oct 03, 2020 at 06:19:18PM -0700, Joe Perches wrote:
> These patches came up because I was looking for
> the location of the declaration of the buffer used
> in kernel/params.c struct kernel_param_ops .get
> functions.
>
> I didn't find it.
>
> I want to see if it's appropriate to convert the
> sprintf family of functions used in these .get
> functions to sysfs_emit.
>
> Patches submitted here:
> https://lore.kernel.org/lkml/5d606519698ce4c8f1203a2b35797d8254c6050a.1600285923.git.joe@perches.com/T/
>
> Anyone know if it's appropriate to change the
> sprintf-like uses in these functions to sysfs_emit
> and/or sysfs_emit_at?
There's a lot of preprocessor magic to wade through.
I'm pretty sure this comes through include/linux/moduleparam.h
and kernel/module.c.
^ permalink raw reply
* Where is the declaration of buffer used in kernel_param_ops .get functions?
From: Joe Perches @ 2020-10-04 1:19 UTC (permalink / raw)
To: Greg KH; +Cc: kvm, linux-kernel, kvm-ppc, rcu, linux-mm, linuxppc-dev
In-Reply-To: <cover.1601770305.git.joe@perches.com>
These patches came up because I was looking for
the location of the declaration of the buffer used
in kernel/params.c struct kernel_param_ops .get
functions.
I didn't find it.
I want to see if it's appropriate to convert the
sprintf family of functions used in these .get
functions to sysfs_emit.
Patches submitted here:
https://lore.kernel.org/lkml/5d606519698ce4c8f1203a2b35797d8254c6050a.1600285923.git.joe@perches.com/T/
Anyone know if it's appropriate to change the
sprintf-like uses in these functions to sysfs_emit
and/or sysfs_emit_at?
^ permalink raw reply
* [PATCH 1/4] KVM: PPC: Book3S HV: Make struct kernel_param_ops definition const
From: Joe Perches @ 2020-10-04 0:18 UTC (permalink / raw)
To: Paul Mackerras
Cc: kvm-ppc, Athira Rajeev, Tianjia Zhang, Ram Pai, linux-kernel,
Sean Christopherson, Bharata B Rao, Davidlohr Bueso,
Nicholas Piggin, Paolo Bonzini, Laurent Dufour, linuxppc-dev
In-Reply-To: <cover.1601770305.git.joe@perches.com>
This should be const, so make it so.
Signed-off-by: Joe Perches <joe@perches.com>
---
arch/powerpc/kvm/book3s_hv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 4ba06a2a306c..2b215852cdc9 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -111,7 +111,7 @@ module_param(one_vm_per_core, bool, S_IRUGO | S_IWUSR);
MODULE_PARM_DESC(one_vm_per_core, "Only run vCPUs from the same VM on a core (requires indep_threads_mode=N)");
#ifdef CONFIG_KVM_XICS
-static struct kernel_param_ops module_param_ops = {
+static const struct kernel_param_ops module_param_ops = {
.set = param_set_int,
.get = param_get_int,
};
--
2.26.0
^ permalink raw reply related
* [PATCH 0/4] treewide: Make definitions of struct kernel_param_ops const
From: Joe Perches @ 2020-10-04 0:18 UTC (permalink / raw)
To: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, Steven Rostedt, Mathieu Desnoyers, Lai Jiangshan,
Joel Fernandes, kvm-ppc, kvm, rcu, linux-mm
Cc: linuxppc-dev, linux-kernel
Using const is good as it reduces data size.
Joe Perches (4):
KVM: PPC: Book3S HV: Make struct kernel_param_ops definition const
kvm x86/mmu: Make struct kernel_param_ops definitions const
rcu/tree: Make struct kernel_param_ops definitions const
mm/zswap: Make struct kernel_param_ops definitions const
arch/powerpc/kvm/book3s_hv.c | 2 +-
arch/x86/kvm/mmu/mmu.c | 4 ++--
kernel/rcu/tree.c | 4 ++--
mm/zswap.c | 6 +++---
4 files changed, 8 insertions(+), 8 deletions(-)
--
2.26.0
^ permalink raw reply
* Re: [PATCH v4 net-next 0/2] Add Seville Ethernet switch to T1040RDB
From: David Miller @ 2020-10-04 0:03 UTC (permalink / raw)
To: vladimir.oltean
Cc: devicetree, andrew, madalin.bucur, netdev, radu-andrei.bulie,
linuxppc-dev, linux-kernel, fido_max, robh+dt, paulus, kuba,
shawnguo
In-Reply-To: <20201002134106.3485970-1-vladimir.oltean@nxp.com>
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Date: Fri, 2 Oct 2020 16:41:04 +0300
> Seville is a DSA switch that is embedded inside the T1040 SoC, and
> supported by the mscc_seville DSA driver inside drivers/net/dsa/ocelot.
>
> This series adds this switch to the SoC's dtsi files and to the T1040RDB
> board file.
>
> I would like to send this series through net-next. There is no conflict
> with other patches submitted to T1040 device tree. Maybe this could at
> least get an ACK from devicetree maintainers.
Series applied, thank you.
^ permalink raw reply
* Re: [PATCH] crypto: talitos - Fix sparse warnings
From: Christophe Leroy @ 2020-10-03 17:15 UTC (permalink / raw)
To: Herbert Xu
Cc: linuxppc-dev, Kim Phillips, Linux Crypto Mailing List,
Horia Geantă
In-Reply-To: <20201002124341.GA1587@gondor.apana.org.au>
Quoting Herbert Xu <herbert@gondor.apana.org.au>:
> On Fri, Oct 02, 2020 at 10:42:23PM +1000, Herbert Xu wrote:
>>
>> I don't think that's a valid optimisation unless it's backed up
>> with real numbers.
>
> Also it only matters on little-endian as they're no-ops on big-endian.
> If I'm right then this driver never even worked on little-endian.
>
The following changes fix the sparse warnings with less churn:
---
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 7c547352a862..992d58a4dbf1 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -460,7 +460,7 @@ DEF_TALITOS2_DONE(ch1_3, TALITOS2_ISR_CH_1_3_DONE)
/*
* locate current (offending) descriptor
*/
-static u32 current_desc_hdr(struct device *dev, int ch)
+static __be32 current_desc_hdr(struct device *dev, int ch)
{
struct talitos_private *priv = dev_get_drvdata(dev);
int tail, iter;
@@ -478,7 +478,7 @@ static u32 current_desc_hdr(struct device *dev, int ch)
iter = tail;
while (priv->chan[ch].fifo[iter].dma_desc != cur_desc &&
- priv->chan[ch].fifo[iter].desc->next_desc != cur_desc) {
+ priv->chan[ch].fifo[iter].desc->next_desc != cpu_to_be32(cur_desc)) {
iter = (iter + 1) & (priv->fifo_len - 1);
if (iter == tail) {
dev_err(dev, "couldn't locate current descriptor\n");
@@ -486,7 +486,7 @@ static u32 current_desc_hdr(struct device *dev, int ch)
}
}
- if (priv->chan[ch].fifo[iter].desc->next_desc == cur_desc) {
+ if (priv->chan[ch].fifo[iter].desc->next_desc == cpu_to_be32(cur_desc)) {
struct talitos_edesc *edesc;
edesc = container_of(priv->chan[ch].fifo[iter].desc,
@@ -501,13 +501,13 @@ static u32 current_desc_hdr(struct device *dev, int ch)
/*
* user diagnostics; report root cause of error based on execution
unit status
*/
-static void report_eu_error(struct device *dev, int ch, u32 desc_hdr)
+static void report_eu_error(struct device *dev, int ch, __be32 desc_hdr)
{
struct talitos_private *priv = dev_get_drvdata(dev);
int i;
if (!desc_hdr)
- desc_hdr = in_be32(priv->chan[ch].reg + TALITOS_DESCBUF);
+ desc_hdr = cpu_to_be32(in_be32(priv->chan[ch].reg + TALITOS_DESCBUF));
switch (desc_hdr & DESC_HDR_SEL0_MASK) {
case DESC_HDR_SEL0_AFEU:
---
Christophe
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox