linux-kernel - Re: [RFC 03/10] PCI: endpoint: Add MSI-X interfaces

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: <27b67ebd-b585-f3a1-091d-a3cdf13f03eb@synopsys.com>
Date:   Tue, 17 Apr 2018 16:51:05 +0100
From:   Gustavo Pimentel <gustavo.pimentel@...opsys.com>
To:     Kishon Vijay Abraham I <kishon@...com>,
        "bhelgaas@...gle.com" <bhelgaas@...gle.com>,
        "lorenzo.pieralisi@....com" <lorenzo.pieralisi@....com>,
        "Joao.Pinto@...opsys.com" <Joao.Pinto@...opsys.com>,
        "jingoohan1@...il.com" <jingoohan1@...il.com>,
        "adouglas@...ence.com" <adouglas@...ence.com>,
        "niklas.cassel@...s.com" <niklas.cassel@...s.com>,
        "jesper.nilsson@...s.com" <jesper.nilsson@...s.com>
Cc:     "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC 03/10] PCI: endpoint: Add MSI-X interfaces

Hi Kishon,

On 17/04/2018 11:24, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Tuesday 10 April 2018 10:44 PM, Gustavo Pimentel wrote:
>> Implements the generic method for calling the get/set callbacks.
>>
>> Adds the PCI_EPC_IRQ_MSIX type.
>>
>> Adds the MSI-X callbacks signatures to the ops structure.
>>
>> Adds sysfs interface for altering the number of MSI-X entries.
>>
>> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@...opsys.com>
>> ---
>>  drivers/pci/endpoint/pci-ep-cfs.c   | 24 ++++++++++++++++
>>  drivers/pci/endpoint/pci-epc-core.c | 57 +++++++++++++++++++++++++++++++++++++
>>  include/linux/pci-epc.h             | 11 ++++++-
>>  include/linux/pci-epf.h             |  1 +
>>  4 files changed, 92 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/endpoint/pci-ep-cfs.c b/drivers/pci/endpoint/pci-ep-cfs.c
>> index 018ea34..d1288a0 100644
>> --- a/drivers/pci/endpoint/pci-ep-cfs.c
>> +++ b/drivers/pci/endpoint/pci-ep-cfs.c
>> @@ -286,6 +286,28 @@ static ssize_t pci_epf_msi_interrupts_show(struct config_item *item,
>>  		       to_pci_epf_group(item)->epf->msi_interrupts);
>>  }
>>  
>> +static ssize_t pci_epf_msix_interrupts_store(struct config_item *item,
>> +					     const char *page, size_t len)
>> +{
>> +	u16 val;
>> +	int ret;
>> +
>> +	ret = kstrtou16(page, 0, &val);
>> +	if (ret)
>> +		return ret;
>> +
>> +	to_pci_epf_group(item)->epf->msix_interrupts = val;
>> +
>> +	return len;
>> +}
>> +
>> +static ssize_t pci_epf_msix_interrupts_show(struct config_item *item,
>> +					    char *page)
>> +{
>> +	return sprintf(page, "%d\n",
>> +		       to_pci_epf_group(item)->epf->msix_interrupts);
>> +}
>> +
>>  PCI_EPF_HEADER_R(vendorid)
>>  PCI_EPF_HEADER_W_u16(vendorid)
>>  
>> @@ -327,6 +349,7 @@ CONFIGFS_ATTR(pci_epf_, subsys_vendor_id);
>>  CONFIGFS_ATTR(pci_epf_, subsys_id);
>>  CONFIGFS_ATTR(pci_epf_, interrupt_pin);
>>  CONFIGFS_ATTR(pci_epf_, msi_interrupts);
>> +CONFIGFS_ATTR(pci_epf_, msix_interrupts);
>>  
>>  static struct configfs_attribute *pci_epf_attrs[] = {
>>  	&pci_epf_attr_vendorid,
>> @@ -340,6 +363,7 @@ static struct configfs_attribute *pci_epf_attrs[] = {
>>  	&pci_epf_attr_subsys_id,
>>  	&pci_epf_attr_interrupt_pin,
>>  	&pci_epf_attr_msi_interrupts,
>> +	&pci_epf_attr_msix_interrupts,
>>  	NULL,
>>  };
>>  
>> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
>> index b0ee427..294a383 100644
>> --- a/drivers/pci/endpoint/pci-epc-core.c
>> +++ b/drivers/pci/endpoint/pci-epc-core.c
>> @@ -218,6 +218,63 @@ int pci_epc_set_msi(struct pci_epc *epc, u8 func_no, u8 interrupts)
>>  EXPORT_SYMBOL_GPL(pci_epc_set_msi);
>>  
>>  /**
>> + * pci_epc_get_msix() - get the number of MSI-X interrupt numbers allocated
>> + * @epc: the EPC device to which MSI-X interrupts was requested
>> + * @func_no: the endpoint function number in the EPC device
>> + *
>> + * Invoke to get the number of MSI-X interrupts allocated by the RC
>> + */
>> +int pci_epc_get_msix(struct pci_epc *epc, u8 func_no)
>> +{
>> +	int interrupt;
>> +	unsigned long flags;
>> +
>> +	if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions)
>> +		return 0;
>> +
>> +	if (!epc->ops->get_msix)
>> +		return 0;
>> +
>> +	spin_lock_irqsave(&epc->lock, flags);
>> +	interrupt = epc->ops->get_msix(epc, func_no);
>> +	spin_unlock_irqrestore(&epc->lock, flags);
>> +
>> +	if (interrupt < 0)
>> +		return 0;
>> +
>> +	return interrupt++;
> 
> return interrupt + 1?

I'll change it.

>> +}
>> +EXPORT_SYMBOL_GPL(pci_epc_get_msix);
>> +
>> +/**
>> + * pci_epc_set_msix() - set the number of MSI-X interrupt numbers required
>> + * @epc: the EPC device on which MSI-X has to be configured
>> + * @func_no: the endpoint function number in the EPC device
>> + * @interrupts: number of MSI-X interrupts required by the EPF
>> + *
>> + * Invoke to set the required number of MSI-X interrupts.
>> + */
>> +int pci_epc_set_msix(struct pci_epc *epc, u8 func_no, u16 interrupts)
>> +{
>> +	int ret;
>> +	unsigned long flags;
>> +
>> +	if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions ||
>> +	    interrupts < 1 || interrupts > 2048)
>> +		return -EINVAL;
>> +
>> +	if (!epc->ops->set_msix)
>> +		return 0;
>> +
>> +	spin_lock_irqsave(&epc->lock, flags);
>> +	ret = epc->ops->set_msix(epc, func_no, interrupts - 1);
>> +	spin_unlock_irqrestore(&epc->lock, flags);
>> +
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(pci_epc_set_msix);
>> +
>> +/**
>>   * pci_epc_unmap_addr() - unmap CPU address from PCI address
>>   * @epc: the EPC device on which address is allocated
>>   * @func_no: the endpoint function number in the EPC device
>> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
>> index af657ca..32e8961 100644
>> --- a/include/linux/pci-epc.h
>> +++ b/include/linux/pci-epc.h
>> @@ -17,6 +17,7 @@ enum pci_epc_irq_type {
>>  	PCI_EPC_IRQ_UNKNOWN,
>>  	PCI_EPC_IRQ_LEGACY,
>>  	PCI_EPC_IRQ_MSI,
>> +	PCI_EPC_IRQ_MSIX,
>>  };
>>  
>>  /**
>> @@ -30,6 +31,10 @@ enum pci_epc_irq_type {
>>   *	     capability register
>>   * @get_msi: ops to get the number of MSI interrupts allocated by the RC from
>>   *	     the MSI capability register
>> + * @set_msix: ops to set the requested number of MSI-X interrupts in the
>> + *	     MSI-X capability register
>> + * @get_msix: ops to get the number of MSI-X interrupts allocated by the RC
>> + *	     from the MSI-X capability register
>>   * @raise_irq: ops to raise a legacy or MSI interrupt
>>   * @start: ops to start the PCI link
>>   * @stop: ops to stop the PCI link
>> @@ -48,8 +53,10 @@ struct pci_epc_ops {
>>  			      phys_addr_t addr);
>>  	int	(*set_msi)(struct pci_epc *epc, u8 func_no, u8 interrupts);
>>  	int	(*get_msi)(struct pci_epc *epc, u8 func_no);
>> +	int	(*set_msix)(struct pci_epc *epc, u8 func_no, u16 interrupts);
>> +	int	(*get_msix)(struct pci_epc *epc, u8 func_no);
> 
> The 1st patch in the series is already using get_msix, set_msix. This patch
> should precede the 1st patch.

Yes, you're right.

> 
> Thanks
> Kishon
> 

Regards,
Gustavo