Re: [PATCH V10 11/12] powerpc/eeh: Don't block PCI config on resetting VF PE

2015-10-30 Thread Wei Yang 
On Fri, Oct 30, 2015 at 04:42:07PM +1100, Alexey Kardashevskiy wrote:
>On 10/26/2015 02:16 PM, Wei Yang wrote:
>>From: Gavin Shan 
>>
>>When passing through SRIOV VF from host to guest via VFIO PCI
>>infrastructure, the VF is resetted by EEH specific backend
>>(pcibios_set_pcie_reset_state()). We can't block the PCI config,
>>otherwise, the reset (FLR or AF FLR), to be completed by PCI
>>config access to the VF, can't be done. Then the VF can't be
>>put into initial state when passing it to the guest and returning
>>back to the host.
>>
>>The patch just doesn't block the VF's PCI config space when doing
>>the reset. It fixes EEH error caused by DMA traffic to bogus DMA
>>address on restarting guest after killing the QEMU process, which
>>includes Mellanox VF passed through from host.
>
>The patch as it is makes sense as a bugfix for our internal tree where the
>EEH VF feature was present at the time when this patch was posted but in this
>patchset is makes more sense to merge it into:
>
>[PATCH V10 08/12] powerpc/powernv: Support EEH reset for VF PE
>
>as it is quite weird within one patchset to introduce a problem  and then fix
>it in a following patch.
>

Sure, got it.

>
>>Reported-by: Alexey Kardashevskiy 
>>Signed-off-by: Gavin Shan 
>>Tested-by: Alexey Kardashevskiy 
>>Signed-off-by: Alexey Kardashevskiy 
>
>Remove "sob: aik@..." please.
>
>
>>---
>>  arch/powerpc/kernel/eeh.c | 9 ++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>>diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
>>index 28e4d73..e1846f5 100644
>>--- a/arch/powerpc/kernel/eeh.c
>>+++ b/arch/powerpc/kernel/eeh.c
>>@@ -745,7 +745,8 @@ int pcibios_set_pcie_reset_state(struct pci_dev *dev, 
>>enum pcie_reset_state stat
>>  case pcie_deassert_reset:
>>  eeh_ops->reset(pe, EEH_RESET_DEACTIVATE);
>>  eeh_unfreeze_pe(pe, false);
>>- eeh_pe_state_clear(pe, EEH_PE_CFG_BLOCKED);
>>+ if (!(pe->type & EEH_PE_VF))
>>+ eeh_pe_state_clear(pe, EEH_PE_CFG_BLOCKED);
>>  eeh_pe_dev_traverse(pe, eeh_restore_dev_state, dev);
>>  eeh_pe_state_clear(pe, EEH_PE_ISOLATED);
>>  break;
>>@@ -753,14 +754,16 @@ int pcibios_set_pcie_reset_state(struct pci_dev *dev, 
>>enum pcie_reset_state stat
>>  eeh_pe_state_mark(pe, EEH_PE_ISOLATED);
>>  eeh_ops->set_option(pe, EEH_OPT_FREEZE_PE);
>>  eeh_pe_dev_traverse(pe, eeh_disable_and_save_dev_state, dev);
>>- eeh_pe_state_mark(pe, EEH_PE_CFG_BLOCKED);
>>+ if (!(pe->type & EEH_PE_VF))
>>+ eeh_pe_state_mark(pe, EEH_PE_CFG_BLOCKED);
>>  eeh_ops->reset(pe, EEH_RESET_HOT);
>>  break;
>>  case pcie_warm_reset:
>>  eeh_pe_state_mark(pe, EEH_PE_ISOLATED);
>>  eeh_ops->set_option(pe, EEH_OPT_FREEZE_PE);
>>  eeh_pe_dev_traverse(pe, eeh_disable_and_save_dev_state, dev);
>>- eeh_pe_state_mark(pe, EEH_PE_CFG_BLOCKED);
>>+ if (!(pe->type & EEH_PE_VF))
>>+ eeh_pe_state_mark(pe, EEH_PE_CFG_BLOCKED);
>>  eeh_ops->reset(pe, EEH_RESET_FUNDAMENTAL);
>>  break;
>>  default:
>>
>
>
>-- 
>Alexey

-- 
Richard Yang
Help you, Help me

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V2 3/3] perf/powerpc :add support for sampling intr machine state

2015-10-30 Thread Madhavan Srinivasan 


On Monday 26 October 2015 06:14 PM, Anju T wrote:
> The registers to sample are passed through the sample_regs_intr bitmask.
> The name and bit position for each register is defined in asm/perf_regs.h.
> This feature can be enabled by using -I option with perf  record command.
> To display the sampled register values use perf script -D.
> The kernel uses the "PERF" register ids to find offset of the register in 
> 'struct pt_regs'.
> CONFIG_HAVE_PERF_REGS will enable sampling of the interrupted machine state.
>
> Signed-off-by: Anju T 
> ---
>  arch/powerpc/Kconfig  |  1 +
>  arch/powerpc/perf/Makefile|  1 +
>  arch/powerpc/perf/perf_regs.c | 87 
> +++
>  tools/perf/config/Makefile|  5 +++
>  4 files changed, 94 insertions(+)
>  create mode 100644 arch/powerpc/perf/perf_regs.c
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 5ef2711..768d700 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -116,6 +116,7 @@ config PPC
>   select GENERIC_ATOMIC64 if PPC32
>   select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
>   select HAVE_PERF_EVENTS
> + select HAVE_PERF_REGS
>   select HAVE_REGS_AND_STACK_ACCESS_API
>   select HAVE_HW_BREAKPOINT if PERF_EVENTS && PPC_BOOK3S_64
>   select ARCH_WANT_IPC_PARSE_VERSION
> diff --git a/arch/powerpc/perf/Makefile b/arch/powerpc/perf/Makefile
> index f9c083a..0d53815 100644
> --- a/arch/powerpc/perf/Makefile
> +++ b/arch/powerpc/perf/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_FSL_EMB_PERF_EVENT) += core-fsl-emb.o
>  obj-$(CONFIG_FSL_EMB_PERF_EVENT_E500) += e500-pmu.o e6500-pmu.o
>  
>  obj-$(CONFIG_HV_PERF_CTRS) += hv-24x7.o hv-gpci.o hv-common.o
> +obj-$(CONFIG_PERF_EVENTS)+= perf_regs.o
>  
>  obj-$(CONFIG_PPC64)  += $(obj64-y)
>  obj-$(CONFIG_PPC32)  += $(obj32-y)
> diff --git a/arch/powerpc/perf/perf_regs.c b/arch/powerpc/perf/perf_regs.c
> new file mode 100644
> index 000..2474dc4
> --- /dev/null
> +++ b/arch/powerpc/perf/perf_regs.c
> @@ -0,0 +1,87 @@
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define PT_REGS_OFFSET(id, r) [id] = offsetof(struct pt_regs, r)
> +
> +#define REG_RESERVED (~((1ULL << PERF_REG_POWERPC_MAX) - 1))
> +
> +static unsigned int pt_regs_offset[PERF_REG_POWERPC_MAX] = {
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR0, gpr[0]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR1, gpr[1]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR2, gpr[2]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR3, gpr[3]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR4, gpr[4]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR5, gpr[5]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR6, gpr[6]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR7, gpr[7]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR8, gpr[8]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR9, gpr[9]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR10, gpr[10]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR11, gpr[11]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR12, gpr[12]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR13, gpr[13]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR14, gpr[14]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR15, gpr[15]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR16, gpr[16]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR17, gpr[17]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR18, gpr[18]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR19, gpr[19]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR20, gpr[20]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR21, gpr[21]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR22, gpr[22]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR23, gpr[23]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR24, gpr[24]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR25, gpr[25]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR26, gpr[26]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR27, gpr[27]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR28, gpr[28]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR29, gpr[29]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR30, gpr[30]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_GPR31, gpr[31]),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_NIP, nip),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_MSR, msr),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_ORIG_R3, orig_gpr3),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_CTR, ctr),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_LNK, link),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_XER, xer),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_CCR, ccr),
> +#ifdef __powerpc64__
> + PT_REGS_OFFSET(PERF_REG_POWERPC_SOFTE, softe),
> +#else
> + PT_REGS_OFFSET(PERF_REG_POWERPC_MQ, mq),
> +#endif
> + PT_REGS_OFFSET(PERF_REG_POWERPC_TRAP, trap),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_DAR, dar),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_DSISR, dsisr),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_RESULT, result),
> +};
> +u64 perf_reg_value(struct pt_regs *regs, int idx)
>

Re: [ANNOUNCE] kexec-tools 2.0.11-rc1

2015-10-30 Thread Simon Horman 
On Thu, Oct 29, 2015 at 07:23:24PM -0500, Scott Wood wrote:
> On Fri, 2015-10-30 at 08:48 +0900, Simon Horman wrote:
> > Hi all,
> > 
> > I am happy to announce the release of kexec-tools 2.0.11-rc1.
> > 
> > This is an incremental feature pre-release.
> > 
> > So long as no serious problems arise I intend to release kexec-tools 2.0.11
> > shortly after the release of the v4.3 kernel, which I expect to occur in
> > the next week or so.  As such testing of 2.0.11-rc1 would be greatly
> > appreciated.
> > 
> > I do not have any outstanding changes for 2.0.11 at this time.
> > And I would like to only accept bug fixes at this time and take take
> > features patches once 2.0.11 has been released.
> 
> As I previously reported, "ppc64: purgatory: Reset primary cpu endian to big-
> endian" breaks book3e, so can http://patchwork.ozlabs.org/patch/527048/ be 
> considered for this release?

Yes of course, sorry for not taking more note of that.

Would it be possible to get a review, e.g. from Michael?
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V10 08/12] powerpc/powernv: Support EEH reset for VF PE

2015-10-30 Thread Wei Yang 
On Fri, Oct 30, 2015 at 03:11:20PM +1100, Alexey Kardashevskiy wrote:
>On 10/26/2015 02:15 PM, Wei Yang wrote:
>>PEs for VFs don't have primary bus. So they have to have their own reset
>>backend, which is used during EEH recovery. The patch implements the reset
>>backend for VF's PE by issuing FLR or AF FLR to the VFs, which are contained
>>in the PE.
>>
>>[gwshan: changelog and code refactoring]
>>Signed-off-by: Wei Yang 
>>Acked-by: Gavin Shan 
>>---
>>  arch/powerpc/include/asm/eeh.h   |   1 +
>>  arch/powerpc/platforms/powernv/eeh-powernv.c | 134 
>> ++-
>>  2 files changed, 134 insertions(+), 1 deletion(-)
>>
>>diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
>>index ec21f8f..331c856 100644
>>--- a/arch/powerpc/include/asm/eeh.h
>>+++ b/arch/powerpc/include/asm/eeh.h
>>@@ -136,6 +136,7 @@ struct eeh_dev {
>>  int pcix_cap;   /* Saved PCIx capability*/
>>  int pcie_cap;   /* Saved PCIe capability*/
>>  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 pci_controller *phb; /* Associated PHB   */
>>diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c 
>>b/arch/powerpc/platforms/powernv/eeh-powernv.c
>>index cfd55dd..017cd72 100644
>>--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
>>+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
>>@@ -404,6 +404,7 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
>>  edev->pcix_cap = pnv_eeh_find_cap(pdn, PCI_CAP_ID_PCIX);
>>  edev->pcie_cap = pnv_eeh_find_cap(pdn, PCI_CAP_ID_EXP);
>>  edev->aer_cap  = pnv_eeh_find_ecap(pdn, PCI_EXT_CAP_ID_ERR);
>>+ edev->af_cap   = pnv_eeh_find_cap(pdn, PCI_CAP_ID_AF);
>>  if ((edev->class_code >> 8) == PCI_CLASS_BRIDGE_PCI) {
>>  edev->mode |= EEH_DEV_BRIDGE;
>>  if (edev->pcie_cap) {
>>@@ -893,6 +894,127 @@ static int pnv_eeh_bridge_reset(struct pci_dev *dev, 
>>int option)
>>  return 0;
>>  }
>>
>>+static void pnv_eeh_wait_for_pending(struct pci_dn *pdn, int pos,
>>+  u16 mask, bool af_flr_rst)
>>+{
>>+ struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
>>+ int status, i;
>>+
>>+ /* Wait for Transaction Pending bit to be cleared */
>>+ for (i = 0; i < 4; i++) {
>>+ eeh_ops->read_config(pdn, pos, 2, );
>
>
>gcc should have complained on using uninitialized @status here.
>

I remove the obj file and re-compile the file, not the warning.
And took a look at other places where read_config() is called. The laster
parameter is not initialized before called.

You see the error during build?

>
>>+ if (!(status & mask))
>>+ return;
>>+
>>+ msleep((1 << i) * 100);
>>+ }
>>+
>>+ pr_warn("%s: Pending transaction while issuing %s FLR to "
>>+ "%04x:%02x:%02x.%01x\n",
>
>Do not wrap user-visible strings.
>

Will change this.

>
>>+ __func__, af_flr_rst ? "AF" : "",
>>+ edev->phb->global_number, pdn->busno,
>>+ PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
>>+}
>>+
>>+static int pnv_eeh_do_flr(struct pci_dn *pdn, int option)
>>+{
>>+ struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
>>+ u32 reg;
>>+
>>+ if (!edev->pcie_cap)
>>+ return -ENOTTY;
>
>
>Can pnv_eeh_do_flr() be really called on a non PCIe device, can we get that
>far? WARN_ON_ONCE() may be?
>

So you suggest to add a WARN_ON_ONCE() in this condition, right?

>
>>+
>>+ eeh_ops->read_config(pdn, edev->pcie_cap + PCI_EXP_DEVCAP, 4, );
>
>
>... and here about uninitialized @reg.
>
>
>>+ if (!(reg & PCI_EXP_DEVCAP_FLR))
>>+ return -ENOTTY;
>>+
>>+ switch (option) {
>>+ case EEH_RESET_HOT:
>>+ case EEH_RESET_FUNDAMENTAL:
>>+ pnv_eeh_wait_for_pending(pdn, edev->pcie_cap + PCI_EXP_DEVSTA,
>>+  PCI_EXP_DEVSTA_TRPND, false);
>>+ eeh_ops->read_config(pdn, edev->pcie_cap + PCI_EXP_DEVCTL,
>>+  4, );
>>+ reg |= PCI_EXP_DEVCTL_BCR_FLR;
>>+ eeh_ops->write_config(pdn, edev->pcie_cap + PCI_EXP_DEVCTL,
>>+   4, reg);
>>+ msleep(EEH_PE_RST_HOLD_TIME);
>>+ break;
>>+ case EEH_RESET_DEACTIVATE:
>>+ eeh_ops->read_config(pdn, edev->pcie_cap + PCI_EXP_DEVCTL,
>>+  4, );
>>+ reg &= ~PCI_EXP_DEVCTL_BCR_FLR;
>>+ eeh_ops->write_config(pdn, edev->pcie_cap + PCI_EXP_DEVCTL,
>>+   4, reg);
>>+

Re: [PATCH V10 06/12] powerpc/powernv: EEH device for VF

2015-10-30 Thread Wei Yang 
On Fri, Oct 30, 2015 at 02:33:49PM +1100, Alexey Kardashevskiy wrote:
>On 10/26/2015 02:15 PM, Wei Yang wrote:
>>VFs and their corresponding pci_dn instances are created and released
>>dynamically as their PF's SRIOV capability is enabled and disabled.
>>The patch creates and releases EEH devices for VFs when creating and
>>releasing their pci_dn instances, which means EEH devices and pci_dn
>>instances have same life cycle. Also, VF's EEH device is identified
>>by (struct eeh_dev::physfn).
>
>
>The add_dev_pci_data() helper (the one you hack) does not create pci_dn
>instances. The add_one_dev_pci_data() helper does.
>

Yes, you are right. The patch here create edev after the pci_dn is created.

So which part in the log you think is not accurate?

>
>>
>>[gwshan: changelog and removed CONFIG_PCI_IOV]
>>Signed-off-by: Wei Yang 
>>Acked-by: Gavin Shan 
>>---
>>  arch/powerpc/include/asm/eeh.h |  1 +
>>  arch/powerpc/kernel/pci_dn.c   | 12 
>>  2 files changed, 13 insertions(+)
>>
>>diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
>>index c5eb86f..6c383ad 100644
>>--- a/arch/powerpc/include/asm/eeh.h
>>+++ b/arch/powerpc/include/asm/eeh.h
>>@@ -140,6 +140,7 @@ struct eeh_dev {
>>  struct pci_controller *phb; /* Associated PHB   */
>>  struct pci_dn *pdn; /* Associated PCI device node   */
>>  struct pci_dev *pdev;   /* Associated PCI device*/
>>+ struct pci_dev *physfn; /* Associated PF PORT   */
>>  struct pci_bus *bus;/* PCI bus for partial hotplug  */
>>  };
>>
>>diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
>>index f771130..f0ddde7 100644
>>--- a/arch/powerpc/kernel/pci_dn.c
>>+++ b/arch/powerpc/kernel/pci_dn.c
>>@@ -180,7 +180,9 @@ static struct pci_dn *add_one_dev_pci_data(struct pci_dn 
>>*parent,
>>  struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
>>  {
>>  #ifdef CONFIG_PCI_IOV
>>+ struct pci_controller *hose = pci_bus_to_host(pdev->bus);
>>  struct pci_dn *parent, *pdn;
>>+ struct eeh_dev *edev;
>>  int i;
>>
>>  /* Only support IOV for now */
>>@@ -206,6 +208,9 @@ struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
>>   __func__, i);
>>  return NULL;
>>  }
>>+ eeh_dev_init(pdn, hose);
>>+ edev = pdn_to_eeh_dev(pdn);
>
>
>In theory, pdn_to_eeh_dev() can return NULL. In this patch, it is not clear
>if pdn->edev gets initialized before or after add_dev_pci_data().
>

Yep, the return value should be checked.

pdn->edev is initialized in eeh_dev_init() which is called in
add_dev_pci_data(). The order is not clear?

>
>
>>+ edev->physfn = pdev;
>>  }
>>  #endif /* CONFIG_PCI_IOV */
>>
>>@@ -254,10 +259,17 @@ void remove_dev_pci_data(struct pci_dev *pdev)
>>  for (i = 0; i < pci_sriov_get_totalvfs(pdev); i++) {
>>  list_for_each_entry_safe(pdn, tmp,
>>  >child_list, list) {
>>+ struct eeh_dev *edev;
>>  if (pdn->busno != pci_iov_virtfn_bus(pdev, i) ||
>>  pdn->devfn != pci_iov_virtfn_devfn(pdev, i))
>>  continue;
>>
>>+ edev = pdn_to_eeh_dev(pdn);
>>+ if (edev) {
>>+ pdn->edev = NULL;
>>+ kfree(edev);
>>+ }
>>+
>>  if (!list_empty(>list))
>>  list_del(>list);
>>
>>
>
>
>-- 
>Alexey
>--
>To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>the body of a message to majord...@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Richard Yang
Help you, Help me

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V10 12/12] powerpc/eeh: Handle hot removed VF when PF is EEH aware

2015-10-30 Thread Wei Yang 
On Fri, Oct 30, 2015 at 04:35:54PM +1100, Alexey Kardashevskiy wrote:
>On 10/26/2015 02:16 PM, Wei Yang wrote:
>>When PF is EEH aware while VFs are not, VFs will be removed during EEH
>>recovery. This is not supported in current code, while will leads to the VF
>>lost.
>>
>>This patch fixes this by adding VFs back. VFs should be added back after PF
>>get recovered properly.
>>
>>Signed-off-by: Wei Yang 
>>Signed-off-by: Alexey Kardashevskiy 
>
>btw please remove my "sob" from this patchset (here and 11/12 at least) as I
>did not "sob" the upstream versions of these and I did not post them and
>there is no public tree of mine with these patches. When I agree that the
>patches are good to go, it will be "reviewed-by" or "acked-by".
>

Sure, I would obey this rule in the future.

>
>>---
>>  arch/powerpc/include/asm/eeh.h   |  6 ++
>>  arch/powerpc/kernel/eeh_dev.c|  1 +
>>  arch/powerpc/kernel/eeh_driver.c | 30 +++---
>>  3 files changed, 30 insertions(+), 7 deletions(-)
>>
>>diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
>>index ea1f13c4..c529a23 100644
>>--- a/arch/powerpc/include/asm/eeh.h
>>+++ b/arch/powerpc/include/asm/eeh.h
>>@@ -127,6 +127,11 @@ static inline bool eeh_pe_passed(struct eeh_pe *pe)
>>  #define EEH_DEV_SYSFS   (1 << 9)/* Sysfs created
>> */
>>  #define EEH_DEV_REMOVED (1 << 10)   /* Removed permanently  
>> */
>>
>>+struct eeh_rmv_data {
>>+ struct list_head edev_list;
>>+ int removed;
>>+};
>>+
>>  struct eeh_dev {
>>  int mode;   /* EEH mode */
>>  int class_code; /* Class code of the device */
>>@@ -139,6 +144,7 @@ struct eeh_dev {
>>  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 edev  */
>>  struct pci_controller *phb; /* Associated PHB   */
>>  struct pci_dn *pdn; /* Associated PCI device node   */
>>  struct pci_dev *pdev;   /* Associated PCI device*/
>>diff --git a/arch/powerpc/kernel/eeh_dev.c b/arch/powerpc/kernel/eeh_dev.c
>>index aabba94..7815095 100644
>>--- a/arch/powerpc/kernel/eeh_dev.c
>>+++ b/arch/powerpc/kernel/eeh_dev.c
>>@@ -67,6 +67,7 @@ void *eeh_dev_init(struct pci_dn *pdn, void *data)
>>  edev->pdn = pdn;
>>  edev->phb = phb;
>>  INIT_LIST_HEAD(>list);
>>+ INIT_LIST_HEAD(>rmv_list);
>>
>>  return NULL;
>>  }
>>diff --git a/arch/powerpc/kernel/eeh_driver.c 
>>b/arch/powerpc/kernel/eeh_driver.c
>>index 99868e2..f2406b4 100644
>>--- a/arch/powerpc/kernel/eeh_driver.c
>>+++ b/arch/powerpc/kernel/eeh_driver.c
>>@@ -420,7 +420,8 @@ static void *eeh_rmv_device(void *data, void *userdata)
>>  struct pci_driver *driver;
>>  struct eeh_dev *edev = (struct eeh_dev *)data;
>>  struct pci_dev *dev = eeh_dev_to_pci_dev(edev);
>>- int *removed = (int *)userdata;
>>+ struct eeh_rmv_data *rmv_data = (struct eeh_rmv_data *)userdata;
>>+ int *removed = rmv_data ? _data->removed : NULL;
>
>
>You just touched @userdata/@removed in [10/12] and now you are touching it
>again.
>
>It feels like this patch is better to be merged into [10/12], this will
>reduce the noise about the userdata pointer changes passed into
>eeh_pe_dev_traverse() and make more sense as "powerpc/eeh: Support error
>recovery for VF PE" without adding VFs back is pretty useless.
>

Agree, will merge them.

>
>
>
>>  struct pci_dn *pdn = eeh_dev_to_pdn(edev);
>>
>>  /*
>>@@ -467,6 +468,9 @@ static void *eeh_rmv_device(void *data, void *userdata)
>>   * required to plug the VF successfully.
>>   */
>>  pdn->pe_number = IODA_INVALID_PE;
>>+
>>+ if (rmv_data)
>>+ list_add(>rmv_list, _data->edev_list);
>>  } else {
>>  pci_lock_rescan_remove();
>>  pci_stop_and_remove_bus_device(dev);
>>@@ -585,11 +589,12 @@ int eeh_pe_reset_and_recover(struct eeh_pe *pe)
>>   * During the reset, udev might be invoked because those affected
>>   * PCI devices will be removed and then added.
>>   */
>>-static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus)
>>+static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
>>+ struct eeh_rmv_data *rmv_data)
>>  {
>>  struct pci_bus *frozen_bus = eeh_pe_bus_get(pe);
>>  struct timeval tstamp;
>>- int cnt, rc, removed = 0;
>>+ int cnt, rc;
>>  struct eeh_dev *edev;
>>
>>  /* pcibios will clear the counter; save the value */
>>@@ -612,7 +617,7 @@ static int eeh_reset_device(struct eeh_pe *pe, struct 
>>pci_bus *bus)
>>

Re: [PATCH V10 06/12] powerpc/powernv: EEH device for VF

2015-10-30 Thread Alexey Kardashevskiy 

On 10/30/2015 05:52 PM, Wei Yang wrote:

On Fri, Oct 30, 2015 at 02:33:49PM +1100, Alexey Kardashevskiy wrote:

On 10/26/2015 02:15 PM, Wei Yang wrote:

VFs and their corresponding pci_dn instances are created and released
dynamically as their PF's SRIOV capability is enabled and disabled.
The patch creates and releases EEH devices for VFs when creating and
releasing their pci_dn instances, which means EEH devices and pci_dn
instances have same life cycle. Also, VF's EEH device is identified
by (struct eeh_dev::physfn).



The add_dev_pci_data() helper (the one you hack) does not create pci_dn
instances. The add_one_dev_pci_data() helper does.



Yes, you are right. The patch here create edev after the pci_dn is created.

So which part in the log you think is not accurate?



The commit log is ok, I just thought loud that eeh_dev_init() could go to 
add_one_dev_pci_data() to make things more clear.








[gwshan: changelog and removed CONFIG_PCI_IOV]
Signed-off-by: Wei Yang 
Acked-by: Gavin Shan 
---
  arch/powerpc/include/asm/eeh.h |  1 +
  arch/powerpc/kernel/pci_dn.c   | 12 
  2 files changed, 13 insertions(+)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index c5eb86f..6c383ad 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -140,6 +140,7 @@ struct eeh_dev {
struct pci_controller *phb; /* Associated PHB   */
struct pci_dn *pdn; /* Associated PCI device node   */
struct pci_dev *pdev;   /* Associated PCI device*/
+   struct pci_dev *physfn; /* Associated PF PORT   */
struct pci_bus *bus;/* PCI bus for partial hotplug  */
  };

diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index f771130..f0ddde7 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -180,7 +180,9 @@ static struct pci_dn *add_one_dev_pci_data(struct pci_dn 
*parent,
  struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
  {
  #ifdef CONFIG_PCI_IOV
+   struct pci_controller *hose = pci_bus_to_host(pdev->bus);
struct pci_dn *parent, *pdn;
+   struct eeh_dev *edev;
int i;

/* Only support IOV for now */
@@ -206,6 +208,9 @@ struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
 __func__, i);
return NULL;
}
+   eeh_dev_init(pdn, hose);
+   edev = pdn_to_eeh_dev(pdn);



In theory, pdn_to_eeh_dev() can return NULL. In this patch, it is not clear
if pdn->edev gets initialized before or after add_dev_pci_data().



Yep, the return value should be checked.


May be BUG_ON will be enough, up to you.




pdn->edev is initialized in eeh_dev_init() which is called in
add_dev_pci_data(). The order is not clear?





+   edev->physfn = pdev;
}
  #endif /* CONFIG_PCI_IOV */

@@ -254,10 +259,17 @@ void remove_dev_pci_data(struct pci_dev *pdev)
for (i = 0; i < pci_sriov_get_totalvfs(pdev); i++) {
list_for_each_entry_safe(pdn, tmp,
>child_list, list) {
+   struct eeh_dev *edev;
if (pdn->busno != pci_iov_virtfn_bus(pdev, i) ||
pdn->devfn != pci_iov_virtfn_devfn(pdev, i))
continue;

+   edev = pdn_to_eeh_dev(pdn);
+   if (edev) {
+   pdn->edev = NULL;
+   kfree(edev);
+   }
+
if (!list_empty(>list))
list_del(>list);







--
Alexey
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V10 04/12] powerpc/pci: Remove VFs prior to PF

2015-10-30 Thread Wei Yang 
On Fri, Oct 30, 2015 at 02:04:12PM +1100, Alexey Kardashevskiy wrote:
>On 10/26/2015 02:15 PM, Wei Yang wrote:
>>As commit ac205b7bb72f ("PCI: make sriov work with hotplug remove") indicates,
>>VFs, which might be hooked to same PCI bus as their PF should be removed
>
>A comma is missing before "should be" (or you did not need a comma after
>"VFs" may be :) ).
>

I think you are right.

>
>>before the PF. Otherwise, the PCI hot unplugging on the PCI bus would
>
>s/on/of/? "Unplugging on" does not make much sense to me in this context at
>least.
>

Sounds I need to improve my English :-)

"on" here means those PCI devices are attached to the PCI bus. So "of" is the
correct word?

Change "unplugging" to "removing" would be better?

>
>>cause kernel crash.
>>
>>The patch applies the above pattern to PowerPC PCI hotplug path.
>>
>>[gwshan: changelog]
>>Signed-off-by: Wei Yang 
>>Acked-by: Gavin Shan 
>>---
>>  arch/powerpc/kernel/pci-hotplug.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>>diff --git a/arch/powerpc/kernel/pci-hotplug.c 
>>b/arch/powerpc/kernel/pci-hotplug.c
>>index 7f9ed0c..59c4361 100644
>>--- a/arch/powerpc/kernel/pci-hotplug.c
>>+++ b/arch/powerpc/kernel/pci-hotplug.c
>>@@ -55,7 +55,7 @@ void pcibios_remove_pci_devices(struct pci_bus *bus)
>>
>>  pr_debug("PCI: Removing devices on bus %04x:%02x\n",
>>   pci_domain_nr(bus),  bus->number);
>>- list_for_each_entry_safe(dev, tmp, >devices, bus_list) {
>>+ list_for_each_entry_safe_reverse(dev, tmp, >devices, bus_list) {
>>  pr_debug("   Removing %s...\n", pci_name(dev));
>>  pci_stop_and_remove_bus_device(dev);
>>  }
>>
>
>
>-- 
>Alexey

-- 
Richard Yang
Help you, Help me

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V10 06/12] powerpc/powernv: EEH device for VF

2015-10-30 Thread Wei Yang 
On Fri, Oct 30, 2015 at 06:36:01PM +1100, Alexey Kardashevskiy wrote:
>On 10/30/2015 05:52 PM, Wei Yang wrote:
>>On Fri, Oct 30, 2015 at 02:33:49PM +1100, Alexey Kardashevskiy wrote:
>>>On 10/26/2015 02:15 PM, Wei Yang wrote:
VFs and their corresponding pci_dn instances are created and released
dynamically as their PF's SRIOV capability is enabled and disabled.
The patch creates and releases EEH devices for VFs when creating and
releasing their pci_dn instances, which means EEH devices and pci_dn
instances have same life cycle. Also, VF's EEH device is identified
by (struct eeh_dev::physfn).
>>>
>>>
>>>The add_dev_pci_data() helper (the one you hack) does not create pci_dn
>>>instances. The add_one_dev_pci_data() helper does.
>>>
>>
>>Yes, you are right. The patch here create edev after the pci_dn is created.
>>
>>So which part in the log you think is not accurate?
>
>
>The commit log is ok, I just thought loud that eeh_dev_init() could go to
>add_one_dev_pci_data() to make things more clear.
>

I thought this is are good suggestion.

My thought is, when we don't have VF, pci_dn and edev are two different thing.
We create pci_dn first and then initialize the edev. So mix the initialization
of them together is not that clear.

Not sure you agree or not.

>
>
>>>

[gwshan: changelog and removed CONFIG_PCI_IOV]
Signed-off-by: Wei Yang 
Acked-by: Gavin Shan 
---
  arch/powerpc/include/asm/eeh.h |  1 +
  arch/powerpc/kernel/pci_dn.c   | 12 
  2 files changed, 13 insertions(+)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index c5eb86f..6c383ad 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -140,6 +140,7 @@ struct eeh_dev {
struct pci_controller *phb; /* Associated PHB   */
struct pci_dn *pdn; /* Associated PCI device node   */
struct pci_dev *pdev;   /* Associated PCI device*/
+   struct pci_dev *physfn; /* Associated PF PORT   */
struct pci_bus *bus;/* PCI bus for partial hotplug  */
  };

diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index f771130..f0ddde7 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -180,7 +180,9 @@ static struct pci_dn *add_one_dev_pci_data(struct 
pci_dn *parent,
  struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
  {
  #ifdef CONFIG_PCI_IOV
+   struct pci_controller *hose = pci_bus_to_host(pdev->bus);
struct pci_dn *parent, *pdn;
+   struct eeh_dev *edev;
int i;

/* Only support IOV for now */
@@ -206,6 +208,9 @@ struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
 __func__, i);
return NULL;
}
+   eeh_dev_init(pdn, hose);
+   edev = pdn_to_eeh_dev(pdn);
>>>
>>>
>>>In theory, pdn_to_eeh_dev() can return NULL. In this patch, it is not clear
>>>if pdn->edev gets initialized before or after add_dev_pci_data().
>>>
>>
>>Yep, the return value should be checked.
>
>May be BUG_ON will be enough, up to you.
>

Yep, thanks.

>
>>
>>pdn->edev is initialized in eeh_dev_init() which is called in
>>add_dev_pci_data(). The order is not clear?
>>
>>>
>>>
+   edev->physfn = pdev;
}
  #endif /* CONFIG_PCI_IOV */

@@ -254,10 +259,17 @@ void remove_dev_pci_data(struct pci_dev *pdev)
for (i = 0; i < pci_sriov_get_totalvfs(pdev); i++) {
list_for_each_entry_safe(pdn, tmp,
>child_list, list) {
+   struct eeh_dev *edev;
if (pdn->busno != pci_iov_virtfn_bus(pdev, i) ||
pdn->devfn != pci_iov_virtfn_devfn(pdev, i))
continue;

+   edev = pdn_to_eeh_dev(pdn);
+   if (edev) {
+   pdn->edev = NULL;
+   kfree(edev);
+   }
+
if (!list_empty(>list))
list_del(>list);


>>>
>
>
>-- 
>Alexey

-- 
Richard Yang
Help you, Help me

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V10 07/12] powerpc/eeh: Create PE for VFs

2015-10-30 Thread Wei Yang 
On Fri, Oct 30, 2015 at 02:46:35PM +1100, Alexey Kardashevskiy wrote:
>On 10/26/2015 02:15 PM, Wei Yang wrote:
>>Current EEH recovery code works with the assumption: the PE has primary
>>bus. Unfortunately, that's not true for VF PEs, which generally contains
>>one or multiple VFs (for VF group case).
>
>What is that "VF group case"? Is not it a "compound PE" thingy which you were
>removing in "SRIOV redesign patchset"?
>

I think you are right.

The commit log is not correct, especially after SRIOV redesign.
Will remove this part.

>The patch might be ok but the commit log above does not explain why the
>existing way of PEs allocation would not work - somehow it works for a
>primary bus now, why would not it work on other buses?
>
>
>>The patch creates PEs for VFs in the weak function
>>pcibios_bus_add_device().Those PEs for VFs are identified with newly
>>introduced flag EEH_PE_VF so that we handle them differently during EEH
>>recovery.
>>
>>[gwshan: changelog and code refactoring]
>>Signed-off-by: Wei Yang 
>>Acked-by: Gavin Shan 
>>---
>>  arch/powerpc/include/asm/eeh.h   |  1 +
>>  arch/powerpc/kernel/eeh_pe.c | 10 --
>>  arch/powerpc/platforms/powernv/eeh-powernv.c | 16 
>>  3 files changed, 25 insertions(+), 2 deletions(-)
>>
>>diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
>>index 6c383ad..ec21f8f 100644
>>--- a/arch/powerpc/include/asm/eeh.h
>>+++ b/arch/powerpc/include/asm/eeh.h
>>@@ -72,6 +72,7 @@ struct pci_dn;
>>  #define EEH_PE_PHB  (1 << 1)/* PHB PE*/
>>  #define EEH_PE_DEVICE   (1 << 2)/* Device PE */
>>  #define EEH_PE_BUS  (1 << 3)/* Bus PE*/
>>+#define EEH_PE_VF(1 << 4)/* VF PE */
>>
>>  #define EEH_PE_ISOLATED (1 << 0)/* Isolated PE  
>> */
>>  #define EEH_PE_RECOVERING   (1 << 1)/* Recovering PE*/
>>diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
>>index 35f0b62..260a701 100644
>>--- a/arch/powerpc/kernel/eeh_pe.c
>>+++ b/arch/powerpc/kernel/eeh_pe.c
>>@@ -299,7 +299,10 @@ static struct eeh_pe *eeh_pe_get_parent(struct eeh_dev 
>>*edev)
>>   * EEH device already having associated PE, but
>>   * the direct parent EEH device doesn't have yet.
>>   */
>>- pdn = pdn ? pdn->parent : NULL;
>>+ if (edev->physfn)
>>+ pdn = pci_get_pdn(edev->physfn);
>>+ else
>>+ pdn = pdn ? pdn->parent : NULL;
>>  while (pdn) {
>>  /* We're poking out of PCI territory */
>>  parent = pdn_to_eeh_dev(pdn);
>>@@ -382,7 +385,10 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
>>  }
>>
>>  /* Create a new EEH PE */
>>- pe = eeh_pe_alloc(edev->phb, EEH_PE_DEVICE);
>>+ if (edev->physfn)
>>+ pe = eeh_pe_alloc(edev->phb, EEH_PE_VF);
>>+ else
>>+ pe = eeh_pe_alloc(edev->phb, EEH_PE_DEVICE);
>>  if (!pe) {
>>  pr_err("%s: out of memory!\n", __func__);
>>  return -ENOMEM;
>>diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c 
>>b/arch/powerpc/platforms/powernv/eeh-powernv.c
>>index 7cf0df8..cfd55dd 100644
>>--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
>>+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
>>@@ -1524,6 +1524,22 @@ static struct eeh_ops pnv_eeh_ops = {
>>  .restore_config = pnv_eeh_restore_config
>>  };
>>
>>+void pcibios_bus_add_device(struct pci_dev *pdev)
>>+{
>>+ struct pci_dn *pdn = pci_get_pdn(pdev);
>>+
>>+ if (!pdev->is_virtfn)
>>+ return;
>>+
>>+ /*
>>+  * The following operations will fail if VF's sysfs files
>>+  * aren't created or its resources aren't finalized.
>>+  */
>>+ eeh_add_device_early(pdn);
>>+ eeh_add_device_late(pdev);
>>+ eeh_sysfs_add_device(pdev);
>>+}
>>+
>>  /**
>>   * eeh_powernv_init - Register platform dependent EEH operations
>>   *
>>
>
>
>-- 
>Alexey

-- 
Richard Yang
Help you, Help me

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V10 08/12] powerpc/powernv: Support EEH reset for VF PE

2015-10-30 Thread Alexey Kardashevskiy 

On 10/30/2015 06:18 PM, Wei Yang wrote:

On Fri, Oct 30, 2015 at 03:11:20PM +1100, Alexey Kardashevskiy wrote:

On 10/26/2015 02:15 PM, Wei Yang wrote:

PEs for VFs don't have primary bus. So they have to have their own reset
backend, which is used during EEH recovery. The patch implements the reset
backend for VF's PE by issuing FLR or AF FLR to the VFs, which are contained
in the PE.

[gwshan: changelog and code refactoring]
Signed-off-by: Wei Yang 
Acked-by: Gavin Shan 
---
  arch/powerpc/include/asm/eeh.h   |   1 +
  arch/powerpc/platforms/powernv/eeh-powernv.c | 134 ++-
  2 files changed, 134 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index ec21f8f..331c856 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -136,6 +136,7 @@ struct eeh_dev {
int pcix_cap;   /* Saved PCIx capability*/
int pcie_cap;   /* Saved PCIe capability*/
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 pci_controller *phb; /* Associated PHB   */
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c 
b/arch/powerpc/platforms/powernv/eeh-powernv.c
index cfd55dd..017cd72 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -404,6 +404,7 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
edev->pcix_cap = pnv_eeh_find_cap(pdn, PCI_CAP_ID_PCIX);
edev->pcie_cap = pnv_eeh_find_cap(pdn, PCI_CAP_ID_EXP);
edev->aer_cap  = pnv_eeh_find_ecap(pdn, PCI_EXT_CAP_ID_ERR);
+   edev->af_cap   = pnv_eeh_find_cap(pdn, PCI_CAP_ID_AF);
if ((edev->class_code >> 8) == PCI_CLASS_BRIDGE_PCI) {
edev->mode |= EEH_DEV_BRIDGE;
if (edev->pcie_cap) {
@@ -893,6 +894,127 @@ static int pnv_eeh_bridge_reset(struct pci_dev *dev, int 
option)
return 0;
  }

+static void pnv_eeh_wait_for_pending(struct pci_dn *pdn, int pos,
+u16 mask, bool af_flr_rst)


Missed this - @af_flr_rst is only used for warnings so better do:
s/bool af_flr_rst/const char *reset_type/
to make it explicit.



+{
+   struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
+   int status, i;
+
+   /* Wait for Transaction Pending bit to be cleared */
+   for (i = 0; i < 4; i++) {
+   eeh_ops->read_config(pdn, pos, 2, );



gcc should have complained on using uninitialized @status here.



I remove the obj file and re-compile the file, not the warning.


Hm. Does not warn me either.


And took a look at other places where read_config() is called. The laster
parameter is not initialized before called.


So? It does not make it right.


You see the error during build?


Why does it matter? We have an undefined behavior here which we should not. 
You could test the return values from read_config() but you do not so at 
least initialize local variables.








+   if (!(status & mask))
+   return;
+
+   msleep((1 << i) * 100);
+   }
+
+   pr_warn("%s: Pending transaction while issuing %s FLR to "
+   "%04x:%02x:%02x.%01x\n",


Do not wrap user-visible strings.



Will change this.




+   __func__, af_flr_rst ? "AF" : "",
+   edev->phb->global_number, pdn->busno,
+   PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
+}
+
+static int pnv_eeh_do_flr(struct pci_dn *pdn, int option)
+{
+   struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
+   u32 reg;
+
+   if (!edev->pcie_cap)
+   return -ENOTTY;



Can pnv_eeh_do_flr() be really called on a non PCIe device, can we get that
far? WARN_ON_ONCE() may be?



So you suggest to add a WARN_ON_ONCE() in this condition, right?


I am asking a question here whether it makes sense or not to add a 
WARN_ON_ONCE or replace "if" with WARN_ON_ONCE or not having pcie_cap 
initialized is possible in this code - which one is it?








+
+   eeh_ops->read_config(pdn, edev->pcie_cap + PCI_EXP_DEVCAP, 4, );



... and here about uninitialized @reg.



+   if (!(reg & PCI_EXP_DEVCAP_FLR))
+   return -ENOTTY;
+
+   switch (option) {
+   case EEH_RESET_HOT:
+   case EEH_RESET_FUNDAMENTAL:
+   pnv_eeh_wait_for_pending(pdn, edev->pcie_cap + PCI_EXP_DEVSTA,
+PCI_EXP_DEVSTA_TRPND, false);
+   eeh_ops->read_config(pdn, edev->pcie_cap + PCI_EXP_DEVCTL,
+4, );
+   reg |= PCI_EXP_DEVCTL_BCR_FLR;
+

Re: [PATCH V10 09/12] powerpc/powernv: Support PCI config restore for VFs

2015-10-30 Thread Wei Yang 
On Fri, Oct 30, 2015 at 03:56:12PM +1100, Alexey Kardashevskiy wrote:
>On 10/26/2015 02:15 PM, Wei Yang wrote:
>>After PE reset, OPAL API opal_pci_reinit() is called on all devices
>>contained in the PE to reinitialize them. However, VFs can't be seen
>>from skiboot firmware. We have to implement the functions, similar
>>those in skiboot firmware, to reinitialize VFs after reset on PE
>>for VFs.
>>
>>[gwshan: changelog and code refactoring]
>>Signed-off-by: Wei Yang 
>>Acked-by: Gavin Shan 
>>---
>>  arch/powerpc/include/asm/pci-bridge.h|  1 +
>>  arch/powerpc/platforms/powernv/eeh-powernv.c | 70 
>> +++-
>>  arch/powerpc/platforms/powernv/pci.c | 18 +++
>>  3 files changed, 88 insertions(+), 1 deletion(-)
>>
>>diff --git a/arch/powerpc/include/asm/pci-bridge.h 
>>b/arch/powerpc/include/asm/pci-bridge.h
>>index 3d7e537..e499d93 100644
>>--- a/arch/powerpc/include/asm/pci-bridge.h
>>+++ b/arch/powerpc/include/asm/pci-bridge.h
>>@@ -219,6 +219,7 @@ struct pci_dn {
>>  #define IODA_INVALID_M64(-1)
>>  int (*m64_map)[PCI_SRIOV_NUM_BARS];
>>  #endif /* CONFIG_PCI_IOV */
>>+ int mps;
>
>int mps; /* maximum payload size */
>?

You are right. Will add this comment in code.

>
>
>>  #endif
>>  struct list_head child_list;
>>  struct list_head list;
>>diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c 
>>b/arch/powerpc/platforms/powernv/eeh-powernv.c
>>index 017cd72..3cc3e76 100644
>>--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
>>+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
>>@@ -1616,6 +1616,67 @@ static int pnv_eeh_next_error(struct eeh_pe **pe)
>>  return ret;
>>  }
>>
>>+static int pnv_eeh_restore_vf_config(struct pci_dn *pdn)
>
>It does not exactly restore it, it just tweaks few bytes.
>
>
>>+{
>>+ struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
>>+ u32 devctl, cmd, cap2, aer_capctl;
>>+ int old_mps;
>>+
>>+ /* Restore MPS */
>>+ if (edev->pcie_cap) {
>>+ old_mps = (ffs(pdn->mps) - 8) << 5;
>>+ eeh_ops->read_config(pdn, edev->pcie_cap + PCI_EXP_DEVCTL,
>>+  2, );
>>+ devctl &= ~PCI_EXP_DEVCTL_PAYLOAD;
>>+ devctl |= old_mps;
>>+ eeh_ops->write_config(pdn, edev->pcie_cap + PCI_EXP_DEVCTL,
>>+   2, devctl);
>>+ }
>>+
>>+ /* Disable Completion Timeout */
>>+ if (edev->pcie_cap) {
>>+ eeh_ops->read_config(pdn, edev->pcie_cap + PCI_EXP_DEVCAP2,
>>+  4, );
>>+ if (cap2 & 0x10) {
>>+ eeh_ops->read_config(pdn,
>>+ edev->pcie_cap + PCI_EXP_DEVCTL2,
>>+ 4, );
>>+ cap2 |= 0x10;
>>+ eeh_ops->write_config(pdn,
>>+ edev->pcie_cap + PCI_EXP_DEVCTL2,
>>+ 4, cap2);
>>+ }
>>+ }
>>+
>>+ /* Enable SERR and parity checking */
>>+ eeh_ops->read_config(pdn, PCI_COMMAND, 2, );
>
>
>No complains from gcc about uninitialized @cmd and others? Cl...
>

No...

>
>>+ cmd |= (PCI_COMMAND_PARITY | PCI_COMMAND_SERR);
>>+ eeh_ops->write_config(pdn, PCI_COMMAND, 2, cmd);
>>+
>>+ /* Enable report various errors */
>>+ if (edev->pcie_cap) {
>>+ eeh_ops->read_config(pdn, edev->pcie_cap + PCI_EXP_DEVCTL,
>>+ 2, );
>>+ devctl &= ~PCI_EXP_DEVCTL_CERE;
>>+ devctl |= (PCI_EXP_DEVCTL_NFERE |
>>+PCI_EXP_DEVCTL_FERE |
>>+PCI_EXP_DEVCTL_URRE);
>>+ eeh_ops->write_config(pdn, edev->pcie_cap + PCI_EXP_DEVCTL,
>>+ 2, devctl);
>>+ }
>>+
>>+ /* Enable ECRC generation and check */
>>+ if (edev->pcie_cap && edev->aer_cap) {
>>+ eeh_ops->read_config(pdn, edev->aer_cap + PCI_ERR_CAP,
>>+ 4, _capctl);
>>+ aer_capctl |= (PCI_ERR_CAP_ECRC_GENE | PCI_ERR_CAP_ECRC_CHKE);
>>+ eeh_ops->write_config(pdn, edev->aer_cap + PCI_ERR_CAP,
>>+ 4, aer_capctl);
>>+ }
>>+
>>+ return 0;
>>+}
>>+
>>  static int pnv_eeh_restore_config(struct pci_dn *pdn)
>>  {
>>  struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
>>@@ -1626,7 +1687,14 @@ static int pnv_eeh_restore_config(struct pci_dn *pdn)
>>  return -EEXIST;
>>
>>  phb = edev->phb->private_data;
>>- ret = opal_pci_reinit(phb->opal_id,
>>+ /*
>>+  * We have to restore the PCI config space after reset since the
>>+  * firmware can't see SRIOV VFs.
>
>
>When I see "restore config space", pci_restore_state() comes to my mind...
>What you do is rather "fixup" but for some reason you do not call this from
>pnv_pci_fixup_vf_mps (which could be more

Re: [PATCH V10 05/12] powerpc/eeh: Cache only BARs, not windows or IOV BARs

2015-10-30 Thread Wei Yang 
On Fri, Oct 30, 2015 at 02:22:43PM +1100, Alexey Kardashevskiy wrote:
>On 10/26/2015 02:15 PM, Wei Yang wrote:
>>EEH address cache, which helps to locate the PCI device according to
>>the given (physical) MMIO address, didn't cover PCI bridges. Also, it
>>shouldn't return PF
>
>"it shouldn't return" is about the cache, right? eeh_addr_cache_get_dev() -
>this guy can "return", the cache cannot.
>

Here I want to say if we cache the PF's IOV BAR, eeh_addr_cache_get_dev()
would return PF when the address is for VF.

>>with address in PF's IOV BARs. Instead, the VFs
>>should be returned.
>>
>>Also, by doing so, it removes the type check in
>>eeh_addr_cache_insert_dev(), since bridge's window would not be cached.
>>
>>The patch restricts the address cache to cover first 7 BARs for the
>>above purposes.
>
>
>I'd better understand something like this :)
>
>This restricts the EEH address cache to use only first 7 BARs. This makes
>__eeh_addr_cache_insert_dev() ignore PCI bridge windows and IOV BARs. As the
>result of this change, eeh_addr_cache_get_dev() will return VFs from VF's
>resource addresses instead of parent PFs.
>
>This removes extra check for a PCI bridge as we limit
>__eeh_addr_cache_insert_dev() to 7 BARs and this effectively excludes PCI
>bridges from being cached.
>

Yep, I think this one is more clear. Would use this one.

>
>>
>>[gwshan: changelog]
>>Signed-off-by: Wei Yang 
>>Acked-by: Gavin Shan 
>>---
>>  arch/powerpc/kernel/eeh_cache.c | 6 +-
>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>
>>diff --git a/arch/powerpc/kernel/eeh_cache.c b/arch/powerpc/kernel/eeh_cache.c
>>index a1e86e1..e6887f0 100644
>>--- a/arch/powerpc/kernel/eeh_cache.c
>>+++ b/arch/powerpc/kernel/eeh_cache.c
>>@@ -196,7 +196,7 @@ static void __eeh_addr_cache_insert_dev(struct pci_dev 
>>*dev)
>>  }
>>
>>  /* Walk resources on this device, poke them into the tree */
>>- for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
>>+ for (i = 0; i <= PCI_ROM_RESOURCE; i++) {
>>  resource_size_t start = pci_resource_start(dev,i);
>>  resource_size_t end = pci_resource_end(dev,i);
>>  unsigned long flags = pci_resource_flags(dev,i);
>>@@ -222,10 +222,6 @@ void eeh_addr_cache_insert_dev(struct pci_dev *dev)
>>  {
>>  unsigned long flags;
>>
>>- /* Ignore PCI bridges */
>>- if ((dev->class >> 16) == PCI_BASE_CLASS_BRIDGE)
>>- return;
>>-
>>  spin_lock_irqsave(_io_addr_cache_root.piar_lock, flags);
>>  __eeh_addr_cache_insert_dev(dev);
>>  spin_unlock_irqrestore(_io_addr_cache_root.piar_lock, flags);
>>
>
>
>-- 
>Alexey

-- 
Richard Yang
Help you, Help me

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 1/1 v3] drivers/nvme: default to 4k device page size

2015-10-30 Thread Nishanth Aravamudan 
On 29.10.2015 [17:20:43 +], Busch, Keith wrote:
> On Thu, Oct 29, 2015 at 08:57:01AM -0700, Nishanth Aravamudan wrote:
> > On 29.10.2015 [04:55:36 -0700], Christoph Hellwig wrote:
> > > We had a quick cht about this issue and I think we simply should
> > > default to a NVMe controler page size of 4k everywhere as that's the
> > > safe default.  This is also what we do for RDMA Memory reigstrations and
> > > it works fine there for SRP and iSER.
> > 
> > So, would that imply changing just the NVMe driver code rather than
> > adding the dma_page_shift API at all? What about
> > architectures that can support the larger page sizes? There is an
> > implied performance impact, at least, of shifting the IO size down.
> 
> It is the safe option, but you're right that it might have a
> measurable performance impact (can you run an experiment?). Maybe we
> should just change the driver to always use MPSMIN for the moment in
> the interest of time, and you can flush out the new API before the
> next merge window.

Given that it's 4K just about everywhere by default (and sort of
implicitly expected to be, I guess), I think I'd prefer we default to
4K. That should mitigate the performance impact (I'll ask our IO team to
do some runs, but since this impacts functionality on some hardware, I
don't think it's too relevant for now). Unless there are NVMe devcies
with a MPSMAX < 4K? 

Something like the following?



We received a bug report recently when DDW (64-bit direct DMA on Power)
is not enabled for NVMe devices. In that case, we fall back to 32-bit
DMA via the IOMMU, which is always done via 4K TCEs (Translation Control
Entries).

The NVMe device driver, though, assumes that the DMA alignment for the
PRP entries will match the device's page size, and that the DMA aligment
matches the kernel's page aligment. On Power, the the IOMMU page size,
as mentioned above, can be 4K, while the device can have a page size of
8K, while the kernel has a page size of 64K. This eventually trips the
BUG_ON in nvme_setup_prps(), as we have a 'dma_len' that is a multiple
of 4K but not 8K (e.g., 0xF000).

In this particular case of page sizes, we clearly want to use the
IOMMU's page size in the driver. And generally, the NVMe driver in this
function should be using the IOMMU's page size for the default device
page size, rather than the kernel's page size. There is not currently an
API to obtain the IOMMU's page size across all architectures and in the
interest of a stop-gap fix to this functional issue, default the NVMe
device page size to 4K, with the intent of adding such an API and
implementation across all architectures in the next merge window.

Signed-off-by: Nishanth Aravamudan 

---
v1 -> v2:
  Based upon feedback from Christoph Hellwig, implement the IOMMU page
  size lookup as a generic DMA API, rather than an architecture-specific
  hack.

v2 -> v3:
  In the interest of fixing the functional problem in the short-term,
  just force the device page size to 4K and work on adding the new API
  in the next merge window.

diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index ccc0c1f93daa..a9a5285bdb39 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -1717,7 +1717,12 @@ static int nvme_configure_admin_queue(struct nvme_dev 
*dev)
u32 aqa;
u64 cap = readq(>bar->cap);
struct nvme_queue *nvmeq;
-   unsigned page_shift = PAGE_SHIFT;
+   /*
+* default to a 4K page size, with the intention to update this
+* path in the future to accomodate architectures with differing
+* kernel and IO page sizes.
+*/
+   unsigned page_shift = 12;
unsigned dev_page_min = NVME_CAP_MPSMIN(cap) + 12;
unsigned dev_page_max = NVME_CAP_MPSMAX(cap) + 12;
 




-Nish

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 1/1 v3] drivers/nvme: default to 4k device page size

2015-10-30 Thread Nishanth Aravamudan 
On 30.10.2015 [21:48:48 +], Keith Busch wrote:
> On Fri, Oct 30, 2015 at 02:35:11PM -0700, Nishanth Aravamudan wrote:
> > Given that it's 4K just about everywhere by default (and sort of
> > implicitly expected to be, I guess), I think I'd prefer we default to
> > 4K. That should mitigate the performance impact (I'll ask our IO team to
> > do some runs, but since this impacts functionality on some hardware, I
> > don't think it's too relevant for now). Unless there are NVMe devcies
> > with a MPSMAX < 4K? 
> 
> Right, I assumed MPSMIN was always 4k for the same reason you mentioned,
> but you can hard code it like you've done in your patch.
> 
> The spec defines MPSMAX such that it's impossible to find a device
> with MPSMAX < 4k.

Great, thanks!

-Nish

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] Endian swap ->count before handing over to the nx-842 compressor

2015-10-30 Thread Ram Pai 
The nx-842 compressor overshoots the output buffer corrupting memory. Verified
that the following patch the issue on a LE system.

Signed-off-by: Ram Pai 
---
 drivers/crypto/nx/nx-842-powernv.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/crypto/nx/nx-842-powernv.c 
b/drivers/crypto/nx/nx-842-powernv.c
index 3750e13..3b80ea7 100644
--- a/drivers/crypto/nx/nx-842-powernv.c
+++ b/drivers/crypto/nx/nx-842-powernv.c
@@ -66,7 +66,7 @@ static void setup_indirect_dde(struct data_descriptor_entry 
*dde,
   unsigned int dde_count, unsigned int byte_count)
 {
dde->flags = 0;
-   dde->count = dde_count;
+   dde->count = cpu_to_be32(dde_count);
dde->index = 0;
dde->length = cpu_to_be32(byte_count);
dde->address = cpu_to_be64(nx842_get_pa(ddl));
-- 
1.7.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 0/5 v3] Fix NVMe driver support on Power with 32-bit DMA

2015-10-30 Thread Nishanth Aravamudan 
On 29.10.2015 [18:49:55 -0700], David Miller wrote:
> From: Nishanth Aravamudan 
> Date: Thu, 29 Oct 2015 08:57:01 -0700
> 
> > So, would that imply changing just the NVMe driver code rather than
> > adding the dma_page_shift API at all? What about
> > architectures that can support the larger page sizes? There is an
> > implied performance impact, at least, of shifting the IO size down.
> > 
> > Sorry for the continuing questions -- I got lots of conflicting feedback
> > on the last series and want to make sure v4 is more acceptable.
> 
> In the long term I would be very happy to see us having a real interface
> for this stuff, just my opinion...

Yep, I think I'll try and balance the two -- fix NVMe for now with a 4K
page size as suggested by Christoph, and then work on the more complete
API for the next merge.

Thanks,
Nish

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 1/1 v3] drivers/nvme: default to 4k device page size

2015-10-30 Thread Keith Busch 
On Fri, Oct 30, 2015 at 02:35:11PM -0700, Nishanth Aravamudan wrote:
> Given that it's 4K just about everywhere by default (and sort of
> implicitly expected to be, I guess), I think I'd prefer we default to
> 4K. That should mitigate the performance impact (I'll ask our IO team to
> do some runs, but since this impacts functionality on some hardware, I
> don't think it's too relevant for now). Unless there are NVMe devcies
> with a MPSMAX < 4K? 

Right, I assumed MPSMIN was always 4k for the same reason you mentioned,
but you can hard code it like you've done in your patch.

The spec defines MPSMAX such that it's impossible to find a device
with MPSMAX < 4k.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 1/2] mpc85xx/lbc: modify suspend/resume entry sequence

2015-10-30 Thread Raghav Dogra 
Modify platform driver suspend/resume to syscore
suspend/resume. This is because p1022ds needs to use
localbus when entering the PCIE resume.

Signed-off-by: Raghav Dogra 
---
 arch/powerpc/sysdev/Makefile  |  2 +-
 arch/powerpc/sysdev/fsl_lbc.c | 51 +--
 2 files changed, 40 insertions(+), 13 deletions(-)

diff --git a/arch/powerpc/sysdev/Makefile b/arch/powerpc/sysdev/Makefile
index f7cb2a1..4c19e614 100644
--- a/arch/powerpc/sysdev/Makefile
+++ b/arch/powerpc/sysdev/Makefile
@@ -18,9 +18,9 @@ obj-$(CONFIG_PPC_PMI) += pmi.o
 obj-$(CONFIG_U3_DART)  += dart_iommu.o
 obj-$(CONFIG_MMIO_NVRAM)   += mmio_nvram.o
 obj-$(CONFIG_FSL_SOC)  += fsl_soc.o fsl_mpic_err.o
+obj-$(CONFIG_FSL_LBC)  += fsl_lbc.o
 obj-$(CONFIG_FSL_PCI)  += fsl_pci.o $(fsl-msi-obj-y)
 obj-$(CONFIG_FSL_PMC)  += fsl_pmc.o
-obj-$(CONFIG_FSL_LBC)  += fsl_lbc.o
 obj-$(CONFIG_FSL_GTM)  += fsl_gtm.o
 obj-$(CONFIG_FSL_85XX_CACHE_SRAM)  += fsl_85xx_l2ctlr.o 
fsl_85xx_cache_sram.o
 obj-$(CONFIG_SIMPLE_GPIO)  += simple_gpio.o
diff --git a/arch/powerpc/sysdev/fsl_lbc.c b/arch/powerpc/sysdev/fsl_lbc.c
index d631022..332d700 100644
--- a/arch/powerpc/sysdev/fsl_lbc.c
+++ b/arch/powerpc/sysdev/fsl_lbc.c
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -354,24 +355,42 @@ err:
 #ifdef CONFIG_SUSPEND
 
 /* save lbc registers */
-static int fsl_lbc_suspend(struct platform_device *pdev, pm_message_t state)
+static int fsl_lbc_syscore_suspend(void)
 {
-   struct fsl_lbc_ctrl *ctrl = dev_get_drvdata(>dev);
-   struct fsl_lbc_regs __iomem *lbc = ctrl->regs;
+   struct fsl_lbc_ctrl *ctrl;
+   struct fsl_lbc_regs __iomem *lbc;
+
+   ctrl = fsl_lbc_ctrl_dev;
+   if (!ctrl)
+   goto out;
+
+   lbc = ctrl->regs;
+   if (!lbc)
+   goto out;
 
ctrl->saved_regs = kmalloc(sizeof(struct fsl_lbc_regs), GFP_KERNEL);
if (!ctrl->saved_regs)
return -ENOMEM;
 
_memcpy_fromio(ctrl->saved_regs, lbc, sizeof(struct fsl_lbc_regs));
+
+out:
return 0;
 }
 
 /* restore lbc registers */
-static int fsl_lbc_resume(struct platform_device *pdev)
+static int fsl_lbc_syscore_resume(void)
 {
-   struct fsl_lbc_ctrl *ctrl = dev_get_drvdata(>dev);
-   struct fsl_lbc_regs __iomem *lbc = ctrl->regs;
+   struct fsl_lbc_ctrl *ctrl;
+   struct fsl_lbc_regs __iomem *lbc;
+
+   ctrl = fsl_lbc_ctrl_dev;
+   if (!ctrl)
+   goto out;
+
+   lbc = ctrl->regs;
+   if (!lbc)
+   goto out;
 
if (ctrl->saved_regs) {
_memcpy_toio(lbc, ctrl->saved_regs,
@@ -379,7 +398,9 @@ static int fsl_lbc_resume(struct platform_device *pdev)
kfree(ctrl->saved_regs);
ctrl->saved_regs = NULL;
}
-   return 0;
+
+out:
+   return;
 }
 #endif /* CONFIG_SUSPEND */
 
@@ -391,20 +412,26 @@ static const struct of_device_id fsl_lbc_match[] = {
{},
 };
 
+#ifdef CONFIG_SUSPEND
+static struct syscore_ops lbc_syscore_pm_ops = {
+   .suspend = fsl_lbc_syscore_suspend,
+   .resume = fsl_lbc_syscore_resume,
+};
+#endif
+
 static struct platform_driver fsl_lbc_ctrl_driver = {
.driver = {
.name = "fsl-lbc",
.of_match_table = fsl_lbc_match,
},
.probe = fsl_lbc_ctrl_probe,
-#ifdef CONFIG_SUSPEND
-   .suspend = fsl_lbc_suspend,
-   .resume  = fsl_lbc_resume,
-#endif
 };
 
 static int __init fsl_lbc_init(void)
 {
+#ifdef CONFIG_SUSPEND
+   register_syscore_ops(_syscore_pm_ops);
+#endif
return platform_driver_register(_lbc_ctrl_driver);
 }
-module_init(fsl_lbc_init);
+arch_initcall(fsl_lbc_init);
-- 
1.9.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 2/2] powerpc/fsl_lbc: removal of dead code

2015-10-30 Thread Raghav Dogra 
The condition check is not used.

Signed-off-by: Raghav Dogra 
---
 arch/powerpc/sysdev/fsl_lbc.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/powerpc/sysdev/fsl_lbc.c b/arch/powerpc/sysdev/fsl_lbc.c
index 332d700..a41d281 100644
--- a/arch/powerpc/sysdev/fsl_lbc.c
+++ b/arch/powerpc/sysdev/fsl_lbc.c
@@ -244,8 +244,6 @@ static irqreturn_t fsl_lbc_ctrl_irq(int irqno, void *data)
if (status & LTESR_CS)
dev_err(ctrl->dev, "Chip select error: "
"LTESR 0x%08X\n", status);
-   if (status & LTESR_UPM)
-   ;
if (status & LTESR_FCT) {
dev_err(ctrl->dev, "FCM command time-out: "
"LTESR 0x%08X\n", status);
-- 
1.9.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] cxl: sparse: add __iomem annotations in vphb.c

2015-10-30 Thread Arnd Bergmann 
On Wednesday 28 October 2015 14:29:39 Andrew Donnellan wrote:
> --- a/drivers/misc/cxl/vphb.c
> +++ b/drivers/misc/cxl/vphb.c
> @@ -128,7 +128,7 @@ static int cxl_pcie_config_info(struct pci_bus *bus, 
> unsigned int devfn,
> return PCIBIOS_BAD_REGISTER_NUMBER;
> addr = cxl_pcie_cfg_addr(phb, bus->number, devfn, offset);
>  
> -   *ioaddr = (void *)(addr & ~0x3ULL);
> +   *ioaddr = (void __iomem *)(addr & ~0x3ULL);
> *shift = ((addr & 0x3) * 8);
> switch (len) {
> case 1:

It would be nice to change the return value of cxl_pcie_cfg_addr to
'void __iomem *' as well, and only do the cast (back and forth) inside 
the calculation, to make it clear that the input type is the same as the
output. Perhaps use a static inline function to wrap it.

> @@ -249,7 +249,7 @@ int cxl_pci_vphb_add(struct cxl_afu *afu)
> /* Setup the PHB using arch provided callback */
> phb->ops = _pcie_pci_ops;
> phb->cfg_addr = afu->afu_desc_mmio + afu->crs_offset;
> -   phb->cfg_data = (void *)(u64)afu->crs_len;
> +   phb->cfg_data = (void __iomem *)afu->crs_len;
> phb->private_data = afu;
> phb->controller_ops = cxl_pci_controller_ops;


This needs a comment to explain why this is correct. I've tried to find it
out by reading the code and could not find any explanation. Also, you
need to cast to an intermediate (uintptr_t) type, as directly converting
a u64 to a pointer of any sort is nonportable, and it would be good to
at least allow compile-testing this code on other architectures.

I suspect that 'phb->cfg_data' is used in this driver in a way that is
incompatible with the other uses of the same variable. Maybe you can
replace it with something like

union {
void __iomem *cfg_data;
u64 cxl_cfg_offset;
};

to make it clear that in this driver it is used as an offset rather than
a pointer.

Arnd
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] nx-842: Ignore bit 3 of condition register returned by icswx

2015-10-30 Thread Ram Pai 
icswx occasionally under heavy load sets bit 3 of condition register 0.
It has no software implication.

Currently that bit is interpreted by the driver as a failure, when
it should have calmly ignored it.

Signed-off-by: Ram Pai 
---
 arch/powerpc/include/asm/icswx.h |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/include/asm/icswx.h b/arch/powerpc/include/asm/icswx.h
index 9f8402b..bce20c7 100644
--- a/arch/powerpc/include/asm/icswx.h
+++ b/arch/powerpc/include/asm/icswx.h
@@ -177,7 +177,7 @@ static inline int icswx(__be32 ccw, struct 
coprocessor_request_block *crb)
: "r" (ccw_reg), "r" (crb)
: "cr0", "memory");
 
-   return (int)((cr >> 28) & 0xf);
+   return (int)((cr >> 28) & 0xe);
 }

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [ANNOUNCE] kexec-tools 2.0.11-rc1

2015-10-30 Thread Scott Wood 
On Fri, 2015-10-30 at 17:12 +0900, Simon Horman wrote:
> On Thu, Oct 29, 2015 at 07:23:24PM -0500, Scott Wood wrote:
> > On Fri, 2015-10-30 at 08:48 +0900, Simon Horman wrote:
> > > Hi all,
> > > 
> > > I am happy to announce the release of kexec-tools 2.0.11-rc1.
> > > 
> > > This is an incremental feature pre-release.
> > > 
> > > So long as no serious problems arise I intend to release kexec-tools 
> > > 2.0.11
> > > shortly after the release of the v4.3 kernel, which I expect to occur in
> > > the next week or so.  As such testing of 2.0.11-rc1 would be greatly
> > > appreciated.
> > > 
> > > I do not have any outstanding changes for 2.0.11 at this time.
> > > And I would like to only accept bug fixes at this time and take take
> > > features patches once 2.0.11 has been released.
> > 
> > As I previously reported, "ppc64: purgatory: Reset primary cpu endian to 
> > big-
> > endian" breaks book3e, so can  http://patchwork.ozlabs.org/patch/527048/be 
> > considered for this release?
> 
> Yes of course, sorry for not taking more note of that.
> 
> Would it be possible to get a review, e.g. from Michael?

I pinged Michael again on the original patch, and CCed him and Samuel and the 
PPC list here.  If they still don't respond, and you still don't want to put 
the fix in without their input, then could you revert the broken patch?

-Scott

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 1/2] mpc85xx/lbc: modify suspend/resume entry sequence

2015-10-30 Thread Scott Wood 
On Fri, 2015-10-30 at 11:54 +0530, Raghav Dogra wrote:
> Modify platform driver suspend/resume to syscore
> suspend/resume. This is because p1022ds needs to use
> localbus when entering the PCIE resume.
> 
> Signed-off-by: Raghav Dogra 
> ---
>  arch/powerpc/sysdev/Makefile  |  2 +-
>  arch/powerpc/sysdev/fsl_lbc.c | 51 +---
> ---
>  2 files changed, 40 insertions(+), 13 deletions(-)

Same comments as on the IFC patch.

-Scott

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev