linux-kernel - Re: [PATCH v3 12/26] PCI: keystone: Prevent ARM32 specific code to be compiled for ARM64

Open Source and information security mailing list archives

Message-ID: <20190412093154.7wj4dmm3oppvv32m@shell.armlinux.org.uk>
Date:   Fri, 12 Apr 2019 10:31:54 +0100
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     Kishon Vijay Abraham I <kishon@...com>
Cc:     Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        devicetree@...r.kernel.org,
        Jesper Nilsson <jesper.nilsson@...s.com>,
        Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Gustavo Pimentel <gustavo.pimentel@...opsys.com>,
        linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...s.com, Minghuan Lian <minghuan.Lian@....com>,
        Rob Herring <robh+dt@...nel.org>,
        Murali Karicheri <m-karicheri2@...com>,
        Jingoo Han <jingoohan1@...il.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Mingkai Hu <mingkai.hu@....com>, linux-omap@...r.kernel.org,
        Roy Zang <roy.zang@....com>,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v3 12/26] PCI: keystone: Prevent ARM32 specific code to
 be compiled for ARM64

On Fri, Apr 12, 2019 at 02:20:06PM +0530, Kishon Vijay Abraham I wrote:
> Hi Lorenzo,
> 
> On 11/04/19 8:33 PM, Lorenzo Pieralisi wrote:
> > On Mon, Mar 25, 2019 at 03:09:33PM +0530, Kishon Vijay Abraham I wrote:
> >> hook_fault_code is an ARM32 specific API for hooking into data abort.
> >> Since pci-keystone.c will be used for AM65X platforms which is an
> >> ARM64 platform,
> > 
> > Hi Kishon,
> > 
> > How is the problem plugged by the fault hook fixed on ARM64 ?
> 
> At least in AM654 platform, I don't see a bus error when PCIe device is not
> connected but returns 0xffffffff. So there is no necessary for hook_fault_code
> in AM654 platform.

... which needs to be explained somewhere, either in the commit
message or comments in the file, so that years later when someone
has to look at this code, they know why it is the way that it is.

When writing commits, please think about whether there is enough
information to understand the change in ten years time.

> 
> Thanks
> Kishon
> 
> > 
> > Thanks,
> > Lorenzo
> > 
> >> allow hook_fault_code to be compiled only for ARM32.
> >>
> >> Signed-off-by: Kishon Vijay Abraham I <kishon@...com>
> >> ---
> >>  drivers/pci/controller/dwc/pci-keystone.c | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> >> index dfe54553d832..93296d434f40 100644
> >> --- a/drivers/pci/controller/dwc/pci-keystone.c
> >> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> >> @@ -710,6 +710,7 @@ static int ks_pcie_config_legacy_irq(struct keystone_pcie *ks_pcie)
> >>  	return ret;
> >>  }
> >>  
> >> +#ifdef CONFIG_ARM
> >>  /*
> >>   * When a PCI device does not exist during config cycles, keystone host gets a
> >>   * bus error instead of returning 0xffffffff. This handler always returns 0
> >> @@ -729,6 +730,7 @@ static int ks_pcie_fault(unsigned long addr, unsigned int fsr,
> >>  
> >>  	return 0;
> >>  }
> >> +#endif
> >>  
> >>  static int __init ks_pcie_init_id(struct keystone_pcie *ks_pcie)
> >>  {
> >> @@ -778,12 +780,14 @@ static int __init ks_pcie_host_init(struct pcie_port *pp)
> >>  	if (ret < 0)
> >>  		return ret;
> >>  
> >> +#ifdef CONFIG_ARM
> >>  	/*
> >>  	 * PCIe access errors that result into OCP errors are caught by ARM as
> >>  	 * "External aborts"
> >>  	 */
> >>  	hook_fault_code(17, ks_pcie_fault, SIGBUS, 0,
> >>  			"Asynchronous external abort");
> >> +#endif
> >>  
> >>  	ks_pcie_start_link(pci);
> >>  	dw_pcie_wait_for_link(pci);
> >> -- 
> >> 2.17.1
> >>
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

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