linux-kernel - Re: [PATCH v4 2/2] PCI: uniphier: Add UniPhier PCIe host controller support

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] [day] [month] [year] [list]

Date:   Wed, 05 Dec 2018 11:13:52 +0900
From:   Kunihiko Hayashi <hayashi.kunihiko@...ionext.com>
To:     Lorenzo Pieralisi <lorenzo.pieralisi@....com>
Cc:     Bjorn Helgaas <bhelgaas@...gle.com>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        <linux-pci@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>,
        Masami Hiramatsu <masami.hiramatsu@...aro.org>,
        Jassi Brar <jaswinder.singh@...aro.org>,
        Gustavo Pimentel <gustavo.pimentel@...opsys.com>
Subject: Re: [PATCH v4 2/2] PCI: uniphier: Add UniPhier PCIe host controller support

Hi Lorenzo,

On Tue, 4 Dec 2018 15:12:27 +0000 <lorenzo.pieralisi@....com> wrote:

> On Wed, Nov 28, 2018 at 01:04:26PM +0900, Kunihiko Hayashi wrote:
> 
> [...]
> 
> > +static void uniphier_pcie_irq_ack(struct irq_data *d)
> > +{
> > +	struct pcie_port *pp = irq_data_get_irq_chip_data(d);
> > +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > +	struct uniphier_pcie_priv *priv = to_uniphier_pcie(pci);
> > +	u32 val;
> > +
> > +	val = readl(priv->base + PCL_RCV_INTX);
> > +	val &= ~PCL_RCV_INTX_ALL_STATUS;
> > +	val |= BIT(irqd_to_hwirq(d) + PCL_RCV_INTX_STATUS_SHIFT);
> > +	writel(val, priv->base + PCL_RCV_INTX);
> > +}
> > +
> > +static void uniphier_pcie_irq_mask(struct irq_data *d)
> > +{
> > +	struct pcie_port *pp = irq_data_get_irq_chip_data(d);
> > +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > +	struct uniphier_pcie_priv *priv = to_uniphier_pcie(pci);
> > +	u32 val;
> > +
> > +	val = readl(priv->base + PCL_RCV_INTX);
> > +	val &= ~PCL_RCV_INTX_ALL_STATUS;
> > +	val |= BIT(irqd_to_hwirq(d) + PCL_RCV_INTX_MASK_SHIFT);
> > +	writel(val, priv->base + PCL_RCV_INTX);
> > +}
> > +
> > +static void uniphier_pcie_irq_unmask(struct irq_data *d)
> > +{
> > +	struct pcie_port *pp = irq_data_get_irq_chip_data(d);
> > +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > +	struct uniphier_pcie_priv *priv = to_uniphier_pcie(pci);
> > +	u32 val;
> > +
> > +	val = readl(priv->base + PCL_RCV_INTX);
> > +	val &= ~PCL_RCV_INTX_ALL_STATUS;
> 
> I have noticed this operation is carried out on ACK/MASK/UNMASK,
> what's its purpose ?

Thanks for pointing out. This is wrong.

The register PCL_RCV_INTX has 3 parts of bits.

#define PCL_RCV_INTX_ALL_ENABLE         GENMASK(19, 16)
#define PCL_RCV_INTX_ALL_MASK           GENMASK(11, 8)
#define PCL_RCV_INTX_ALL_STATUS         GENMASK(3, 0)

In the mask/unmask operation,
We should use PCL_RCV_INTX_ALL_MASK to represent 'mask bits',
not PCL_RCV_INTX_ALL_STATUS.
And the ack operation is effective by writing 1 to 'status bits'.

Thank you,

---
Best Regards,
Kunihiko Hayashi