[PATCH] nitro_enclaves: set master in the procedure of NE probe

[PATCH] nitro_enclaves: set master in the procedure of NE probe

[Longpeng(Mike)]

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.

Signed-off-by: Longpeng(Mike) <longpeng2@huawei.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);
+
 	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

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

[Paraschiv, Andra-Irina]

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@huawei.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.

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

[Longpeng (Mike, Cloud Infrastructure Service Product Dept.)]

在 2021/1/20 18:27, Paraschiv, Andra-Irina 写道:
> 
> 
> 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.
> 
Will update the commit message in V2.

>>
>> Signed-off-by: Longpeng(Mike) <longpeng2@huawei.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].
> 
No need to call pci_clear_master.

> 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.