linux-kernel - Re: [PATCH RFC 10/13] dmaengine: sdxi: Add PCI driver 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <87o6ri49e4.fsf@AUSNATLYNCH.amd.com>
Date: Wed, 10 Sep 2025 10:25:39 -0500
From: Nathan Lynch <nathan.lynch@....com>
To: Mario Limonciello <mario.limonciello@....com>, Vinod Koul
	<vkoul@...nel.org>
CC: Wei Huang <wei.huang2@....com>, Bjorn Helgaas <bhelgaas@...gle.com>,
	<linux-pci@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<dmaengine@...r.kernel.org>
Subject: Re: [PATCH RFC 10/13] dmaengine: sdxi: Add PCI driver support

Mario Limonciello <mario.limonciello@....com> writes:
> On 9/5/2025 1:48 PM, Nathan Lynch via B4 Relay wrote:
>> +static int sdxi_pci_map(struct sdxi_dev *sdxi)
>> +{
>> +	struct pci_dev *pdev = sdxi_to_pci_dev(sdxi);
>> +	int bars, ret;
>> +
>> +	bars = 1 << MMIO_CTL_REGS_BAR | 1 << MMIO_DOORBELL_BAR;
>> +	ret = pcim_iomap_regions(pdev, bars, SDXI_DRV_NAME);
>> +	if (ret) {
>> +		sdxi_err(sdxi, "pcim_iomap_regions failed (%d)\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	sdxi->dbs_bar = pci_resource_start(pdev, MMIO_DOORBELL_BAR);
>> +
>> +	/* FIXME: pcim_iomap_table may return NULL, and it's deprecated. */
>> +	sdxi->ctrl_regs = pcim_iomap_table(pdev)[MMIO_CTL_REGS_BAR];
>> +	sdxi->dbs = pcim_iomap_table(pdev)[MMIO_DOORBELL_BAR];
>> +	if (!sdxi->ctrl_regs || !sdxi->dbs) {
>> +		sdxi_err(sdxi, "pcim_iomap_table failed\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static void sdxi_pci_unmap(struct sdxi_dev *sdxi)
>> +{
>> +	struct pci_dev *pdev = sdxi_to_pci_dev(sdxi);
>> +
>> +	pcim_iounmap(pdev, sdxi->ctrl_regs);
>> +	pcim_iounmap(pdev, sdxi->dbs);
>> +}
>> +
>> +static int sdxi_pci_init(struct sdxi_dev *sdxi)
>> +{
>> +	struct pci_dev *pdev = sdxi_to_pci_dev(sdxi);
>> +	struct device *dev = &pdev->dev;
>> +	int dma_bits = 64;
>> +	int ret;
>> +
>> +	ret = pcim_enable_device(pdev);
>> +	if (ret) {
>> +		sdxi_err(sdxi, "pcim_enbale_device failed\n");
>> +		return ret;
>> +	}
>> +
>> +	pci_set_master(pdev);
>
> Does pci_set_master() need to come before dma_set_mask_and_coherent() 
> and sdxi_pci_map()?

Yes, I think so. I'll audit this code for conformance to the
initialization sequence described in Documentation/pci/pci.rst.

>
> If so; there should be an error handling path that does
> pci_clear_master().

Right.

>
>> +	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(dma_bits));
>> +	if (ret) {
>> +		sdxi_err(sdxi, "failed to set DMA mask & coherent bits\n");
>> +		return ret;
>> +	}
>> +
>> +	ret = sdxi_pci_map(sdxi);
>> +	if (ret) {
>> +		sdxi_err(sdxi, "failed to map device IO resources\n");
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static void sdxi_pci_exit(struct sdxi_dev *sdxi)
>> +{
>> +	sdxi_pci_unmap(sdxi);
>
> I think you need a pci_clear_master() here.

OK, will fix.