linux-kernel - RE: [PATCH 2/2] usb: dwc3: Add driver for Xilinx platforms

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: <DM6PR02MB589878EB45E441F87B6D177CC1520@DM6PR02MB5898.namprd02.prod.outlook.com>
Date:   Fri, 28 Aug 2020 17:53:22 +0000
From:   Manish Narani <MNARANI@...inx.com>
To:     Robin Murphy <robin.murphy@....com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        Michal Simek <michals@...inx.com>,
        "balbi@...nel.org" <balbi@...nel.org>,
        "p.zabel@...gutronix.de" <p.zabel@...gutronix.de>
CC:     "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        git <git@...inx.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: RE: [PATCH 2/2] usb: dwc3: Add driver for Xilinx platforms

Hi Robin,

Thanks for the review. Please find my comment below inline.

> -----Original Message-----
> From: Robin Murphy <robin.murphy@....com>
> Sent: Friday, August 28, 2020 12:17 AM
> To: Manish Narani <MNARANI@...inx.com>; gregkh@...uxfoundation.org;
> robh+dt@...nel.org; Michal Simek <michals@...inx.com>; balbi@...nel.org;
> p.zabel@...gutronix.de
> Cc: devicetree@...r.kernel.org; linux-usb@...r.kernel.org; linux-
> kernel@...r.kernel.org; git <git@...inx.com>; linux-arm-
> kernel@...ts.infradead.org
> Subject: Re: [PATCH 2/2] usb: dwc3: Add driver for Xilinx platforms
> 
> On 2020-08-26 19:44, Manish Narani wrote:
> [...]
> > +	/*
> > +	 * This routes the usb dma traffic to go through CCI path instead
> > +	 * of reaching DDR directly. This traffic routing is needed to
> > +	 * make SMMU and CCI work with USB dma.
> > +	 */
> > +	if (of_dma_is_coherent(dev->of_node) || dev->iommu_group) {
> > +		reg = readl(priv_data->regs + XLNX_USB_COHERENCY);
> > +		reg |= XLNX_USB_COHERENCY_ENABLE;
> > +		writel(reg, priv_data->regs + XLNX_USB_COHERENCY);
> > +	}
> 
> This looks rather suspect - coherency should be based on coherency, not
> on whether an IOMMU group is present. If the device isn't described as
> coherent in the DT, then any SMMU mappings will end up using attributes
> that will downgrade traffic to be non-snooping anyway. And if the SMMU
> is enabled but not translating (e.g. "iommu.passthrough=1") then
> enabling hardware coherency when the DMA layer hasn't been told about it
> can potentially lead to nasty subtle problems and data loss.

May be the description needs to be updated in this. This is not the actual coherency enabling bit, but this is needed when coherency is enabled.
This is a register inside Xilinx USB controller which handles USB (which is in LPD) traffic route switching from LPD (Low Power Domain) to FPD (Full Power Domain)  path in the Xilinx SoC in either of the below scenarios:
1. Device is described coherent in  DT.
2. SMMU is enabled.

I will update the same in v2.

Thanks,
Manish