diff mbox series

[2/2] PCI: rcar: Return all Fs from read which triggered an exception

Message ID 20220116022549.456486-2-marek.vasut@gmail.com (mailing list archive)
State Superseded
Headers show
Series [1/2] PCI: rcar: Finish transition to L1 state in rcar_pcie_config_access() | expand

Commit Message

Marek Vasut Jan. 16, 2022, 2:25 a.m. UTC 
From: Marek Vasut <marek.vasut+renesas@gmail.com>

In case the controller is transitioning to L1 in rcar_pcie_config_access(),
any read/write access to PCIECDR triggers asynchronous external abort. This
is because the transition to L1 link state must be manually finished by the
driver. The PCIe IP can transition back from L1 state to L0 on its own.

The current asynchronous external abort hook implementation restarts
the instruction which finally triggered the fault, which can be a
different instruction than the read/write instruction which started
the faulting access. Usually the instruction which finally triggers
the fault is one which has some data dependency on the result of the
read/write. In case of read, the read value after fixup is undefined,
while a read value of faulting read should be all Fs.

It is possible to enforce the fault using 'isb' instruction placed
right after the read/write instruction which started the faulting
access. Add custom register accessors which perform the read/write
followed immediately by 'isb'.

This way, the fault always happens on the 'isb' and in case of read,
which is located one instruction before the 'isb', it is now possible
to fix up the return value of the read in the asynchronous external
abort hook and make that read return all Fs.

Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Krzysztof Wilczyński <kw@linux.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: linux-renesas-soc@vger.kernel.org
---
 drivers/pci/controller/pcie-rcar-host.c | 67 ++++++++++++++++++++++++-
 1 file changed, 65 insertions(+), 2 deletions(-)

Comments

kernel test robot Jan. 16, 2022, 6:30 p.m. UTC | #1 
Hi,

I love your patch! Perhaps something to improve:

[auto build test WARNING on helgaas-pci/next]
[also build test WARNING on next-20220116]
[cannot apply to v5.16]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/marek-vasut-gmail-com/PCI-rcar-Finish-transition-to-L1-state-in-rcar_pcie_config_access/20220116-102631
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: arm-randconfig-c002-20220116 (https://download.01.org/0day-ci/archive/20220117/202201170248.g8zqyL3O-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/f784bebf4a058d633fc77579892309b12dd33edb
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review marek-vasut-gmail-com/PCI-rcar-Finish-transition-to-L1-state-in-rcar_pcie_config_access/20220116-102631
        git checkout f784bebf4a058d633fc77579892309b12dd33edb
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arm SHELL=/bin/bash drivers/pci/controller/

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

All warnings (new ones prefixed by >>):

>> drivers/pci/controller/pcie-rcar-host.c:109:6: warning: no previous prototype for 'rcar_pci_write_reg_workaround' [-Wmissing-prototypes]
     109 | void rcar_pci_write_reg_workaround(struct rcar_pcie *pcie, u32 val, unsigned int reg)
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/pci/controller/pcie-rcar-host.c:121:5: warning: no previous prototype for 'rcar_pci_read_reg_workaround' [-Wmissing-prototypes]
     121 | u32 rcar_pci_read_reg_workaround(struct rcar_pcie *pcie, unsigned int reg)
         |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   /tmp/cc0JJaba.s: Assembler messages:
   /tmp/cc0JJaba.s:1425: Error: selected processor does not support `isb' in ARM mode
   /tmp/cc0JJaba.s:1486: Error: selected processor does not support `isb' in ARM mode
   /tmp/cc0JJaba.s:2594: Error: selected processor does not support `isb' in ARM mode
   /tmp/cc0JJaba.s:2622: Error: symbol `rcar_pci_read_reg_workaround_start' is already defined
   /tmp/cc0JJaba.s:2624: Error: selected processor does not support `isb' in ARM mode


vim +/rcar_pci_write_reg_workaround +109 drivers/pci/controller/pcie-rcar-host.c

   108	
 > 109	void rcar_pci_write_reg_workaround(struct rcar_pcie *pcie, u32 val, unsigned int reg)
   110	{
   111	#ifdef CONFIG_ARM
   112		asm volatile(
   113			"	str %0, [%1]\n"
   114			"	isb\n"
   115		::"r"(val), "r"(pcie->base + reg):"memory");
   116	#else
   117		rcar_pci_write_reg(pcie, val, reg);
   118	#endif
   119	}
   120	
 > 121	u32 rcar_pci_read_reg_workaround(struct rcar_pcie *pcie, unsigned int reg)
   122	{
   123	#ifdef CONFIG_ARM
   124		u32 val;
   125	
   126		asm volatile(
   127			"rcar_pci_read_reg_workaround_start:\n"
   128			"1:	ldr %0, [%1]\n"
   129			"	isb\n"
   130		: "=r"(val):"r"(pcie->base + reg):"memory");
   131	
   132		return val;
   133	#else
   134		return rcar_pci_read_reg(pcie, reg);
   135	#endif
   136	}
   137	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot Jan. 16, 2022, 6:51 p.m. UTC | #2 
Hi,

I love your patch! Perhaps something to improve:

[auto build test WARNING on helgaas-pci/next]
[also build test WARNING on next-20220116]
[cannot apply to v5.16]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/marek-vasut-gmail-com/PCI-rcar-Finish-transition-to-L1-state-in-rcar_pcie_config_access/20220116-102631
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: arm-randconfig-c002-20220116 (https://download.01.org/0day-ci/archive/20220117/202201170230.U9LeY58M-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project c63a3175c2947e8c1a2d3bbe16a8586600705c54)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/0day-ci/linux/commit/f784bebf4a058d633fc77579892309b12dd33edb
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review marek-vasut-gmail-com/PCI-rcar-Finish-transition-to-L1-state-in-rcar_pcie_config_access/20220116-102631
        git checkout f784bebf4a058d633fc77579892309b12dd33edb
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/pci/controller/

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

All warnings (new ones prefixed by >>):

>> drivers/pci/controller/pcie-rcar-host.c:109:6: warning: no previous prototype for function 'rcar_pci_write_reg_workaround' [-Wmissing-prototypes]
   void rcar_pci_write_reg_workaround(struct rcar_pcie *pcie, u32 val, unsigned int reg)
        ^
   drivers/pci/controller/pcie-rcar-host.c:109:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void rcar_pci_write_reg_workaround(struct rcar_pcie *pcie, u32 val, unsigned int reg)
   ^
   static 
>> drivers/pci/controller/pcie-rcar-host.c:121:5: warning: no previous prototype for function 'rcar_pci_read_reg_workaround' [-Wmissing-prototypes]
   u32 rcar_pci_read_reg_workaround(struct rcar_pcie *pcie, unsigned int reg)
       ^
   drivers/pci/controller/pcie-rcar-host.c:121:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   u32 rcar_pci_read_reg_workaround(struct rcar_pcie *pcie, unsigned int reg)
   ^
   static 
   drivers/pci/controller/pcie-rcar-host.c:114:4: error: instruction requires: data-barriers
                   "       isb\n"
                    ^
   <inline asm>:2:2: note: instantiated into assembly here
           isb
           ^
   drivers/pci/controller/pcie-rcar-host.c:129:4: error: instruction requires: data-barriers
                   "       isb\n"
                    ^
   <inline asm>:3:2: note: instantiated into assembly here
           isb
           ^
   drivers/pci/controller/pcie-rcar-host.c:114:4: error: instruction requires: data-barriers
                   "       isb\n"
                    ^
   <inline asm>:2:2: note: instantiated into assembly here
           isb
           ^
   drivers/pci/controller/pcie-rcar-host.c:127:3: error: symbol 'rcar_pci_read_reg_workaround_start' is already defined
                   "rcar_pci_read_reg_workaround_start:\n"
                   ^
   <inline asm>:1:2: note: instantiated into assembly here
           rcar_pci_read_reg_workaround_start:
           ^
   drivers/pci/controller/pcie-rcar-host.c:129:4: error: instruction requires: data-barriers
                   "       isb\n"
                    ^
   <inline asm>:3:2: note: instantiated into assembly here
           isb
           ^
   2 warnings and 5 errors generated.


vim +/rcar_pci_write_reg_workaround +109 drivers/pci/controller/pcie-rcar-host.c

   108	
 > 109	void rcar_pci_write_reg_workaround(struct rcar_pcie *pcie, u32 val, unsigned int reg)
   110	{
   111	#ifdef CONFIG_ARM
   112		asm volatile(
   113			"	str %0, [%1]\n"
   114			"	isb\n"
   115		::"r"(val), "r"(pcie->base + reg):"memory");
   116	#else
   117		rcar_pci_write_reg(pcie, val, reg);
   118	#endif
   119	}
   120	
 > 121	u32 rcar_pci_read_reg_workaround(struct rcar_pcie *pcie, unsigned int reg)
   122	{
   123	#ifdef CONFIG_ARM
   124		u32 val;
   125	
   126		asm volatile(
   127			"rcar_pci_read_reg_workaround_start:\n"
   128			"1:	ldr %0, [%1]\n"
   129			"	isb\n"
   130		: "=r"(val):"r"(pcie->base + reg):"memory");
   131	
   132		return val;
   133	#else
   134		return rcar_pci_read_reg(pcie, reg);
   135	#endif
   136	}
   137	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot Jan. 16, 2022, 8:43 p.m. UTC | #3 
Hi,

I love your patch! Yet something to improve:

[auto build test ERROR on helgaas-pci/next]
[also build test ERROR on next-20220116]
[cannot apply to v5.16]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/marek-vasut-gmail-com/PCI-rcar-Finish-transition-to-L1-state-in-rcar_pcie_config_access/20220116-102631
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: arm-randconfig-c002-20220116 (https://download.01.org/0day-ci/archive/20220117/202201170402.sPT8Gtpa-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/f784bebf4a058d633fc77579892309b12dd33edb
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review marek-vasut-gmail-com/PCI-rcar-Finish-transition-to-L1-state-in-rcar_pcie_config_access/20220116-102631
        git checkout f784bebf4a058d633fc77579892309b12dd33edb
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arm SHELL=/bin/bash

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

All errors (new ones prefixed by >>):

   drivers/pci/controller/pcie-rcar-host.c:109:6: warning: no previous prototype for 'rcar_pci_write_reg_workaround' [-Wmissing-prototypes]
     109 | void rcar_pci_write_reg_workaround(struct rcar_pcie *pcie, u32 val, unsigned int reg)
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/pci/controller/pcie-rcar-host.c:121:5: warning: no previous prototype for 'rcar_pci_read_reg_workaround' [-Wmissing-prototypes]
     121 | u32 rcar_pci_read_reg_workaround(struct rcar_pcie *pcie, unsigned int reg)
         |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   /tmp/ccquRjDl.s: Assembler messages:
   /tmp/ccquRjDl.s:1425: Error: selected processor does not support `isb' in ARM mode
   /tmp/ccquRjDl.s:1486: Error: selected processor does not support `isb' in ARM mode
   /tmp/ccquRjDl.s:2594: Error: selected processor does not support `isb' in ARM mode
>> /tmp/ccquRjDl.s:2622: Error: symbol `rcar_pci_read_reg_workaround_start' is already defined
   /tmp/ccquRjDl.s:2624: Error: selected processor does not support `isb' in ARM mode

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/drivers/pci/controller/pcie-rcar-host.c b/drivers/pci/controller/pcie-rcar-host.c
index 0be58a91ddea..6da5af92568d 100644
--- a/drivers/pci/controller/pcie-rcar-host.c
+++ b/drivers/pci/controller/pcie-rcar-host.c
@@ -106,6 +106,35 @@  static u32 rcar_read_conf(struct rcar_pcie *pcie, int where)
 	return val >> shift;
 }
 
+void rcar_pci_write_reg_workaround(struct rcar_pcie *pcie, u32 val, unsigned int reg)
+{
+#ifdef CONFIG_ARM
+	asm volatile(
+		"	str %0, [%1]\n"
+		"	isb\n"
+	::"r"(val), "r"(pcie->base + reg):"memory");
+#else
+	rcar_pci_write_reg(pcie, val, reg);
+#endif
+}
+
+u32 rcar_pci_read_reg_workaround(struct rcar_pcie *pcie, unsigned int reg)
+{
+#ifdef CONFIG_ARM
+	u32 val;
+
+	asm volatile(
+		"rcar_pci_read_reg_workaround_start:\n"
+		"1:	ldr %0, [%1]\n"
+		"	isb\n"
+	: "=r"(val):"r"(pcie->base + reg):"memory");
+
+	return val;
+#else
+	return rcar_pci_read_reg(pcie, reg);
+#endif
+}
+
 /* Serialization is provided by 'pci_lock' in drivers/pci/access.c */
 static int rcar_pcie_config_access(struct rcar_pcie_host *host,
 		unsigned char access_type, struct pci_bus *bus,
@@ -178,9 +207,9 @@  static int rcar_pcie_config_access(struct rcar_pcie_host *host,
 		return PCIBIOS_DEVICE_NOT_FOUND;
 
 	if (access_type == RCAR_PCI_ACCESS_READ)
-		*data = rcar_pci_read_reg(pcie, PCIECDR);
+		*data = rcar_pci_read_reg_workaround(pcie, PCIECDR);
 	else
-		rcar_pci_write_reg(pcie, *data, PCIECDR);
+		rcar_pci_write_reg_workaround(pcie, *data, PCIECDR);
 
 	/* Disable the configuration access */
 	rcar_pci_write_reg(pcie, 0, PCIECCTLR);
@@ -1090,7 +1119,11 @@  static struct platform_driver rcar_pcie_driver = {
 static int rcar_pcie_aarch32_abort_handler(unsigned long addr,
 		unsigned int fsr, struct pt_regs *regs)
 {
+	extern u32 *rcar_pci_read_reg_workaround_start;
+	unsigned long pc = instruction_pointer(regs);
+	unsigned long instr = *(unsigned long *)pc;
 	unsigned long flags;
+	u32 reg, val;
 	int ret = 0;
 
 	spin_lock_irqsave(&pmsr_lock, flags);
@@ -1100,6 +1133,36 @@  static int rcar_pcie_aarch32_abort_handler(unsigned long addr,
 	if (ret)
 		goto unlock_exit;
 
+	/*
+	 * Test whether the faulting instruction is 'isb' and if
+	 * so, test whether it is the 'isb' instruction within
+	 * rcar_pci_read_reg_workaround() asm volatile()
+	 * implementation of read access. If it is, fix it up.
+	 */
+	instr &= ~0xf;
+	if ((instr == 0xf57ff060 || instr == 0xf3bf8f60) &&
+	    (pc == (u32)&rcar_pci_read_reg_workaround_start + 4)) {
+		/*
+		 * If the instruction being executed was a read,
+		 * make it look like it read all-ones.
+		 */
+		instr = *(unsigned long *)(pc - 4);
+		reg = (instr >> 12) & 15;
+
+		if ((instr & 0x0c100000) == 0x04100000) {
+			if (instr & 0x00400000)
+				val = 255;
+			else
+				val = -1;
+
+			regs->uregs[reg] = val;
+			regs->ARM_pc += 4;
+		} else if ((instr & 0x0e100090) == 0x00100090) {
+			regs->uregs[reg] = -1;
+			regs->ARM_pc += 4;
+		}
+	}
+
 unlock_exit:
 	spin_unlock_irqrestore(&pmsr_lock, flags);
 	return ret;