netdev - Re: [PATCH v4 net-next 02/19] ionic: Add hardware init and device commands

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]

Date:   Tue, 23 Jul 2019 17:25:00 -0700
From:   Shannon Nelson <snelson@...sando.io>
To:     Saeed Mahameed <saeedm@...lanox.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>
Subject: Re: [PATCH v4 net-next 02/19] ionic: Add hardware init and device
 commands

On 7/23/19 4:47 PM, Saeed Mahameed wrote:
> On Mon, 2019-07-22 at 14:40 -0700, Shannon Nelson wrote:
>> The ionic device has a small set of PCI registers, including a
>> device control and data space, and a large set of message
>> commands.
>>
>> Signed-off-by: Shannon Nelson <snelson@...sando.io>
>> ---
>>   drivers/net/ethernet/pensando/ionic/Makefile  |    2 +-
>>   drivers/net/ethernet/pensando/ionic/ionic.h   |   20 +
>>   .../net/ethernet/pensando/ionic/ionic_bus.h   |    1 +
>>   .../ethernet/pensando/ionic/ionic_bus_pci.c   |  140 +-
>>   .../ethernet/pensando/ionic/ionic_debugfs.c   |   67 +
>>   .../ethernet/pensando/ionic/ionic_debugfs.h   |   28 +
>>   .../net/ethernet/pensando/ionic/ionic_dev.c   |  132 +
>>   .../net/ethernet/pensando/ionic/ionic_dev.h   |  144 +
>>   .../net/ethernet/pensando/ionic/ionic_if.h    | 2552
>> +++++++++++++++++
>>   .../net/ethernet/pensando/ionic/ionic_main.c  |  296 ++
>>   .../net/ethernet/pensando/ionic/ionic_regs.h  |  133 +
>>   11 files changed, 3512 insertions(+), 3 deletions(-)
>>   create mode 100644
>> drivers/net/ethernet/pensando/ionic/ionic_debugfs.c
>>   create mode 100644
>> drivers/net/ethernet/pensando/ionic/ionic_debugfs.h
>>   create mode 100644 drivers/net/ethernet/pensando/ionic/ionic_dev.c
>>   create mode 100644 drivers/net/ethernet/pensando/ionic/ionic_dev.h
>>   create mode 100644 drivers/net/ethernet/pensando/ionic/ionic_if.h
>>   create mode 100644 drivers/net/ethernet/pensando/ionic/ionic_regs.h
>>
> [...]
>   
>>   static void ionic_remove(struct pci_dev *pdev)
>>   {
>>   	struct ionic *ionic = pci_get_drvdata(pdev);
>>   
>> -	devm_kfree(&pdev->dev, ionic);
>> +	if (ionic) {
> nit, in case you are doing another re-spin  maybe early return here:
> if (!ionic)
>       return;
> //do stuff

Sure

>
>> +		ionic_reset(ionic);
>> +		ionic_dev_teardown(ionic);
>> +		ionic_unmap_bars(ionic);
>> +		pci_release_regions(pdev);
>> +		pci_clear_master(pdev);
>> +		pci_disable_sriov(pdev);
>> +		pci_disable_device(pdev);
>> +		ionic_debugfs_del_dev(ionic);
>> +		mutex_destroy(&ionic->dev_cmd_lock);
>> +
>> +		devm_kfree(&pdev->dev, ionic);
>> +	}
>>   }
>>
> [...]
>
>>   
>> +
>> +/* Devcmd Interface */
>> +u8 ionic_dev_cmd_status(struct ionic_dev *idev)
>> +{
>> +	return ioread8(&idev->dev_cmd_regs->comp.comp.status);
>> +}
>> +
>> +bool ionic_dev_cmd_done(struct ionic_dev *idev)
>> +{
>> +	return ioread32(&idev->dev_cmd_regs->done) & DEV_CMD_DONE;
>> +}
>> +
>> +void ionic_dev_cmd_comp(struct ionic_dev *idev, union dev_cmd_comp
>> *comp)
>> +{
>> +	memcpy_fromio(comp, &idev->dev_cmd_regs->comp, sizeof(*comp));
>> +}
>> +
>> +void ionic_dev_cmd_go(struct ionic_dev *idev, union dev_cmd *cmd)
>> +{
>> +	memcpy_toio(&idev->dev_cmd_regs->cmd, cmd, sizeof(*cmd));
>> +	iowrite32(0, &idev->dev_cmd_regs->done);
>> +	iowrite32(1, &idev->dev_cmd_regs->doorbell);
>> +}
>> +
>> +/* Device commands */
>> +void ionic_dev_cmd_identify(struct ionic_dev *idev, u8 ver)
>> +{
>> +	union dev_cmd cmd = {
>> +		.identify.opcode = CMD_OPCODE_IDENTIFY,
>> +		.identify.ver = ver,
>> +	};
>> +
>> +	ionic_dev_cmd_go(idev, &cmd);
>> +}
>> +
>> +void ionic_dev_cmd_init(struct ionic_dev *idev)
>> +{
>> +	union dev_cmd cmd = {
>> +		.init.opcode = CMD_OPCODE_INIT,
>> +		.init.type = 0,
>> +	};
>> +
>> +	ionic_dev_cmd_go(idev, &cmd);
>> +}
>> +
>> +void ionic_dev_cmd_reset(struct ionic_dev *idev)
>> +{
>> +	union dev_cmd cmd = {
>> +		.reset.opcode = CMD_OPCODE_RESET,
>> +	};
>> +
>> +	ionic_dev_cmd_go(idev, &cmd);
>> +}
> [...]
>
>> +int ionic_dev_cmd_wait(struct ionic *ionic, unsigned long
>> max_seconds)
>> +{
>> +	struct ionic_dev *idev = &ionic->idev;
>> +	unsigned long max_wait, start_time, duration;
>> +	int opcode;
>> +	int done;
>> +	int err;
>> +
>> +	WARN_ON(in_interrupt());
>> +
>> +	/* Wait for dev cmd to complete, retrying if we get EAGAIN,
>> +	 * but don't wait any longer than max_seconds.
>> +	 */
>> +	max_wait = jiffies + (max_seconds * HZ);
>> +try_again:
>> +	start_time = jiffies;
>> +	do {
>> +		done = ionic_dev_cmd_done(idev);
> READ_ONCE required here ? to read from coherent memory modified
> by the device and read by the driver ?

Good idea, I'll add that in.

>
>> +		if (done)
>> +			break;
>> +		msleep(20);
>> +	} while (!done && time_before(jiffies, max_wait));
> so your command interface is busy polling based, i am relating here to
> Dave's comment regarding async command completion, is it possible to
> have interrupt (MSIX?) based command completion in this hw ?

As I wrote elsewhere, this is only the low-level dev_cmd that does 
polling; the adminq does a wait that is completed by an MSI-x handler.

sln