linux-kernel - Re: [PATCH RFC 09/13] dmaengine: sdxi: Add core device management code

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: <877bxyxfai.fsf@AUSNATLYNCH.amd.com>
Date: Tue, 16 Sep 2025 16:23:33 -0500
From: Nathan Lynch <nathan.lynch@....com>
To: Jonathan Cameron <jonathan.cameron@...wei.com>
CC: Vinod Koul <vkoul@...nel.org>, Wei Huang <wei.huang2@....com>, "Mario
 Limonciello" <mario.limonciello@....com>, Bjorn Helgaas
	<bhelgaas@...gle.com>, <linux-pci@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <dmaengine@...r.kernel.org>
Subject: Re: [PATCH RFC 09/13] dmaengine: sdxi: Add core device management code

Jonathan Cameron <jonathan.cameron@...wei.com> writes:
> On Fri, 05 Sep 2025 13:48:32 -0500
> Nathan Lynch via B4 Relay <devnull+nathan.lynch.amd.com@...nel.org> wrote:
>
>> From: Nathan Lynch <nathan.lynch@....com>
>> 
>> Add code that manages device initialization and exit and provides
>> entry points for the PCI driver code to come.
>
> I'd prefer a patch series that started with the PCI device and built up
> functionality for the stuff found earlier + in this patch on top of it.
> Doing that allows each patch to be fully tested and reviewed on it's
> own.

Right, I think you made this point elsewhere too -- I'll try organizing
the series as you suggest.


>> +/* Refer to "Activation of the SDXI Function by Software". */
>> +static int sdxi_fn_activate(struct sdxi_dev *sdxi)
>> +{
>> +	const struct sdxi_dev_ops *ops = sdxi->dev_ops;
>> +	u64 cxt_l2;
>> +	u64 cap0;
>> +	u64 cap1;
>> +	u64 ctl2;
>
> Combine these u64 declarations on one line.

Sure.


>> +void sdxi_device_exit(struct sdxi_dev *sdxi)
>> +{
>> +	sdxi_working_cxt_exit(sdxi->dma_cxt);
>> +
>> +	/* Walk sdxi->cxt_array freeing any allocated rows. */
>> +	for (size_t i = 0; i < L2_TABLE_ENTRIES; ++i) {
>> +		if (!sdxi->cxt_array[i])
>> +			continue;
>> +		/* When a context is released its entry in the table should be NULL. */
>> +		for (size_t j = 0; j < L1_TABLE_ENTRIES; ++j) {
>> +			struct sdxi_cxt *cxt = sdxi->cxt_array[i][j];
>> +
>> +			if (!cxt)
>> +				continue;
>> +			if (cxt->id != 0)  /* admin context shutdown is last */
>> +				sdxi_working_cxt_exit(cxt);
>> +			sdxi->cxt_array[i][j] = NULL;
>> +		}
>> +		if (i != 0)  /* another special case for admin cxt */
>> +			kfree(sdxi->cxt_array[i]);
>> +	}
>> +
>> +	sdxi_working_cxt_exit(sdxi->admin_cxt);
>> +	kfree(sdxi->cxt_array[0]);  /* ugh */
>
> The constraints here need to be described a little more clearly.

Heh, I think I was tired of finding and fixing memory leaks when I wrote
these comments. If this part of the code survives to the next version
I'll improve them, but I plan to rework how we track contexts using a
per-device xarray, so this code should become a single xa_for_each()
loop followed by the admin context cleanup.