linux-kernel - Re: [PATCH V3 02/11] accel/amdxdna: Add a new driver for AMD AI Engine

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: <f81addf5-f9eb-5d39-313e-b18788cd8418@quicinc.com>
Date: Fri, 4 Oct 2024 11:21:31 -0600
From: Jeffrey Hugo <quic_jhugo@...cinc.com>
To: Lizhi Hou <lizhi.hou@....com>, <ogabbay@...nel.org>,
        <dri-devel@...ts.freedesktop.org>
CC: <linux-kernel@...r.kernel.org>, <min.ma@....com>, <max.zhen@....com>,
        <sonal.santan@....com>, <king.tam@....com>,
        Narendra Gutta
	<VenkataNarendraKumar.Gutta@....com>,
        George Yang <George.Yang@....com>
Subject: Re: [PATCH V3 02/11] accel/amdxdna: Add a new driver for AMD AI
 Engine

On 9/11/2024 12:05 PM, Lizhi Hou wrote:
> diff --git a/drivers/accel/amdxdna/aie2_pci.c b/drivers/accel/amdxdna/aie2_pci.c
> new file mode 100644
> index 000000000000..e21b32557fc2
> --- /dev/null
> +++ b/drivers/accel/amdxdna/aie2_pci.c
> @@ -0,0 +1,183 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2023-2024, Advanced Micro Devices, Inc.
> + */
> +
> +#include <drm/drm_device.h>
> +#include <drm/drm_managed.h>
> +#include <drm/drm_print.h>
> +#include <linux/errno.h>
> +#include <linux/firmware.h>
> +#include <linux/iommu.h>
> +#include <linux/pci.h>
> +
> +#include "amdxdna_pci_drv.h"
> +#include "aie2_pci.h"

Feels like a blank line is missing here.

> +static void aie2_hw_stop(struct amdxdna_dev *xdna)
> +{
> +	struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
> +	struct amdxdna_dev_hdl *ndev = xdna->dev_handle;
> +
> +	aie2_psp_stop(ndev->psp_hdl);
> +	aie2_smu_fini(ndev);
> +	pci_disable_device(pdev);
> +}
> diff --git a/drivers/accel/amdxdna/aie2_pci.h b/drivers/accel/amdxdna/aie2_pci.h
> new file mode 100644
> index 000000000000..b3c360a10c04
> --- /dev/null
> +++ b/drivers/accel/amdxdna/aie2_pci.h
> @@ -0,0 +1,130 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2023-2024, Advanced Micro Devices, Inc.
> + */
> +
> +#ifndef _AIE2_PCI_H_
> +#define _AIE2_PCI_H_
> +
> +#define AIE2_INTERVAL	20000	/* us */
> +#define AIE2_TIMEOUT	1000000	/* us */
> +
> +/* Firmware determines device memory base address and size */
> +#define AIE2_DEVM_BASE	0x4000000
> +#define AIE2_DEVM_SIZE	(64 * SZ_1M)

SZ_64M? Then the code, and reader's brains, are not doing multiplication.

> diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c b/drivers/accel/amdxdna/amdxdna_pci_drv.c
> new file mode 100644
> index 000000000000..33133f2503cd
> --- /dev/null
> +++ b/drivers/accel/amdxdna/amdxdna_pci_drv.c
> @@ -0,0 +1,128 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2022-2024, Advanced Micro Devices, Inc.
> + */
> +
> +#include <drm/drm_accel.h>
> +#include <drm/drm_drv.h>
> +#include <drm/drm_gem.h>
> +#include <drm/drm_ioctl.h>
> +#include <drm/drm_managed.h>
> +#include <linux/pci.h>
> +
> +#include "amdxdna_pci_drv.h"
> +
> +/*
> + *  Bind the driver base on PCI class and later use the (device_id, rev_id)
> + *  pair as a key to select the devices. The devices with same device_id
> + *  have very similar interface to host driver.
> + */

Looks like this comment didn't get updated when the class matching was 
removed.

> +static const struct pci_device_id pci_ids[] = {
> +	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, 0x1502) },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, 0x17f0) },
> +	{0}
> +};
> +
> +MODULE_DEVICE_TABLE(pci, pci_ids);