| 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
| ||
|
Date: Mon, 3 Dec 2018 16:41:50 +0000
From: Lorenzo Pieralisi <lorenzo.pieralisi@....com>
To: Hanjie Lin <hanjie.lin@...ogic.com>,
Bjorn Helgaas <bhelgaas@...gle.com>
Cc: Yue Wang <yue.wang@...ogic.com>,
Kevin Hilman <khilman@...libre.com>,
Carlo Caione <carlo@...one.org>,
Jerome Brunet <jbrunet@...libre.com>,
Rob Herring <robh@...nel.org>,
Gustavo Pimentel <gustavo.pimentel@...opsys.com>,
Shawn Lin <shawn.lin@...k-chips.com>,
Philippe Ombredanne <pombredanne@...b.com>,
Cyrille Pitchen <cyrille.pitchen@...e-electrons.com>,
linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
linux-amlogic@...ts.infradead.org,
Yixun Lan <yixun.lan@...ogic.com>,
Liang Yang <liang.yang@...ogic.com>,
Jianxin Pan <jianxin.pan@...ogic.com>,
Qiufang Dai <qiufang.dai@...ogic.com>,
Jian Hu <jian.hu@...ogic.com>
Subject: Re: [PATCH v6 2/2] PCI: amlogic: Add the Amlogic Meson PCIe
controller driver
On Thu, Nov 22, 2018 at 04:53:54PM +0800, Hanjie Lin wrote:
[...]
> +static int meson_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
> + u32 *val)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +
> + /*
> + * there is a bug of MESON AXG pcie controller that software can not
> + * programe PCI_CLASS_DEVICE register, so we must return a fake right
> + * value to ensure driver could probe successfully.
> + */
> + if (where == PCI_CLASS_REVISION) {
> + *val = readl(pci->dbi_base + PCI_CLASS_REVISION);
> + /* keep revision id */
> + *val &= PCI_CLASS_REVISION_MASK;
> + *val |= PCI_CLASS_BRIDGE_PCI << 16;
> + return PCIBIOS_SUCCESSFUL;
> + }
As I said before, this looks broken. If this code (or other drivers with
the same broken assumptions, eg dwc/pcie-qcom.c) carries out a, say,
byte sized config access of eg PCI_CLASS_DEVICE you will get junk out of
it according to your comment above.
I would like to pick Bjorn's brain on this to see what we can really do
to fix this (and other) drivers.
Thanks,
Lorenzo
> + return dw_pcie_read(pci->dbi_base + where, size, val);
> +}
> +
> +static int meson_pcie_wr_own_conf(struct pcie_port *pp, int where,
> + int size, u32 val)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +
> + return dw_pcie_write(pci->dbi_base + where, size, val);
> +}
> +
> +static int meson_pcie_link_up(struct dw_pcie *pci)
> +{
> + struct meson_pcie *mp = to_meson_pcie(pci);
> + struct device *dev = pci->dev;
> + u32 smlh_up = 0;
> + u32 ltssm_up = 0;
> + u32 rdlh_up = 0;
> + u32 speed_okay = 0;
> + u32 cnt = 0;
> + u32 state12, state17;
> +
> + while (smlh_up == 0 || rdlh_up == 0 || ltssm_up == 0 ||
> + speed_okay == 0) {
> + udelay(20);
> +
> + state12 = meson_cfg_readl(mp, PCIE_CFG_STATUS12);
> + state17 = meson_cfg_readl(mp, PCIE_CFG_STATUS17);
> + smlh_up = IS_SMLH_LINK_UP(state12);
> + rdlh_up = IS_RDLH_LINK_UP(state12);
> + ltssm_up = IS_LTSSM_UP(state12);
> +
> + if (PM_CURRENT_STATE(state17) < PCIE_GEN3)
> + speed_okay = 1;
> +
> + if (smlh_up)
> + dev_dbg(dev, "smlh_link_up is on\n");
> + if (rdlh_up)
> + dev_dbg(dev, "rdlh_link_up is on\n");
> + if (ltssm_up)
> + dev_dbg(dev, "ltssm_up is on\n");
> + if (speed_okay)
> + dev_dbg(dev, "speed_okay\n");
> +
> + cnt++;
> +
> + if (cnt >= WAIT_LINKUP_TIMEOUT) {
> + dev_err(dev, "Error: Wait linkup timeout.\n");
> + return 0;
> + }
> + }
> +
> + return 1;
> +}
> +
> +static int meson_pcie_host_init(struct pcie_port *pp)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct meson_pcie *mp = to_meson_pcie(pci);
> + int ret;
> +
> + ret = meson_pcie_establish_link(mp);
> + if (ret)
> + return ret;
> +
> + meson_pcie_enable_interrupts(mp);
> +
> + return 0;
> +}
> +
> +static const struct dw_pcie_host_ops meson_pcie_host_ops = {
> + .rd_own_conf = meson_pcie_rd_own_conf,
> + .wr_own_conf = meson_pcie_wr_own_conf,
> + .host_init = meson_pcie_host_init,
> +};
> +
> +static int meson_add_pcie_port(struct meson_pcie *mp,
> + struct platform_device *pdev)
> +{
> + struct dw_pcie *pci = &mp->pci;
> + struct pcie_port *pp = &pci->pp;
> + struct device *dev = &pdev->dev;
> + int ret;
> +
> + if (IS_ENABLED(CONFIG_PCI_MSI)) {
> + pp->msi_irq = platform_get_irq(pdev, 0);
> + if (pp->msi_irq < 0) {
> + dev_err(dev, "failed to get msi irq\n");
> + return pp->msi_irq;
> + }
> + }
> +
> + pp->ops = &meson_pcie_host_ops;
> + pci->dbi_base = mp->mem_res.elbi_base;
> +
> + ret = dw_pcie_host_init(pp);
> + if (ret) {
> + dev_err(dev, "failed to initialize host\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct dw_pcie_ops dw_pcie_ops = {
> + .link_up = meson_pcie_link_up,
> +};
> +
> +static int meson_pcie_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct dw_pcie *pci;
> + struct meson_pcie *mp;
> + int ret;
> +
> + mp = devm_kzalloc(dev, sizeof(*mp), GFP_KERNEL);
> + if (!mp)
> + return -ENOMEM;
> +
> + pci = &mp->pci;
> + pci->dev = dev;
> + pci->ops = &dw_pcie_ops;
> +
> + mp->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> + if (IS_ERR(mp->reset_gpio)) {
> + dev_err(dev, "Get reset gpio failed\n");
> + return PTR_ERR(mp->reset_gpio);
> + }
> +
> + ret = meson_pcie_get_resets(mp);
> + if (ret) {
> + dev_err(dev, "Get reset resource failed, %d\n", ret);
> + return ret;
> + }
> +
> + ret = meson_pcie_get_mems(pdev, mp);
> + if (ret) {
> + dev_err(dev, "Get memory resource failed, %d\n", ret);
> + return ret;
> + }
> +
> + meson_pcie_power_on(mp);
> + meson_pcie_reset(mp);
> +
> + ret = meson_pcie_probe_clocks(mp);
> + if (ret) {
> + dev_err(dev, "Init clock resources failed, %d\n", ret);
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, mp);
> +
> + ret = meson_add_pcie_port(mp, pdev);
> + if (ret < 0) {
> + dev_err(dev, "Add PCIE port failed, %d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct of_device_id meson_pcie_of_match[] = {
> + {
> + .compatible = "amlogic,axg-pcie",
> + },
> + {},
> +};
> +
> +static struct platform_driver meson_pcie_driver = {
> + .probe = meson_pcie_probe,
> + .driver = {
> + .name = "meson-pcie",
> + .of_match_table = meson_pcie_of_match,
> + },
> +};
> +
> +builtin_platform_driver(meson_pcie_driver);
> --
> 2.7.4
>
Powered by blists - more mailing lists