linux-kernel - Re: [PATCH] nitro_enclaves: set master in the procedure of NE probe

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:   Wed, 20 Jan 2021 12:27:51 +0200
From:   "Paraschiv, Andra-Irina" <andraprs@...zon.com>
To:     "Longpeng(Mike)" <longpeng2@...wei.com>
CC:     <arei.gonglei@...wei.com>, <linux-kernel@...r.kernel.org>,
        <lexnv@...zon.com>, <alcioa@...zon.com>,
        Greg KH <gregkh@...uxfoundation.org>
Subject: Re: [PATCH] nitro_enclaves: set master in the procedure of NE probe



On 19/01/2021 05:30, Longpeng(Mike) wrote:
> According the PCI spec:
>    Bus Master Enable – Controls the ability of a PCI Express
>    Endpoint to issue Memory and I/O Read/Write Requests, and
>    the ability of a Root or Switch Port to forward Memory and
>    I/O Read/Write Requests in the Upstream direction
>
> Set BusMaster to make the driver to be PCI conformant.

Could update the commit title and message body to reflect more the why 
and what for the change. One option can be:

nitro_enclaves: Set Bus Master for the NE PCI device

Enable Bus Master for the NE PCI device, according to the PCI spec
for submitting memory or I/O requests:
   Bus Master Enable ...



Please include the changelog in the commit message for the next revision(s).

+ Greg in CC, as the patches for the Nitro Enclaves kernel driver are 
first included in the char misc tree, then in the linux next and finally 
in the mainline.

>
> Signed-off-by: Longpeng(Mike) <longpeng2@...wei.com>
> ---
>   drivers/virt/nitro_enclaves/ne_pci_dev.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/drivers/virt/nitro_enclaves/ne_pci_dev.c b/drivers/virt/nitro_enclaves/ne_pci_dev.c
> index b9c1de4..143207e 100644
> --- a/drivers/virt/nitro_enclaves/ne_pci_dev.c
> +++ b/drivers/virt/nitro_enclaves/ne_pci_dev.c
> @@ -480,6 +480,8 @@ static int ne_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>                  goto free_ne_pci_dev;
>          }
>
> +       pci_set_master(pdev);

I was looking if we need the reverse for this - pci_clear_master() [1] - 
on the error and remove / shutdown codebase paths, but 
pci_disable_device() seems to include the bus master disable logic [2][3].

Thanks,
Andra

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/pci.c?h=v5.11-rc4#n4312
[2] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/pci.c?h=v5.11-rc4#n2104
[3] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/pci.c?h=v5.11-rc4#n4242

> +
>          rc = pci_request_regions_exclusive(pdev, "nitro_enclaves");
>          if (rc < 0) {
>                  dev_err(&pdev->dev, "Error in pci request regions [rc=%d]\n", rc);
> --
> 1.8.3.1
>




Amazon Development Center (Romania) S.R.L. registered office: 27A Sf. Lazar Street, UBC5, floor 2, Iasi, Iasi County, 700045, Romania. Registered in Romania. Registration number J22/2621/2005.