linux-kernel - Re: [PATCH v1 5/5] pci: keystone: add pcie driver based on designware core driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <53875399.2020700@ti.com>
Date:	Thu, 29 May 2014 11:34:49 -0400
From:	Murali Karicheri <m-karicheri2@...com>
To:	Arnd Bergmann <arnd@...db.de>
CC:	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	"Strashko, Grygorii" <grygorii.strashko@...com>,
	Mohit Kumar <mohit.kumar@...com>,
	Jingoo Han <jg1.han@...sung.com>,
	"Shilimkar, Santosh" <santosh.shilimkar@...com>,
	Bjorn Helgaas <bhelgaas@...gle.com>
Subject: Re: [PATCH v1 5/5] pci: keystone: add pcie driver based on designware
 core driver

On 5/15/2014 12:23 PM, Arnd Bergmann wrote:
> On Thursday 15 May 2014 12:01:32 Murali Karicheri wrote:
>
>> +static int
>> +keystone_pcie_fault(unsigned long addr, unsigned int fsr,
>> +		struct pt_regs *regs)
>> +{
>> +	unsigned long instr = *(unsigned long *) instruction_pointer(regs);
>> +
>> +	if ((instr & 0x0e100090) == 0x00100090) {
>> +		int reg = (instr >> 12) & 15;
>> +
>> +		regs->uregs[reg] = -1;
>> +		regs->ARM_pc += 4;
>> +	}
>> +
>> +	return 0;
>> +}
> This needs to check in the PCI host registers what happened and do
> some appropriate action. If the fault was not caused by PCIe, it
> should not be ignored here but get passed on to the next handler.
>
>
Arnd,

The PCI controller is not able to detect the abort error from the EP and 
this gets
reported to ARM as an Asynchronous external abort. PCI fault handler 
needs to
consume this fault, otherwise it can't work on the Keystone h/w.

For example pci-imx6.c has

/*  Added for PCI abort handling */
static int imx6q_pcie_abort_handler(unsigned long addr,
                 unsigned int fsr, struct pt_regs *regs)
{
         return 0;
}

Other examples are

1) arch/arm/mach-cns3xxx/pci.c
static int cns3xxx_pcie_abort_handler(unsigned long addr, unsigned int fsr,
                                       struct pt_regs *regs)
{
         if (fsr & (1 << 10))
                 regs->ARM_pc += 4;
         return 0;
}
2) arch/.arm/plat-iop/pci.c
/*
  * When a PCI device does not exist during config cycles, the 80200 gets a
  * bus error instead of returning 0xffffffff. This handler simply returns.
  */
static int
iop3xx_pci_abort(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
{
         DBG("PCI abort: address = 0x%08lx fsr = 0x%03x PC = 0x%08lx LR 
= 0x%08lx\n",
                 addr, fsr, regs->ARM_pc, regs->ARM_lr);

         /*
          * If it was an imprecise abort, then we need to correct the
          * return address to be _after_ the instruction.
          */
         if (fsr & (1 << 10))
                 regs->ARM_pc += 4;

         return 0;
}

I suggest we document this as done in plat-iop. Let me know as I need to 
post my
v2 of the patch this week that incorporates all the comments so far on v1.

Murali
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/