* Re: [PATCH 00/25] Change tty_port(standard)_install's return type
From: Alan Cox @ 2018-09-12 14:41 UTC (permalink / raw)
To: Jaejoong Kim
Cc: linux-um, netdev, linux-mmc, linux-s390, devel, greybus-dev,
linuxppc-dev, linux-serial, sparclinux, linux-usb,
linux-bluetooth, linux-kernel, Greg Kroah-Hartman, Jiri Slaby
In-Reply-To: <1536029091-4426-1-git-send-email-climbbb.kim@gmail.com>
On Tue, 4 Sep 2018 11:44:26 +0900
Jaejoong Kim <climbbb.kim@gmail.com> wrote:
> Many drivers with tty use the tty_stand_install(). But, there is no
> need to handle the error, since it always returns 0.
And what happens if another change means it can fail again. It's just a
property of the current implementation that it can't. It used to fail.
This seems to be a ton of unneccessary churn that will end up just having
to be reversed again some day in the future.
Alan
^ permalink raw reply
* Re: [PATCH 0/2] sriov enablement on s390
From: Bjorn Helgaas @ 2018-09-12 13:02 UTC (permalink / raw)
To: Sebastian Ott
Cc: Bjorn Helgaas, linux-pci, Arnd Bergmann, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman, Russell Currey, linuxppc-dev
In-Reply-To: <20180912123411.23229-1-sebott@linux.ibm.com>
[+cc Arnd, powerpc folks]
On Wed, Sep 12, 2018 at 02:34:09PM +0200, Sebastian Ott wrote:
> Hello Bjorn,
>
> On s390 we currently handle SRIOV within firmware. Which means
> that the PF is under firmware control and not visible to operating
> systems. SRIOV enablement happens within firmware and VFs are
> passed through to logical partitions.
>
> I'm working on a new mode were the PF is under operating system
> control (including SRIOV enablement). However we still need
> firmware support to access the VFs. The way this is supposed
> to work is that when firmware traps the SRIOV enablement it
> will present machine checks to the logical partition that
> triggered the SRIOV enablement and provide the VFs via hotplug
> events.
>
> The problem I'm faced with is that the VF detection code in
> sriov_enable leads to unusable functions in s390.
We're moving away from the weak function implementation style. Can
you take a look at Arnd's work here, which uses pci_host_bridge
callbacks instead?
https://lkml.kernel.org/r/20180817102645.3839621-1-arnd@arndb.de
I cc'd some powerpc folks because they also have a fair amount of
arch-specific SR-IOV code that might one day move in this direction.
> Sebastian Ott (2):
> pci: provide pcibios_sriov_add_vfs
> s390/pci: handle function enumeration after sriov enablement
>
> arch/s390/pci/pci.c | 11 +++++++++++
> drivers/pci/iov.c | 43 +++++++++++++++++++++++++++++++------------
> include/linux/pci.h | 2 ++
> 3 files changed, 44 insertions(+), 12 deletions(-)
>
> --
> 2.13.4
>
^ permalink raw reply
* Re: [PATCH v2 5/5] PCI/powerpc/eeh: Add pcibios hooks for preparing to rescan
From: Benjamin Herrenschmidt @ 2018-09-12 10:39 UTC (permalink / raw)
To: Sergey Miroshnichenko, Sam Bobroff; +Cc: linuxppc-dev, linux
In-Reply-To: <61e8e5b9-0ed2-1848-d676-64bcbd3b9cbc@yadro.com>
On Mon, 2018-09-10 at 19:00 +0300, Sergey Miroshnichenko wrote:
>
> Yes, missing a real EEH event is possible, unfortunately, and it is
> indeed worth mentioning.
>
> To reduce this probability the next patchset I'll post in a few days
> among other things puts all the affected device drivers to pause during
> rescan, mainly because of moving BARs and bridge windows, but it will
> also help here a bit.
How do you deal with moving BARs etc... within the segmenting
restrictions of EEH ?
It's a horrible mess right now and I don't know if the current code can
even work properly to be honest.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 5/5] PCI/powerpc/eeh: Add pcibios hooks for preparing to rescan
From: Benjamin Herrenschmidt @ 2018-09-12 10:37 UTC (permalink / raw)
To: Sergey Miroshnichenko, linuxppc-dev; +Cc: linux
In-Reply-To: <20180905154008.11130-6-s.miroshnichenko@yadro.com>
On Wed, 2018-09-05 at 18:40 +0300, Sergey Miroshnichenko wrote:
> Reading an empty slot returns all ones, which triggers a false
> EEH error event on PowerNV.
>
> New callbacks pcibios_rescan_prepare/done are introduced to
> pause/resume the EEH during rescan.
This freaks me out a bit. EEH will still happen in HW, you will just
drop the notifications. Disabling the interrupt globally is not a great
idea either as it will affect other unrelated devices or busses.
Just marking the specific bus might work if we deal with unfreezing it
properly but you need some locking there, your code doesn't seem to
have any.
Ben.
>
> Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
> ---
> arch/powerpc/include/asm/eeh.h | 2 ++
> arch/powerpc/kernel/eeh.c | 14 ++++++++++++++
> arch/powerpc/platforms/powernv/eeh-powernv.c | 20 ++++++++++++++++++++
> drivers/pci/probe.c | 14 ++++++++++++++
> include/linux/pci.h | 2 ++
> 5 files changed, 52 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
> index 219637ea69a1..926c3e31df99 100644
> --- a/arch/powerpc/include/asm/eeh.h
> +++ b/arch/powerpc/include/asm/eeh.h
> @@ -219,6 +219,8 @@ struct eeh_ops {
> int (*next_error)(struct eeh_pe **pe);
> int (*restore_config)(struct pci_dn *pdn);
> int (*notify_resume)(struct pci_dn *pdn);
> + int (*pause)(struct pci_bus *bus);
> + int (*resume)(struct pci_bus *bus);
> };
>
> extern int eeh_subsystem_flags;
> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> index 6ebba3e48b01..dce9b0978cb5 100644
> --- a/arch/powerpc/kernel/eeh.c
> +++ b/arch/powerpc/kernel/eeh.c
> @@ -1831,3 +1831,17 @@ static int __init eeh_init_proc(void)
> return 0;
> }
> __initcall(eeh_init_proc);
> +
> +void pcibios_rescan_prepare(struct pci_bus *bus)
> +{
> + if (eeh_ops && eeh_ops->pause) {
> + eeh_ops->pause(bus);
> + }
> +}
> +
> +void pcibios_rescan_done(struct pci_bus *bus)
> +{
> + if (eeh_ops && eeh_ops->resume) {
> + eeh_ops->resume(bus);
> + }
> +}
> diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
> index 3c1beae29f2d..9c9213d92550 100644
> --- a/arch/powerpc/platforms/powernv/eeh-powernv.c
> +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
> @@ -59,6 +59,24 @@ void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
> eeh_sysfs_add_device(pdev);
> }
>
> +static int pnv_eeh_pause(struct pci_bus *bus)
> +{
> + struct pci_controller *hose = pci_bus_to_host(bus);
> + struct pnv_phb *phb = hose->private_data;
> + phb->flags &= ~PNV_PHB_FLAG_EEH;
> + disable_irq(eeh_event_irq);
> + return 0;
> +}
> +
> +static int pnv_eeh_resume(struct pci_bus *bus)
> +{
> + struct pci_controller *hose = pci_bus_to_host(bus);
> + struct pnv_phb *phb = hose->private_data;
> + enable_irq(eeh_event_irq);
> + phb->flags |= PNV_PHB_FLAG_EEH;
> + return 0;
> +}
> +
> static int pnv_eeh_init(void)
> {
> struct pci_controller *hose;
> @@ -1710,6 +1728,8 @@ static struct eeh_ops pnv_eeh_ops = {
> .write_config = pnv_eeh_write_config,
> .next_error = pnv_eeh_next_error,
> .restore_config = pnv_eeh_restore_config,
> + .pause = pnv_eeh_pause,
> + .resume = pnv_eeh_resume,
> .notify_resume = NULL
> };
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index ec784009a36b..203368566896 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2893,6 +2893,14 @@ void __weak pcibios_remove_bus(struct pci_bus *bus)
> {
> }
>
> +void __weak pcibios_rescan_prepare(struct pci_bus *bus)
> +{
> +}
> +
> +void __weak pcibios_rescan_done(struct pci_bus *bus)
> +{
> +}
> +
> struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
> struct pci_ops *ops, void *sysdata, struct list_head *resources)
> {
> @@ -3147,9 +3155,15 @@ unsigned int pci_rescan_bus_bridge_resize(struct pci_dev *bridge)
> unsigned int pci_rescan_bus(struct pci_bus *bus)
> {
> unsigned int max;
> + struct pci_bus *root = bus;
> + while (!pci_is_root_bus(root)) {
> + root = root->parent;
> + }
>
> + pcibios_rescan_prepare(root);
> max = pci_scan_child_bus(bus);
> pci_assign_unassigned_bus_resources(bus);
> + pcibios_rescan_done(root);
> pci_bus_add_devices(bus);
>
> return max;
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index e72ca8dd6241..d7fe72aa53b3 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1952,6 +1952,8 @@ void pcibios_penalize_isa_irq(int irq, int active);
> int pcibios_alloc_irq(struct pci_dev *dev);
> void pcibios_free_irq(struct pci_dev *dev);
> resource_size_t pcibios_default_alignment(void);
> +void pcibios_rescan_prepare(struct pci_bus *bus);
> +void pcibios_rescan_done(struct pci_bus *bus);
>
> #ifdef CONFIG_HIBERNATE_CALLBACKS
> extern struct dev_pm_ops pcibios_pm_ops;
^ permalink raw reply
* Re: [PATCH 4/5] powerpc/powernv/pci: Enable reassigning the bus numbers
From: Benjamin Herrenschmidt @ 2018-09-12 10:35 UTC (permalink / raw)
To: Sergey Miroshnichenko, linuxppc-dev; +Cc: linux
In-Reply-To: <20180905154008.11130-5-s.miroshnichenko@yadro.com>
On Wed, 2018-09-05 at 18:40 +0300, Sergey Miroshnichenko wrote:
> PowerNV doesn't depend on PCIe topology info from DT anymore, and now
> it is able to enumerate the fabric and assign the bus numbers.
No it's not, at least unless we drop P7 support.
P7 has constraints on the bus ranges being aligned power-of-two for the
PE assignment to work, which is why we have to honor the firmware
provided numbers.
Additionally, this breaks the mapping between the firmware idea of the
bus numbers and Linux idea. This will probably break all of the SR-IOV
stuff.
Now we should probably fix it all by removing the FW bits completely
and doing it all from Linux, though we really need to better handle how
we deal with the segmented MMIO space.
I would also be weary of what other parts of the code depends on that
matching between the FW bdfn and the Linux bdfn.
Cheers,
Ben.
> Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
> ---
> arch/powerpc/platforms/powernv/pci.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
> index 6d4280086a08..f6eaca3123cd 100644
> --- a/arch/powerpc/platforms/powernv/pci.c
> +++ b/arch/powerpc/platforms/powernv/pci.c
> @@ -1104,6 +1104,7 @@ void __init pnv_pci_init(void)
> struct device_node *np;
>
> pci_add_flags(PCI_CAN_SKIP_ISA_ALIGN);
> + pci_add_flags(PCI_REASSIGN_ALL_BUS);
>
> /* If we don't have OPAL, eg. in sim, just skip PCI probe */
> if (!firmware_has_feature(FW_FEATURE_OPAL))
^ permalink raw reply
* Re: [PATCH 2/5] powerpc/pci: Create pci_dn on demand
From: Benjamin Herrenschmidt @ 2018-09-12 10:33 UTC (permalink / raw)
To: Sergey Miroshnichenko, linuxppc-dev; +Cc: linux
In-Reply-To: <20180905154008.11130-3-s.miroshnichenko@yadro.com>
On Wed, 2018-09-05 at 18:40 +0300, Sergey Miroshnichenko wrote:
> The pci_dn structures can be created not only from DT, but also
> directly from newly discovered PCIe devices, so allocate them
> dynamically.
I'd rather we moved towards killing pci_dn completely to be honest :)
> Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
> ---
> arch/powerpc/kernel/pci_dn.c | 69 +++++++++++++++++++++++++++---------
> 1 file changed, 52 insertions(+), 17 deletions(-)
>
> diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
> index ab147a1909c8..5ce752874827 100644
> --- a/arch/powerpc/kernel/pci_dn.c
> +++ b/arch/powerpc/kernel/pci_dn.c
> @@ -33,6 +33,8 @@
> #include <asm/firmware.h>
> #include <asm/eeh.h>
>
> +static struct pci_dn* create_pdn(struct pci_dev *pdev, struct pci_dn *parent);
> +
> /*
> * The function is used to find the firmware data of one
> * specific PCI device, which is attached to the indicated
> @@ -58,6 +60,10 @@ static struct pci_dn *pci_bus_to_pdn(struct pci_bus *bus)
> pbus = pbus->parent;
> }
>
> + if (!pbus->self && !pci_is_root_bus(pbus)) {
> + return NULL;
> + }
> +
> /*
> * Except virtual bus, all PCI buses should
> * have device nodes.
> @@ -65,13 +71,16 @@ static struct pci_dn *pci_bus_to_pdn(struct pci_bus *bus)
> dn = pci_bus_to_OF_node(pbus);
> pdn = dn ? PCI_DN(dn) : NULL;
>
> + if (!pdn && pbus->self) {
> + pdn = pbus->self->dev.archdata.pci_data;
> + }
> +
> return pdn;
> }
>
> struct pci_dn *pci_get_pdn_by_devfn(struct pci_bus *bus,
> int devfn)
> {
> - struct device_node *dn = NULL;
> struct pci_dn *parent, *pdn;
> struct pci_dev *pdev = NULL;
>
> @@ -80,17 +89,10 @@ struct pci_dn *pci_get_pdn_by_devfn(struct pci_bus *bus,
> if (pdev->devfn == devfn) {
> if (pdev->dev.archdata.pci_data)
> return pdev->dev.archdata.pci_data;
> -
> - dn = pci_device_to_OF_node(pdev);
> break;
> }
> }
>
> - /* Fast path: fetch from device node */
> - pdn = dn ? PCI_DN(dn) : NULL;
> - if (pdn)
> - return pdn;
> -
> /* Slow path: fetch from firmware data hierarchy */
> parent = pci_bus_to_pdn(bus);
> if (!parent)
> @@ -128,16 +130,9 @@ struct pci_dn *pci_get_pdn(struct pci_dev *pdev)
> if (!parent)
> return NULL;
>
> - list_for_each_entry(pdn, &parent->child_list, list) {
> - if (pdn->busno == pdev->bus->number &&
> - pdn->devfn == pdev->devfn)
> - return pdn;
> - }
> -
> - return NULL;
> + return create_pdn(pdev, parent);
> }
>
> -#ifdef CONFIG_PCI_IOV
> static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
> int vf_index,
> int busno, int devfn)
> @@ -164,7 +159,47 @@ static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
>
> return pdn;
> }
> -#endif
> +
> +static struct pci_dn* create_pdn(struct pci_dev *pdev, struct pci_dn *parent)
> +{
> + struct pci_dn *pdn = NULL;
> +
> + pdn = add_one_dev_pci_data(parent, 0, pdev->bus->number, pdev->devfn);
> + dev_info(&pdev->dev, "Create a new pdn for devfn %2x\n", pdev->devfn / 8);
> +
> + if (pdn)
> + {
> + u32 class_code;
> + u16 device_id;
> + u16 vendor_id;
> +
> + struct eeh_dev *edev = eeh_dev_init(pdn);
> + if (!edev) {
> + kfree(pdn);
> + dev_err(&pdev->dev, "%s:%d: Failed to allocate edev\n", __func__, __LINE__);
> + return NULL;
> + }
> +
> + pdn->busno = pdev->bus->busn_res.start;
> +
> + pci_bus_read_config_word(pdev->bus, pdev->devfn, PCI_VENDOR_ID, &vendor_id);
> + pdn->vendor_id = vendor_id;
> +
> + pci_bus_read_config_word(pdev->bus, pdev->devfn, PCI_DEVICE_ID, &device_id);
> + pdn->device_id = device_id;
> +
> + pci_bus_read_config_dword(pdev->bus, pdev->devfn, PCI_CLASS_REVISION, &class_code);
> + class_code >>= 8;
> + pdn->class_code = class_code;
> +
> + pdn->pci_ext_config_space = 0;
> + pdev->dev.archdata.pci_data = pdn;
> + } else {
> + dev_err(&pdev->dev, "%s:%d: Failed to allocate pdn\n", __func__, __LINE__);
> + }
> +
> + return pdn;
> +}
>
> struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
> {
^ permalink raw reply
* Re: [PATCH v3 0/6] powerpc/powernv/pci: Discover surprise-hotplugged PCIe devices during rescan
From: Oliver @ 2018-09-12 9:49 UTC (permalink / raw)
To: Sergey Miroshnichenko; +Cc: linuxppc-dev, linux
In-Reply-To: <20180911115620.10507-1-s.miroshnichenko@yadro.com>
On Tue, Sep 11, 2018 at 9:56 PM, Sergey Miroshnichenko
<s.miroshnichenko@yadro.com> wrote:
> This patchset allows hotplugged PCIe devices to be enumerated during a bus
> rescan being issued via sysfs on PowerNV platforms, when the "Presence
> Detect Changed" interrupt is not available.
Seems to be on par with the sysfs slot power hack that pnv_php uses.
> As a first part of our work on adding support for hotplugging PCIe bridges
> full of devices (without special requirement such as Hot-Plug Controller,
> reservation of bus numbers and memory regions by firmware, etc.), this
> serie is intended to solve the first two problems of the listed below:
>
> I PowerNV doesn't discover new hotplugged PCIe devices
> II EEH is falsely triggered when poking empty slots during the PCIe rescan
We avoid this problem in pnv_php by having OPAL to do the rescan and
Linux requests
a FDT fragment of everything under the slot. I'm don't think it's a
great system, but
it keeps firmware and the OS on the same page.
> III The PCI subsystem is not prepared to runtime changes of BAR addresses
> IV Device drivers don't track changes of their BAR addresses
> V BARs of working devices don't move to make space for new ones
I'm having a really hard to figuring out what would make this
necessary. Keep in mind
that each PHB has it's own set of bus numbers and it's own MMIO space,
so it's not
like you're short on either.
How are you planning on making this sort of live-device-migration work? And what
are you trying to do that makes the added complexity worth it?
> Tested on:
> - POWER8 PowerNV+OPAL ppc64le (our Vesnin server) w/ and w/o pci=realloc;
> - POWER8 IBM 8247-42L (pSeries);
> - POWER8 IBM 8247-42L (PowerNV+OPAL) w/ and w/o pci=realloc.
>
> Changes since v2:
> - Don't reassign bus numbers on PowerNV by default (to retain the default
> behavior), but only when pci=realloc is passed;
> - Less code affected;
> - pci_add_device_node_info is refactored with add_one_dev_pci_data;
> - Minor code cleanup.
>
> Changes since v1:
> - Fixed build for ppc64le and ppc64be when CONFIG_PCI_IOV is disabled;
> - Fixed build for ppc64e when CONFIG_EEH is disabled;
> - Fixed code style warnings.
>
> Sergey Miroshnichenko (6):
> powerpc/pci: Access PCI config space directly w/o pci_dn
> powerpc/pci: Create pci_dn on demand
> powerpc/pci: Use DT to create pci_dn for root bridges only
> powerpc/powernv/pci: Enable reassigning the bus numbers
> PCI/powerpc/eeh: Add pcibios hooks for preparing to rescan
> powerpc/pci: Reduce code duplication in pci_add_device_node_info
>
> arch/powerpc/include/asm/eeh.h | 2 +
> arch/powerpc/kernel/eeh.c | 12 ++
> arch/powerpc/kernel/pci_dn.c | 119 ++++++++++++++-----
> arch/powerpc/kernel/rtas_pci.c | 97 ++++++++++-----
> arch/powerpc/platforms/powernv/eeh-powernv.c | 22 ++++
> arch/powerpc/platforms/powernv/pci.c | 64 ++++++----
> drivers/pci/probe.c | 14 +++
> include/linux/pci.h | 2 +
> 8 files changed, 253 insertions(+), 79 deletions(-)
>
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH net] powerpc: use big endian to hash len and proto in csum_ipv6_magic
From: Xin Long @ 2018-09-12 6:27 UTC (permalink / raw)
To: davem
Cc: network dev, linuxppc-dev, Christophe Leroy, Michael Ellerman,
Roopa Prabhu
In-Reply-To: <20180911.230105.563027666901362955.davem@davemloft.net>
On Wed, Sep 12, 2018 at 2:01 PM David Miller <davem@davemloft.net> wrote:
>
> From: Xin Long <lucien.xin@gmail.com>
> Date: Sat, 8 Sep 2018 18:15:12 +0800
>
> > The function csum_ipv6_magic doesn't convert len and proto to big
> > endian before doing ipv6 csum hash, which is not consistent with
> > RFC and other arches.
> >
> > Jianlin found it when ICMPv6 packets from other hosts were dropped
> > in the powerpc64 system.
> >
> > This patch is to fix it by using instruction 'lwbrx' to do this
> > conversion in powerpc32/64 csum_ipv6_magic.
> >
> > Fixes: e9c4943a107b ("powerpc: Implement csum_ipv6_magic in assembly")
> > Reported-by: Jianlin Shi <jishi@redhat.com>
> > Signed-off-by: Xin Long <lucien.xin@gmail.com>
>
> Xin, please address the feedback you were given.
Christophe posted another one,
https://lore.kernel.org/patchwork/patch/983905/
Sorry, I didn't notice netdev wasn't in its CC-list.
^ permalink raw reply
* Re: [PATCH net] powerpc: use big endian to hash len and proto in csum_ipv6_magic
From: Christophe LEROY @ 2018-09-12 6:27 UTC (permalink / raw)
To: David Miller, lucien.xin; +Cc: netdev, linuxppc-dev, mpe, roopa
In-Reply-To: <20180911.230105.563027666901362955.davem@davemloft.net>
Le 12/09/2018 à 08:01, David Miller a écrit :
> From: Xin Long <lucien.xin@gmail.com>
> Date: Sat, 8 Sep 2018 18:15:12 +0800
>
>> The function csum_ipv6_magic doesn't convert len and proto to big
>> endian before doing ipv6 csum hash, which is not consistent with
>> RFC and other arches.
>>
>> Jianlin found it when ICMPv6 packets from other hosts were dropped
>> in the powerpc64 system.
>>
>> This patch is to fix it by using instruction 'lwbrx' to do this
>> conversion in powerpc32/64 csum_ipv6_magic.
>>
>> Fixes: e9c4943a107b ("powerpc: Implement csum_ipv6_magic in assembly")
>> Reported-by: Jianlin Shi <jishi@redhat.com>
>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>
> Xin, please address the feedback you were given.
I submitted an alternative fix, and Lucien Xin gave its Tested-by:
See https://patchwork.ozlabs.org/patch/967868/
Christophe
^ permalink raw reply
* Re: [PATCH v2] powerpc: Avoid code patching freed init sections
From: Christophe LEROY @ 2018-09-12 6:23 UTC (permalink / raw)
To: Michael Neuling, mpe
Cc: linuxppc-dev, Nicholas Piggin, paulus, Haren Myneni,
Michal Suchánek
In-Reply-To: <20180912052058.10062-1-mikey@neuling.org>
Le 12/09/2018 à 07:20, Michael Neuling a écrit :
> This stops us from doing code patching in init sections after they've
> been freed.
>
> In this chain:
> kvm_guest_init() ->
> kvm_use_magic_page() ->
> fault_in_pages_readable() ->
> __get_user() ->
> __get_user_nocheck() ->
> barrier_nospec();
>
> We have a code patching location at barrier_nospec() and
> kvm_guest_init() is an init function. This whole chain gets inlined,
> so when we free the init section (hence kvm_guest_init()), this code
> goes away and hence should no longer be patched.
>
> We seen this as userspace memory corruption when using a memory
> checker while doing partition migration testing on powervm (this
> starts the code patching post migration via
> /sys/kernel/mobility/migration). In theory, it could also happen when
> using /sys/kernel/debug/powerpc/barrier_nospec.
>
> With this patch there is a small change of a race if we code patch
> between the init section being freed and setting SYSTEM_RUNNING (in
> kernel_init()) but that seems like an impractical time and small
> window for any code patching to occur.
>
> cc: stable@vger.kernel.org # 4.13+
> Signed-off-by: Michael Neuling <mikey@neuling.org>
>
> ---
> For stable I've marked this as v4.13+ since that's when we refactored
> code-patching.c but it could go back even further than that. In
> reality though, I think we can only hit this since the first
> spectre/meltdown changes.
>
> v2:
> Print when we skip an address
> ---
> arch/powerpc/lib/code-patching.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
> index 850f3b8f4d..68254e7f17 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -23,11 +23,33 @@
> #include <asm/code-patching.h>
> #include <asm/setup.h>
>
> +
This blank line is not needed
> +static inline bool in_init_section(unsigned int *patch_addr)
> +{
> + if (patch_addr < (unsigned int *)__init_begin)
> + return false;
> + if (patch_addr >= (unsigned int *)__init_end)
> + return false;
> + return true;
> +}
Can we use the existing function init_section_contains() instead of this
new function ?
> +
> +static inline bool init_freed(void)
> +{
> + return (system_state >= SYSTEM_RUNNING);
> +}
> +
I would call this function differently, for instance init_is_finished(),
because as you mentionned it doesn't exactly mean that init memory is freed.
> static int __patch_instruction(unsigned int *exec_addr, unsigned int instr,
> unsigned int *patch_addr)
> {
> int err;
>
> + /* Make sure we aren't patching a freed init section */
> + if (in_init_section(patch_addr) && init_freed()) {
The test must be done on exec_addr, not on patch_addr, as patch_addr is
the address where the instruction as been remapped RW for allowing its
modification.
Also I think it should be tested the other way round, because the
init_freed() is a simpler test which will be false most of the time once
the system is running so it should be checked first.
> + printk(KERN_DEBUG "Skipping init section patching addr: 0x%lx\n",
Maybe use pr_debug() instead.
> + (unsigned long)patch_addr);
Please align second line as per Codying style.
> + return 0;
> + }
> +
> __put_user_size(instr, patch_addr, 4, err);
> if (err)
> return err;
>
I think it would be better to put this verification in
patch_instruction() instead, to avoid RW mapping/unmapping the
instruction to patch when we are not going to do the patching.
Christophe
^ permalink raw reply
* Re: [PATCH net] powerpc: use big endian to hash len and proto in csum_ipv6_magic
From: David Miller @ 2018-09-12 6:01 UTC (permalink / raw)
To: lucien.xin; +Cc: netdev, linuxppc-dev, christophe.leroy, mpe, roopa
In-Reply-To: <9183876a4a8ff0099686521d60f395a5230b67ed.1536401712.git.lucien.xin@gmail.com>
From: Xin Long <lucien.xin@gmail.com>
Date: Sat, 8 Sep 2018 18:15:12 +0800
> The function csum_ipv6_magic doesn't convert len and proto to big
> endian before doing ipv6 csum hash, which is not consistent with
> RFC and other arches.
>
> Jianlin found it when ICMPv6 packets from other hosts were dropped
> in the powerpc64 system.
>
> This patch is to fix it by using instruction 'lwbrx' to do this
> conversion in powerpc32/64 csum_ipv6_magic.
>
> Fixes: e9c4943a107b ("powerpc: Implement csum_ipv6_magic in assembly")
> Reported-by: Jianlin Shi <jishi@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Xin, please address the feedback you were given.
Thank you.
^ permalink raw reply
* [PATCH v2] powerpc: Avoid code patching freed init sections
From: Michael Neuling @ 2018-09-12 5:20 UTC (permalink / raw)
To: mpe
Cc: linuxppc-dev, Nicholas Piggin, paulus, Haren Myneni, mikey,
Michal Suchánek, Christophe LEROY
This stops us from doing code patching in init sections after they've
been freed.
In this chain:
kvm_guest_init() ->
kvm_use_magic_page() ->
fault_in_pages_readable() ->
__get_user() ->
__get_user_nocheck() ->
barrier_nospec();
We have a code patching location at barrier_nospec() and
kvm_guest_init() is an init function. This whole chain gets inlined,
so when we free the init section (hence kvm_guest_init()), this code
goes away and hence should no longer be patched.
We seen this as userspace memory corruption when using a memory
checker while doing partition migration testing on powervm (this
starts the code patching post migration via
/sys/kernel/mobility/migration). In theory, it could also happen when
using /sys/kernel/debug/powerpc/barrier_nospec.
With this patch there is a small change of a race if we code patch
between the init section being freed and setting SYSTEM_RUNNING (in
kernel_init()) but that seems like an impractical time and small
window for any code patching to occur.
cc: stable@vger.kernel.org # 4.13+
Signed-off-by: Michael Neuling <mikey@neuling.org>
---
For stable I've marked this as v4.13+ since that's when we refactored
code-patching.c but it could go back even further than that. In
reality though, I think we can only hit this since the first
spectre/meltdown changes.
v2:
Print when we skip an address
---
arch/powerpc/lib/code-patching.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 850f3b8f4d..68254e7f17 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -23,11 +23,33 @@
#include <asm/code-patching.h>
#include <asm/setup.h>
+
+static inline bool in_init_section(unsigned int *patch_addr)
+{
+ if (patch_addr < (unsigned int *)__init_begin)
+ return false;
+ if (patch_addr >= (unsigned int *)__init_end)
+ return false;
+ return true;
+}
+
+static inline bool init_freed(void)
+{
+ return (system_state >= SYSTEM_RUNNING);
+}
+
static int __patch_instruction(unsigned int *exec_addr, unsigned int instr,
unsigned int *patch_addr)
{
int err;
+ /* Make sure we aren't patching a freed init section */
+ if (in_init_section(patch_addr) && init_freed()) {
+ printk(KERN_DEBUG "Skipping init section patching addr: 0x%lx\n",
+ (unsigned long)patch_addr);
+ return 0;
+ }
+
__put_user_size(instr, patch_addr, 4, err);
if (err)
return err;
--
2.17.1
^ permalink raw reply related
* [PATCH 00/14] EEH refactoring 3
From: Sam Bobroff @ 2018-09-12 1:23 UTC (permalink / raw)
To: linuxppc-dev
Hello everyone,
Here is another set of minor fixes and cleanups for the EEH code. There should
be no significant changes in behaviour.
I'm not sure if all of these are worth doing, and I don't want to add
unnecessary churn so please comment if you have an opinion or better approach.
The set is based on powerpc next.
Cheers,
Sam
Sam Bobroff (14):
powerpc/eeh: Fix possible null deref in eeh_dump_dev_log()
powerpc/eeh: Fix null deref for devices removed during EEH
powerpc/eeh: Fix use of EEH_PE_KEEP on wrong field
powerpc/eeh: Cleanup EEH_POSTPONED_PROBE
powerpc/eeh: Cleanup unused field in eeh_dev
powerpc/eeh: Cleanup eeh_add_virt_device()
powerpc/eeh: Cleanup list_head field names
powerpc/eeh: Cleanup field names in eeh_rmv_data
powerpc/eeh: Cleanup logic in eeh_rmv_from_parent_pe()
powerpc/eeh: Cleanup eeh_enabled()
powerpc/eeh: Cleanup unnecessary eeh_pe_state_mark_with_cfg()
powerpc/eeh: Cleanup eeh_pe_state_mark()
powerpc/eeh: Cleanup eeh_ops.wait_state()
powerpc/eeh: Cleanup control flow in eeh_handle_normal_event()
arch/powerpc/include/asm/eeh.h | 24 +-
arch/powerpc/include/asm/ppc-pci.h | 1 +
arch/powerpc/kernel/eeh.c | 42 ++--
arch/powerpc/kernel/eeh_dev.c | 2 -
arch/powerpc/kernel/eeh_driver.c | 237 +++++++++----------
arch/powerpc/kernel/eeh_pe.c | 160 +++++++------
arch/powerpc/platforms/powernv/eeh-powernv.c | 62 +----
arch/powerpc/platforms/pseries/eeh_pseries.c | 66 +-----
arch/powerpc/platforms/pseries/msi.c | 3 +-
arch/powerpc/platforms/pseries/pci.c | 1 +
drivers/pci/hotplug/pnv_php.c | 2 +-
11 files changed, 250 insertions(+), 350 deletions(-)
--
2.19.0.2.gcad72f5712
^ permalink raw reply
* [PATCH 13/14] powerpc/eeh: Cleanup eeh_ops.wait_state()
From: Sam Bobroff @ 2018-09-12 1:23 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1536715396.git.sbobroff@linux.ibm.com>
The wait_state member of eeh_ops does not need to be platform
dependent; it's just logic around eeh_ops.get_state(). Therefore,
merge the two (slightly different!) platform versions into a new
function, eeh_wait_state() and remove the eeh_ops member.
While doing this, also correct:
* The wait logic, so that it never waits longer than max_wait.
* The wait logic, so that it never waits less than
EEH_STATE_MIN_WAIT_TIME.
* One call site where the result is treated like a bit field before
it's checked for negative error values.
* In pseries_eeh_get_state(), rename the "state" parameter to "delay"
because that's what it is.
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
arch/powerpc/include/asm/eeh.h | 4 +-
arch/powerpc/kernel/eeh.c | 9 ++-
arch/powerpc/kernel/eeh_driver.c | 2 +-
arch/powerpc/kernel/eeh_pe.c | 51 +++++++++++++++
arch/powerpc/platforms/powernv/eeh-powernv.c | 38 -----------
arch/powerpc/platforms/pseries/eeh_pseries.c | 66 ++------------------
6 files changed, 62 insertions(+), 108 deletions(-)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 247f09ce44de..8b596d096ebe 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -205,9 +205,8 @@ struct eeh_ops {
void* (*probe)(struct pci_dn *pdn, void *data);
int (*set_option)(struct eeh_pe *pe, int option);
int (*get_pe_addr)(struct eeh_pe *pe);
- int (*get_state)(struct eeh_pe *pe, int *state);
+ int (*get_state)(struct eeh_pe *pe, int *delay);
int (*reset)(struct eeh_pe *pe, int option);
- int (*wait_state)(struct eeh_pe *pe, int max_wait);
int (*get_log)(struct eeh_pe *pe, int severity, char *drv_log, unsigned long len);
int (*configure_bridge)(struct eeh_pe *pe);
int (*err_inject)(struct eeh_pe *pe, int type, int func,
@@ -264,6 +263,7 @@ typedef void *(*eeh_edev_traverse_func)(struct eeh_dev *edev, void *flag);
typedef void *(*eeh_pe_traverse_func)(struct eeh_pe *pe, void *flag);
void eeh_set_pe_aux_size(int size);
int eeh_phb_pe_create(struct pci_controller *phb);
+int eeh_wait_state(struct eeh_pe *pe, int max_wait);
struct eeh_pe *eeh_phb_pe_get(struct pci_controller *phb);
struct eeh_pe *eeh_pe_next(struct eeh_pe *pe, struct eeh_pe *root);
struct eeh_pe *eeh_pe_get(struct pci_controller *phb,
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 90e718f58676..c1c6ae18cfed 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -681,7 +681,7 @@ int eeh_pci_enable(struct eeh_pe *pe, int function)
/* Check if the request is finished successfully */
if (active_flag) {
- rc = eeh_ops->wait_state(pe, PCI_BUS_RESET_WAIT_MSEC);
+ rc = eeh_wait_state(pe, PCI_BUS_RESET_WAIT_MSEC);
if (rc < 0)
return rc;
@@ -920,16 +920,15 @@ int eeh_pe_reset_full(struct eeh_pe *pe)
break;
/* Wait until the PE is in a functioning state */
- state = eeh_ops->wait_state(pe, PCI_BUS_RESET_WAIT_MSEC);
- if (eeh_state_active(state))
- break;
-
+ state = eeh_wait_state(pe, PCI_BUS_RESET_WAIT_MSEC);
if (state < 0) {
pr_warn("%s: Unrecoverable slot failure on PHB#%x-PE#%x",
__func__, pe->phb->global_number, pe->addr);
ret = -ENOTRECOVERABLE;
break;
}
+ if (eeh_state_active(state))
+ break;
/* Set error in case this is our last attempt */
ret = -EIO;
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index c827617613c1..e7f757cd839b 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -836,7 +836,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
/* Get the current PCI slot state. This can take a long time,
* sometimes over 300 seconds for certain systems.
*/
- rc = eeh_ops->wait_state(pe, MAX_WAIT_FOR_RECOVERY*1000);
+ rc = eeh_wait_state(pe, MAX_WAIT_FOR_RECOVERY*1000);
if (rc < 0 || rc == EEH_STATE_NOT_SUPPORT) {
pr_warn("EEH: Permanent failure\n");
goto hard_fail;
diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index e43dcefbe73f..6fa2032e0594 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -108,6 +108,57 @@ int eeh_phb_pe_create(struct pci_controller *phb)
return 0;
}
+/**
+ * eeh_wait_state - Wait for PE state
+ * @pe: EEH PE
+ * @max_wait: maximal period in millisecond
+ *
+ * Wait for the state of associated PE. It might take some time
+ * to retrieve the PE's state.
+ */
+int eeh_wait_state(struct eeh_pe *pe, int max_wait)
+{
+ int ret;
+ int mwait;
+
+ /*
+ * According to PAPR, the state of PE might be temporarily
+ * unavailable. Under the circumstance, we have to wait
+ * for indicated time determined by firmware. The maximal
+ * wait time is 5 minutes, which is acquired from the original
+ * EEH implementation. Also, the original implementation
+ * also defined the minimal wait time as 1 second.
+ */
+#define EEH_STATE_MIN_WAIT_TIME (1000)
+#define EEH_STATE_MAX_WAIT_TIME (300 * 1000)
+
+ while (1) {
+ ret = eeh_ops->get_state(pe, &mwait);
+
+ if (ret != EEH_STATE_UNAVAILABLE)
+ return ret;
+
+ if (max_wait <= 0) {
+ pr_warn("%s: Timeout when getting PE's state (%d)\n",
+ __func__, max_wait);
+ return EEH_STATE_NOT_SUPPORT;
+ }
+
+ if (mwait < EEH_STATE_MIN_WAIT_TIME) {
+ pr_warn("%s: Firmware returned bad wait value %d\n",
+ __func__, mwait);
+ mwait = EEH_STATE_MIN_WAIT_TIME;
+ } else if (mwait > EEH_STATE_MAX_WAIT_TIME) {
+ pr_warn("%s: Firmware returned too long wait value %d\n",
+ __func__, mwait);
+ mwait = EEH_STATE_MAX_WAIT_TIME;
+ }
+
+ msleep(min(mwait, max_wait));
+ max_wait -= mwait;
+ }
+}
+
/**
* eeh_phb_pe_get - Retrieve PHB PE based on the given PHB
* @phb: PCI controller
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index fd1db9f286f1..abc0be7507c8 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -1133,43 +1133,6 @@ static int pnv_eeh_reset(struct eeh_pe *pe, int option)
return pnv_eeh_bridge_reset(bus->self, option);
}
-/**
- * pnv_eeh_wait_state - Wait for PE state
- * @pe: EEH PE
- * @max_wait: maximal period in millisecond
- *
- * Wait for the state of associated PE. It might take some time
- * to retrieve the PE's state.
- */
-static int pnv_eeh_wait_state(struct eeh_pe *pe, int max_wait)
-{
- int ret;
- int mwait;
-
- while (1) {
- ret = pnv_eeh_get_state(pe, &mwait);
-
- /*
- * If the PE's state is temporarily unavailable,
- * we have to wait for the specified time. Otherwise,
- * the PE's state will be returned immediately.
- */
- if (ret != EEH_STATE_UNAVAILABLE)
- return ret;
-
- if (max_wait <= 0) {
- pr_warn("%s: Timeout getting PE#%x's state (%d)\n",
- __func__, pe->addr, max_wait);
- return EEH_STATE_NOT_SUPPORT;
- }
-
- max_wait -= mwait;
- msleep(mwait);
- }
-
- return EEH_STATE_NOT_SUPPORT;
-}
-
/**
* pnv_eeh_get_log - Retrieve error log
* @pe: EEH PE
@@ -1688,7 +1651,6 @@ static struct eeh_ops pnv_eeh_ops = {
.get_pe_addr = pnv_eeh_get_pe_addr,
.get_state = pnv_eeh_get_state,
.reset = pnv_eeh_reset,
- .wait_state = pnv_eeh_wait_state,
.get_log = pnv_eeh_get_log,
.configure_bridge = pnv_eeh_configure_bridge,
.err_inject = pnv_eeh_err_inject,
diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
index 823cb27efa8b..c9e5ca4afb26 100644
--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
@@ -438,7 +438,7 @@ static int pseries_eeh_get_pe_addr(struct eeh_pe *pe)
/**
* pseries_eeh_get_state - Retrieve PE state
* @pe: EEH PE
- * @state: return value
+ * @delay: suggested time to wait if state is unavailable
*
* Retrieve the state of the specified PE. On RTAS compliant
* pseries platform, there already has one dedicated RTAS function
@@ -448,7 +448,7 @@ static int pseries_eeh_get_pe_addr(struct eeh_pe *pe)
* RTAS calls for the purpose, we need to try the new one and back
* to the old one if the new one couldn't work properly.
*/
-static int pseries_eeh_get_state(struct eeh_pe *pe, int *state)
+static int pseries_eeh_get_state(struct eeh_pe *pe, int *delay)
{
int config_addr;
int ret;
@@ -499,7 +499,8 @@ static int pseries_eeh_get_state(struct eeh_pe *pe, int *state)
break;
case 5:
if (rets[2]) {
- if (state) *state = rets[2];
+ if (delay)
+ *delay = rets[2];
result = EEH_STATE_UNAVAILABLE;
} else {
result = EEH_STATE_NOT_SUPPORT;
@@ -553,64 +554,6 @@ static int pseries_eeh_reset(struct eeh_pe *pe, int option)
return ret;
}
-/**
- * pseries_eeh_wait_state - Wait for PE state
- * @pe: EEH PE
- * @max_wait: maximal period in millisecond
- *
- * Wait for the state of associated PE. It might take some time
- * to retrieve the PE's state.
- */
-static int pseries_eeh_wait_state(struct eeh_pe *pe, int max_wait)
-{
- int ret;
- int mwait;
-
- /*
- * According to PAPR, the state of PE might be temporarily
- * unavailable. Under the circumstance, we have to wait
- * for indicated time determined by firmware. The maximal
- * wait time is 5 minutes, which is acquired from the original
- * EEH implementation. Also, the original implementation
- * also defined the minimal wait time as 1 second.
- */
-#define EEH_STATE_MIN_WAIT_TIME (1000)
-#define EEH_STATE_MAX_WAIT_TIME (300 * 1000)
-
- while (1) {
- ret = pseries_eeh_get_state(pe, &mwait);
-
- /*
- * If the PE's state is temporarily unavailable,
- * we have to wait for the specified time. Otherwise,
- * the PE's state will be returned immediately.
- */
- if (ret != EEH_STATE_UNAVAILABLE)
- return ret;
-
- if (max_wait <= 0) {
- pr_warn("%s: Timeout when getting PE's state (%d)\n",
- __func__, max_wait);
- return EEH_STATE_NOT_SUPPORT;
- }
-
- if (mwait <= 0) {
- pr_warn("%s: Firmware returned bad wait value %d\n",
- __func__, mwait);
- mwait = EEH_STATE_MIN_WAIT_TIME;
- } else if (mwait > EEH_STATE_MAX_WAIT_TIME) {
- pr_warn("%s: Firmware returned too long wait value %d\n",
- __func__, mwait);
- mwait = EEH_STATE_MAX_WAIT_TIME;
- }
-
- max_wait -= mwait;
- msleep(mwait);
- }
-
- return EEH_STATE_NOT_SUPPORT;
-}
-
/**
* pseries_eeh_get_log - Retrieve error log
* @pe: EEH PE
@@ -849,7 +792,6 @@ static struct eeh_ops pseries_eeh_ops = {
.get_pe_addr = pseries_eeh_get_pe_addr,
.get_state = pseries_eeh_get_state,
.reset = pseries_eeh_reset,
- .wait_state = pseries_eeh_wait_state,
.get_log = pseries_eeh_get_log,
.configure_bridge = pseries_eeh_configure_bridge,
.err_inject = NULL,
--
2.19.0.2.gcad72f5712
^ permalink raw reply related
* [PATCH 04/14] powerpc/eeh: Cleanup EEH_POSTPONED_PROBE
From: Sam Bobroff @ 2018-09-12 1:23 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1536715396.git.sbobroff@linux.ibm.com>
Currently a flag, EEH_POSTPONED_PROBE, is used to prevent an incorrect
message "EEH: No capable adapters found" from being displayed during
the boot of powernv systems.
It is necessary because, on powernv, the call to eeh_probe_devices()
made from eeh_init() is too early and EEH can't yet be enabled. A
second call is made later from eeh_pnv_post_init(), which succeeds.
(On pseries, the first call succeeds because PCI devices are set up
early enough and no second call is made.)
This can be simplified by moving the early call to eeh_probe_devices()
from eeh_init() (where it's seen by both platforms) to
pSeries_final_fixup(), so that each platform only calls
eeh_probe_devices() once, at a point where it can succeed.
This is slightly later in the boot sequence, but but still early
enough and it is now in the same place in the sequence for both
platforms (the pcibios_fixup hook).
The display of the message can be cleaned up as well, by moving it
into eeh_probe_devices().
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
arch/powerpc/include/asm/eeh.h | 1 -
arch/powerpc/kernel/eeh.c | 18 ++++++------------
arch/powerpc/platforms/powernv/eeh-powernv.c | 14 --------------
arch/powerpc/platforms/pseries/pci.c | 1 +
4 files changed, 7 insertions(+), 27 deletions(-)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 219637ea69a1..147f0117e56f 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -43,7 +43,6 @@ struct pci_dn;
#define EEH_VALID_PE_ZERO 0x10 /* PE#0 is valid */
#define EEH_ENABLE_IO_FOR_LOG 0x20 /* Enable IO for log */
#define EEH_EARLY_DUMP_LOG 0x40 /* Dump log immediately */
-#define EEH_POSTPONED_PROBE 0x80 /* Powernv may postpone device probe */
/*
* Delay for PE reset, all in ms
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 69754af506dd..25e7384fdaba 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -1041,6 +1041,11 @@ void eeh_probe_devices(void)
pdn = hose->pci_data;
traverse_pci_dn(pdn, eeh_ops->probe, NULL);
}
+ if (eeh_enabled())
+ pr_info("EEH: PCI Enhanced I/O Error Handling Enabled\n");
+ else
+ pr_info("EEH: No capable adapters found\n");
+
}
/**
@@ -1084,18 +1089,7 @@ static int eeh_init(void)
eeh_dev_phb_init_dynamic(hose);
/* Initialize EEH event */
- ret = eeh_event_init();
- if (ret)
- return ret;
-
- eeh_probe_devices();
-
- if (eeh_enabled())
- pr_info("EEH: PCI Enhanced I/O Error Handling Enabled\n");
- else if (!eeh_has_flag(EEH_POSTPONED_PROBE))
- pr_info("EEH: No capable adapters found\n");
-
- return ret;
+ return eeh_event_init();
}
core_initcall_sync(eeh_init);
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 3c1beae29f2d..d0764f2c0733 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -223,14 +223,6 @@ int pnv_eeh_post_init(void)
eeh_probe_devices();
eeh_addr_cache_build();
- if (eeh_has_flag(EEH_POSTPONED_PROBE)) {
- eeh_clear_flag(EEH_POSTPONED_PROBE);
- if (eeh_enabled())
- pr_info("EEH: PCI Enhanced I/O Error Handling Enabled\n");
- else
- pr_info("EEH: No capable adapters found\n");
- }
-
/* Register OPAL event notifier */
eeh_event_irq = opal_event_request(ilog2(OPAL_EVENT_PCI_ERROR));
if (eeh_event_irq < 0) {
@@ -391,12 +383,6 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
if ((pdn->class_code >> 8) == PCI_CLASS_BRIDGE_ISA)
return NULL;
- /* Skip if we haven't probed yet */
- if (phb->ioda.pe_rmap[config_addr] == IODA_INVALID_PE) {
- eeh_add_flag(EEH_POSTPONED_PROBE);
- return NULL;
- }
-
/* Initialize eeh device */
edev->class_code = pdn->class_code;
edev->mode &= 0xFFFFFF00;
diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c
index eab96637d6cf..41d8a4d1d02e 100644
--- a/arch/powerpc/platforms/pseries/pci.c
+++ b/arch/powerpc/platforms/pseries/pci.c
@@ -239,6 +239,7 @@ void __init pSeries_final_fixup(void)
{
pSeries_request_regions();
+ eeh_probe_devices();
eeh_addr_cache_build();
#ifdef CONFIG_PCI_IOV
--
2.19.0.2.gcad72f5712
^ permalink raw reply related
* [PATCH 14/14] powerpc/eeh: Cleanup control flow in eeh_handle_normal_event()
From: Sam Bobroff @ 2018-09-12 1:23 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1536715396.git.sbobroff@linux.ibm.com>
Rather than mixing "if (state)" blocks and gotos, convert entirely to
"if (state)" blocks to make the state machine behaviour clearer.
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
arch/powerpc/kernel/eeh_driver.c | 196 +++++++++++++++----------------
1 file changed, 94 insertions(+), 102 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index e7f757cd839b..9446248eb6b8 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -808,10 +808,8 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
pr_err("EEH: PHB#%x-PE#%x has failed %d times in the last hour and has been permanently disabled.\n",
pe->phb->global_number, pe->addr,
pe->freeze_count);
- goto hard_fail;
+ result = PCI_ERS_RESULT_DISCONNECT;
}
- pr_warn("EEH: This PCI device has failed %d times in the last hour and will be permanently disabled after %d failures.\n",
- pe->freeze_count, eeh_max_freezes);
/* Walk the various device drivers attached to this slot through
* a reset sequence, giving each an opportunity to do what it needs
@@ -823,31 +821,39 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
* the error. Override the result if necessary to have partially
* hotplug for this case.
*/
- pr_info("EEH: Notify device drivers to shutdown\n");
- eeh_set_channel_state(pe, pci_channel_io_frozen);
- eeh_set_irq_state(pe, false);
- eeh_pe_report("error_detected(IO frozen)", pe, eeh_report_error,
- &result);
- if ((pe->type & EEH_PE_PHB) &&
- result != PCI_ERS_RESULT_NONE &&
- result != PCI_ERS_RESULT_NEED_RESET)
- result = PCI_ERS_RESULT_NEED_RESET;
+ if (result != PCI_ERS_RESULT_DISCONNECT) {
+ pr_warn("EEH: This PCI device has failed %d times in the last hour and will be permanently disabled after %d failures.\n",
+ pe->freeze_count, eeh_max_freezes);
+ pr_info("EEH: Notify device drivers to shutdown\n");
+ eeh_set_channel_state(pe, pci_channel_io_frozen);
+ eeh_set_irq_state(pe, false);
+ eeh_pe_report("error_detected(IO frozen)", pe,
+ eeh_report_error, &result);
+ if ((pe->type & EEH_PE_PHB) &&
+ result != PCI_ERS_RESULT_NONE &&
+ result != PCI_ERS_RESULT_NEED_RESET)
+ result = PCI_ERS_RESULT_NEED_RESET;
+ }
/* Get the current PCI slot state. This can take a long time,
* sometimes over 300 seconds for certain systems.
*/
- rc = eeh_wait_state(pe, MAX_WAIT_FOR_RECOVERY*1000);
- if (rc < 0 || rc == EEH_STATE_NOT_SUPPORT) {
- pr_warn("EEH: Permanent failure\n");
- goto hard_fail;
+ if (result != PCI_ERS_RESULT_DISCONNECT) {
+ rc = eeh_wait_state(pe, MAX_WAIT_FOR_RECOVERY*1000);
+ if (rc < 0 || rc == EEH_STATE_NOT_SUPPORT) {
+ pr_warn("EEH: Permanent failure\n");
+ result = PCI_ERS_RESULT_DISCONNECT;
+ }
}
/* Since rtas may enable MMIO when posting the error log,
* don't post the error log until after all dev drivers
* have been informed.
*/
- pr_info("EEH: Collect temporary log\n");
- eeh_slot_error_detail(pe, EEH_LOG_TEMP);
+ if (result != PCI_ERS_RESULT_DISCONNECT) {
+ pr_info("EEH: Collect temporary log\n");
+ eeh_slot_error_detail(pe, EEH_LOG_TEMP);
+ }
/* If all device drivers were EEH-unaware, then shut
* down all of the device drivers, and hope they
@@ -859,7 +865,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
if (rc) {
pr_warn("%s: Unable to reset, err=%d\n",
__func__, rc);
- goto hard_fail;
+ result = PCI_ERS_RESULT_DISCONNECT;
}
}
@@ -868,9 +874,9 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
pr_info("EEH: Enable I/O for affected devices\n");
rc = eeh_pci_enable(pe, EEH_OPT_THAW_MMIO);
- if (rc < 0)
- goto hard_fail;
- if (rc) {
+ if (rc < 0) {
+ result = PCI_ERS_RESULT_DISCONNECT;
+ } else if (rc) {
result = PCI_ERS_RESULT_NEED_RESET;
} else {
pr_info("EEH: Notify device drivers to resume I/O\n");
@@ -884,9 +890,9 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
pr_info("EEH: Enabled DMA for affected devices\n");
rc = eeh_pci_enable(pe, EEH_OPT_THAW_DMA);
- if (rc < 0)
- goto hard_fail;
- if (rc) {
+ if (rc < 0) {
+ result = PCI_ERS_RESULT_DISCONNECT;
+ } else if (rc) {
result = PCI_ERS_RESULT_NEED_RESET;
} else {
/*
@@ -899,12 +905,6 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
}
}
- /* If any device has a hard failure, then shut off everything. */
- if (result == PCI_ERS_RESULT_DISCONNECT) {
- pr_warn("EEH: Device driver gave up\n");
- goto hard_fail;
- }
-
/* If any device called out for a reset, then reset the slot */
if (result == PCI_ERS_RESULT_NEED_RESET) {
pr_info("EEH: Reset without hotplug activity\n");
@@ -912,89 +912,81 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
if (rc) {
pr_warn("%s: Cannot reset, err=%d\n",
__func__, rc);
- goto hard_fail;
+ result = PCI_ERS_RESULT_DISCONNECT;
+ } else {
+ result = PCI_ERS_RESULT_NONE;
+ eeh_set_channel_state(pe, pci_channel_io_normal);
+ eeh_set_irq_state(pe, true);
+ eeh_pe_report("slot_reset", pe, eeh_report_reset,
+ &result);
}
-
- pr_info("EEH: Notify device drivers "
- "the completion of reset\n");
- result = PCI_ERS_RESULT_NONE;
- eeh_set_channel_state(pe, pci_channel_io_normal);
- eeh_set_irq_state(pe, true);
- eeh_pe_report("slot_reset", pe, eeh_report_reset, &result);
}
- /* All devices should claim they have recovered by now. */
- if ((result != PCI_ERS_RESULT_RECOVERED) &&
- (result != PCI_ERS_RESULT_NONE)) {
- pr_warn("EEH: Not recovered\n");
- goto hard_fail;
- }
-
- /*
- * For those hot removed VFs, we should add back them after PF get
- * recovered properly.
- */
- list_for_each_entry_safe(edev, tmp, &rmv_data.removed_vf_list,
- rmv_entry) {
- eeh_add_virt_device(edev);
- list_del(&edev->rmv_entry);
- }
-
- /* Tell all device drivers that they can resume operations */
- pr_info("EEH: Notify device driver to resume\n");
- eeh_set_channel_state(pe, pci_channel_io_normal);
- eeh_set_irq_state(pe, true);
- eeh_pe_report("resume", pe, eeh_report_resume, NULL);
- eeh_for_each_pe(pe, tmp_pe) {
- eeh_pe_for_each_dev(tmp_pe, edev, tmp) {
- edev->mode &= ~EEH_DEV_NO_HANDLER;
- edev->in_error = false;
+ if ((result == PCI_ERS_RESULT_RECOVERED) ||
+ (result == PCI_ERS_RESULT_NONE)) {
+ /*
+ * For those hot removed VFs, we should add back them after PF
+ * get recovered properly.
+ */
+ list_for_each_entry_safe(edev, tmp, &rmv_data.removed_vf_list,
+ rmv_entry) {
+ eeh_add_virt_device(edev);
+ list_del(&edev->rmv_entry);
}
- }
- pr_info("EEH: Recovery successful.\n");
- goto final;
+ /* Tell all device drivers that they can resume operations */
+ pr_info("EEH: Notify device driver to resume\n");
+ eeh_set_channel_state(pe, pci_channel_io_normal);
+ eeh_set_irq_state(pe, true);
+ eeh_pe_report("resume", pe, eeh_report_resume, NULL);
+ eeh_for_each_pe(pe, tmp_pe) {
+ eeh_pe_for_each_dev(tmp_pe, edev, tmp) {
+ edev->mode &= ~EEH_DEV_NO_HANDLER;
+ edev->in_error = false;
+ }
+ }
-hard_fail:
- /*
- * About 90% of all real-life EEH failures in the field
- * are due to poorly seated PCI cards. Only 10% or so are
- * due to actual, failed cards.
- */
- pr_err("EEH: Unable to recover from failure from PHB#%x-PE#%x.\n"
- "Please try reseating or replacing it\n",
- pe->phb->global_number, pe->addr);
+ pr_info("EEH: Recovery successful.\n");
+ } else {
+ /*
+ * About 90% of all real-life EEH failures in the field
+ * are due to poorly seated PCI cards. Only 10% or so are
+ * due to actual, failed cards.
+ */
+ pr_err("EEH: Unable to recover from failure from PHB#%x-PE#%x.\n"
+ "Please try reseating or replacing it\n",
+ pe->phb->global_number, pe->addr);
- eeh_slot_error_detail(pe, EEH_LOG_PERM);
+ eeh_slot_error_detail(pe, EEH_LOG_PERM);
- /* Notify all devices that they're about to go down. */
- eeh_set_channel_state(pe, pci_channel_io_perm_failure);
- eeh_set_irq_state(pe, false);
- eeh_pe_report("error_detected(permanent failure)", pe,
- eeh_report_failure, NULL);
+ /* Notify all devices that they're about to go down. */
+ eeh_set_channel_state(pe, pci_channel_io_perm_failure);
+ eeh_set_irq_state(pe, false);
+ eeh_pe_report("error_detected(permanent failure)", pe,
+ eeh_report_failure, NULL);
- /* Mark the PE to be removed permanently */
- eeh_pe_state_mark(pe, EEH_PE_REMOVED);
+ /* Mark the PE to be removed permanently */
+ eeh_pe_state_mark(pe, EEH_PE_REMOVED);
- /*
- * Shut down the device drivers for good. We mark
- * all removed devices correctly to avoid access
- * the their PCI config any more.
- */
- if (pe->type & EEH_PE_VF) {
- eeh_pe_dev_traverse(pe, eeh_rmv_device, NULL);
- eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
- } else {
- eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
- eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
+ /*
+ * Shut down the device drivers for good. We mark
+ * all removed devices correctly to avoid access
+ * the their PCI config any more.
+ */
+ if (pe->type & EEH_PE_VF) {
+ eeh_pe_dev_traverse(pe, eeh_rmv_device, NULL);
+ eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
+ } else {
+ eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
+ eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
- pci_lock_rescan_remove();
- pci_hp_remove_devices(bus);
- pci_unlock_rescan_remove();
- /* The passed PE should no longer be used */
- return;
+ pci_lock_rescan_remove();
+ pci_hp_remove_devices(bus);
+ pci_unlock_rescan_remove();
+ /* The passed PE should no longer be used */
+ return;
+ }
}
-final:
eeh_pe_state_clear(pe, EEH_PE_RECOVERING);
}
--
2.19.0.2.gcad72f5712
^ permalink raw reply related
* [PATCH 08/14] powerpc/eeh: Cleanup field names in eeh_rmv_data
From: Sam Bobroff @ 2018-09-12 1:23 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1536715396.git.sbobroff@linux.ibm.com>
Change the name of the fields in eeh_rmv_data to clarify their usage.
Change "edev_list" to "removed_vf_list" because it does not contain
generic edevs, but rather only edevs that contain virtual functions
(which need to be removed during recovery).
Similarly, change "removed" to "removed_dev_count" because it is a
count of any removed devices, not just those in the above list.
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
arch/powerpc/kernel/eeh_driver.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 7859af897058..ffe8293d1f06 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -35,8 +35,8 @@
#include <asm/rtas.h>
struct eeh_rmv_data {
- struct list_head edev_list;
- int removed;
+ struct list_head removed_vf_list;
+ int removed_dev_count;
};
static int eeh_result_priority(enum pci_ers_result result)
@@ -502,7 +502,6 @@ static void *eeh_rmv_device(struct eeh_dev *edev, void *userdata)
struct pci_driver *driver;
struct pci_dev *dev = eeh_dev_to_pci_dev(edev);
struct eeh_rmv_data *rmv_data = (struct eeh_rmv_data *)userdata;
- int *removed = rmv_data ? &rmv_data->removed : NULL;
/*
* Actually, we should remove the PCI bridges as well.
@@ -524,7 +523,7 @@ static void *eeh_rmv_device(struct eeh_dev *edev, void *userdata)
if (eeh_dev_removed(edev))
return NULL;
- if (removed) {
+ if (rmv_data) {
if (eeh_pe_passed(edev->pe))
return NULL;
driver = eeh_pcid_get(dev);
@@ -543,8 +542,8 @@ static void *eeh_rmv_device(struct eeh_dev *edev, void *userdata)
pr_debug("EEH: Removing %s without EEH sensitive driver\n",
pci_name(dev));
edev->mode |= EEH_DEV_DISCONNECTED;
- if (removed)
- (*removed)++;
+ if (rmv_data)
+ rmv_data->removed_dev_count++;
if (edev->physfn) {
#ifdef CONFIG_PCI_IOV
@@ -560,7 +559,7 @@ static void *eeh_rmv_device(struct eeh_dev *edev, void *userdata)
pdn->pe_number = IODA_INVALID_PE;
#endif
if (rmv_data)
- list_add(&edev->rmv_entry, &rmv_data->edev_list);
+ list_add(&edev->rmv_entry, &rmv_data->removed_vf_list);
} else {
pci_lock_rescan_remove();
pci_stop_and_remove_bus_device(dev);
@@ -729,7 +728,7 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
* the device up before the scripts have taken it down,
* potentially weird things happen.
*/
- if (!driver_eeh_aware || rmv_data->removed) {
+ if (!driver_eeh_aware || rmv_data->removed_dev_count) {
pr_info("EEH: Sleep 5s ahead of %s hotplug\n",
(driver_eeh_aware ? "partial" : "complete"));
ssleep(5);
@@ -791,7 +790,8 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
struct eeh_pe *tmp_pe;
int rc = 0;
enum pci_ers_result result = PCI_ERS_RESULT_NONE;
- struct eeh_rmv_data rmv_data = {LIST_HEAD_INIT(rmv_data.edev_list), 0};
+ struct eeh_rmv_data rmv_data =
+ {LIST_HEAD_INIT(rmv_data.removed_vf_list), 0};
bus = eeh_pe_bus_get(pe);
if (!bus) {
@@ -934,7 +934,8 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
* For those hot removed VFs, we should add back them after PF get
* recovered properly.
*/
- list_for_each_entry_safe(edev, tmp, &rmv_data.edev_list, rmv_entry) {
+ list_for_each_entry_safe(edev, tmp, &rmv_data.removed_vf_list,
+ rmv_entry) {
eeh_add_virt_device(edev);
list_del(&edev->rmv_entry);
}
--
2.19.0.2.gcad72f5712
^ permalink raw reply related
* [PATCH 06/14] powerpc/eeh: Cleanup eeh_add_virt_device()
From: Sam Bobroff @ 2018-09-12 1:23 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1536715396.git.sbobroff@linux.ibm.com>
Remove the unnecessary cast through void * on the first parameter and
remove the unused second parameter (always NULL).
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
arch/powerpc/kernel/eeh_driver.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 7766766bab57..cc300eb9585c 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -469,10 +469,9 @@ static enum pci_ers_result eeh_report_failure(struct eeh_dev *edev,
return rc;
}
-static void *eeh_add_virt_device(void *data, void *userdata)
+static void *eeh_add_virt_device(struct eeh_dev *edev)
{
struct pci_driver *driver;
- struct eeh_dev *edev = (struct eeh_dev *)data;
struct pci_dev *dev = eeh_dev_to_pci_dev(edev);
struct pci_dn *pdn = eeh_dev_to_pdn(edev);
@@ -743,7 +742,7 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
edev = list_first_entry(&pe->edevs, struct eeh_dev, list);
eeh_pe_traverse(pe, eeh_pe_detach_dev, NULL);
if (pe->type & EEH_PE_VF) {
- eeh_add_virt_device(edev, NULL);
+ eeh_add_virt_device(edev);
} else {
if (!driver_eeh_aware)
eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
@@ -936,7 +935,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
* recovered properly.
*/
list_for_each_entry_safe(edev, tmp, &rmv_data.edev_list, rmv_list) {
- eeh_add_virt_device(edev, NULL);
+ eeh_add_virt_device(edev);
list_del(&edev->rmv_list);
}
--
2.19.0.2.gcad72f5712
^ permalink raw reply related
* [PATCH 03/14] powerpc/eeh: Fix use of EEH_PE_KEEP on wrong field
From: Sam Bobroff @ 2018-09-12 1:23 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1536715396.git.sbobroff@linux.ibm.com>
eeh_add_to_parent_pe() sometimes removes the EEH_PE_KEEP flag, but it
incorrectly removes it from pe->type, instead of pe->state.
However, rather than clearing it from the correct field, remove it.
Inspection of the code shows that it can't ever have had any effect
(even if it had been cleared from the correct field), because the
field is never tested after it is cleared by the statement in
question.
The clear statement was added by commit 807a827d4e74 ("powerpc/eeh:
Keep PE during hotplug"), but it didn't explain why it was necessary.
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
arch/powerpc/kernel/eeh_pe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index 1b238ecc553e..210d239a9395 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -379,7 +379,7 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
while (parent) {
if (!(parent->type & EEH_PE_INVALID))
break;
- parent->type &= ~(EEH_PE_INVALID | EEH_PE_KEEP);
+ parent->type &= ~EEH_PE_INVALID;
parent = parent->parent;
}
--
2.19.0.2.gcad72f5712
^ permalink raw reply related
* [PATCH 11/14] powerpc/eeh: Cleanup unnecessary eeh_pe_state_mark_with_cfg()
From: Sam Bobroff @ 2018-09-12 1:23 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1536715396.git.sbobroff@linux.ibm.com>
The function eeh_pe_state_mark_with_cfg() just performs the work of
eeh_pe_state_mark() and then, conditionally, the work of
eeh_pe_state_clear(). However it is only ever called with a constant
state such that the condition is always true, so replace it by direct
calls.
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
arch/powerpc/kernel/eeh.c | 6 ++++--
arch/powerpc/kernel/eeh_pe.c | 22 ----------------------
2 files changed, 4 insertions(+), 24 deletions(-)
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 25e7384fdaba..dd5ac2ad141e 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -830,7 +830,8 @@ int pcibios_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state stat
eeh_pe_state_clear(pe, EEH_PE_ISOLATED);
break;
case pcie_hot_reset:
- eeh_pe_state_mark_with_cfg(pe, EEH_PE_ISOLATED);
+ eeh_pe_state_mark(pe, EEH_PE_ISOLATED);
+ eeh_pe_state_clear(pe, EEH_PE_CFG_BLOCKED);
eeh_ops->set_option(pe, EEH_OPT_FREEZE_PE);
eeh_pe_dev_traverse(pe, eeh_disable_and_save_dev_state, dev);
if (!(pe->type & EEH_PE_VF))
@@ -838,7 +839,8 @@ int pcibios_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state stat
eeh_ops->reset(pe, EEH_RESET_HOT);
break;
case pcie_warm_reset:
- eeh_pe_state_mark_with_cfg(pe, EEH_PE_ISOLATED);
+ eeh_pe_state_mark(pe, EEH_PE_ISOLATED);
+ eeh_pe_state_clear(pe, EEH_PE_CFG_BLOCKED);
eeh_ops->set_option(pe, EEH_OPT_FREEZE_PE);
eeh_pe_dev_traverse(pe, eeh_disable_and_save_dev_state, dev);
if (!(pe->type & EEH_PE_VF))
diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index 78f125d24bd0..2b376718237f 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -670,28 +670,6 @@ void eeh_pe_state_clear(struct eeh_pe *pe, int state)
eeh_pe_traverse(pe, __eeh_pe_state_clear, &state);
}
-/**
- * eeh_pe_state_mark_with_cfg - Mark PE state with unblocked config space
- * @pe: PE
- * @state: PE state to be set
- *
- * Set specified flag to PE and its child PEs. The PCI config space
- * of some PEs is blocked automatically when EEH_PE_ISOLATED is set,
- * which isn't needed in some situations. The function allows to set
- * the specified flag to indicated PEs without blocking their PCI
- * config space.
- */
-void eeh_pe_state_mark_with_cfg(struct eeh_pe *pe, int state)
-{
- eeh_pe_traverse(pe, __eeh_pe_state_mark, &state);
- if (!(state & EEH_PE_ISOLATED))
- return;
-
- /* Clear EEH_PE_CFG_BLOCKED, which might be set just now */
- state = EEH_PE_CFG_BLOCKED;
- eeh_pe_traverse(pe, __eeh_pe_state_clear, &state);
-}
-
/*
* Some PCI bridges (e.g. PLX bridges) have primary/secondary
* buses assigned explicitly by firmware, and we probably have
--
2.19.0.2.gcad72f5712
^ permalink raw reply related
* [PATCH 10/14] powerpc/eeh: Cleanup eeh_enabled()
From: Sam Bobroff @ 2018-09-12 1:23 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1536715396.git.sbobroff@linux.ibm.com>
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
arch/powerpc/include/asm/eeh.h | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index b48b08ed9be3..247f09ce44de 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -241,11 +241,7 @@ static inline bool eeh_has_flag(int flag)
static inline bool eeh_enabled(void)
{
- if (eeh_has_flag(EEH_FORCE_DISABLED) ||
- !eeh_has_flag(EEH_ENABLED))
- return false;
-
- return true;
+ return eeh_has_flag(EEH_ENABLED) && !eeh_has_flag(EEH_FORCE_DISABLED);
}
static inline void eeh_serialize_lock(unsigned long *flags)
--
2.19.0.2.gcad72f5712
^ permalink raw reply related
* [PATCH 12/14] powerpc/eeh: Cleanup eeh_pe_state_mark()
From: Sam Bobroff @ 2018-09-12 1:23 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1536715396.git.sbobroff@linux.ibm.com>
Currently, eeh_pe_state_mark() marks a PE (and it's children) with a
state and then performs additional processing if that state included
EEH_PE_ISOLATED.
The state parameter is always a constant at the call site, so
rearrange eeh_pe_state_mark() into two functions and just call the
appropriate one at each site.
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
arch/powerpc/include/asm/ppc-pci.h | 1 +
arch/powerpc/kernel/eeh.c | 8 +--
arch/powerpc/kernel/eeh_driver.c | 10 ++-
arch/powerpc/kernel/eeh_pe.c | 70 +++++++++-----------
arch/powerpc/platforms/powernv/eeh-powernv.c | 8 +--
drivers/pci/hotplug/pnv_php.c | 2 +-
6 files changed, 46 insertions(+), 53 deletions(-)
diff --git a/arch/powerpc/include/asm/ppc-pci.h b/arch/powerpc/include/asm/ppc-pci.h
index 726288048652..f67da277d652 100644
--- a/arch/powerpc/include/asm/ppc-pci.h
+++ b/arch/powerpc/include/asm/ppc-pci.h
@@ -58,6 +58,7 @@ void eeh_save_bars(struct eeh_dev *edev);
int rtas_write_config(struct pci_dn *, int where, int size, u32 val);
int rtas_read_config(struct pci_dn *, int where, int size, u32 *val);
void eeh_pe_state_mark(struct eeh_pe *pe, int state);
+void eeh_pe_mark_isolated(struct eeh_pe *pe);
void eeh_pe_state_clear(struct eeh_pe *pe, int state);
void eeh_pe_state_mark_with_cfg(struct eeh_pe *pe, int state);
void eeh_pe_dev_mode_mark(struct eeh_pe *pe, int mode);
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index dd5ac2ad141e..90e718f58676 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -404,7 +404,7 @@ static int eeh_phb_check_failure(struct eeh_pe *pe)
}
/* Isolate the PHB and send event */
- eeh_pe_state_mark(phb_pe, EEH_PE_ISOLATED);
+ eeh_pe_mark_isolated(phb_pe);
eeh_serialize_unlock(flags);
pr_err("EEH: PHB#%x failure detected, location: %s\n",
@@ -563,7 +563,7 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
* with other functions on this device, and functions under
* bridges.
*/
- eeh_pe_state_mark(pe, EEH_PE_ISOLATED);
+ eeh_pe_mark_isolated(pe);
eeh_serialize_unlock(flags);
/* Most EEH events are due to device driver bugs. Having
@@ -830,7 +830,7 @@ int pcibios_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state stat
eeh_pe_state_clear(pe, EEH_PE_ISOLATED);
break;
case pcie_hot_reset:
- eeh_pe_state_mark(pe, EEH_PE_ISOLATED);
+ eeh_pe_mark_isolated(pe);
eeh_pe_state_clear(pe, EEH_PE_CFG_BLOCKED);
eeh_ops->set_option(pe, EEH_OPT_FREEZE_PE);
eeh_pe_dev_traverse(pe, eeh_disable_and_save_dev_state, dev);
@@ -839,7 +839,7 @@ int pcibios_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state stat
eeh_ops->reset(pe, EEH_RESET_HOT);
break;
case pcie_warm_reset:
- eeh_pe_state_mark(pe, EEH_PE_ISOLATED);
+ eeh_pe_mark_isolated(pe);
eeh_pe_state_clear(pe, EEH_PE_CFG_BLOCKED);
eeh_ops->set_option(pe, EEH_OPT_FREEZE_PE);
eeh_pe_dev_traverse(pe, eeh_disable_and_save_dev_state, dev);
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index ffe8293d1f06..c827617613c1 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -1029,7 +1029,7 @@ void eeh_handle_special_event(void)
phb_pe = eeh_phb_pe_get(hose);
if (!phb_pe) continue;
- eeh_pe_state_mark(phb_pe, EEH_PE_ISOLATED);
+ eeh_pe_mark_isolated(phb_pe);
}
eeh_serialize_unlock(flags);
@@ -1044,11 +1044,9 @@ void eeh_handle_special_event(void)
/* Purge all events of the PHB */
eeh_remove_event(pe, true);
- if (rc == EEH_NEXT_ERR_DEAD_PHB)
- eeh_pe_state_mark(pe, EEH_PE_ISOLATED);
- else
- eeh_pe_state_mark(pe,
- EEH_PE_ISOLATED | EEH_PE_RECOVERING);
+ if (rc != EEH_NEXT_ERR_DEAD_PHB)
+ eeh_pe_state_mark(pe, EEH_PE_RECOVERING);
+ eeh_pe_mark_isolated(pe);
eeh_serialize_unlock(flags);
diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index 2b376718237f..e43dcefbe73f 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -540,56 +540,50 @@ void eeh_pe_update_time_stamp(struct eeh_pe *pe)
}
/**
- * __eeh_pe_state_mark - Mark the state for the PE
- * @data: EEH PE
- * @flag: state
+ * eeh_pe_state_mark - Mark specified state for PE and its associated device
+ * @pe: EEH PE
*
- * The function is used to mark the indicated state for the given
- * PE. Also, the associated PCI devices will be put into IO frozen
- * state as well.
+ * EEH error affects the current PE and its child PEs. The function
+ * is used to mark appropriate state for the affected PEs and the
+ * associated devices.
*/
-static void *__eeh_pe_state_mark(struct eeh_pe *pe, void *flag)
+void eeh_pe_state_mark(struct eeh_pe *root, int state)
{
- int state = *((int *)flag);
- struct eeh_dev *edev, *tmp;
- struct pci_dev *pdev;
-
- /* Keep the state of permanently removed PE intact */
- if (pe->state & EEH_PE_REMOVED)
- return NULL;
-
- pe->state |= state;
-
- /* Offline PCI devices if applicable */
- if (!(state & EEH_PE_ISOLATED))
- return NULL;
-
- eeh_pe_for_each_dev(pe, edev, tmp) {
- pdev = eeh_dev_to_pci_dev(edev);
- if (pdev)
- pdev->error_state = pci_channel_io_frozen;
- }
-
- /* Block PCI config access if required */
- if (pe->state & EEH_PE_CFG_RESTRICTED)
- pe->state |= EEH_PE_CFG_BLOCKED;
+ struct eeh_pe *pe;
- return NULL;
+ eeh_for_each_pe(root, pe)
+ if (!(pe->state & EEH_PE_REMOVED))
+ pe->state |= state;
}
+EXPORT_SYMBOL_GPL(eeh_pe_state_mark);
/**
- * eeh_pe_state_mark - Mark specified state for PE and its associated device
+ * eeh_pe_mark_isolated
* @pe: EEH PE
*
- * EEH error affects the current PE and its child PEs. The function
- * is used to mark appropriate state for the affected PEs and the
- * associated devices.
+ * Record that a PE has been isolated by marking the PE and it's children as
+ * EEH_PE_ISOLATED (and EEH_PE_CFG_BLOCKED, if required) and their PCI devices
+ * as pci_channel_io_frozen.
*/
-void eeh_pe_state_mark(struct eeh_pe *pe, int state)
+void eeh_pe_mark_isolated(struct eeh_pe *root)
{
- eeh_pe_traverse(pe, __eeh_pe_state_mark, &state);
+ struct eeh_pe *pe;
+ struct eeh_dev *edev;
+ struct pci_dev *pdev;
+
+ eeh_pe_state_mark(root, EEH_PE_ISOLATED);
+ eeh_for_each_pe(root, pe) {
+ list_for_each_entry(edev, &pe->edevs, entry) {
+ pdev = eeh_dev_to_pci_dev(edev);
+ if (pdev)
+ pdev->error_state = pci_channel_io_frozen;
+ }
+ /* Block PCI config access if required */
+ if (pe->state & EEH_PE_CFG_RESTRICTED)
+ pe->state |= EEH_PE_CFG_BLOCKED;
+ }
}
-EXPORT_SYMBOL_GPL(eeh_pe_state_mark);
+EXPORT_SYMBOL_GPL(eeh_pe_mark_isolated);
static void *__eeh_pe_dev_mode_mark(struct eeh_dev *edev, void *flag)
{
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index a7e59dbf2696..fd1db9f286f1 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -590,7 +590,7 @@ static int pnv_eeh_get_phb_state(struct eeh_pe *pe)
EEH_STATE_MMIO_ENABLED |
EEH_STATE_DMA_ENABLED);
} else if (!(pe->state & EEH_PE_ISOLATED)) {
- eeh_pe_state_mark(pe, EEH_PE_ISOLATED);
+ eeh_pe_mark_isolated(pe);
pnv_eeh_get_phb_diag(pe);
if (eeh_has_flag(EEH_EARLY_DUMP_LOG))
@@ -692,7 +692,7 @@ static int pnv_eeh_get_pe_state(struct eeh_pe *pe)
if (phb->freeze_pe)
phb->freeze_pe(phb, pe->addr);
- eeh_pe_state_mark(pe, EEH_PE_ISOLATED);
+ eeh_pe_mark_isolated(pe);
pnv_eeh_get_phb_diag(pe);
if (eeh_has_flag(EEH_EARLY_DUMP_LOG))
@@ -1597,7 +1597,7 @@ static int pnv_eeh_next_error(struct eeh_pe **pe)
if ((ret == EEH_NEXT_ERR_FROZEN_PE ||
ret == EEH_NEXT_ERR_FENCED_PHB) &&
!((*pe)->state & EEH_PE_ISOLATED)) {
- eeh_pe_state_mark(*pe, EEH_PE_ISOLATED);
+ eeh_pe_mark_isolated(*pe);
pnv_eeh_get_phb_diag(*pe);
if (eeh_has_flag(EEH_EARLY_DUMP_LOG))
@@ -1626,7 +1626,7 @@ static int pnv_eeh_next_error(struct eeh_pe **pe)
}
/* We possibly migrate to another PE */
- eeh_pe_state_mark(*pe, EEH_PE_ISOLATED);
+ eeh_pe_mark_isolated(*pe);
}
/*
diff --git a/drivers/pci/hotplug/pnv_php.c b/drivers/pci/hotplug/pnv_php.c
index 6c2e8d7307c6..9555338f39c5 100644
--- a/drivers/pci/hotplug/pnv_php.c
+++ b/drivers/pci/hotplug/pnv_php.c
@@ -738,7 +738,7 @@ static irqreturn_t pnv_php_interrupt(int irq, void *data)
pe = edev ? edev->pe : NULL;
if (pe) {
eeh_serialize_lock(&flags);
- eeh_pe_state_mark(pe, EEH_PE_ISOLATED);
+ eeh_pe_mark_isolated(pe);
eeh_serialize_unlock(flags);
eeh_pe_set_option(pe, EEH_OPT_FREEZE_PE);
}
--
2.19.0.2.gcad72f5712
^ permalink raw reply related
* [PATCH 05/14] powerpc/eeh: Cleanup unused field in eeh_dev
From: Sam Bobroff @ 2018-09-12 1:23 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1536715396.git.sbobroff@linux.ibm.com>
The 'bus' member of struct eeh_dev is assigned to once but never used,
so remove it.
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
arch/powerpc/include/asm/eeh.h | 1 -
arch/powerpc/kernel/eeh_driver.c | 1 -
2 files changed, 2 deletions(-)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 147f0117e56f..703d1f96ee8b 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -147,7 +147,6 @@ struct eeh_dev {
struct pci_dev *pdev; /* Associated PCI device */
bool in_error; /* Error flag for edev */
struct pci_dev *physfn; /* Associated SRIOV PF */
- struct pci_bus *bus; /* PCI bus for partial hotplug */
};
static inline struct pci_dn *eeh_dev_to_pdn(struct eeh_dev *edev)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 4115d353c349..7766766bab57 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -543,7 +543,6 @@ static void *eeh_rmv_device(struct eeh_dev *edev, void *userdata)
/* Remove it from PCI subsystem */
pr_debug("EEH: Removing %s without EEH sensitive driver\n",
pci_name(dev));
- edev->bus = dev->bus;
edev->mode |= EEH_DEV_DISCONNECTED;
if (removed)
(*removed)++;
--
2.19.0.2.gcad72f5712
^ permalink raw reply related
* [PATCH 07/14] powerpc/eeh: Cleanup list_head field names
From: Sam Bobroff @ 2018-09-12 1:23 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1536715396.git.sbobroff@linux.ibm.com>
Instances of struct eeh_pe are placed in a tree structure using the
fields "child_list" and "child", so place these next to each other
in the definition.
The field "child" is a list entry, so remove the unnecessary and
misleading use of the list initializer, LIST_HEAD(), on it.
The eeh_dev struct contains two list entry fields, called "list" and
"rmv_list". Rename them to "entry" and "rmv_entry" and, as above, stop
initializing them with LIST_HEAD().
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
arch/powerpc/include/asm/eeh.h | 12 ++++++------
arch/powerpc/kernel/eeh_dev.c | 2 --
arch/powerpc/kernel/eeh_driver.c | 10 +++++-----
arch/powerpc/kernel/eeh_pe.c | 11 +++++------
arch/powerpc/platforms/powernv/eeh-powernv.c | 2 +-
arch/powerpc/platforms/pseries/msi.c | 3 ++-
6 files changed, 19 insertions(+), 21 deletions(-)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 703d1f96ee8b..b48b08ed9be3 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -98,13 +98,13 @@ struct eeh_pe {
atomic_t pass_dev_cnt; /* Count of passed through devs */
struct eeh_pe *parent; /* Parent PE */
void *data; /* PE auxillary data */
- struct list_head child_list; /* Link PE to the child list */
- struct list_head edevs; /* Link list of EEH devices */
- struct list_head child; /* Child PEs */
+ struct list_head child_list; /* List of PEs below this PE */
+ struct list_head child; /* Memb. child_list/eeh_phb_pe */
+ struct list_head edevs; /* List of eeh_dev in this PE */
};
#define eeh_pe_for_each_dev(pe, edev, tmp) \
- list_for_each_entry_safe(edev, tmp, &pe->edevs, list)
+ list_for_each_entry_safe(edev, tmp, &pe->edevs, entry)
#define eeh_for_each_pe(root, pe) \
for (pe = root; pe; pe = eeh_pe_next(pe, root))
@@ -141,8 +141,8 @@ struct eeh_dev {
int aer_cap; /* Saved AER capability */
int af_cap; /* Saved AF capability */
struct eeh_pe *pe; /* Associated PE */
- struct list_head list; /* Form link list in the PE */
- struct list_head rmv_list; /* Record the removed edevs */
+ struct list_head entry; /* Membership in eeh_pe.edevs */
+ struct list_head rmv_entry; /* Membership in rmv_list */
struct pci_dn *pdn; /* Associated PCI device node */
struct pci_dev *pdev; /* Associated PCI device */
bool in_error; /* Error flag for edev */
diff --git a/arch/powerpc/kernel/eeh_dev.c b/arch/powerpc/kernel/eeh_dev.c
index a34e6912c15e..d8c90f3284b5 100644
--- a/arch/powerpc/kernel/eeh_dev.c
+++ b/arch/powerpc/kernel/eeh_dev.c
@@ -60,8 +60,6 @@ struct eeh_dev *eeh_dev_init(struct pci_dn *pdn)
/* Associate EEH device with OF node */
pdn->edev = edev;
edev->pdn = pdn;
- INIT_LIST_HEAD(&edev->list);
- INIT_LIST_HEAD(&edev->rmv_list);
return edev;
}
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index cc300eb9585c..7859af897058 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -404,7 +404,7 @@ static void *eeh_dev_restore_state(struct eeh_dev *edev, void *userdata)
* EEH device is created.
*/
if (edev->pe && (edev->pe->state & EEH_PE_CFG_RESTRICTED)) {
- if (list_is_last(&edev->list, &edev->pe->edevs))
+ if (list_is_last(&edev->entry, &edev->pe->edevs))
eeh_pe_restore_bars(edev->pe);
return NULL;
@@ -560,7 +560,7 @@ static void *eeh_rmv_device(struct eeh_dev *edev, void *userdata)
pdn->pe_number = IODA_INVALID_PE;
#endif
if (rmv_data)
- list_add(&edev->rmv_list, &rmv_data->edev_list);
+ list_add(&edev->rmv_entry, &rmv_data->edev_list);
} else {
pci_lock_rescan_remove();
pci_stop_and_remove_bus_device(dev);
@@ -739,7 +739,7 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
* PE. We should disconnect it so the binding can be
* rebuilt when adding PCI devices.
*/
- edev = list_first_entry(&pe->edevs, struct eeh_dev, list);
+ edev = list_first_entry(&pe->edevs, struct eeh_dev, entry);
eeh_pe_traverse(pe, eeh_pe_detach_dev, NULL);
if (pe->type & EEH_PE_VF) {
eeh_add_virt_device(edev);
@@ -934,9 +934,9 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
* For those hot removed VFs, we should add back them after PF get
* recovered properly.
*/
- list_for_each_entry_safe(edev, tmp, &rmv_data.edev_list, rmv_list) {
+ list_for_each_entry_safe(edev, tmp, &rmv_data.edev_list, rmv_entry) {
eeh_add_virt_device(edev);
- list_del(&edev->rmv_list);
+ list_del(&edev->rmv_entry);
}
/* Tell all device drivers that they can resume operations */
diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index 210d239a9395..7d6d93cd67e1 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -75,7 +75,6 @@ static struct eeh_pe *eeh_pe_alloc(struct pci_controller *phb, int type)
pe->type = type;
pe->phb = phb;
INIT_LIST_HEAD(&pe->child_list);
- INIT_LIST_HEAD(&pe->child);
INIT_LIST_HEAD(&pe->edevs);
pe->data = (void *)pe + ALIGN(sizeof(struct eeh_pe),
@@ -360,7 +359,7 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
edev->pe = pe;
/* Put the edev to PE */
- list_add_tail(&edev->list, &pe->edevs);
+ list_add_tail(&edev->entry, &pe->edevs);
pr_debug("EEH: Add %04x:%02x:%02x.%01x to Bus PE#%x\n",
pdn->phb->global_number,
pdn->busno,
@@ -369,7 +368,7 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
pe->addr);
return 0;
} else if (pe && (pe->type & EEH_PE_INVALID)) {
- list_add_tail(&edev->list, &pe->edevs);
+ list_add_tail(&edev->entry, &pe->edevs);
edev->pe = pe;
/*
* We're running to here because of PCI hotplug caused by
@@ -429,7 +428,7 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
* link the EEH device accordingly.
*/
list_add_tail(&pe->child, &parent->child_list);
- list_add_tail(&edev->list, &pe->edevs);
+ list_add_tail(&edev->entry, &pe->edevs);
edev->pe = pe;
pr_debug("EEH: Add %04x:%02x:%02x.%01x to "
"Device PE#%x, Parent PE#%x\n",
@@ -469,7 +468,7 @@ int eeh_rmv_from_parent_pe(struct eeh_dev *edev)
/* Remove the EEH device */
pe = eeh_dev_to_pe(edev);
edev->pe = NULL;
- list_del(&edev->list);
+ list_del(&edev->entry);
/*
* Check if the parent PE includes any EEH devices.
@@ -945,7 +944,7 @@ struct pci_bus *eeh_pe_bus_get(struct eeh_pe *pe)
return pe->bus;
/* Retrieve the parent PCI bus of first (top) PCI device */
- edev = list_first_entry_or_null(&pe->edevs, struct eeh_dev, list);
+ edev = list_first_entry_or_null(&pe->edevs, struct eeh_dev, entry);
pdev = eeh_dev_to_pci_dev(edev);
if (pdev)
return pdev->bus;
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index d0764f2c0733..a7e59dbf2696 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -1040,7 +1040,7 @@ static int pnv_eeh_reset_vf_pe(struct eeh_pe *pe, int option)
int ret;
/* The VF PE should have only one child device */
- edev = list_first_entry_or_null(&pe->edevs, struct eeh_dev, list);
+ edev = list_first_entry_or_null(&pe->edevs, struct eeh_dev, entry);
pdn = eeh_dev_to_pdn(edev);
if (!pdn)
return -ENXIO;
diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
index b7496948129e..8011b4129e3a 100644
--- a/arch/powerpc/platforms/pseries/msi.c
+++ b/arch/powerpc/platforms/pseries/msi.c
@@ -203,7 +203,8 @@ static struct device_node *find_pe_dn(struct pci_dev *dev, int *total)
/* Get the top level device in the PE */
edev = pdn_to_eeh_dev(PCI_DN(dn));
if (edev->pe)
- edev = list_first_entry(&edev->pe->edevs, struct eeh_dev, list);
+ edev = list_first_entry(&edev->pe->edevs, struct eeh_dev,
+ entry);
dn = pci_device_to_OF_node(edev->pdev);
if (!dn)
return NULL;
--
2.19.0.2.gcad72f5712
^ permalink raw reply related
* [PATCH 09/14] powerpc/eeh: Cleanup logic in eeh_rmv_from_parent_pe()
From: Sam Bobroff @ 2018-09-12 1:23 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1536715396.git.sbobroff@linux.ibm.com>
Move the call to eeh_dev_to_pe() up, so that later it's clear that
"pe" isn't NULL.
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
arch/powerpc/kernel/eeh_pe.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index 7d6d93cd67e1..78f125d24bd0 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -456,7 +456,8 @@ int eeh_rmv_from_parent_pe(struct eeh_dev *edev)
int cnt;
struct pci_dn *pdn = eeh_dev_to_pdn(edev);
- if (!edev->pe) {
+ pe = eeh_dev_to_pe(edev);
+ if (!pe) {
pr_debug("%s: No PE found for device %04x:%02x:%02x.%01x\n",
__func__, pdn->phb->global_number,
pdn->busno,
@@ -466,7 +467,6 @@ int eeh_rmv_from_parent_pe(struct eeh_dev *edev)
}
/* Remove the EEH device */
- pe = eeh_dev_to_pe(edev);
edev->pe = NULL;
list_del(&edev->entry);
--
2.19.0.2.gcad72f5712
^ 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