LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/2] powerpc/83xx: map IMMR with a BAT.
From: Christophe Leroy @ 2019-08-23 12:50 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, oss,
	galak
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <b51b96090138aba1920d2cf7c0e0e348667f9a69.1566564560.git.christophe.leroy@c-s.fr>

On mpc83xx with a QE, IMMR is 2Mbytes.
On mpc83xx without a QE, IMMR is 1Mbytes.
Each driver will map a part of it to access the registers it needs.
Some driver will map the same part of IMMR as other drivers.

In order to reduce TLB misses, map the full IMMR with a BAT.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/platforms/83xx/misc.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/powerpc/platforms/83xx/misc.c b/arch/powerpc/platforms/83xx/misc.c
index f46d7bf3b140..1e395b01c535 100644
--- a/arch/powerpc/platforms/83xx/misc.c
+++ b/arch/powerpc/platforms/83xx/misc.c
@@ -18,6 +18,8 @@
 #include <sysdev/fsl_soc.h>
 #include <sysdev/fsl_pci.h>
 
+#include <mm/mmu_decl.h>
+
 #include "mpc83xx.h"
 
 static __be32 __iomem *restart_reg_base;
@@ -145,6 +147,14 @@ void __init mpc83xx_setup_arch(void)
 	if (ppc_md.progress)
 		ppc_md.progress("mpc83xx_setup_arch()", 0);
 
+	if (!__map_without_bats) {
+		int immrsize = IS_ENABLED(CONFIG_QUICC_ENGINE) ? SZ_2M : SZ_1M;
+
+		ioremap_bot = ALIGN_DOWN(ioremap_bot - immrsize, immrsize);
+		setbat(-1, ioremap_bot, get_immrbase(), immrsize, PAGE_KERNEL_NCG);
+		update_bats();
+	}
+
 	mpc83xx_setup_pci();
 }
 
-- 
2.13.3


^ permalink raw reply related

* [PATCH 1/2] powerpc/32s: automatically allocate BAT in setbat()
From: Christophe Leroy @ 2019-08-23 12:50 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, oss,
	galak
  Cc: linuxppc-dev, linux-kernel

If no BAT is given to setbat(), select an available BAT.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/mm/book3s32/mmu.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/mm/book3s32/mmu.c b/arch/powerpc/mm/book3s32/mmu.c
index 50a1991d257f..5f08b93ecffd 100644
--- a/arch/powerpc/mm/book3s32/mmu.c
+++ b/arch/powerpc/mm/book3s32/mmu.c
@@ -251,9 +251,18 @@ void __init setbat(int index, unsigned long virt, phys_addr_t phys,
 {
 	unsigned int bl;
 	int wimgxpp;
-	struct ppc_bat *bat = BATS[index];
+	struct ppc_bat *bat;
 	unsigned long flags = pgprot_val(prot);
 
+	if (index == -1)
+		index = find_free_bat();
+	if (index == -1) {
+		pr_err("%s: no BAT available for mapping 0x%llx\n", __func__,
+		       (unsigned long long)phys);
+		return;
+	}
+	bat = BATS[index];
+
 	if ((flags & _PAGE_NO_CACHE) ||
 	    (cpu_has_feature(CPU_FTR_NEED_COHERENT) == 0))
 		flags &= ~_PAGE_COHERENT;
-- 
2.13.3


^ permalink raw reply related

* Re: [PATCH v2 01/10] PCI: designware-ep: Add multiple PFs support for DWC
From: Andrew Murray @ 2019-08-23 13:25 UTC (permalink / raw)
  To: Xiaowei Bao
  Cc: mark.rutland, roy.zang, lorenzo.pieralisi, arnd, devicetree,
	gregkh, linuxppc-dev, linux-pci, linux-kernel, kishon,
	minghuan.Lian, robh+dt, gustavo.pimentel, jingoohan1, bhelgaas,
	leoyang.li, shawnguo, mingkai.hu, linux-arm-kernel
In-Reply-To: <20190822112242.16309-1-xiaowei.bao@nxp.com>

On Thu, Aug 22, 2019 at 07:22:33PM +0800, Xiaowei Bao wrote:
> Add multiple PFs support for DWC, different PF have different config space
> we use pf-offset property which get from the DTS to access the different pF
> config space.

It looks like you're missing a --cover-letter again.

> 
> Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> ---
> v2:
>  - Remove duplicate redundant code.
>  - Reimplement the PF config space access way.
> 
>  drivers/pci/controller/dwc/pcie-designware-ep.c | 122 ++++++++++++++++--------
>  drivers/pci/controller/dwc/pcie-designware.c    |  59 ++++++++----
>  drivers/pci/controller/dwc/pcie-designware.h    |  11 ++-
>  3 files changed, 134 insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 2bf5a35..3e2b740 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -19,12 +19,17 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
>  	pci_epc_linkup(epc);
>  }
>  
> -static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar,
> -				   int flags)
> +static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, u8 func_no,
> +				   enum pci_barno bar, int flags)
>  {
>  	u32 reg;
> +	unsigned int func_offset = 0;
> +	struct dw_pcie_ep *ep = &pci->ep;
>  
> -	reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> +	if (ep->ops->func_conf_select)
> +		func_offset = ep->ops->func_conf_select(ep, func_no);
> +
> +	reg = func_offset + PCI_BASE_ADDRESS_0 + (4 * bar);

This pattern of checking if func_conf_select exists and using it to get an
offset is repeated a lot throughout this file. You could move this
functionality into a new function (similar to dw_pcie_read_dbi etc). Or
perhaps a new variant of dw_pcie_writel_ should be created that writes takes
a func_no argument.
 

>  	dw_pcie_dbi_ro_wr_en(pci);
>  	dw_pcie_writel_dbi2(pci, reg, 0x0);
>  	dw_pcie_writel_dbi(pci, reg, 0x0);


> @@ -235,7 +257,7 @@ static int dw_pcie_ep_map_addr(struct pci_epc *epc, u8 func_no,
>  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>  
> -	ret = dw_pcie_ep_outbound_atu(ep, addr, pci_addr, size);
> +	ret = dw_pcie_ep_outbound_atu(ep, func_no, addr, pci_addr, size);
>  	if (ret) {
>  		dev_err(pci->dev, "Failed to enable address\n");
>  		return ret;
> @@ -249,11 +271,15 @@ static int dw_pcie_ep_get_msi(struct pci_epc *epc, u8 func_no)
>  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>  	u32 val, reg;
> +	unsigned int func_offset = 0;
> +
> +	if (ep->ops->func_conf_select)
> +		func_offset = ep->ops->func_conf_select(ep, func_no);
>  
>  	if (!ep->msi_cap)
>  		return -EINVAL;
>  
> -	reg = ep->msi_cap + PCI_MSI_FLAGS;
> +	reg = ep->msi_cap + func_offset + PCI_MSI_FLAGS;

This makes me nervous.

From a PCI viewpoint, each function has it's own capability structure and
within each function there may exist a MSI capability. Yet what we're doing
here is using dw_pcie_ep_find_capability to get the list of capabilities for
function 0, and then applying offsets from that for subsequent functions. I.e.
we're applying DW specific knowledge to find the correct capability, rather
than following the general PCI approach.

I think the above hunk shouldn't be required - but instead
dw_pcie_ep_find_capability is updated to take a func_no parameter.

Have I understood this correctly?

>  	val = dw_pcie_readw_dbi(pci, reg);
>  	if (!(val & PCI_MSI_FLAGS_ENABLE))
>  		return -EINVAL;
> @@ -268,11 +294,15 @@ static int dw_pcie_ep_set_msi(struct pci_epc *epc, u8 func_no, u8 interrupts)
>  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>  	u32 val, reg;
> +	unsigned int func_offset = 0;
> +
> +	if (ep->ops->func_conf_select)
> +		func_offset = ep->ops->func_conf_select(ep, func_no);
>  
>  	if (!ep->msi_cap)
>  		return -EINVAL;
>  
> -	reg = ep->msi_cap + PCI_MSI_FLAGS;
> +	reg = ep->msi_cap + func_offset + PCI_MSI_FLAGS;
>  	val = dw_pcie_readw_dbi(pci, reg);
>  	val &= ~PCI_MSI_FLAGS_QMASK;
>  	val |= (interrupts << 1) & PCI_MSI_FLAGS_QMASK;
> @@ -288,11 +318,15 @@ static int dw_pcie_ep_get_msix(struct pci_epc *epc, u8 func_no)
>  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>  	u32 val, reg;
> +	unsigned int func_offset = 0;
> +
> +	if (ep->ops->func_conf_select)
> +		func_offset = ep->ops->func_conf_select(ep, func_no);
>  
>  	if (!ep->msix_cap)
>  		return -EINVAL;
>  
> -	reg = ep->msix_cap + PCI_MSIX_FLAGS;
> +	reg = ep->msix_cap + func_offset + PCI_MSIX_FLAGS;

Same for MSIX.

>  	val = dw_pcie_readw_dbi(pci, reg);
>  	if (!(val & PCI_MSIX_FLAGS_ENABLE))
>  		return -EINVAL;
> @@ -307,11 +341,15 @@ static int dw_pcie_ep_set_msix(struct pci_epc *epc, u8 func_no, u16 interrupts)
>  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>  	u32 val, reg;
> +	unsigned int func_offset = 0;
> +
> +	if (ep->ops->func_conf_select)
> +		func_offset = ep->ops->func_conf_select(ep, func_no);
>  
>  	if (!ep->msix_cap)
>  		return -EINVAL;
>  
> -	reg = ep->msix_cap + PCI_MSIX_FLAGS;
> +	reg = ep->msix_cap + func_offset + PCI_MSIX_FLAGS;
>  	val = dw_pcie_readw_dbi(pci, reg);
>  	val &= ~PCI_MSIX_FLAGS_QSIZE;
>  	val |= interrupts;
> @@ -398,29 +436,33 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>  	struct pci_epc *epc = ep->epc;
>  	unsigned int aligned_offset;
> +	unsigned int func_offset = 0;
>  	u16 msg_ctrl, msg_data;
>  	u32 msg_addr_lower, msg_addr_upper, reg;
>  	u64 msg_addr;
>  	bool has_upper;
>  	int ret;
>  
> +	if (ep->ops->func_conf_select)
> +		func_offset = ep->ops->func_conf_select(ep, func_no);
> +

You could probably move this hunk below the test for msi_cap to save some
cycles.

>  	if (!ep->msi_cap)
>  		return -EINVAL;
>  
>  	/* Raise MSI per the PCI Local Bus Specification Revision 3.0, 6.8.1. */
> -	reg = ep->msi_cap + PCI_MSI_FLAGS;
> +	reg = ep->msi_cap + func_offset + PCI_MSI_FLAGS;
>  	msg_ctrl = dw_pcie_readw_dbi(pci, reg);
>  	has_upper = !!(msg_ctrl & PCI_MSI_FLAGS_64BIT);
> -	reg = ep->msi_cap + PCI_MSI_ADDRESS_LO;
> +	reg = ep->msi_cap + func_offset + PCI_MSI_ADDRESS_LO;
>  	msg_addr_lower = dw_pcie_readl_dbi(pci, reg);
>  	if (has_upper) {
> -		reg = ep->msi_cap + PCI_MSI_ADDRESS_HI;
> +		reg = ep->msi_cap + func_offset + PCI_MSI_ADDRESS_HI;
>  		msg_addr_upper = dw_pcie_readl_dbi(pci, reg);
> -		reg = ep->msi_cap + PCI_MSI_DATA_64;
> +		reg = ep->msi_cap + func_offset + PCI_MSI_DATA_64;
>  		msg_data = dw_pcie_readw_dbi(pci, reg);
>  	} else {
>  		msg_addr_upper = 0;
> -		reg = ep->msi_cap + PCI_MSI_DATA_32;
> +		reg = ep->msi_cap + func_offset + PCI_MSI_DATA_32;
>  		msg_data = dw_pcie_readw_dbi(pci, reg);
>  	}
>  	aligned_offset = msg_addr_lower & (epc->mem->page_size - 1);



> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 7d25102..305e73d 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -158,9 +158,10 @@ static void dw_pcie_writel_ob_unroll(struct dw_pcie *pci, u32 index, u32 reg,
>  	dw_pcie_writel_atu(pci, offset + reg, val);
>  }
>  
> -static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, int index,
> -					     int type, u64 cpu_addr,
> -					     u64 pci_addr, u32 size)
> +static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
> +					     int index, int type,
> +					     u64 cpu_addr, u64 pci_addr,
> +					     u32 size)
>  {
>  	u32 retries, val;
>  
> @@ -175,7 +176,7 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, int index,
>  	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_TARGET,
>  				 upper_32_bits(pci_addr));
>  	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1,
> -				 type);
> +				 type | PCIE_ATU_FUNC_NUM(func_no));

Much better :)

>  	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL2,
>  				 PCIE_ATU_ENABLE);
>  
> @@ -194,8 +195,9 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, int index,
>  	dev_err(pci->dev, "Outbound iATU is not being enabled\n");
>  }
>  
> -void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
> -			       u64 cpu_addr, u64 pci_addr, u32 size)
> +static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
> +					int index, int type, u64 cpu_addr,
> +					u64 pci_addr, u32 size)
>  {
>  	u32 retries, val;
>  
> @@ -203,8 +205,8 @@ void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
>  		cpu_addr = pci->ops->cpu_addr_fixup(pci, cpu_addr);
>  
>  	if (pci->iatu_unroll_enabled) {
> -		dw_pcie_prog_outbound_atu_unroll(pci, index, type, cpu_addr,
> -						 pci_addr, size);
> +		dw_pcie_prog_outbound_atu_unroll(pci, func_no, index, type,
> +						 cpu_addr, pci_addr, size);
>  		return;
>  	}
>  


> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index ffed084..a0fdbf7 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -71,9 +71,11 @@
>  #define PCIE_ATU_TYPE_IO		0x2
>  #define PCIE_ATU_TYPE_CFG0		0x4
>  #define PCIE_ATU_TYPE_CFG1		0x5
> +#define PCIE_ATU_FUNC_NUM(pf)           (pf << 20)

"Macro argument 'pf' may be better as '(pf)' to avoid precedence issues"

>  #define PCIE_ATU_CR2			0x908
>  #define PCIE_ATU_ENABLE			BIT(31)
>  #define PCIE_ATU_BAR_MODE_ENABLE	BIT(30)
> +#define PCIE_ATU_FUNC_NUM_MATCH_EN      BIT(19)
>  #define PCIE_ATU_LOWER_BASE		0x90C
>  #define PCIE_ATU_UPPER_BASE		0x910
>  #define PCIE_ATU_LIMIT			0x914
> @@ -197,6 +199,7 @@ struct dw_pcie_ep_ops {
>  	int	(*raise_irq)(struct dw_pcie_ep *ep, u8 func_no,
>  			     enum pci_epc_irq_type type, u16 interrupt_num);
>  	const struct pci_epc_features* (*get_features)(struct dw_pcie_ep *ep);
> +	unsigned int (*func_conf_select)(struct dw_pcie_ep *ep, u8 func_no);

Given that this function will return an offset, I'm not sure the name you
have is suitable. Something like get_pf_offset or similar is more descriptive.

Thanks,

Andrew Murray

>  };
>  
>  struct dw_pcie_ep {
> @@ -265,8 +268,12 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci);
>  void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
>  			       int type, u64 cpu_addr, u64 pci_addr,
>  			       u32 size);
> -int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, int index, int bar,
> -			     u64 cpu_addr, enum dw_pcie_as_type as_type);
> +void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> +				  int type, u64 cpu_addr, u64 pci_addr,
> +				  u32 size);
> +int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> +			     int bar, u64 cpu_addr,
> +			     enum dw_pcie_as_type as_type);
>  void dw_pcie_disable_atu(struct dw_pcie *pci, int index,
>  			 enum dw_pcie_region_type type);
>  void dw_pcie_setup(struct dw_pcie *pci);
> -- 
> 2.9.5
> 

^ permalink raw reply

* Re: [PATCH v2 02/10] PCI: designware-ep: Add the doorbell mode of MSI-X in EP mode
From: Andrew Murray @ 2019-08-23 13:35 UTC (permalink / raw)
  To: Xiaowei Bao
  Cc: mark.rutland, roy.zang, lorenzo.pieralisi, arnd, devicetree,
	gregkh, linuxppc-dev, linux-pci, linux-kernel, kishon,
	minghuan.Lian, robh+dt, gustavo.pimentel, jingoohan1, bhelgaas,
	leoyang.li, shawnguo, mingkai.hu, linux-arm-kernel
In-Reply-To: <20190822112242.16309-2-xiaowei.bao@nxp.com>

On Thu, Aug 22, 2019 at 07:22:34PM +0800, Xiaowei Bao wrote:
> Add the doorbell mode of MSI-X in EP mode.
> 
> Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> ---
> v2:
>  - Remove the macro of no used.
> 
>  drivers/pci/controller/dwc/pcie-designware-ep.c | 14 ++++++++++++++
>  drivers/pci/controller/dwc/pcie-designware.h    | 12 ++++++++++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 3e2b740..b8388f8 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -480,6 +480,20 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
>  	return 0;
>  }
>  
> +int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8 func_no,
> +				       u16 interrupt_num)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +	u32 msg_data;
> +
> +	msg_data = (func_no << PCIE_MSIX_DOORBELL_PF_SHIFT) |
> +		   (interrupt_num - 1);
> +
> +	dw_pcie_writel_dbi(pci, PCIE_MSIX_DOORBELL, msg_data);
> +
> +	return 0;
> +}
> +
>  int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
>  			      u16 interrupt_num)
>  {
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index a0fdbf7..895a9ef 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -88,6 +88,9 @@
>  #define PCIE_MISC_CONTROL_1_OFF		0x8BC
>  #define PCIE_DBI_RO_WR_EN		BIT(0)
>  
> +#define PCIE_MSIX_DOORBELL		0x948
> +#define PCIE_MSIX_DOORBELL_PF_SHIFT	24
> +
>  /*
>   * iATU Unroll-specific register definitions
>   * From 4.80 core version the address translation will be made by unroll
> @@ -400,6 +403,8 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
>  			     u8 interrupt_num);
>  int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
>  			     u16 interrupt_num);
> +int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8 func_no,
> +				       u16 interrupt_num);
>  void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar);
>  #else
>  static inline void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
> @@ -432,6 +437,13 @@ static inline int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
>  	return 0;
>  }
>  
> +static inline int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep,
> +						     u8 func_no,
> +						     u16 interrupt_num)
> +{
> +	return 0;
> +}
> +

Looks OK to me.

Reviewed-by: Andrew Murray <andrew.murray@arm.com>

>  static inline void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
>  {
>  }
> -- 
> 2.9.5
> 

^ permalink raw reply

* Re: [PATCH v2 03/10] PCI: designware-ep: Move the function of getting MSI capability forward
From: Andrew Murray @ 2019-08-23 13:38 UTC (permalink / raw)
  To: Xiaowei Bao
  Cc: mark.rutland, roy.zang, lorenzo.pieralisi, arnd, devicetree,
	gregkh, linuxppc-dev, linux-pci, linux-kernel, kishon,
	minghuan.Lian, robh+dt, gustavo.pimentel, jingoohan1, bhelgaas,
	leoyang.li, shawnguo, mingkai.hu, linux-arm-kernel
In-Reply-To: <20190822112242.16309-3-xiaowei.bao@nxp.com>

On Thu, Aug 22, 2019 at 07:22:35PM +0800, Xiaowei Bao wrote:
> Move the function of getting MSI capability to the front of init
> function, because the init function of the EP platform driver will use
> the return value by the function of getting MSI capability.
> 
> Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>

Reviewed-by: Andrew Murray <andrew.murray@arm.com>

> ---
> v2:
>  - No change.
> 
>  drivers/pci/controller/dwc/pcie-designware-ep.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index b8388f8..0a6c199 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -656,6 +656,10 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>  	if (ret < 0)
>  		epc->max_functions = 1;
>  
> +	ep->msi_cap = dw_pcie_ep_find_capability(pci, PCI_CAP_ID_MSI);
> +
> +	ep->msix_cap = dw_pcie_ep_find_capability(pci, PCI_CAP_ID_MSIX);
> +
>  	if (ep->ops->ep_init)
>  		ep->ops->ep_init(ep);
>  
> @@ -672,9 +676,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>  		dev_err(dev, "Failed to reserve memory for MSI/MSI-X\n");
>  		return -ENOMEM;
>  	}
> -	ep->msi_cap = dw_pcie_ep_find_capability(pci, PCI_CAP_ID_MSI);
> -
> -	ep->msix_cap = dw_pcie_ep_find_capability(pci, PCI_CAP_ID_MSIX);
>  
>  	offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
>  	if (offset) {
> -- 
> 2.9.5
> 

^ permalink raw reply

* Re: [PATCH v2 05/10] PCI: layerscape: Fix some format issue of the code
From: Andrew Murray @ 2019-08-23 13:45 UTC (permalink / raw)
  To: Xiaowei Bao
  Cc: mark.rutland, roy.zang, lorenzo.pieralisi, arnd, devicetree,
	gregkh, linuxppc-dev, linux-pci, linux-kernel, kishon,
	minghuan.Lian, robh+dt, gustavo.pimentel, jingoohan1, bhelgaas,
	leoyang.li, shawnguo, mingkai.hu, linux-arm-kernel
In-Reply-To: <20190822112242.16309-5-xiaowei.bao@nxp.com>

On Thu, Aug 22, 2019 at 07:22:37PM +0800, Xiaowei Bao wrote:
> Fix some format issue of the code in EP driver.
> 
> Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>

Reviewed-by: Andrew Murray <andrew.murray@arm.com>


> ---
> v2:
>  - No change.
> 
>  drivers/pci/controller/dwc/pci-layerscape-ep.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> index be61d96..4e92a95 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -62,7 +62,7 @@ static void ls_pcie_ep_init(struct dw_pcie_ep *ep)
>  }
>  
>  static int ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> -				  enum pci_epc_irq_type type, u16 interrupt_num)
> +				enum pci_epc_irq_type type, u16 interrupt_num)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>  
> @@ -86,7 +86,7 @@ static const struct dw_pcie_ep_ops pcie_ep_ops = {
>  };
>  
>  static int __init ls_add_pcie_ep(struct ls_pcie_ep *pcie,
> -					struct platform_device *pdev)
> +				 struct platform_device *pdev)
>  {
>  	struct dw_pcie *pci = pcie->pci;
>  	struct device *dev = pci->dev;
> -- 
> 2.9.5
> 

^ permalink raw reply

* Re: [PATCH v2 07/10] PCI: layerscape: Modify the MSIX to the doorbell way
From: Andrew Murray @ 2019-08-23 13:58 UTC (permalink / raw)
  To: Xiaowei Bao
  Cc: mark.rutland, roy.zang, lorenzo.pieralisi, arnd, devicetree,
	gregkh, linuxppc-dev, linux-pci, linux-kernel, kishon,
	minghuan.Lian, robh+dt, gustavo.pimentel, jingoohan1, bhelgaas,
	leoyang.li, shawnguo, mingkai.hu, linux-arm-kernel
In-Reply-To: <20190822112242.16309-7-xiaowei.bao@nxp.com>

On Thu, Aug 22, 2019 at 07:22:39PM +0800, Xiaowei Bao wrote:
> The layerscape platform use the doorbell way to trigger MSIX
> interrupt in EP mode.
> 

I have no problems with this patch, however...

Are you able to add to this message a reason for why you are making this
change? Did dw_pcie_ep_raise_msix_irq not work when func_no != 0? Or did
it work yet dw_pcie_ep_raise_msix_irq_doorbell is more efficient?

Thanks,

Andrew Murray

> Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> ---
> v2:
>  - No change.
> 
>  drivers/pci/controller/dwc/pci-layerscape-ep.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> index 8461f62..7ca5fe8 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -74,7 +74,8 @@ static int ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
>  	case PCI_EPC_IRQ_MSI:
>  		return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
>  	case PCI_EPC_IRQ_MSIX:
> -		return dw_pcie_ep_raise_msix_irq(ep, func_no, interrupt_num);
> +		return dw_pcie_ep_raise_msix_irq_doorbell(ep, func_no,
> +							  interrupt_num);
>  	default:
>  		dev_err(pci->dev, "UNKNOWN IRQ type\n");
>  		return -EINVAL;
> -- 
> 2.9.5
> 

^ permalink raw reply

* Re: [PATCH v4 1/3] dt-bindings: pci: layerscape-pci: add compatible strings "fsl,ls1028a-pcie"
From: Lorenzo Pieralisi @ 2019-08-23 14:04 UTC (permalink / raw)
  To: Xiaowei Bao
  Cc: mark.rutland, roy.zang, devicetree, linux-pci, zhiqiang.hou,
	linuxppc-dev, linux-kernel, leoyang.li, minghuan.Lian, robh+dt,
	linux-arm-kernel, bhelgaas, shawnguo, mingkai.hu
In-Reply-To: <20190823082643.10903-1-xiaowei.bao@nxp.com>

On Fri, Aug 23, 2019 at 04:26:41PM +0800, Xiaowei Bao wrote:
> Add the PCIe compatible string for LS1028A
> 
> Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
> v2:
>  - No change.
> v3:
>  - No change.
> v4:
>  - No change.
> 
>  Documentation/devicetree/bindings/pci/layerscape-pci.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> index e20ceaa..99a386e 100644
> --- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> +++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> @@ -21,6 +21,7 @@ Required properties:
>          "fsl,ls1046a-pcie"
>          "fsl,ls1043a-pcie"
>          "fsl,ls1012a-pcie"
> +        "fsl,ls1028a-pcie"
>    EP mode:
>  	"fsl,ls1046a-pcie-ep", "fsl,ls-pcie-ep"
>  - reg: base addresses and lengths of the PCIe controller register blocks.

This series does not apply to v5.3-rc1, what is it based on ?

Lorenzo

^ permalink raw reply

* [PATCH] powerpc/mm/radix: remove useless kernel messages
From: Qian Cai @ 2019-08-23 14:22 UTC (permalink / raw)
  To: mpe; +Cc: linux-kernel, Qian Cai, aneesh.kumar, paulus, linuxppc-dev

Booting a POWER9 PowerNV system generates a few messages below with
"____ptrval____" due to the pointers printed without a specifier
extension (i.e unadorned %p) are hashed to prevent leaking information
about the kernel memory layout.

radix-mmu: Initializing Radix MMU
radix-mmu: Partition table (____ptrval____)
radix-mmu: Mapped 0x0000000000000000-0x0000000040000000 with 1.00 GiB
pages (exec)
radix-mmu: Mapped 0x0000000040000000-0x0000002000000000 with 1.00 GiB
pages
radix-mmu: Mapped 0x0000200000000000-0x0000202000000000 with 1.00 GiB
pages
radix-mmu: Process table (____ptrval____) and radix root for kernel:
(____ptrval____)

Signed-off-by: Qian Cai <cai@lca.pw>
---
 arch/powerpc/mm/book3s64/radix_pgtable.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c
index b4ca9e95e678..b6692ee9411d 100644
--- a/arch/powerpc/mm/book3s64/radix_pgtable.c
+++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
@@ -386,7 +386,6 @@ static void __init radix_init_pgtable(void)
 	 * physical address here.
 	 */
 	register_process_table(__pa(process_tb), 0, PRTB_SIZE_SHIFT - 12);
-	pr_info("Process table %p and radix root for kernel: %p\n", process_tb, init_mm.pgd);
 	asm volatile("ptesync" : : : "memory");
 	asm volatile(PPC_TLBIE_5(%0,%1,2,1,1) : :
 		     "r" (TLBIEL_INVAL_SET_LPID), "r" (0));
@@ -420,7 +419,6 @@ static void __init radix_init_partition_table(void)
 	mmu_partition_table_set_entry(0, dw0, 0);
 
 	pr_info("Initializing Radix MMU\n");
-	pr_info("Partition table %p\n", partition_tb);
 }
 
 void __init radix_init_native(void)
-- 
1.8.3.1


^ permalink raw reply related

* Re: [PATCH v2 08/10] PCI: layerscape: Add EP mode support for ls1088a and ls2088a
From: Andrew Murray @ 2019-08-23 14:27 UTC (permalink / raw)
  To: Xiaowei Bao
  Cc: mark.rutland, roy.zang, lorenzo.pieralisi, arnd, devicetree,
	gregkh, linuxppc-dev, linux-pci, linux-kernel, kishon,
	minghuan.Lian, robh+dt, gustavo.pimentel, jingoohan1, bhelgaas,
	leoyang.li, shawnguo, mingkai.hu, linux-arm-kernel
In-Reply-To: <20190822112242.16309-8-xiaowei.bao@nxp.com>

On Thu, Aug 22, 2019 at 07:22:40PM +0800, Xiaowei Bao wrote:
> Add PCIe EP mode support for ls1088a and ls2088a, there are some
> difference between LS1 and LS2 platform, so refactor the code of
> the EP driver.
> 
> Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> ---
> v2:
>  - New mechanism for layerscape EP driver.

Was there a v1 of this patch?

> 
>  drivers/pci/controller/dwc/pci-layerscape-ep.c | 76 ++++++++++++++++++++------
>  1 file changed, 58 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> index 7ca5fe8..2a66f07 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -20,27 +20,29 @@
>  
>  #define PCIE_DBI2_OFFSET		0x1000	/* DBI2 base address*/
>  
> -struct ls_pcie_ep {
> -	struct dw_pcie		*pci;
> -	struct pci_epc_features	*ls_epc;
> +#define to_ls_pcie_ep(x)	dev_get_drvdata((x)->dev)
> +
> +struct ls_pcie_ep_drvdata {
> +	u32				func_offset;
> +	const struct dw_pcie_ep_ops	*ops;
> +	const struct dw_pcie_ops	*dw_pcie_ops;
>  };
>  
> -#define to_ls_pcie_ep(x)	dev_get_drvdata((x)->dev)
> +struct ls_pcie_ep {
> +	struct dw_pcie			*pci;
> +	struct pci_epc_features		*ls_epc;
> +	const struct ls_pcie_ep_drvdata *drvdata;
> +};
>  
>  static int ls_pcie_establish_link(struct dw_pcie *pci)
>  {
>  	return 0;
>  }
>  
> -static const struct dw_pcie_ops ls_pcie_ep_ops = {
> +static const struct dw_pcie_ops dw_ls_pcie_ep_ops = {
>  	.start_link = ls_pcie_establish_link,
>  };
>  
> -static const struct of_device_id ls_pcie_ep_of_match[] = {
> -	{ .compatible = "fsl,ls-pcie-ep",},
> -	{ },
> -};
> -
>  static const struct pci_epc_features*
>  ls_pcie_ep_get_features(struct dw_pcie_ep *ep)
>  {
> @@ -82,10 +84,44 @@ static int ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
>  	}
>  }
>  
> -static const struct dw_pcie_ep_ops pcie_ep_ops = {
> +static unsigned int ls_pcie_ep_func_conf_select(struct dw_pcie_ep *ep,
> +						u8 func_no)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +	struct ls_pcie_ep *pcie = to_ls_pcie_ep(pci);
> +	u8 header_type;
> +
> +	header_type = ioread8(pci->dbi_base + PCI_HEADER_TYPE);
> +
> +	if (header_type & (1 << 7))
> +		return pcie->drvdata->func_offset * func_no;
> +	else
> +		return 0;

It looks like there isn't a PCI define for multi function, the nearest I
could find was PCI_HEADER_TYPE_MULTIDEVICE in hotplug/ibmphp.h. A comment
above the test might be helpful to explain the test.

As the ls_pcie_ep_drvdata structures are static, the unset .func_offset
will be initialised to 0, so you could just drop the test above.

However something to the effect of the following may help spot
misconfiguration:

WARN_ON(func_no && !pcie->drvdata->func_offset);
return pcie->drvdata->func_offset * func_no;

The WARN is probably quite useful as if you are attempting to use
non-zero functions and func_offset isn't set - then things may appear to work
normally but actually will break horribly.

Thanks,

Andrew Murray

> +}
> +
> +static const struct dw_pcie_ep_ops ls_pcie_ep_ops = {
>  	.ep_init = ls_pcie_ep_init,
>  	.raise_irq = ls_pcie_ep_raise_irq,
>  	.get_features = ls_pcie_ep_get_features,
> +	.func_conf_select = ls_pcie_ep_func_conf_select,
> +};
> +
> +static const struct ls_pcie_ep_drvdata ls1_ep_drvdata = {
> +	.ops = &ls_pcie_ep_ops,
> +	.dw_pcie_ops = &dw_ls_pcie_ep_ops,
> +};
> +
> +static const struct ls_pcie_ep_drvdata ls2_ep_drvdata = {
> +	.func_offset = 0x20000,
> +	.ops = &ls_pcie_ep_ops,
> +	.dw_pcie_ops = &dw_ls_pcie_ep_ops,
> +};
> +
> +static const struct of_device_id ls_pcie_ep_of_match[] = {
> +	{ .compatible = "fsl,ls1046a-pcie-ep", .data = &ls1_ep_drvdata },
> +	{ .compatible = "fsl,ls1088a-pcie-ep", .data = &ls2_ep_drvdata },
> +	{ .compatible = "fsl,ls2088a-pcie-ep", .data = &ls2_ep_drvdata },
> +	{ },
>  };
>  
>  static int __init ls_add_pcie_ep(struct ls_pcie_ep *pcie,
> @@ -98,7 +134,7 @@ static int __init ls_add_pcie_ep(struct ls_pcie_ep *pcie,
>  	int ret;
>  
>  	ep = &pci->ep;
> -	ep->ops = &pcie_ep_ops;
> +	ep->ops = pcie->drvdata->ops;
>  
>  	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space");
>  	if (!res)
> @@ -137,14 +173,11 @@ static int __init ls_pcie_ep_probe(struct platform_device *pdev)
>  	if (!ls_epc)
>  		return -ENOMEM;
>  
> -	dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
> -	pci->dbi_base = devm_pci_remap_cfg_resource(dev, dbi_base);
> -	if (IS_ERR(pci->dbi_base))
> -		return PTR_ERR(pci->dbi_base);
> +	pcie->drvdata = of_device_get_match_data(dev);
>  
> -	pci->dbi_base2 = pci->dbi_base + PCIE_DBI2_OFFSET;
>  	pci->dev = dev;
> -	pci->ops = &ls_pcie_ep_ops;
> +	pci->ops = pcie->drvdata->dw_pcie_ops;
> +
>  	pcie->pci = pci;
>  
>  	ls_epc->linkup_notifier = false,
> @@ -152,6 +185,13 @@ static int __init ls_pcie_ep_probe(struct platform_device *pdev)
>  
>  	pcie->ls_epc = ls_epc;
>  
> +	dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
> +	pci->dbi_base = devm_pci_remap_cfg_resource(dev, dbi_base);
> +	if (IS_ERR(pci->dbi_base))
> +		return PTR_ERR(pci->dbi_base);
> +
> +	pci->dbi_base2 = pci->dbi_base + PCIE_DBI2_OFFSET;
> +
>  	platform_set_drvdata(pdev, pcie);
>  
>  	ret = ls_add_pcie_ep(pcie, pdev);
> -- 
> 2.9.5
> 

^ permalink raw reply

* Re: [PATCH kernel] vfio/spapr_tce: Fix incorrect tce_iommu_group memory free
From: Alex Williamson @ 2019-08-23 14:40 UTC (permalink / raw)
  To: Paul Mackerras
  Cc: kvm, Jose Ricardo Ziviani, Alexey Kardashevskiy, kvm-ppc,
	linuxppc-dev, David Gibson
In-Reply-To: <20190823053241.hogc44em2ccwdwq4@oak.ozlabs.ibm.com>

On Fri, 23 Aug 2019 15:32:41 +1000
Paul Mackerras <paulus@ozlabs.org> wrote:

> On Mon, Aug 19, 2019 at 11:51:17AM +1000, Alexey Kardashevskiy wrote:
> > The @tcegrp variable is used in 1) a loop over attached groups
> > 2) it stores a pointer to a newly allocated tce_iommu_group if 1) found
> > nothing. However the error handler does not distinguish how we got there
> > and incorrectly releases memory for a found+incompatible group.
> > 
> > This fixes it by adding another error handling case.
> > 
> > Fixes: 0bd971676e68 ("powerpc/powernv/npu: Add compound IOMMU groups")
> > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>  
> 
> Good catch.  This is potentially nasty since it is a double free.
> Alex, are you going to take this, or would you prefer it goes via
> Michael Ellerman's tree?
> 
> Reviewed-by: Paul Mackerras <paulus@ozlabs.org>

I can take it, I've got it queued, but was hoping for an ack/review by
you or David.  I'll add the R-b and push it out to my next branch.
Thanks,

Alex

^ permalink raw reply

* Re: [PATCH 3/3] powerpc: use __builtin_trap() in BUG/WARN macros.
From: Christophe Leroy @ 2019-08-23 15:35 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: linuxppc-dev, Paul Mackerras, linux-kernel
In-Reply-To: <20190819154531.GM31406@gate.crashing.org>



On 08/19/2019 03:45 PM, Segher Boessenkool wrote:
> On Mon, Aug 19, 2019 at 05:05:46PM +0200, Christophe Leroy wrote:
>> Le 19/08/2019 à 16:37, Segher Boessenkool a écrit :
>>> On Mon, Aug 19, 2019 at 04:08:43PM +0200, Christophe Leroy wrote:
>>>> Le 19/08/2019 à 15:23, Segher Boessenkool a écrit :
>>>>> On Mon, Aug 19, 2019 at 01:06:31PM +0000, Christophe Leroy wrote:
>>>>>> Note that we keep using an assembly text using "twi 31, 0, 0" for
>>>>>> inconditional traps because GCC drops all code after
>>>>>> __builtin_trap() when the condition is always true at build time.
>>>>>
>>>>> As I said, it can also do this for conditional traps, if it can prove
>>>>> the condition is always true.
>>>>
>>>> But we have another branch for 'always true' and 'always false' using
>>>> __builtin_constant_p(), which don't use __builtin_trap(). Is there
>>>> anything wrong with that ?:
>>>
>>> The compiler might not realise it is constant when it evaluates the
>>> __builtin_constant_p, but only realises it later.  As the documentation
>>> for the builtin says:
>>>    A return of 0 does not indicate that the
>>>    value is _not_ a constant, but merely that GCC cannot prove it is a
>>>    constant with the specified value of the '-O' option.
>>
>> So you mean GCC would not be able to prove that
>> __builtin_constant_p(cond) is always true but it would be able to prove
>> that if (cond)  is always true ?
> 
> Not sure what you mean, sorry.
> 
>> And isn't there a away to tell GCC that '__builtin_trap()' is
>> recoverable in our case ?
> 
> No, GCC knows that a trap will never fall through.
> 
>>> I think it may work if you do
>>>
>>> #define BUG_ON(x) do {						\
>>> 	if (__builtin_constant_p(x)) {				\
>>> 		if (x)						\
>>> 			BUG();					\
>>> 	} else {						\
>>> 		BUG_ENTRY("", 0);				\
>>> 		if (x)						\
>>> 			__builtin_trap();			\
>>> 	}							\
>>> } while (0)
>>
>> It doesn't work:
> 
> You need to make a BUG_ENTRY so that it refers to the *following* trap
> instruction, if you go this way.
> 
>>> I don't know how BUG_ENTRY works exactly.
>>
>> It's basic, maybe too basic: it adds an inline asm with a label, and
>> adds a .long in the __bug_table section with the address of that label.
>>
>> When putting it after the __builtin_trap(), I changed it to using the
>> address before the one of the label which is always the twxx instruction
>> as far as I can see.
>>
>> #define BUG_ENTRY(insn, flags, ...)			\
>> 	__asm__ __volatile__(				\
>> 		"1:	" insn "\n"			\
>> 		".section __bug_table,\"aw\"\n"		\
>> 		"2:\t" PPC_LONG "1b, %0\n"		\
>> 		"\t.short %1, %2\n"			\
>> 		".org 2b+%3\n"				\
>> 		".previous\n"				\
>> 		: : "i" (__FILE__), "i" (__LINE__),	\
>> 		  "i" (flags),				\
>> 		  "i" (sizeof(struct bug_entry)),	\
>> 		  ##__VA_ARGS__)
> 
> #define MY_BUG_ENTRY(lab, flags)			\
> 	__asm__ __volatile__(				\
> 		".section __bug_table,\"aw\"\n"		\
> 		"2:\t" PPC_LONG "%4, %0\n"		\
> 		"\t.short %1, %2\n"			\
> 		".org 2b+%3\n"				\
> 		".previous\n"				\
> 		: : "i" (__FILE__), "i" (__LINE__),	\
> 		  "i" (flags),				\
> 		  "i" (sizeof(struct bug_entry)),	\
> 		  "i" (lab))
> 
> called as
> 
> #define BUG_ON(x) do {						\
> 	MY_BUG_ENTRY(&&lab, 0);					\
> 	lab: if (x)						\
> 		__builtin_trap();				\
> } while (0)
> 
> not sure how reliable that works -- *if* it works, I just typed that in
> without testing or anything -- but hopefully you get the idea.
> 

I've not been able to make it work. GCC puts the label (.L2 and .L6) 
outside of the function, so the instruction preceding the label is blr, 
not the trap.

#define _EMIT_BUG_ENTRY				\
	".section __bug_table,\"aw\"\n"		\
	"2:\t" PPC_LONG "%4, %0\n"		\
	"\t.short %1, %2\n"			\
	".org 2b+%3\n"				\
	".previous\n"

#define BUG_ENTRY(flags, label)				\
	__asm__ __volatile__(				\
		_EMIT_BUG_ENTRY				\
		: : "i" (__FILE__), "i" (__LINE__),	\
		  "i" (flags),				\
		  "i" (sizeof(struct bug_entry)),	\
		  "i" (label - 4))

#define __recoverable_trap()	asm volatile ("twi 31, 0, 0;");

#define __WARN_FLAGS(flags) do {				\
	__label__ label;					\
	BUG_ENTRY(BUGFLAG_WARNING | (flags), &&label);		\
	__recoverable_trap();					\
	label: ;						\
} while (0)

#define WARN_ON(x) ({						\
	int __ret_warn_on = !!(x);				\
	if (__builtin_constant_p(__ret_warn_on)) {		\
		if (__ret_warn_on)				\
			__WARN_TAINT(TAINT_WARN);		\
	} else {						\
		__label__ label;				\
		BUG_ENTRY(BUGFLAG_WARNING | BUGFLAG_TAINT(TAINT_WARN), &&label);	\
		if (__ret_warn_on)				\
			__builtin_trap();			\
		label: ;					\
	}							\
	unlikely(__ret_warn_on);				\
})

void test_warn1(unsigned long long a)
{
	WARN_ON(a);
}

void test_warn2(unsigned long a)
{
	WARN_ON(a);
}

00000000 <test_warn1>:
    0:	7c 63 23 78 	or      r3,r3,r4
    4:	0f 03 00 00 	twnei   r3,0
    8:	4e 80 00 20 	blr

0000000c <test_warn2>:
    c:	0f 03 00 00 	twnei   r3,0
   10:	4e 80 00 20 	blr

RELOCATION RECORDS FOR [__bug_table]:
OFFSET   TYPE              VALUE
00000000 R_PPC_ADDR32      .text+0x00000008
00000004 R_PPC_ADDR32      .rodata.str1.4
0000000c R_PPC_ADDR32      .text+0x00000010
00000010 R_PPC_ADDR32      .rodata.str1.4


	.file	"test.c"
	.section	".text"
.Ltext0:
	.align 2
	.globl test_warn1
	.type	test_warn1, @function
test_warn1:
.LFB598:
	.file 1 "arch/powerpc/mm/test.c"
	.loc 1 34 0
.LBB2:
.LBB3:
	.loc 1 35 0
#APP
  # 35 "arch/powerpc/mm/test.c" 1
	.section __bug_table,"aw"
2:	.long .L2-4, .LC0
	.short 35, 2305
.org 2b+12
.previous

  # 0 "" 2
#NO_APP
	or 3,3,4
	twnei 3,0
	blr
.L3:
.L2:
.LBE3:
.LBE2:
.LFE598:
	.size	test_warn1, .-test_warn1
	.align 2
	.globl test_warn2
	.type	test_warn2, @function
test_warn2:
.LFB599:
	.loc 1 39 0
.LBB4:
.LBB5:
	.loc 1 40 0
#APP
  # 40 "arch/powerpc/mm/test.c" 1
	.section __bug_table,"aw"
2:	.long .L6-4, .LC0
	.short 40, 2305
.org 2b+12
.previous

  # 0 "" 2
#NO_APP
	twnei 3,0
	blr
.L7:
.L6:
.LBE5:
.LBE4:
.LFE599:

Any idea ?

Christophe

^ permalink raw reply

* Re: [PATCH v2] powerpc/powernv: Add ultravisor message log interface
From: kbuild test robot @ 2019-08-23 16:08 UTC (permalink / raw)
  To: Claudio Carvalho
  Cc: Madhavan Srinivasan, Michael Anderson, Ram Pai, Claudio Carvalho,
	kvm-ppc, linuxppc-dev, Ryan Grimm, Oliver O'Halloran,
	kbuild-all
In-Reply-To: <20190823060654.28842-1-cclaudio@linux.ibm.com>

[-- Attachment #1: Type: text/plain, Size: 8045 bytes --]

Hi Claudio,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[cannot apply to v5.3-rc5 next-20190823]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Claudio-Carvalho/powerpc-powernv-Add-ultravisor-message-log-interface/20190823-214650
config: powerpc-defconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=powerpc 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from arch/powerpc/include/asm/lppaca.h:48:0,
                    from arch/powerpc/include/asm/paca.h:17,
                    from arch/powerpc/include/asm/current.h:13,
                    from include/linux/mutex.h:14,
                    from include/linux/kernfs.h:12,
                    from include/linux/sysfs.h:16,
                    from include/linux/kobject.h:20,
                    from include/linux/device.h:16,
                    from arch/powerpc/include/asm/io.h:27,
                    from arch/powerpc/platforms/powernv/opal-msglog.c:8:
   arch/powerpc/platforms/powernv/opal-msglog.c: In function 'opal_msglog_init':
>> arch/powerpc/platforms/powernv/opal-msglog.c:159:27: error: 'FW_FEATURE_ULTRAVISOR' undeclared (first use in this function); did you mean 'FW_FEATURE_ALWAYS'?
     if (firmware_has_feature(FW_FEATURE_ULTRAVISOR))
                              ^
   arch/powerpc/include/asm/firmware.h:120:25: note: in definition of macro 'firmware_has_feature'
     ((FW_FEATURE_ALWAYS & (feature)) ||    \
                            ^~~~~~~
   arch/powerpc/platforms/powernv/opal-msglog.c:159:27: note: each undeclared identifier is reported only once for each function it appears in
     if (firmware_has_feature(FW_FEATURE_ULTRAVISOR))
                              ^
   arch/powerpc/include/asm/firmware.h:120:25: note: in definition of macro 'firmware_has_feature'
     ((FW_FEATURE_ALWAYS & (feature)) ||    \
                            ^~~~~~~
   arch/powerpc/platforms/powernv/opal-msglog.c: In function 'opal_msglog_sysfs_init':
   arch/powerpc/platforms/powernv/opal-msglog.c:181:27: error: 'FW_FEATURE_ULTRAVISOR' undeclared (first use in this function); did you mean 'FW_FEATURE_ALWAYS'?
     if (firmware_has_feature(FW_FEATURE_ULTRAVISOR))
                              ^
   arch/powerpc/include/asm/firmware.h:120:25: note: in definition of macro 'firmware_has_feature'
     ((FW_FEATURE_ALWAYS & (feature)) ||    \
                            ^~~~~~~

vim +159 arch/powerpc/platforms/powernv/opal-msglog.c

   > 8	#include <asm/io.h>
     9	#include <asm/opal.h>
    10	#include <linux/debugfs.h>
    11	#include <linux/of.h>
    12	#include <linux/types.h>
    13	#include <asm/barrier.h>
    14	#include <asm/firmware.h>
    15	
    16	/* OPAL in-memory console. Defined in OPAL source at core/console.c */
    17	struct memcons {
    18		__be64 magic;
    19	#define MEMCONS_MAGIC	0x6630696567726173L
    20		__be64 obuf_phys;
    21		__be64 ibuf_phys;
    22		__be32 obuf_size;
    23		__be32 ibuf_size;
    24		__be32 out_pos;
    25	#define MEMCONS_OUT_POS_WRAP	0x80000000u
    26	#define MEMCONS_OUT_POS_MASK	0x00ffffffu
    27		__be32 in_prod;
    28		__be32 in_cons;
    29	};
    30	
    31	static struct memcons *opal_memcons = NULL;
    32	static struct memcons *opal_uv_memcons;
    33	
    34	static ssize_t msglog_copy(struct memcons *memcons, const char *bin_attr_name,
    35				   char *to, loff_t pos, size_t count)
    36	{
    37		const char *conbuf;
    38		ssize_t ret;
    39		size_t first_read = 0;
    40		uint32_t out_pos, avail;
    41	
    42		if (!memcons)
    43			return -ENODEV;
    44	
    45		out_pos = be32_to_cpu(READ_ONCE(memcons->out_pos));
    46	
    47		/* Now we've read out_pos, put a barrier in before reading the new
    48		 * data it points to in conbuf. */
    49		smp_rmb();
    50	
    51		conbuf = phys_to_virt(be64_to_cpu(memcons->obuf_phys));
    52	
    53		/* When the buffer has wrapped, read from the out_pos marker to the end
    54		 * of the buffer, and then read the remaining data as in the un-wrapped
    55		 * case. */
    56		if (out_pos & MEMCONS_OUT_POS_WRAP) {
    57	
    58			out_pos &= MEMCONS_OUT_POS_MASK;
    59			avail = be32_to_cpu(memcons->obuf_size) - out_pos;
    60	
    61			ret = memory_read_from_buffer(to, count, &pos,
    62					conbuf + out_pos, avail);
    63	
    64			if (ret < 0)
    65				goto out;
    66	
    67			first_read = ret;
    68			to += first_read;
    69			count -= first_read;
    70			pos -= avail;
    71	
    72			if (count <= 0)
    73				goto out;
    74		}
    75	
    76		/* Sanity check. The firmware should not do this to us. */
    77		if (out_pos > be32_to_cpu(memcons->obuf_size)) {
    78			pr_err("OPAL: %s corruption. Aborting read.\n", bin_attr_name);
    79			return -EINVAL;
    80		}
    81	
    82		ret = memory_read_from_buffer(to, count, &pos, conbuf, out_pos);
    83	
    84		if (ret < 0)
    85			goto out;
    86	
    87		ret += first_read;
    88	out:
    89		return ret;
    90	}
    91	
    92	#define BIN_ATTR_NAME_OPAL	"msglog"
    93	#define BIN_ATTR_NAME_UV	"uv_msglog"
    94	
    95	ssize_t opal_msglog_copy(char *to, loff_t pos, size_t count)
    96	{
    97		return msglog_copy(opal_memcons, BIN_ATTR_NAME_OPAL, to, pos,
    98				   count);
    99	}
   100	
   101	static ssize_t opal_msglog_read(struct file *file, struct kobject *kobj,
   102					struct bin_attribute *bin_attr, char *to,
   103					loff_t pos, size_t count)
   104	{
   105		return msglog_copy(opal_memcons, BIN_ATTR_NAME_OPAL, to, pos,
   106				   count);
   107	}
   108	
   109	static ssize_t opal_uv_msglog_read(struct file *file, struct kobject *kobj,
   110					   struct bin_attribute *bin_attr, char *to,
   111					   loff_t pos, size_t count)
   112	{
   113		return msglog_copy(opal_uv_memcons, BIN_ATTR_NAME_UV, to, pos,
   114				   count);
   115	}
   116	
   117	static struct bin_attribute opal_msglog_attr = {
   118		.attr = {.name = BIN_ATTR_NAME_OPAL, .mode = 0400},
   119		.read = opal_msglog_read
   120	};
   121	
   122	static struct bin_attribute opal_uv_msglog_attr = {
   123		.attr = {.name = BIN_ATTR_NAME_UV, .mode = 0400},
   124		.read = opal_uv_msglog_read
   125	};
   126	
   127	static void __init msglog_init(struct memcons **memcons,
   128				       struct bin_attribute *bin_attr,
   129				       const char *dt_prop_name)
   130	{
   131		u64 memcons_addr;
   132	
   133		if (of_property_read_u64(opal_node, dt_prop_name, &memcons_addr)) {
   134			pr_warn("OPAL: Property '%s' not found, no message log\n",
   135				dt_prop_name);
   136			return;
   137		}
   138	
   139		*memcons = phys_to_virt(memcons_addr);
   140		if (!(*memcons)) {
   141			pr_warn("OPAL: '%s' address is invalid\n", dt_prop_name);
   142			return;
   143		}
   144	
   145		if (be64_to_cpu((*memcons)->magic) != MEMCONS_MAGIC) {
   146			pr_warn("OPAL: '%s' version is invalid\n", dt_prop_name);
   147			*memcons = NULL;
   148			return;
   149		}
   150	
   151		/* Report maximum size */
   152		bin_attr->size = be32_to_cpu((*memcons)->ibuf_size) +
   153				 be32_to_cpu((*memcons)->obuf_size);
   154	}
   155	
   156	void __init opal_msglog_init(void)
   157	{
   158		msglog_init(&opal_memcons, &opal_msglog_attr, "ibm,opal-memcons");
 > 159		if (firmware_has_feature(FW_FEATURE_ULTRAVISOR))
   160			msglog_init(&opal_uv_memcons, &opal_uv_msglog_attr,
   161				    "ibm,opal-uv-memcons");
   162	}
   163	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25334 bytes --]

^ permalink raw reply

* [PATCH v5 0/7] kexec: add generic support for elf kernel images
From: Sven Schnelle @ 2019-08-23 19:49 UTC (permalink / raw)
  To: kexec; +Cc: Sven Schnelle, Helge Deller, linuxppc-dev, Thiago Jung Bauermann

Changes to v4:
 - rebase on current powerpc/merge tree
 - fix syscall name in commit message
 - remove a few unused #defines in arch/powerpc/kernel/kexec_elf_64.c

Changes to v3:
 - add support for 32-bit ELF files

Changes to v2:
 - use git format-patch -C

Changes to v1:
 - split up patch into smaller pieces
 - rebase onto powerpc/next
 - remove unused variable in kexec_elf_load()

Changes to RFC version:
 - remove unused Elf_Rel macro
 - remove section header parsing
 - remove PURGATORY_STACK_SIZE
 - change order of elf_*_to_cpu() functions
 - remove elf_addr_to_cpu macro

Sven Schnelle (7):
  kexec: add KEXEC_ELF
  kexec_elf: change order of elf_*_to_cpu() functions
  kexec_elf: remove parsing of section headers
  kexec_elf: remove PURGATORY_STACK_SIZE
  kexec_elf: remove Elf_Rel macro
  kexec_elf: remove unused variable in kexec_elf_load()
  kexec_elf: support 32 bit ELF files

 arch/Kconfig                                  |   3 +
 arch/powerpc/Kconfig                          |   1 +
 arch/powerpc/kernel/kexec_elf_64.c            | 545 +-----------------
 include/linux/kexec.h                         |  23 +
 kernel/Makefile                               |   1 +
 .../kexec_elf_64.c => kernel/kexec_elf.c      | 394 +++----------
 6 files changed, 115 insertions(+), 852 deletions(-)
 copy arch/powerpc/kernel/kexec_elf_64.c => kernel/kexec_elf.c (50%)

-- 
2.23.0.rc1


^ permalink raw reply

* [PATCH v5 2/7] kexec_elf: change order of elf_*_to_cpu() functions
From: Sven Schnelle @ 2019-08-23 19:49 UTC (permalink / raw)
  To: kexec; +Cc: Sven Schnelle, Helge Deller, linuxppc-dev, Thiago Jung Bauermann
In-Reply-To: <20190823194919.30916-1-svens@stackframe.org>

Change the order to have a 64/32/16 order, no functional change.

Signed-off-by: Sven Schnelle <svens@stackframe.org>
Reviewed-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
 kernel/kexec_elf.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c
index 26c6310167a0..34376fbc55be 100644
--- a/kernel/kexec_elf.c
+++ b/kernel/kexec_elf.c
@@ -44,22 +44,22 @@ static uint64_t elf64_to_cpu(const struct elfhdr *ehdr, uint64_t value)
 	return value;
 }
 
-static uint16_t elf16_to_cpu(const struct elfhdr *ehdr, uint16_t value)
+static uint32_t elf32_to_cpu(const struct elfhdr *ehdr, uint32_t value)
 {
 	if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB)
-		value = le16_to_cpu(value);
+		value = le32_to_cpu(value);
 	else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB)
-		value = be16_to_cpu(value);
+		value = be32_to_cpu(value);
 
 	return value;
 }
 
-static uint32_t elf32_to_cpu(const struct elfhdr *ehdr, uint32_t value)
+static uint16_t elf16_to_cpu(const struct elfhdr *ehdr, uint16_t value)
 {
 	if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB)
-		value = le32_to_cpu(value);
+		value = le16_to_cpu(value);
 	else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB)
-		value = be32_to_cpu(value);
+		value = be16_to_cpu(value);
 
 	return value;
 }
-- 
2.23.0.rc1


^ permalink raw reply related

* [PATCH v5 6/7] kexec_elf: remove unused variable in kexec_elf_load()
From: Sven Schnelle @ 2019-08-23 19:49 UTC (permalink / raw)
  To: kexec; +Cc: Sven Schnelle, Helge Deller, linuxppc-dev, Thiago Jung Bauermann
In-Reply-To: <20190823194919.30916-1-svens@stackframe.org>

base was never assigned, so we can remove it.

Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>
Reviewed-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
 kernel/kexec_elf.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c
index 6c806ce96ac1..85f2bd177d6e 100644
--- a/kernel/kexec_elf.c
+++ b/kernel/kexec_elf.c
@@ -363,7 +363,7 @@ int kexec_elf_load(struct kimage *image, struct elfhdr *ehdr,
 			 struct kexec_buf *kbuf,
 			 unsigned long *lowest_load_addr)
 {
-	unsigned long base = 0, lowest_addr = UINT_MAX;
+	unsigned long lowest_addr = UINT_MAX;
 	int ret;
 	size_t i;
 
@@ -385,7 +385,7 @@ int kexec_elf_load(struct kimage *image, struct elfhdr *ehdr,
 		kbuf->bufsz = size;
 		kbuf->memsz = phdr->p_memsz;
 		kbuf->buf_align = phdr->p_align;
-		kbuf->buf_min = phdr->p_paddr + base;
+		kbuf->buf_min = phdr->p_paddr;
 		kbuf->mem = KEXEC_BUF_MEM_UNKNOWN;
 		ret = kexec_add_buffer(kbuf);
 		if (ret)
@@ -396,9 +396,6 @@ int kexec_elf_load(struct kimage *image, struct elfhdr *ehdr,
 			lowest_addr = load_addr;
 	}
 
-	/* Update entry point to reflect new load address. */
-	ehdr->e_entry += base;
-
 	*lowest_load_addr = lowest_addr;
 	ret = 0;
  out:
-- 
2.23.0.rc1


^ permalink raw reply related

* [PATCH v5 1/7] kexec: add KEXEC_ELF
From: Sven Schnelle @ 2019-08-23 19:49 UTC (permalink / raw)
  To: kexec; +Cc: Sven Schnelle, Helge Deller, linuxppc-dev, Thiago Jung Bauermann
In-Reply-To: <20190823194919.30916-1-svens@stackframe.org>

Right now powerpc provides an implementation to read elf files
with the kexec_file_load() syscall. Make that available as a public
kexec interface so it can be re-used on other architectures.

Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
 arch/Kconfig                                  |   3 +
 arch/powerpc/Kconfig                          |   1 +
 arch/powerpc/kernel/kexec_elf_64.c            | 545 +-----------------
 include/linux/kexec.h                         |  24 +
 kernel/Makefile                               |   1 +
 .../kexec_elf_64.c => kernel/kexec_elf.c      | 183 ++----
 6 files changed, 70 insertions(+), 687 deletions(-)
 copy arch/powerpc/kernel/kexec_elf_64.c => kernel/kexec_elf.c (75%)

diff --git a/arch/Kconfig b/arch/Kconfig
index a7b57dd42c26..01244412f393 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -18,6 +18,9 @@ config KEXEC_CORE
 	select CRASH_CORE
 	bool
 
+config KEXEC_ELF
+	bool
+
 config HAVE_IMA_KEXEC
 	bool
 
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 7cb51bba4985..fbb0b305e1be 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -511,6 +511,7 @@ config KEXEC_FILE
 	select KEXEC_CORE
 	select HAVE_IMA_KEXEC
 	select BUILD_BIN2C
+	select KEXEC_ELF
 	depends on PPC64
 	depends on CRYPTO=y
 	depends on CRYPTO_SHA256=y
diff --git a/arch/powerpc/kernel/kexec_elf_64.c b/arch/powerpc/kernel/kexec_elf_64.c
index 83cf7b852876..3072fd6dbe94 100644
--- a/arch/powerpc/kernel/kexec_elf_64.c
+++ b/arch/powerpc/kernel/kexec_elf_64.c
@@ -23,541 +23,6 @@
 #include <linux/slab.h>
 #include <linux/types.h>
 
-#define PURGATORY_STACK_SIZE	(16 * 1024)
-
-#define elf_addr_to_cpu	elf64_to_cpu
-
-#ifndef Elf_Rel
-#define Elf_Rel		Elf64_Rel
-#endif /* Elf_Rel */
-
-struct elf_info {
-	/*
-	 * Where the ELF binary contents are kept.
-	 * Memory managed by the user of the struct.
-	 */
-	const char *buffer;
-
-	const struct elfhdr *ehdr;
-	const struct elf_phdr *proghdrs;
-	struct elf_shdr *sechdrs;
-};
-
-static inline bool elf_is_elf_file(const struct elfhdr *ehdr)
-{
-       return memcmp(ehdr->e_ident, ELFMAG, SELFMAG) == 0;
-}
-
-static uint64_t elf64_to_cpu(const struct elfhdr *ehdr, uint64_t value)
-{
-	if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB)
-		value = le64_to_cpu(value);
-	else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB)
-		value = be64_to_cpu(value);
-
-	return value;
-}
-
-static uint16_t elf16_to_cpu(const struct elfhdr *ehdr, uint16_t value)
-{
-	if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB)
-		value = le16_to_cpu(value);
-	else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB)
-		value = be16_to_cpu(value);
-
-	return value;
-}
-
-static uint32_t elf32_to_cpu(const struct elfhdr *ehdr, uint32_t value)
-{
-	if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB)
-		value = le32_to_cpu(value);
-	else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB)
-		value = be32_to_cpu(value);
-
-	return value;
-}
-
-/**
- * elf_is_ehdr_sane - check that it is safe to use the ELF header
- * @buf_len:	size of the buffer in which the ELF file is loaded.
- */
-static bool elf_is_ehdr_sane(const struct elfhdr *ehdr, size_t buf_len)
-{
-	if (ehdr->e_phnum > 0 && ehdr->e_phentsize != sizeof(struct elf_phdr)) {
-		pr_debug("Bad program header size.\n");
-		return false;
-	} else if (ehdr->e_shnum > 0 &&
-		   ehdr->e_shentsize != sizeof(struct elf_shdr)) {
-		pr_debug("Bad section header size.\n");
-		return false;
-	} else if (ehdr->e_ident[EI_VERSION] != EV_CURRENT ||
-		   ehdr->e_version != EV_CURRENT) {
-		pr_debug("Unknown ELF version.\n");
-		return false;
-	}
-
-	if (ehdr->e_phoff > 0 && ehdr->e_phnum > 0) {
-		size_t phdr_size;
-
-		/*
-		 * e_phnum is at most 65535 so calculating the size of the
-		 * program header cannot overflow.
-		 */
-		phdr_size = sizeof(struct elf_phdr) * ehdr->e_phnum;
-
-		/* Sanity check the program header table location. */
-		if (ehdr->e_phoff + phdr_size < ehdr->e_phoff) {
-			pr_debug("Program headers at invalid location.\n");
-			return false;
-		} else if (ehdr->e_phoff + phdr_size > buf_len) {
-			pr_debug("Program headers truncated.\n");
-			return false;
-		}
-	}
-
-	if (ehdr->e_shoff > 0 && ehdr->e_shnum > 0) {
-		size_t shdr_size;
-
-		/*
-		 * e_shnum is at most 65536 so calculating
-		 * the size of the section header cannot overflow.
-		 */
-		shdr_size = sizeof(struct elf_shdr) * ehdr->e_shnum;
-
-		/* Sanity check the section header table location. */
-		if (ehdr->e_shoff + shdr_size < ehdr->e_shoff) {
-			pr_debug("Section headers at invalid location.\n");
-			return false;
-		} else if (ehdr->e_shoff + shdr_size > buf_len) {
-			pr_debug("Section headers truncated.\n");
-			return false;
-		}
-	}
-
-	return true;
-}
-
-static int elf_read_ehdr(const char *buf, size_t len, struct elfhdr *ehdr)
-{
-	struct elfhdr *buf_ehdr;
-
-	if (len < sizeof(*buf_ehdr)) {
-		pr_debug("Buffer is too small to hold ELF header.\n");
-		return -ENOEXEC;
-	}
-
-	memset(ehdr, 0, sizeof(*ehdr));
-	memcpy(ehdr->e_ident, buf, sizeof(ehdr->e_ident));
-	if (!elf_is_elf_file(ehdr)) {
-		pr_debug("No ELF header magic.\n");
-		return -ENOEXEC;
-	}
-
-	if (ehdr->e_ident[EI_CLASS] != ELF_CLASS) {
-		pr_debug("Not a supported ELF class.\n");
-		return -ENOEXEC;
-	} else  if (ehdr->e_ident[EI_DATA] != ELFDATA2LSB &&
-		ehdr->e_ident[EI_DATA] != ELFDATA2MSB) {
-		pr_debug("Not a supported ELF data format.\n");
-		return -ENOEXEC;
-	}
-
-	buf_ehdr = (struct elfhdr *) buf;
-	if (elf16_to_cpu(ehdr, buf_ehdr->e_ehsize) != sizeof(*buf_ehdr)) {
-		pr_debug("Bad ELF header size.\n");
-		return -ENOEXEC;
-	}
-
-	ehdr->e_type      = elf16_to_cpu(ehdr, buf_ehdr->e_type);
-	ehdr->e_machine   = elf16_to_cpu(ehdr, buf_ehdr->e_machine);
-	ehdr->e_version   = elf32_to_cpu(ehdr, buf_ehdr->e_version);
-	ehdr->e_entry     = elf_addr_to_cpu(ehdr, buf_ehdr->e_entry);
-	ehdr->e_phoff     = elf_addr_to_cpu(ehdr, buf_ehdr->e_phoff);
-	ehdr->e_shoff     = elf_addr_to_cpu(ehdr, buf_ehdr->e_shoff);
-	ehdr->e_flags     = elf32_to_cpu(ehdr, buf_ehdr->e_flags);
-	ehdr->e_phentsize = elf16_to_cpu(ehdr, buf_ehdr->e_phentsize);
-	ehdr->e_phnum     = elf16_to_cpu(ehdr, buf_ehdr->e_phnum);
-	ehdr->e_shentsize = elf16_to_cpu(ehdr, buf_ehdr->e_shentsize);
-	ehdr->e_shnum     = elf16_to_cpu(ehdr, buf_ehdr->e_shnum);
-	ehdr->e_shstrndx  = elf16_to_cpu(ehdr, buf_ehdr->e_shstrndx);
-
-	return elf_is_ehdr_sane(ehdr, len) ? 0 : -ENOEXEC;
-}
-
-/**
- * elf_is_phdr_sane - check that it is safe to use the program header
- * @buf_len:	size of the buffer in which the ELF file is loaded.
- */
-static bool elf_is_phdr_sane(const struct elf_phdr *phdr, size_t buf_len)
-{
-
-	if (phdr->p_offset + phdr->p_filesz < phdr->p_offset) {
-		pr_debug("ELF segment location wraps around.\n");
-		return false;
-	} else if (phdr->p_offset + phdr->p_filesz > buf_len) {
-		pr_debug("ELF segment not in file.\n");
-		return false;
-	} else if (phdr->p_paddr + phdr->p_memsz < phdr->p_paddr) {
-		pr_debug("ELF segment address wraps around.\n");
-		return false;
-	}
-
-	return true;
-}
-
-static int elf_read_phdr(const char *buf, size_t len, struct elf_info *elf_info,
-			 int idx)
-{
-	/* Override the const in proghdrs, we are the ones doing the loading. */
-	struct elf_phdr *phdr = (struct elf_phdr *) &elf_info->proghdrs[idx];
-	const char *pbuf;
-	struct elf_phdr *buf_phdr;
-
-	pbuf = buf + elf_info->ehdr->e_phoff + (idx * sizeof(*buf_phdr));
-	buf_phdr = (struct elf_phdr *) pbuf;
-
-	phdr->p_type   = elf32_to_cpu(elf_info->ehdr, buf_phdr->p_type);
-	phdr->p_offset = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_offset);
-	phdr->p_paddr  = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_paddr);
-	phdr->p_vaddr  = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_vaddr);
-	phdr->p_flags  = elf32_to_cpu(elf_info->ehdr, buf_phdr->p_flags);
-
-	/*
-	 * The following fields have a type equivalent to Elf_Addr
-	 * both in 32 bit and 64 bit ELF.
-	 */
-	phdr->p_filesz = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_filesz);
-	phdr->p_memsz  = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_memsz);
-	phdr->p_align  = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_align);
-
-	return elf_is_phdr_sane(phdr, len) ? 0 : -ENOEXEC;
-}
-
-/**
- * elf_read_phdrs - read the program headers from the buffer
- *
- * This function assumes that the program header table was checked for sanity.
- * Use elf_is_ehdr_sane() if it wasn't.
- */
-static int elf_read_phdrs(const char *buf, size_t len,
-			  struct elf_info *elf_info)
-{
-	size_t phdr_size, i;
-	const struct elfhdr *ehdr = elf_info->ehdr;
-
-	/*
-	 * e_phnum is at most 65535 so calculating the size of the
-	 * program header cannot overflow.
-	 */
-	phdr_size = sizeof(struct elf_phdr) * ehdr->e_phnum;
-
-	elf_info->proghdrs = kzalloc(phdr_size, GFP_KERNEL);
-	if (!elf_info->proghdrs)
-		return -ENOMEM;
-
-	for (i = 0; i < ehdr->e_phnum; i++) {
-		int ret;
-
-		ret = elf_read_phdr(buf, len, elf_info, i);
-		if (ret) {
-			kfree(elf_info->proghdrs);
-			elf_info->proghdrs = NULL;
-			return ret;
-		}
-	}
-
-	return 0;
-}
-
-/**
- * elf_is_shdr_sane - check that it is safe to use the section header
- * @buf_len:	size of the buffer in which the ELF file is loaded.
- */
-static bool elf_is_shdr_sane(const struct elf_shdr *shdr, size_t buf_len)
-{
-	bool size_ok;
-
-	/* SHT_NULL headers have undefined values, so we can't check them. */
-	if (shdr->sh_type == SHT_NULL)
-		return true;
-
-	/* Now verify sh_entsize */
-	switch (shdr->sh_type) {
-	case SHT_SYMTAB:
-		size_ok = shdr->sh_entsize == sizeof(Elf_Sym);
-		break;
-	case SHT_RELA:
-		size_ok = shdr->sh_entsize == sizeof(Elf_Rela);
-		break;
-	case SHT_DYNAMIC:
-		size_ok = shdr->sh_entsize == sizeof(Elf_Dyn);
-		break;
-	case SHT_REL:
-		size_ok = shdr->sh_entsize == sizeof(Elf_Rel);
-		break;
-	case SHT_NOTE:
-	case SHT_PROGBITS:
-	case SHT_HASH:
-	case SHT_NOBITS:
-	default:
-		/*
-		 * This is a section whose entsize requirements
-		 * I don't care about.  If I don't know about
-		 * the section I can't care about it's entsize
-		 * requirements.
-		 */
-		size_ok = true;
-		break;
-	}
-
-	if (!size_ok) {
-		pr_debug("ELF section with wrong entry size.\n");
-		return false;
-	} else if (shdr->sh_addr + shdr->sh_size < shdr->sh_addr) {
-		pr_debug("ELF section address wraps around.\n");
-		return false;
-	}
-
-	if (shdr->sh_type != SHT_NOBITS) {
-		if (shdr->sh_offset + shdr->sh_size < shdr->sh_offset) {
-			pr_debug("ELF section location wraps around.\n");
-			return false;
-		} else if (shdr->sh_offset + shdr->sh_size > buf_len) {
-			pr_debug("ELF section not in file.\n");
-			return false;
-		}
-	}
-
-	return true;
-}
-
-static int elf_read_shdr(const char *buf, size_t len, struct elf_info *elf_info,
-			 int idx)
-{
-	struct elf_shdr *shdr = &elf_info->sechdrs[idx];
-	const struct elfhdr *ehdr = elf_info->ehdr;
-	const char *sbuf;
-	struct elf_shdr *buf_shdr;
-
-	sbuf = buf + ehdr->e_shoff + idx * sizeof(*buf_shdr);
-	buf_shdr = (struct elf_shdr *) sbuf;
-
-	shdr->sh_name      = elf32_to_cpu(ehdr, buf_shdr->sh_name);
-	shdr->sh_type      = elf32_to_cpu(ehdr, buf_shdr->sh_type);
-	shdr->sh_addr      = elf_addr_to_cpu(ehdr, buf_shdr->sh_addr);
-	shdr->sh_offset    = elf_addr_to_cpu(ehdr, buf_shdr->sh_offset);
-	shdr->sh_link      = elf32_to_cpu(ehdr, buf_shdr->sh_link);
-	shdr->sh_info      = elf32_to_cpu(ehdr, buf_shdr->sh_info);
-
-	/*
-	 * The following fields have a type equivalent to Elf_Addr
-	 * both in 32 bit and 64 bit ELF.
-	 */
-	shdr->sh_flags     = elf_addr_to_cpu(ehdr, buf_shdr->sh_flags);
-	shdr->sh_size      = elf_addr_to_cpu(ehdr, buf_shdr->sh_size);
-	shdr->sh_addralign = elf_addr_to_cpu(ehdr, buf_shdr->sh_addralign);
-	shdr->sh_entsize   = elf_addr_to_cpu(ehdr, buf_shdr->sh_entsize);
-
-	return elf_is_shdr_sane(shdr, len) ? 0 : -ENOEXEC;
-}
-
-/**
- * elf_read_shdrs - read the section headers from the buffer
- *
- * This function assumes that the section header table was checked for sanity.
- * Use elf_is_ehdr_sane() if it wasn't.
- */
-static int elf_read_shdrs(const char *buf, size_t len,
-			  struct elf_info *elf_info)
-{
-	size_t shdr_size, i;
-
-	/*
-	 * e_shnum is at most 65536 so calculating
-	 * the size of the section header cannot overflow.
-	 */
-	shdr_size = sizeof(struct elf_shdr) * elf_info->ehdr->e_shnum;
-
-	elf_info->sechdrs = kzalloc(shdr_size, GFP_KERNEL);
-	if (!elf_info->sechdrs)
-		return -ENOMEM;
-
-	for (i = 0; i < elf_info->ehdr->e_shnum; i++) {
-		int ret;
-
-		ret = elf_read_shdr(buf, len, elf_info, i);
-		if (ret) {
-			kfree(elf_info->sechdrs);
-			elf_info->sechdrs = NULL;
-			return ret;
-		}
-	}
-
-	return 0;
-}
-
-/**
- * elf_read_from_buffer - read ELF file and sets up ELF header and ELF info
- * @buf:	Buffer to read ELF file from.
- * @len:	Size of @buf.
- * @ehdr:	Pointer to existing struct which will be populated.
- * @elf_info:	Pointer to existing struct which will be populated.
- *
- * This function allows reading ELF files with different byte order than
- * the kernel, byte-swapping the fields as needed.
- *
- * Return:
- * On success returns 0, and the caller should call elf_free_info(elf_info) to
- * free the memory allocated for the section and program headers.
- */
-int elf_read_from_buffer(const char *buf, size_t len, struct elfhdr *ehdr,
-			 struct elf_info *elf_info)
-{
-	int ret;
-
-	ret = elf_read_ehdr(buf, len, ehdr);
-	if (ret)
-		return ret;
-
-	elf_info->buffer = buf;
-	elf_info->ehdr = ehdr;
-	if (ehdr->e_phoff > 0 && ehdr->e_phnum > 0) {
-		ret = elf_read_phdrs(buf, len, elf_info);
-		if (ret)
-			return ret;
-	}
-	if (ehdr->e_shoff > 0 && ehdr->e_shnum > 0) {
-		ret = elf_read_shdrs(buf, len, elf_info);
-		if (ret) {
-			kfree(elf_info->proghdrs);
-			return ret;
-		}
-	}
-
-	return 0;
-}
-
-/**
- * elf_free_info - free memory allocated by elf_read_from_buffer
- */
-void elf_free_info(struct elf_info *elf_info)
-{
-	kfree(elf_info->proghdrs);
-	kfree(elf_info->sechdrs);
-	memset(elf_info, 0, sizeof(*elf_info));
-}
-/**
- * build_elf_exec_info - read ELF executable and check that we can use it
- */
-static int build_elf_exec_info(const char *buf, size_t len, struct elfhdr *ehdr,
-			       struct elf_info *elf_info)
-{
-	int i;
-	int ret;
-
-	ret = elf_read_from_buffer(buf, len, ehdr, elf_info);
-	if (ret)
-		return ret;
-
-	/* Big endian vmlinux has type ET_DYN. */
-	if (ehdr->e_type != ET_EXEC && ehdr->e_type != ET_DYN) {
-		pr_err("Not an ELF executable.\n");
-		goto error;
-	} else if (!elf_info->proghdrs) {
-		pr_err("No ELF program header.\n");
-		goto error;
-	}
-
-	for (i = 0; i < ehdr->e_phnum; i++) {
-		/*
-		 * Kexec does not support loading interpreters.
-		 * In addition this check keeps us from attempting
-		 * to kexec ordinay executables.
-		 */
-		if (elf_info->proghdrs[i].p_type == PT_INTERP) {
-			pr_err("Requires an ELF interpreter.\n");
-			goto error;
-		}
-	}
-
-	return 0;
-error:
-	elf_free_info(elf_info);
-	return -ENOEXEC;
-}
-
-static int elf64_probe(const char *buf, unsigned long len)
-{
-	struct elfhdr ehdr;
-	struct elf_info elf_info;
-	int ret;
-
-	ret = build_elf_exec_info(buf, len, &ehdr, &elf_info);
-	if (ret)
-		return ret;
-
-	elf_free_info(&elf_info);
-
-	return elf_check_arch(&ehdr) ? 0 : -ENOEXEC;
-}
-
-/**
- * elf_exec_load - load ELF executable image
- * @lowest_load_addr:	On return, will be the address where the first PT_LOAD
- *			section will be loaded in memory.
- *
- * Return:
- * 0 on success, negative value on failure.
- */
-static int elf_exec_load(struct kimage *image, struct elfhdr *ehdr,
-			 struct elf_info *elf_info,
-			 unsigned long *lowest_load_addr)
-{
-	unsigned long base = 0, lowest_addr = UINT_MAX;
-	int ret;
-	size_t i;
-	struct kexec_buf kbuf = { .image = image, .buf_max = ppc64_rma_size,
-				  .top_down = false };
-
-	/* Read in the PT_LOAD segments. */
-	for (i = 0; i < ehdr->e_phnum; i++) {
-		unsigned long load_addr;
-		size_t size;
-		const struct elf_phdr *phdr;
-
-		phdr = &elf_info->proghdrs[i];
-		if (phdr->p_type != PT_LOAD)
-			continue;
-
-		size = phdr->p_filesz;
-		if (size > phdr->p_memsz)
-			size = phdr->p_memsz;
-
-		kbuf.buffer = (void *) elf_info->buffer + phdr->p_offset;
-		kbuf.bufsz = size;
-		kbuf.memsz = phdr->p_memsz;
-		kbuf.buf_align = phdr->p_align;
-		kbuf.buf_min = phdr->p_paddr + base;
-		kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
-		ret = kexec_add_buffer(&kbuf);
-		if (ret)
-			goto out;
-		load_addr = kbuf.mem;
-
-		if (load_addr < lowest_addr)
-			lowest_addr = load_addr;
-	}
-
-	/* Update entry point to reflect new load address. */
-	ehdr->e_entry += base;
-
-	*lowest_load_addr = lowest_addr;
-	ret = 0;
- out:
-	return ret;
-}
-
 static void *elf64_load(struct kimage *image, char *kernel_buf,
 			unsigned long kernel_len, char *initrd,
 			unsigned long initrd_len, char *cmdline,
@@ -570,18 +35,18 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
 	void *fdt;
 	const void *slave_code;
 	struct elfhdr ehdr;
-	struct elf_info elf_info;
+	struct kexec_elf_info elf_info;
 	struct kexec_buf kbuf = { .image = image, .buf_min = 0,
 				  .buf_max = ppc64_rma_size };
 	struct kexec_buf pbuf = { .image = image, .buf_min = 0,
 				  .buf_max = ppc64_rma_size, .top_down = true,
 				  .mem = KEXEC_BUF_MEM_UNKNOWN };
 
-	ret = build_elf_exec_info(kernel_buf, kernel_len, &ehdr, &elf_info);
+	ret = kexec_build_elf_info(kernel_buf, kernel_len, &ehdr, &elf_info);
 	if (ret)
 		goto out;
 
-	ret = elf_exec_load(image, &ehdr, &elf_info, &kernel_load_addr);
+	ret = kexec_elf_load(image, &ehdr, &elf_info, &kbuf, &kernel_load_addr);
 	if (ret)
 		goto out;
 
@@ -648,13 +113,13 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
 		pr_err("Error setting up the purgatory.\n");
 
 out:
-	elf_free_info(&elf_info);
+	kexec_free_elf_info(&elf_info);
 
 	/* Make kimage_file_post_load_cleanup free the fdt buffer for us. */
 	return ret ? ERR_PTR(ret) : fdt;
 }
 
 const struct kexec_file_ops kexec_elf64_ops = {
-	.probe = elf64_probe,
+	.probe = kexec_elf_probe,
 	.load = elf64_load,
 };
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index b9b1bc5f9669..da2a6b1d69e7 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -216,6 +216,30 @@ extern int crash_prepare_elf64_headers(struct crash_mem *mem, int kernel_map,
 				       void **addr, unsigned long *sz);
 #endif /* CONFIG_KEXEC_FILE */
 
+#ifdef CONFIG_KEXEC_ELF
+struct kexec_elf_info {
+	/*
+	 * Where the ELF binary contents are kept.
+	 * Memory managed by the user of the struct.
+	 */
+	const char *buffer;
+
+	const struct elfhdr *ehdr;
+	const struct elf_phdr *proghdrs;
+	struct elf_shdr *sechdrs;
+};
+
+int kexec_build_elf_info(const char *buf, size_t len, struct elfhdr *ehdr,
+			       struct kexec_elf_info *elf_info);
+
+int kexec_elf_load(struct kimage *image, struct elfhdr *ehdr,
+			 struct kexec_elf_info *elf_info,
+			 struct kexec_buf *kbuf,
+			 unsigned long *lowest_load_addr);
+
+void kexec_free_elf_info(struct kexec_elf_info *elf_info);
+int kexec_elf_probe(const char *buf, unsigned long len);
+#endif
 struct kimage {
 	kimage_entry_t head;
 	kimage_entry_t *entry;
diff --git a/kernel/Makefile b/kernel/Makefile
index ef0d95a190b4..48c5376d290a 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -64,6 +64,7 @@ obj-$(CONFIG_CRASH_CORE) += crash_core.o
 obj-$(CONFIG_KEXEC_CORE) += kexec_core.o
 obj-$(CONFIG_KEXEC) += kexec.o
 obj-$(CONFIG_KEXEC_FILE) += kexec_file.o
+obj-$(CONFIG_KEXEC_ELF) += kexec_elf.o
 obj-$(CONFIG_BACKTRACE_SELF_TEST) += backtracetest.o
 obj-$(CONFIG_COMPAT) += compat.o
 obj-$(CONFIG_CGROUPS) += cgroup/
diff --git a/arch/powerpc/kernel/kexec_elf_64.c b/kernel/kexec_elf.c
similarity index 75%
copy from arch/powerpc/kernel/kexec_elf_64.c
copy to kernel/kexec_elf.c
index 83cf7b852876..26c6310167a0 100644
--- a/arch/powerpc/kernel/kexec_elf_64.c
+++ b/kernel/kexec_elf.c
@@ -17,9 +17,7 @@
 
 #include <linux/elf.h>
 #include <linux/kexec.h>
-#include <linux/libfdt.h>
 #include <linux/module.h>
-#include <linux/of_fdt.h>
 #include <linux/slab.h>
 #include <linux/types.h>
 
@@ -31,21 +29,9 @@
 #define Elf_Rel		Elf64_Rel
 #endif /* Elf_Rel */
 
-struct elf_info {
-	/*
-	 * Where the ELF binary contents are kept.
-	 * Memory managed by the user of the struct.
-	 */
-	const char *buffer;
-
-	const struct elfhdr *ehdr;
-	const struct elf_phdr *proghdrs;
-	struct elf_shdr *sechdrs;
-};
-
 static inline bool elf_is_elf_file(const struct elfhdr *ehdr)
 {
-       return memcmp(ehdr->e_ident, ELFMAG, SELFMAG) == 0;
+	return memcmp(ehdr->e_ident, ELFMAG, SELFMAG) == 0;
 }
 
 static uint64_t elf64_to_cpu(const struct elfhdr *ehdr, uint64_t value)
@@ -206,7 +192,8 @@ static bool elf_is_phdr_sane(const struct elf_phdr *phdr, size_t buf_len)
 	return true;
 }
 
-static int elf_read_phdr(const char *buf, size_t len, struct elf_info *elf_info,
+static int elf_read_phdr(const char *buf, size_t len,
+			 struct kexec_elf_info *elf_info,
 			 int idx)
 {
 	/* Override the const in proghdrs, we are the ones doing the loading. */
@@ -241,7 +228,7 @@ static int elf_read_phdr(const char *buf, size_t len, struct elf_info *elf_info,
  * Use elf_is_ehdr_sane() if it wasn't.
  */
 static int elf_read_phdrs(const char *buf, size_t len,
-			  struct elf_info *elf_info)
+			  struct kexec_elf_info *elf_info)
 {
 	size_t phdr_size, i;
 	const struct elfhdr *ehdr = elf_info->ehdr;
@@ -332,7 +319,8 @@ static bool elf_is_shdr_sane(const struct elf_shdr *shdr, size_t buf_len)
 	return true;
 }
 
-static int elf_read_shdr(const char *buf, size_t len, struct elf_info *elf_info,
+static int elf_read_shdr(const char *buf, size_t len,
+			 struct kexec_elf_info *elf_info,
 			 int idx)
 {
 	struct elf_shdr *shdr = &elf_info->sechdrs[idx];
@@ -369,7 +357,7 @@ static int elf_read_shdr(const char *buf, size_t len, struct elf_info *elf_info,
  * Use elf_is_ehdr_sane() if it wasn't.
  */
 static int elf_read_shdrs(const char *buf, size_t len,
-			  struct elf_info *elf_info)
+			  struct kexec_elf_info *elf_info)
 {
 	size_t shdr_size, i;
 
@@ -408,11 +396,13 @@ static int elf_read_shdrs(const char *buf, size_t len,
  * the kernel, byte-swapping the fields as needed.
  *
  * Return:
- * On success returns 0, and the caller should call elf_free_info(elf_info) to
- * free the memory allocated for the section and program headers.
+ * On success returns 0, and the caller should call
+ * kexec_free_elf_info(elf_info) to free the memory allocated for the section
+ * and program headers.
  */
-int elf_read_from_buffer(const char *buf, size_t len, struct elfhdr *ehdr,
-			 struct elf_info *elf_info)
+static int elf_read_from_buffer(const char *buf, size_t len,
+				struct elfhdr *ehdr,
+				struct kexec_elf_info *elf_info)
 {
 	int ret;
 
@@ -439,19 +429,19 @@ int elf_read_from_buffer(const char *buf, size_t len, struct elfhdr *ehdr,
 }
 
 /**
- * elf_free_info - free memory allocated by elf_read_from_buffer
+ * kexec_free_elf_info - free memory allocated by elf_read_from_buffer
  */
-void elf_free_info(struct elf_info *elf_info)
+void kexec_free_elf_info(struct kexec_elf_info *elf_info)
 {
 	kfree(elf_info->proghdrs);
 	kfree(elf_info->sechdrs);
 	memset(elf_info, 0, sizeof(*elf_info));
 }
 /**
- * build_elf_exec_info - read ELF executable and check that we can use it
+ * kexec_build_elf_info - read ELF executable and check that we can use it
  */
-static int build_elf_exec_info(const char *buf, size_t len, struct elfhdr *ehdr,
-			       struct elf_info *elf_info)
+int kexec_build_elf_info(const char *buf, size_t len, struct elfhdr *ehdr,
+			       struct kexec_elf_info *elf_info)
 {
 	int i;
 	int ret;
@@ -483,42 +473,42 @@ static int build_elf_exec_info(const char *buf, size_t len, struct elfhdr *ehdr,
 
 	return 0;
 error:
-	elf_free_info(elf_info);
+	kexec_free_elf_info(elf_info);
 	return -ENOEXEC;
 }
 
-static int elf64_probe(const char *buf, unsigned long len)
+
+int kexec_elf_probe(const char *buf, unsigned long len)
 {
 	struct elfhdr ehdr;
-	struct elf_info elf_info;
+	struct kexec_elf_info elf_info;
 	int ret;
 
-	ret = build_elf_exec_info(buf, len, &ehdr, &elf_info);
+	ret = kexec_build_elf_info(buf, len, &ehdr, &elf_info);
 	if (ret)
 		return ret;
 
-	elf_free_info(&elf_info);
+	kexec_free_elf_info(&elf_info);
 
 	return elf_check_arch(&ehdr) ? 0 : -ENOEXEC;
 }
 
 /**
- * elf_exec_load - load ELF executable image
+ * kexec_elf_load - load ELF executable image
  * @lowest_load_addr:	On return, will be the address where the first PT_LOAD
  *			section will be loaded in memory.
  *
  * Return:
  * 0 on success, negative value on failure.
  */
-static int elf_exec_load(struct kimage *image, struct elfhdr *ehdr,
-			 struct elf_info *elf_info,
+int kexec_elf_load(struct kimage *image, struct elfhdr *ehdr,
+			 struct kexec_elf_info *elf_info,
+			 struct kexec_buf *kbuf,
 			 unsigned long *lowest_load_addr)
 {
 	unsigned long base = 0, lowest_addr = UINT_MAX;
 	int ret;
 	size_t i;
-	struct kexec_buf kbuf = { .image = image, .buf_max = ppc64_rma_size,
-				  .top_down = false };
 
 	/* Read in the PT_LOAD segments. */
 	for (i = 0; i < ehdr->e_phnum; i++) {
@@ -534,16 +524,16 @@ static int elf_exec_load(struct kimage *image, struct elfhdr *ehdr,
 		if (size > phdr->p_memsz)
 			size = phdr->p_memsz;
 
-		kbuf.buffer = (void *) elf_info->buffer + phdr->p_offset;
-		kbuf.bufsz = size;
-		kbuf.memsz = phdr->p_memsz;
-		kbuf.buf_align = phdr->p_align;
-		kbuf.buf_min = phdr->p_paddr + base;
-		kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
-		ret = kexec_add_buffer(&kbuf);
+		kbuf->buffer = (void *) elf_info->buffer + phdr->p_offset;
+		kbuf->bufsz = size;
+		kbuf->memsz = phdr->p_memsz;
+		kbuf->buf_align = phdr->p_align;
+		kbuf->buf_min = phdr->p_paddr + base;
+		kbuf->mem = KEXEC_BUF_MEM_UNKNOWN;
+		ret = kexec_add_buffer(kbuf);
 		if (ret)
 			goto out;
-		load_addr = kbuf.mem;
+		load_addr = kbuf->mem;
 
 		if (load_addr < lowest_addr)
 			lowest_addr = load_addr;
@@ -557,104 +547,3 @@ static int elf_exec_load(struct kimage *image, struct elfhdr *ehdr,
  out:
 	return ret;
 }
-
-static void *elf64_load(struct kimage *image, char *kernel_buf,
-			unsigned long kernel_len, char *initrd,
-			unsigned long initrd_len, char *cmdline,
-			unsigned long cmdline_len)
-{
-	int ret;
-	unsigned int fdt_size;
-	unsigned long kernel_load_addr;
-	unsigned long initrd_load_addr = 0, fdt_load_addr;
-	void *fdt;
-	const void *slave_code;
-	struct elfhdr ehdr;
-	struct elf_info elf_info;
-	struct kexec_buf kbuf = { .image = image, .buf_min = 0,
-				  .buf_max = ppc64_rma_size };
-	struct kexec_buf pbuf = { .image = image, .buf_min = 0,
-				  .buf_max = ppc64_rma_size, .top_down = true,
-				  .mem = KEXEC_BUF_MEM_UNKNOWN };
-
-	ret = build_elf_exec_info(kernel_buf, kernel_len, &ehdr, &elf_info);
-	if (ret)
-		goto out;
-
-	ret = elf_exec_load(image, &ehdr, &elf_info, &kernel_load_addr);
-	if (ret)
-		goto out;
-
-	pr_debug("Loaded the kernel at 0x%lx\n", kernel_load_addr);
-
-	ret = kexec_load_purgatory(image, &pbuf);
-	if (ret) {
-		pr_err("Loading purgatory failed.\n");
-		goto out;
-	}
-
-	pr_debug("Loaded purgatory at 0x%lx\n", pbuf.mem);
-
-	if (initrd != NULL) {
-		kbuf.buffer = initrd;
-		kbuf.bufsz = kbuf.memsz = initrd_len;
-		kbuf.buf_align = PAGE_SIZE;
-		kbuf.top_down = false;
-		kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
-		ret = kexec_add_buffer(&kbuf);
-		if (ret)
-			goto out;
-		initrd_load_addr = kbuf.mem;
-
-		pr_debug("Loaded initrd at 0x%lx\n", initrd_load_addr);
-	}
-
-	fdt_size = fdt_totalsize(initial_boot_params) * 2;
-	fdt = kmalloc(fdt_size, GFP_KERNEL);
-	if (!fdt) {
-		pr_err("Not enough memory for the device tree.\n");
-		ret = -ENOMEM;
-		goto out;
-	}
-	ret = fdt_open_into(initial_boot_params, fdt, fdt_size);
-	if (ret < 0) {
-		pr_err("Error setting up the new device tree.\n");
-		ret = -EINVAL;
-		goto out;
-	}
-
-	ret = setup_new_fdt(image, fdt, initrd_load_addr, initrd_len, cmdline);
-	if (ret)
-		goto out;
-
-	fdt_pack(fdt);
-
-	kbuf.buffer = fdt;
-	kbuf.bufsz = kbuf.memsz = fdt_size;
-	kbuf.buf_align = PAGE_SIZE;
-	kbuf.top_down = true;
-	kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
-	ret = kexec_add_buffer(&kbuf);
-	if (ret)
-		goto out;
-	fdt_load_addr = kbuf.mem;
-
-	pr_debug("Loaded device tree at 0x%lx\n", fdt_load_addr);
-
-	slave_code = elf_info.buffer + elf_info.proghdrs[0].p_offset;
-	ret = setup_purgatory(image, slave_code, fdt, kernel_load_addr,
-			      fdt_load_addr);
-	if (ret)
-		pr_err("Error setting up the purgatory.\n");
-
-out:
-	elf_free_info(&elf_info);
-
-	/* Make kimage_file_post_load_cleanup free the fdt buffer for us. */
-	return ret ? ERR_PTR(ret) : fdt;
-}
-
-const struct kexec_file_ops kexec_elf64_ops = {
-	.probe = elf64_probe,
-	.load = elf64_load,
-};
-- 
2.23.0.rc1


^ permalink raw reply related

* [PATCH v5 3/7] kexec_elf: remove parsing of section headers
From: Sven Schnelle @ 2019-08-23 19:49 UTC (permalink / raw)
  To: kexec; +Cc: Sven Schnelle, Helge Deller, linuxppc-dev, Thiago Jung Bauermann
In-Reply-To: <20190823194919.30916-1-svens@stackframe.org>

We're not using them, so we can drop the parsing.

Signed-off-by: Sven Schnelle <svens@stackframe.org>
Reviewed-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
 include/linux/kexec.h |   1 -
 kernel/kexec_elf.c    | 137 ------------------------------------------
 2 files changed, 138 deletions(-)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index da2a6b1d69e7..f0b809258ed3 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -226,7 +226,6 @@ struct kexec_elf_info {
 
 	const struct elfhdr *ehdr;
 	const struct elf_phdr *proghdrs;
-	struct elf_shdr *sechdrs;
 };
 
 int kexec_build_elf_info(const char *buf, size_t len, struct elfhdr *ehdr,
diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c
index 34376fbc55be..137037603117 100644
--- a/kernel/kexec_elf.c
+++ b/kernel/kexec_elf.c
@@ -257,134 +257,6 @@ static int elf_read_phdrs(const char *buf, size_t len,
 	return 0;
 }
 
-/**
- * elf_is_shdr_sane - check that it is safe to use the section header
- * @buf_len:	size of the buffer in which the ELF file is loaded.
- */
-static bool elf_is_shdr_sane(const struct elf_shdr *shdr, size_t buf_len)
-{
-	bool size_ok;
-
-	/* SHT_NULL headers have undefined values, so we can't check them. */
-	if (shdr->sh_type == SHT_NULL)
-		return true;
-
-	/* Now verify sh_entsize */
-	switch (shdr->sh_type) {
-	case SHT_SYMTAB:
-		size_ok = shdr->sh_entsize == sizeof(Elf_Sym);
-		break;
-	case SHT_RELA:
-		size_ok = shdr->sh_entsize == sizeof(Elf_Rela);
-		break;
-	case SHT_DYNAMIC:
-		size_ok = shdr->sh_entsize == sizeof(Elf_Dyn);
-		break;
-	case SHT_REL:
-		size_ok = shdr->sh_entsize == sizeof(Elf_Rel);
-		break;
-	case SHT_NOTE:
-	case SHT_PROGBITS:
-	case SHT_HASH:
-	case SHT_NOBITS:
-	default:
-		/*
-		 * This is a section whose entsize requirements
-		 * I don't care about.  If I don't know about
-		 * the section I can't care about it's entsize
-		 * requirements.
-		 */
-		size_ok = true;
-		break;
-	}
-
-	if (!size_ok) {
-		pr_debug("ELF section with wrong entry size.\n");
-		return false;
-	} else if (shdr->sh_addr + shdr->sh_size < shdr->sh_addr) {
-		pr_debug("ELF section address wraps around.\n");
-		return false;
-	}
-
-	if (shdr->sh_type != SHT_NOBITS) {
-		if (shdr->sh_offset + shdr->sh_size < shdr->sh_offset) {
-			pr_debug("ELF section location wraps around.\n");
-			return false;
-		} else if (shdr->sh_offset + shdr->sh_size > buf_len) {
-			pr_debug("ELF section not in file.\n");
-			return false;
-		}
-	}
-
-	return true;
-}
-
-static int elf_read_shdr(const char *buf, size_t len,
-			 struct kexec_elf_info *elf_info,
-			 int idx)
-{
-	struct elf_shdr *shdr = &elf_info->sechdrs[idx];
-	const struct elfhdr *ehdr = elf_info->ehdr;
-	const char *sbuf;
-	struct elf_shdr *buf_shdr;
-
-	sbuf = buf + ehdr->e_shoff + idx * sizeof(*buf_shdr);
-	buf_shdr = (struct elf_shdr *) sbuf;
-
-	shdr->sh_name      = elf32_to_cpu(ehdr, buf_shdr->sh_name);
-	shdr->sh_type      = elf32_to_cpu(ehdr, buf_shdr->sh_type);
-	shdr->sh_addr      = elf_addr_to_cpu(ehdr, buf_shdr->sh_addr);
-	shdr->sh_offset    = elf_addr_to_cpu(ehdr, buf_shdr->sh_offset);
-	shdr->sh_link      = elf32_to_cpu(ehdr, buf_shdr->sh_link);
-	shdr->sh_info      = elf32_to_cpu(ehdr, buf_shdr->sh_info);
-
-	/*
-	 * The following fields have a type equivalent to Elf_Addr
-	 * both in 32 bit and 64 bit ELF.
-	 */
-	shdr->sh_flags     = elf_addr_to_cpu(ehdr, buf_shdr->sh_flags);
-	shdr->sh_size      = elf_addr_to_cpu(ehdr, buf_shdr->sh_size);
-	shdr->sh_addralign = elf_addr_to_cpu(ehdr, buf_shdr->sh_addralign);
-	shdr->sh_entsize   = elf_addr_to_cpu(ehdr, buf_shdr->sh_entsize);
-
-	return elf_is_shdr_sane(shdr, len) ? 0 : -ENOEXEC;
-}
-
-/**
- * elf_read_shdrs - read the section headers from the buffer
- *
- * This function assumes that the section header table was checked for sanity.
- * Use elf_is_ehdr_sane() if it wasn't.
- */
-static int elf_read_shdrs(const char *buf, size_t len,
-			  struct kexec_elf_info *elf_info)
-{
-	size_t shdr_size, i;
-
-	/*
-	 * e_shnum is at most 65536 so calculating
-	 * the size of the section header cannot overflow.
-	 */
-	shdr_size = sizeof(struct elf_shdr) * elf_info->ehdr->e_shnum;
-
-	elf_info->sechdrs = kzalloc(shdr_size, GFP_KERNEL);
-	if (!elf_info->sechdrs)
-		return -ENOMEM;
-
-	for (i = 0; i < elf_info->ehdr->e_shnum; i++) {
-		int ret;
-
-		ret = elf_read_shdr(buf, len, elf_info, i);
-		if (ret) {
-			kfree(elf_info->sechdrs);
-			elf_info->sechdrs = NULL;
-			return ret;
-		}
-	}
-
-	return 0;
-}
-
 /**
  * elf_read_from_buffer - read ELF file and sets up ELF header and ELF info
  * @buf:	Buffer to read ELF file from.
@@ -417,14 +289,6 @@ static int elf_read_from_buffer(const char *buf, size_t len,
 		if (ret)
 			return ret;
 	}
-	if (ehdr->e_shoff > 0 && ehdr->e_shnum > 0) {
-		ret = elf_read_shdrs(buf, len, elf_info);
-		if (ret) {
-			kfree(elf_info->proghdrs);
-			return ret;
-		}
-	}
-
 	return 0;
 }
 
@@ -434,7 +298,6 @@ static int elf_read_from_buffer(const char *buf, size_t len,
 void kexec_free_elf_info(struct kexec_elf_info *elf_info)
 {
 	kfree(elf_info->proghdrs);
-	kfree(elf_info->sechdrs);
 	memset(elf_info, 0, sizeof(*elf_info));
 }
 /**
-- 
2.23.0.rc1


^ permalink raw reply related

* [PATCH v5 4/7] kexec_elf: remove PURGATORY_STACK_SIZE
From: Sven Schnelle @ 2019-08-23 19:49 UTC (permalink / raw)
  To: kexec; +Cc: Sven Schnelle, Helge Deller, linuxppc-dev, Thiago Jung Bauermann
In-Reply-To: <20190823194919.30916-1-svens@stackframe.org>

It's not used anywhere so just drop it.

Signed-off-by: Sven Schnelle <svens@stackframe.org>
Reviewed-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
 kernel/kexec_elf.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c
index 137037603117..87935bd5e2ba 100644
--- a/kernel/kexec_elf.c
+++ b/kernel/kexec_elf.c
@@ -21,8 +21,6 @@
 #include <linux/slab.h>
 #include <linux/types.h>
 
-#define PURGATORY_STACK_SIZE	(16 * 1024)
-
 #define elf_addr_to_cpu	elf64_to_cpu
 
 #ifndef Elf_Rel
-- 
2.23.0.rc1


^ permalink raw reply related

* [PATCH v5 7/7] kexec_elf: support 32 bit ELF files
From: Sven Schnelle @ 2019-08-23 19:49 UTC (permalink / raw)
  To: kexec; +Cc: Sven Schnelle, Helge Deller, linuxppc-dev, Thiago Jung Bauermann
In-Reply-To: <20190823194919.30916-1-svens@stackframe.org>

The powerpc version only supported 64 bit. Add some
code to switch decoding of fields during runtime so
we can kexec a 32 bit kernel from a 64 bit kernel and
vice versa.

Signed-off-by: Sven Schnelle <svens@stackframe.org>
Reviewed-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
 kernel/kexec_elf.c | 57 ++++++++++++++++++++++++++++++++++------------
 1 file changed, 42 insertions(+), 15 deletions(-)

diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c
index 85f2bd177d6e..d3689632e8b9 100644
--- a/kernel/kexec_elf.c
+++ b/kernel/kexec_elf.c
@@ -21,8 +21,6 @@
 #include <linux/slab.h>
 #include <linux/types.h>
 
-#define elf_addr_to_cpu	elf64_to_cpu
-
 static inline bool elf_is_elf_file(const struct elfhdr *ehdr)
 {
 	return memcmp(ehdr->e_ident, ELFMAG, SELFMAG) == 0;
@@ -152,9 +150,6 @@ static int elf_read_ehdr(const char *buf, size_t len, struct elfhdr *ehdr)
 	ehdr->e_type      = elf16_to_cpu(ehdr, buf_ehdr->e_type);
 	ehdr->e_machine   = elf16_to_cpu(ehdr, buf_ehdr->e_machine);
 	ehdr->e_version   = elf32_to_cpu(ehdr, buf_ehdr->e_version);
-	ehdr->e_entry     = elf_addr_to_cpu(ehdr, buf_ehdr->e_entry);
-	ehdr->e_phoff     = elf_addr_to_cpu(ehdr, buf_ehdr->e_phoff);
-	ehdr->e_shoff     = elf_addr_to_cpu(ehdr, buf_ehdr->e_shoff);
 	ehdr->e_flags     = elf32_to_cpu(ehdr, buf_ehdr->e_flags);
 	ehdr->e_phentsize = elf16_to_cpu(ehdr, buf_ehdr->e_phentsize);
 	ehdr->e_phnum     = elf16_to_cpu(ehdr, buf_ehdr->e_phnum);
@@ -162,6 +157,24 @@ static int elf_read_ehdr(const char *buf, size_t len, struct elfhdr *ehdr)
 	ehdr->e_shnum     = elf16_to_cpu(ehdr, buf_ehdr->e_shnum);
 	ehdr->e_shstrndx  = elf16_to_cpu(ehdr, buf_ehdr->e_shstrndx);
 
+	switch (ehdr->e_ident[EI_CLASS]) {
+	case ELFCLASS64:
+		ehdr->e_entry = elf64_to_cpu(ehdr, buf_ehdr->e_entry);
+		ehdr->e_phoff = elf64_to_cpu(ehdr, buf_ehdr->e_phoff);
+		ehdr->e_shoff = elf64_to_cpu(ehdr, buf_ehdr->e_shoff);
+		break;
+
+	case ELFCLASS32:
+		ehdr->e_entry = elf32_to_cpu(ehdr, buf_ehdr->e_entry);
+		ehdr->e_phoff = elf32_to_cpu(ehdr, buf_ehdr->e_phoff);
+		ehdr->e_shoff = elf32_to_cpu(ehdr, buf_ehdr->e_shoff);
+		break;
+
+	default:
+		pr_debug("Unknown ELF class.\n");
+		return -EINVAL;
+	}
+
 	return elf_is_ehdr_sane(ehdr, len) ? 0 : -ENOEXEC;
 }
 
@@ -192,6 +205,7 @@ static int elf_read_phdr(const char *buf, size_t len,
 {
 	/* Override the const in proghdrs, we are the ones doing the loading. */
 	struct elf_phdr *phdr = (struct elf_phdr *) &elf_info->proghdrs[idx];
+	const struct elfhdr *ehdr = elf_info->ehdr;
 	const char *pbuf;
 	struct elf_phdr *buf_phdr;
 
@@ -199,18 +213,31 @@ static int elf_read_phdr(const char *buf, size_t len,
 	buf_phdr = (struct elf_phdr *) pbuf;
 
 	phdr->p_type   = elf32_to_cpu(elf_info->ehdr, buf_phdr->p_type);
-	phdr->p_offset = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_offset);
-	phdr->p_paddr  = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_paddr);
-	phdr->p_vaddr  = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_vaddr);
 	phdr->p_flags  = elf32_to_cpu(elf_info->ehdr, buf_phdr->p_flags);
 
-	/*
-	 * The following fields have a type equivalent to Elf_Addr
-	 * both in 32 bit and 64 bit ELF.
-	 */
-	phdr->p_filesz = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_filesz);
-	phdr->p_memsz  = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_memsz);
-	phdr->p_align  = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_align);
+	switch (ehdr->e_ident[EI_CLASS]) {
+	case ELFCLASS64:
+		phdr->p_offset = elf64_to_cpu(ehdr, buf_phdr->p_offset);
+		phdr->p_paddr  = elf64_to_cpu(ehdr, buf_phdr->p_paddr);
+		phdr->p_vaddr  = elf64_to_cpu(ehdr, buf_phdr->p_vaddr);
+		phdr->p_filesz = elf64_to_cpu(ehdr, buf_phdr->p_filesz);
+		phdr->p_memsz  = elf64_to_cpu(ehdr, buf_phdr->p_memsz);
+		phdr->p_align  = elf64_to_cpu(ehdr, buf_phdr->p_align);
+		break;
+
+	case ELFCLASS32:
+		phdr->p_offset = elf32_to_cpu(ehdr, buf_phdr->p_offset);
+		phdr->p_paddr  = elf32_to_cpu(ehdr, buf_phdr->p_paddr);
+		phdr->p_vaddr  = elf32_to_cpu(ehdr, buf_phdr->p_vaddr);
+		phdr->p_filesz = elf32_to_cpu(ehdr, buf_phdr->p_filesz);
+		phdr->p_memsz  = elf32_to_cpu(ehdr, buf_phdr->p_memsz);
+		phdr->p_align  = elf32_to_cpu(ehdr, buf_phdr->p_align);
+		break;
+
+	default:
+		pr_debug("Unknown ELF class.\n");
+		return -EINVAL;
+	}
 
 	return elf_is_phdr_sane(phdr, len) ? 0 : -ENOEXEC;
 }
-- 
2.23.0.rc1


^ permalink raw reply related

* [PATCH v5 5/7] kexec_elf: remove Elf_Rel macro
From: Sven Schnelle @ 2019-08-23 19:49 UTC (permalink / raw)
  To: kexec; +Cc: Sven Schnelle, Helge Deller, linuxppc-dev, Thiago Jung Bauermann
In-Reply-To: <20190823194919.30916-1-svens@stackframe.org>

It wasn't used anywhere, so lets drop it.

Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>
Reviewed-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
 kernel/kexec_elf.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c
index 87935bd5e2ba..6c806ce96ac1 100644
--- a/kernel/kexec_elf.c
+++ b/kernel/kexec_elf.c
@@ -23,10 +23,6 @@
 
 #define elf_addr_to_cpu	elf64_to_cpu
 
-#ifndef Elf_Rel
-#define Elf_Rel		Elf64_Rel
-#endif /* Elf_Rel */
-
 static inline bool elf_is_elf_file(const struct elfhdr *ehdr)
 {
 	return memcmp(ehdr->e_ident, ELFMAG, SELFMAG) == 0;
-- 
2.23.0.rc1


^ permalink raw reply related

* Re: [PATCH kernel] vfio/spapr_tce: Fix incorrect tce_iommu_group memory free
From: Alex Williamson @ 2019-08-23 20:44 UTC (permalink / raw)
  To: Alexey Kardashevskiy
  Cc: Jose Ricardo Ziviani, linuxppc-dev, kvm, kvm-ppc, David Gibson
In-Reply-To: <20190819015117.94878-1-aik@ozlabs.ru>

On Mon, 19 Aug 2019 11:51:17 +1000
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> The @tcegrp variable is used in 1) a loop over attached groups
> 2) it stores a pointer to a newly allocated tce_iommu_group if 1) found
> nothing. However the error handler does not distinguish how we got there
> and incorrectly releases memory for a found+incompatible group.
> 
> This fixes it by adding another error handling case.
> 
> Fixes: 0bd971676e68 ("powerpc/powernv/npu: Add compound IOMMU groups")
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---

Applied to vfio next branch with Paul's R-b.  Thanks,

Alex

> 
> The bug is there since 2157e7b82f3b but it would not appear in practice
> before 0bd971676e68, hence that "Fixes". Or it still should be
> 157e7b82f3b ("vfio: powerpc/spapr: Register memory and define IOMMU v2")
> ?
> 
> Found it when tried adding a "compound PE" (GPU + NPUs) to a container
> with a passed through xHCI host. The compatibility test (->create_table
> should be equal) treats them as incompatible which might a bug (or
> we are just suboptimal here) on its own.
> 
> ---
>  drivers/vfio/vfio_iommu_spapr_tce.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
> index 8ce9ad21129f..babef8b00daf 100644
> --- a/drivers/vfio/vfio_iommu_spapr_tce.c
> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
> @@ -1234,7 +1234,7 @@ static long tce_iommu_take_ownership_ddw(struct tce_container *container,
>  static int tce_iommu_attach_group(void *iommu_data,
>  		struct iommu_group *iommu_group)
>  {
> -	int ret;
> +	int ret = 0;
>  	struct tce_container *container = iommu_data;
>  	struct iommu_table_group *table_group;
>  	struct tce_iommu_group *tcegrp = NULL;
> @@ -1287,13 +1287,13 @@ static int tce_iommu_attach_group(void *iommu_data,
>  			!table_group->ops->release_ownership) {
>  		if (container->v2) {
>  			ret = -EPERM;
> -			goto unlock_exit;
> +			goto free_exit;
>  		}
>  		ret = tce_iommu_take_ownership(container, table_group);
>  	} else {
>  		if (!container->v2) {
>  			ret = -EPERM;
> -			goto unlock_exit;
> +			goto free_exit;
>  		}
>  		ret = tce_iommu_take_ownership_ddw(container, table_group);
>  		if (!tce_groups_attached(container) && !container->tables[0])
> @@ -1305,10 +1305,11 @@ static int tce_iommu_attach_group(void *iommu_data,
>  		list_add(&tcegrp->next, &container->group_list);
>  	}
>  
> -unlock_exit:
> +free_exit:
>  	if (ret && tcegrp)
>  		kfree(tcegrp);
>  
> +unlock_exit:
>  	mutex_unlock(&container->lock);
>  
>  	return ret;


^ permalink raw reply

* Re: cleanup the dma_pgprot handling
From: Paul Burton @ 2019-08-23 21:58 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Shawn Anastasio, linux-m68k@lists.linux-m68k.org, Will Deacon,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	Russell King, linux-mips@vger.kernel.org,
	iommu@lists.linux-foundation.org, Geert Uytterhoeven,
	Catalin Marinas, James Hogan, Guan Xuetao,
	linux-arm-kernel@lists.infradead.org, Robin Murphy
In-Reply-To: <20190816070754.15653-1-hch@lst.de>

Hi Christoph,

On Fri, Aug 16, 2019 at 09:07:48AM +0200, Christoph Hellwig wrote:
> I'd still like to hear a confirmation from the mips folks how
> the write combibe attribute can or can't work with the KSEG1
> uncached segment.

Quoting section 4.8 "Cacheability and Coherency Attributes and Access
Types" of "MIPS Architecture Volume 1: Introduction to the MIPS32
Architecture" (MD00080, revision 6.01):

https://www.mips.com/?do-download=introduction-to-the-mips32-architecture-v6-01

> Memory access types are specified by architecturally-defined and
> implementation-specific Cacheability and Coherency Attribute bits
> (CCAs) generated by the MMU for the access.
>
> Slightly different cacheability and coherency attributes such as
> “cached coherent, update on write” and “cached coherent, exclusive on
> write” can map to the same memory access type; in this case they both
> map to cached coherent. In order to map to the same access type, the
> fundamental mechanisms of both CCAs must be the same.
>
> When the operation of the instruction is affected, the instructions
> are described in terms of memory access types. The load and store
> operations in a processor proceed according to the specific CCA of the
> reference, however, and the pseudocode for load and store common
> functions uses the CCA value rather than the corresponding memory
> access type.

So I believe uncached & uncached accelerated are another case like that
described above - they're 2 different CCAs but the same "access type",
namely uncached.

Section 4.9 then goes on to forbid mixing access types, but not CCAs.

It would be nice if the precise mapping from CCA to access type was
provided, but I don't see that anywhere. I can check with the
architecture team to be sure, but to my knowledge we're fine to mix
access via kseg1 (ie. uncached) & mappings with CCA=7 (uncached
accelerated).

Thanks,
    Paul

^ permalink raw reply

* RE: [PATCH v2 01/10] PCI: designware-ep: Add multiple PFs support for DWC
From: Xiaowei Bao @ 2019-08-23 23:50 UTC (permalink / raw)
  To: Andrew Murray
  Cc: mark.rutland@arm.com, Roy Zang, lorenzo.pieralisi@arm.co,
	arnd@arndb.de, devicetree@vger.kernel.org,
	gregkh@linuxfoundation.org, linuxppc-dev@lists.ozlabs.org,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	kishon@ti.com, M.h. Lian, robh+dt@kernel.org,
	gustavo.pimentel@synopsys.com, jingoohan1@gmail.com,
	bhelgaas@google.com, Leo Li, shawnguo@kernel.org, Mingkai Hu,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190823132526.GD14582@e119886-lin.cambridge.arm.com>



> -----Original Message-----
> From: Andrew Murray <andrew.murray@arm.com>
> Sent: 2019年8月23日 21:25
> To: Xiaowei Bao <xiaowei.bao@nxp.com>
> Cc: bhelgaas@google.com; robh+dt@kernel.org; mark.rutland@arm.com;
> shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>; kishon@ti.com;
> lorenzo.pieralisi@arm.co; arnd@arndb.de; gregkh@linuxfoundation.org; M.h.
> Lian <minghuan.lian@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>; Roy
> Zang <roy.zang@nxp.com>; jingoohan1@gmail.com;
> gustavo.pimentel@synopsys.com; linux-pci@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH v2 01/10] PCI: designware-ep: Add multiple PFs support
> for DWC
> 
> On Thu, Aug 22, 2019 at 07:22:33PM +0800, Xiaowei Bao wrote:
> > Add multiple PFs support for DWC, different PF have different config
> > space we use pf-offset property which get from the DTS to access the
> > different pF config space.
> 
> It looks like you're missing a --cover-letter again.
> 
> >
> > Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> > ---
> > v2:
> >  - Remove duplicate redundant code.
> >  - Reimplement the PF config space access way.
> >
> >  drivers/pci/controller/dwc/pcie-designware-ep.c | 122
> ++++++++++++++++--------
> >  drivers/pci/controller/dwc/pcie-designware.c    |  59 ++++++++----
> >  drivers/pci/controller/dwc/pcie-designware.h    |  11 ++-
> >  3 files changed, 134 insertions(+), 58 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > index 2bf5a35..3e2b740 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > @@ -19,12 +19,17 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
> >  	pci_epc_linkup(epc);
> >  }
> >
> > -static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno
> bar,
> > -				   int flags)
> > +static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, u8 func_no,
> > +				   enum pci_barno bar, int flags)
> >  {
> >  	u32 reg;
> > +	unsigned int func_offset = 0;
> > +	struct dw_pcie_ep *ep = &pci->ep;
> >
> > -	reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> > +	if (ep->ops->func_conf_select)
> > +		func_offset = ep->ops->func_conf_select(ep, func_no);
> > +
> > +	reg = func_offset + PCI_BASE_ADDRESS_0 + (4 * bar);
> 
> This pattern of checking if func_conf_select exists and using it to get an offset
> is repeated a lot throughout this file. You could move this functionality into a
> new function (similar to dw_pcie_read_dbi etc). Or perhaps a new variant of
> dw_pcie_writel_ should be created that writes takes a func_no argument.

Thanks for your comments, I thought about this method before, but there is a issue
about the method of access the different func config space, due to our platform use
this method that different func have different offset from dbi_base to access the
different config space, but others platform maybe use the way that write a register
to implement different func config space access, so I think reserve a callback function 
to different platform to implement the own method, my point is that, if use register 
method they can implement the code in this function and return offset is 0, if use 
offset method, they can return the offset value which can be use by dw_pcie_ep driver.
 
> 
> 
> >  	dw_pcie_dbi_ro_wr_en(pci);
> >  	dw_pcie_writel_dbi2(pci, reg, 0x0);
> >  	dw_pcie_writel_dbi(pci, reg, 0x0);
> 
> 
> > @@ -235,7 +257,7 @@ static int dw_pcie_ep_map_addr(struct pci_epc
> *epc, u8 func_no,
> >  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> >  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> >
> > -	ret = dw_pcie_ep_outbound_atu(ep, addr, pci_addr, size);
> > +	ret = dw_pcie_ep_outbound_atu(ep, func_no, addr, pci_addr, size);
> >  	if (ret) {
> >  		dev_err(pci->dev, "Failed to enable address\n");
> >  		return ret;
> > @@ -249,11 +271,15 @@ static int dw_pcie_ep_get_msi(struct pci_epc
> *epc, u8 func_no)
> >  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> >  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> >  	u32 val, reg;
> > +	unsigned int func_offset = 0;
> > +
> > +	if (ep->ops->func_conf_select)
> > +		func_offset = ep->ops->func_conf_select(ep, func_no);
> >
> >  	if (!ep->msi_cap)
> >  		return -EINVAL;
> >
> > -	reg = ep->msi_cap + PCI_MSI_FLAGS;
> > +	reg = ep->msi_cap + func_offset + PCI_MSI_FLAGS;
> 
> This makes me nervous.
> 
> From a PCI viewpoint, each function has it's own capability structure and
> within each function there may exist a MSI capability. Yet what we're doing
> here is using dw_pcie_ep_find_capability to get the list of capabilities for
> function 0, and then applying offsets from that for subsequent functions. I.e.
> we're applying DW specific knowledge to find the correct capability, rather
> than following the general PCI approach.
> 
> I think the above hunk shouldn't be required - but instead
> dw_pcie_ep_find_capability is updated to take a func_no parameter.
> 
> Have I understood this correctly?

Yes, this is a issue, I think the different func maybe have different capability,
but the dw_pcie_ep_find_capability function is called by dw_pcie_ep_init 
function, we can't add func_no parameter to dw_pcie_ep_find_capability,
I will try to fix it use a new patch, I think move this function to ep_init callback
function If better, thanks. 


> 
> >  	val = dw_pcie_readw_dbi(pci, reg);
> >  	if (!(val & PCI_MSI_FLAGS_ENABLE))
> >  		return -EINVAL;
> > @@ -268,11 +294,15 @@ static int dw_pcie_ep_set_msi(struct pci_epc
> *epc, u8 func_no, u8 interrupts)
> >  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> >  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> >  	u32 val, reg;
> > +	unsigned int func_offset = 0;
> > +
> > +	if (ep->ops->func_conf_select)
> > +		func_offset = ep->ops->func_conf_select(ep, func_no);
> >
> >  	if (!ep->msi_cap)
> >  		return -EINVAL;
> >
> > -	reg = ep->msi_cap + PCI_MSI_FLAGS;
> > +	reg = ep->msi_cap + func_offset + PCI_MSI_FLAGS;
> >  	val = dw_pcie_readw_dbi(pci, reg);
> >  	val &= ~PCI_MSI_FLAGS_QMASK;
> >  	val |= (interrupts << 1) & PCI_MSI_FLAGS_QMASK; @@ -288,11 +318,15
> > @@ static int dw_pcie_ep_get_msix(struct pci_epc *epc, u8 func_no)
> >  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> >  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> >  	u32 val, reg;
> > +	unsigned int func_offset = 0;
> > +
> > +	if (ep->ops->func_conf_select)
> > +		func_offset = ep->ops->func_conf_select(ep, func_no);
> >
> >  	if (!ep->msix_cap)
> >  		return -EINVAL;
> >
> > -	reg = ep->msix_cap + PCI_MSIX_FLAGS;
> > +	reg = ep->msix_cap + func_offset + PCI_MSIX_FLAGS;
> 
> Same for MSIX.

Yes.

> 
> >  	val = dw_pcie_readw_dbi(pci, reg);
> >  	if (!(val & PCI_MSIX_FLAGS_ENABLE))
> >  		return -EINVAL;
> > @@ -307,11 +341,15 @@ static int dw_pcie_ep_set_msix(struct pci_epc
> *epc, u8 func_no, u16 interrupts)
> >  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> >  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> >  	u32 val, reg;
> > +	unsigned int func_offset = 0;
> > +
> > +	if (ep->ops->func_conf_select)
> > +		func_offset = ep->ops->func_conf_select(ep, func_no);
> >
> >  	if (!ep->msix_cap)
> >  		return -EINVAL;
> >
> > -	reg = ep->msix_cap + PCI_MSIX_FLAGS;
> > +	reg = ep->msix_cap + func_offset + PCI_MSIX_FLAGS;
> >  	val = dw_pcie_readw_dbi(pci, reg);
> >  	val &= ~PCI_MSIX_FLAGS_QSIZE;
> >  	val |= interrupts;
> > @@ -398,29 +436,33 @@ int dw_pcie_ep_raise_msi_irq(struct
> dw_pcie_ep *ep, u8 func_no,
> >  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> >  	struct pci_epc *epc = ep->epc;
> >  	unsigned int aligned_offset;
> > +	unsigned int func_offset = 0;
> >  	u16 msg_ctrl, msg_data;
> >  	u32 msg_addr_lower, msg_addr_upper, reg;
> >  	u64 msg_addr;
> >  	bool has_upper;
> >  	int ret;
> >
> > +	if (ep->ops->func_conf_select)
> > +		func_offset = ep->ops->func_conf_select(ep, func_no);
> > +
> 
> You could probably move this hunk below the test for msi_cap to save some
> cycles.

Sorry, I didn't understand the means, please explain it detailly, thanks a lot, ^_^
> 
> >  	if (!ep->msi_cap)
> >  		return -EINVAL;
> >
> >  	/* Raise MSI per the PCI Local Bus Specification Revision 3.0, 6.8.1. */
> > -	reg = ep->msi_cap + PCI_MSI_FLAGS;
> > +	reg = ep->msi_cap + func_offset + PCI_MSI_FLAGS;
> >  	msg_ctrl = dw_pcie_readw_dbi(pci, reg);
> >  	has_upper = !!(msg_ctrl & PCI_MSI_FLAGS_64BIT);
> > -	reg = ep->msi_cap + PCI_MSI_ADDRESS_LO;
> > +	reg = ep->msi_cap + func_offset + PCI_MSI_ADDRESS_LO;
> >  	msg_addr_lower = dw_pcie_readl_dbi(pci, reg);
> >  	if (has_upper) {
> > -		reg = ep->msi_cap + PCI_MSI_ADDRESS_HI;
> > +		reg = ep->msi_cap + func_offset + PCI_MSI_ADDRESS_HI;
> >  		msg_addr_upper = dw_pcie_readl_dbi(pci, reg);
> > -		reg = ep->msi_cap + PCI_MSI_DATA_64;
> > +		reg = ep->msi_cap + func_offset + PCI_MSI_DATA_64;
> >  		msg_data = dw_pcie_readw_dbi(pci, reg);
> >  	} else {
> >  		msg_addr_upper = 0;
> > -		reg = ep->msi_cap + PCI_MSI_DATA_32;
> > +		reg = ep->msi_cap + func_offset + PCI_MSI_DATA_32;
> >  		msg_data = dw_pcie_readw_dbi(pci, reg);
> >  	}
> >  	aligned_offset = msg_addr_lower & (epc->mem->page_size - 1);
> 
> 
> 
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c
> > b/drivers/pci/controller/dwc/pcie-designware.c
> > index 7d25102..305e73d 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -158,9 +158,10 @@ static void dw_pcie_writel_ob_unroll(struct
> dw_pcie *pci, u32 index, u32 reg,
> >  	dw_pcie_writel_atu(pci, offset + reg, val);  }
> >
> > -static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, int
> index,
> > -					     int type, u64 cpu_addr,
> > -					     u64 pci_addr, u32 size)
> > +static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8
> func_no,
> > +					     int index, int type,
> > +					     u64 cpu_addr, u64 pci_addr,
> > +					     u32 size)
> >  {
> >  	u32 retries, val;
> >
> > @@ -175,7 +176,7 @@ static void
> dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, int index,
> >  	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_TARGET,
> >  				 upper_32_bits(pci_addr));
> >  	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1,
> > -				 type);
> > +				 type | PCIE_ATU_FUNC_NUM(func_no));
> 
> Much better :)

Do you mean that use the expression "a? b:c"

> 
> >  	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL2,
> >  				 PCIE_ATU_ENABLE);
> >
> > @@ -194,8 +195,9 @@ static void
> dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, int index,
> >  	dev_err(pci->dev, "Outbound iATU is not being enabled\n");  }
> >
> > -void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
> > -			       u64 cpu_addr, u64 pci_addr, u32 size)
> > +static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8
> func_no,
> > +					int index, int type, u64 cpu_addr,
> > +					u64 pci_addr, u32 size)
> >  {
> >  	u32 retries, val;
> >
> > @@ -203,8 +205,8 @@ void dw_pcie_prog_outbound_atu(struct dw_pcie
> *pci, int index, int type,
> >  		cpu_addr = pci->ops->cpu_addr_fixup(pci, cpu_addr);
> >
> >  	if (pci->iatu_unroll_enabled) {
> > -		dw_pcie_prog_outbound_atu_unroll(pci, index, type, cpu_addr,
> > -						 pci_addr, size);
> > +		dw_pcie_prog_outbound_atu_unroll(pci, func_no, index, type,
> > +						 cpu_addr, pci_addr, size);
> >  		return;
> >  	}
> >
> 
> 
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h
> > b/drivers/pci/controller/dwc/pcie-designware.h
> > index ffed084..a0fdbf7 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -71,9 +71,11 @@
> >  #define PCIE_ATU_TYPE_IO		0x2
> >  #define PCIE_ATU_TYPE_CFG0		0x4
> >  #define PCIE_ATU_TYPE_CFG1		0x5
> > +#define PCIE_ATU_FUNC_NUM(pf)           (pf << 20)
> 
> "Macro argument 'pf' may be better as '(pf)' to avoid precedence issues"
> 
> >  #define PCIE_ATU_CR2			0x908
> >  #define PCIE_ATU_ENABLE			BIT(31)
> >  #define PCIE_ATU_BAR_MODE_ENABLE	BIT(30)
> > +#define PCIE_ATU_FUNC_NUM_MATCH_EN      BIT(19)
> >  #define PCIE_ATU_LOWER_BASE		0x90C
> >  #define PCIE_ATU_UPPER_BASE		0x910
> >  #define PCIE_ATU_LIMIT			0x914
> > @@ -197,6 +199,7 @@ struct dw_pcie_ep_ops {
> >  	int	(*raise_irq)(struct dw_pcie_ep *ep, u8 func_no,
> >  			     enum pci_epc_irq_type type, u16 interrupt_num);
> >  	const struct pci_epc_features* (*get_features)(struct dw_pcie_ep
> > *ep);
> > +	unsigned int (*func_conf_select)(struct dw_pcie_ep *ep, u8 func_no);
> 
> Given that this function will return an offset, I'm not sure the name you have
> is suitable. Something like get_pf_offset or similar is more descriptive.

As above explain, my initial view is that this function can return 0 or offset depends on
the platform implement mechanism, so I named it func_conf_select, I think add a
comment for this function, like this:
/*
 * provide a method to implement the method of different func config space access,
 * if use offset method, return the offset from dbi_base, if your register method, implement
 * the code in this callback function and return 0.
 */
How about it?

> 
> Thanks,
> 
> Andrew Murray
> 
> >  };
> >
> >  struct dw_pcie_ep {
> > @@ -265,8 +268,12 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci);
> > void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
> >  			       int type, u64 cpu_addr, u64 pci_addr,
> >  			       u32 size);
> > -int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, int index, int bar,
> > -			     u64 cpu_addr, enum dw_pcie_as_type as_type);
> > +void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int
> index,
> > +				  int type, u64 cpu_addr, u64 pci_addr,
> > +				  u32 size);
> > +int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> > +			     int bar, u64 cpu_addr,
> > +			     enum dw_pcie_as_type as_type);
> >  void dw_pcie_disable_atu(struct dw_pcie *pci, int index,
> >  			 enum dw_pcie_region_type type);
> >  void dw_pcie_setup(struct dw_pcie *pci);
> > --
> > 2.9.5
> >

^ permalink raw reply

* RE: [PATCH v2 02/10] PCI: designware-ep: Add the doorbell mode of MSI-X in EP mode
From: Xiaowei Bao @ 2019-08-23 23:51 UTC (permalink / raw)
  To: Andrew Murray
  Cc: mark.rutland@arm.com, Roy Zang, lorenzo.pieralisi@arm.co,
	arnd@arndb.de, devicetree@vger.kernel.org,
	gregkh@linuxfoundation.org, linuxppc-dev@lists.ozlabs.org,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	kishon@ti.com, M.h. Lian, robh+dt@kernel.org,
	gustavo.pimentel@synopsys.com, jingoohan1@gmail.com,
	bhelgaas@google.com, Leo Li, shawnguo@kernel.org, Mingkai Hu,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190823133540.GE14582@e119886-lin.cambridge.arm.com>



> -----Original Message-----
> From: Andrew Murray <andrew.murray@arm.com>
> Sent: 2019年8月23日 21:36
> To: Xiaowei Bao <xiaowei.bao@nxp.com>
> Cc: bhelgaas@google.com; robh+dt@kernel.org; mark.rutland@arm.com;
> shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>; kishon@ti.com;
> lorenzo.pieralisi@arm.co; arnd@arndb.de; gregkh@linuxfoundation.org; M.h.
> Lian <minghuan.lian@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>; Roy
> Zang <roy.zang@nxp.com>; jingoohan1@gmail.com;
> gustavo.pimentel@synopsys.com; linux-pci@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH v2 02/10] PCI: designware-ep: Add the doorbell mode of
> MSI-X in EP mode
> 
> On Thu, Aug 22, 2019 at 07:22:34PM +0800, Xiaowei Bao wrote:
> > Add the doorbell mode of MSI-X in EP mode.
> >
> > Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> > ---
> > v2:
> >  - Remove the macro of no used.
> >
> >  drivers/pci/controller/dwc/pcie-designware-ep.c | 14 ++++++++++++++
> >  drivers/pci/controller/dwc/pcie-designware.h    | 12 ++++++++++++
> >  2 files changed, 26 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > index 3e2b740..b8388f8 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > @@ -480,6 +480,20 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep
> *ep, u8 func_no,
> >  	return 0;
> >  }
> >
> > +int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8
> func_no,
> > +				       u16 interrupt_num)
> > +{
> > +	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > +	u32 msg_data;
> > +
> > +	msg_data = (func_no << PCIE_MSIX_DOORBELL_PF_SHIFT) |
> > +		   (interrupt_num - 1);
> > +
> > +	dw_pcie_writel_dbi(pci, PCIE_MSIX_DOORBELL, msg_data);
> > +
> > +	return 0;
> > +}
> > +
> >  int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> >  			      u16 interrupt_num)
> >  {
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h
> > b/drivers/pci/controller/dwc/pcie-designware.h
> > index a0fdbf7..895a9ef 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -88,6 +88,9 @@
> >  #define PCIE_MISC_CONTROL_1_OFF		0x8BC
> >  #define PCIE_DBI_RO_WR_EN		BIT(0)
> >
> > +#define PCIE_MSIX_DOORBELL		0x948
> > +#define PCIE_MSIX_DOORBELL_PF_SHIFT	24
> > +
> >  /*
> >   * iATU Unroll-specific register definitions
> >   * From 4.80 core version the address translation will be made by
> > unroll @@ -400,6 +403,8 @@ int dw_pcie_ep_raise_msi_irq(struct
> dw_pcie_ep *ep, u8 func_no,
> >  			     u8 interrupt_num);
> >  int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> >  			     u16 interrupt_num);
> > +int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8
> func_no,
> > +				       u16 interrupt_num);
> >  void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar);
> > #else  static inline void dw_pcie_ep_linkup(struct dw_pcie_ep *ep) @@
> > -432,6 +437,13 @@ static inline int dw_pcie_ep_raise_msix_irq(struct
> dw_pcie_ep *ep, u8 func_no,
> >  	return 0;
> >  }
> >
> > +static inline int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep
> *ep,
> > +						     u8 func_no,
> > +						     u16 interrupt_num)
> > +{
> > +	return 0;
> > +}
> > +
> 
> Looks OK to me.
> 
> Reviewed-by: Andrew Murray <andrew.murray@arm.com>

Thanks a lot.

> 
> >  static inline void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum
> > pci_barno bar)  {  }
> > --
> > 2.9.5
> >

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox