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: <53F44178.7070409@huawei.com>
Date:	Wed, 20 Aug 2014 14:34:32 +0800
From:	Yijing Wang <wangyijing@...wei.com>
To:	"Bharat.Bhushan@...escale.com" <Bharat.Bhushan@...escale.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:	Xinwei Hu <huxinwei@...wei.com>, Wuyun <wuyun.wu@...wei.com>,
	"Bjorn Helgaas" <bhelgaas@...gle.com>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	"Paul.Mundt@...wei.com" <Paul.Mundt@...wei.com>,
	"James E.J. Bottomley" <jejb@...isc-linux.org>,
	Marc Zyngier <marc.zyngier@....com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Russell King <linux@....linux.org.uk>,
	"linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
	"arnab.basu@...escale.com" <arnab.basu@...escale.com>,
	"virtualization@...ts.linux-foundation.org" 
	<virtualization@...ts.linux-foundation.org>,
	Hanjun Guo <guohanjun@...wei.com>
Subject: Re: [RFC PATCH 09/11] PCI/MSI: refactor PCI MSI driver

>> @@ -1025,21 +1059,52 @@ int pci_msi_enabled(void)
>>  }
>>  EXPORT_SYMBOL(pci_msi_enabled);
>>
>> -void pci_msi_init_pci_dev(struct pci_dev *dev)
>> +static struct msi_ops pci_msi = {
>> +	.msi_set_enable = msi_set_enable,
>> +	.msi_setup_entry = msi_setup_entry,
>> +	.msix_setup_entries = msix_setup_entries,
>> +	.msi_mask_irq = default_msi_mask_irq,
>> +	.msix_mask_irq = default_msix_mask_irq,
>> +	.msi_read_message = __read_msi_msg,
>> +	.msi_write_message = __write_msi_msg,
>> +	.msi_set_intx =  pci_intx_for_msi,
>> +};
> 
> Ahh, want to be sure I am understanding this correctly. So if I have a non-pci driver "xyz" which wants to use separate ops then I need to have a all these functions in that driver. Something like driver/xyz/msi.c

Yes, because different MSI device has different MSI hardware registers, so every MSI type should provide its own msi_ops, or its own msi_driver in my new proposal.


> 
> Thanks
> -Bharat
> 
>> +
>> +struct msi_irqs *alloc_msi_irqs(void *data, struct msi_ops *ops)
>>  {
>> -	INIT_LIST_HEAD(&dev->msi_list);
>> +	struct msi_irqs *msi;
>> +
>> +	msi = kzalloc(sizeof(struct msi_irqs), GFP_KERNEL);
>> +	if (!msi)
>> +		return NULL;
>>
>> +	INIT_LIST_HEAD(&msi->msi_list);
>> +	msi->data = data;
>> +	msi->ops = ops;
>> +	return msi;
>> +}
>> +
>> +void pci_msi_init_pci_dev(struct pci_dev *dev)
>> +{
>>  	/* Disable the msi hardware to avoid screaming interrupts
>>  	 * during boot.  This is the power on reset default so
>>  	 * usually this should be a noop.
>>  	 */
>>  	dev->msi_cap = pci_find_capability(dev, PCI_CAP_ID_MSI);
>> -	if (dev->msi_cap)
>> -		msi_set_enable(dev, 0);
>> -
>>  	dev->msix_cap = pci_find_capability(dev, PCI_CAP_ID_MSIX);
>> -	if (dev->msix_cap)
>> -		msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0);
>> +
>> +	if (dev->msi_cap || dev->msix_cap) {
>> +		dev->msi = alloc_msi_irqs(dev, &pci_msi);
>> +		if (!dev->msi)
>> +			return;
>> +
>> +		dev->msi->node = dev_to_node(&dev->dev);
>> +		if (dev->msi_cap)
>> +			msi_set_enable(dev->msi, 0, MSI_TYPE);
>> +
>> +		if (dev->msix_cap)
>> +			msi_set_enable(dev->msi, 0, MSIX_TYPE);
>> +	}
>>  }
>>
>>  /**
>> @@ -1060,13 +1125,13 @@ int pci_enable_msi_range(struct pci_dev *dev, int
>> minvec, int maxvec)
>>  	int rc;
>>  	struct msi_desc *entry;
>>
>> -	if (dev->current_state != PCI_D0)
>> +	if (dev->current_state != PCI_D0 || !dev->msi)
>>  		return -EINVAL;
>>
>> -	WARN_ON(!!dev->msi_enabled);
>> +	WARN_ON(!!pci_dev_msi_enabled(dev, MSI_TYPE));
>>
>>  	/* Check whether driver already requested MSI-X irqs */
>> -	if (dev->msix_enabled) {
>> +	if (pci_dev_msi_enabled(dev, MSIX_TYPE)) {
>>  		dev_info(&dev->dev,
>>  			 "can't enable MSI (MSI-X already enabled)\n");
>>  		return -EINVAL;
>> @@ -1095,7 +1160,7 @@ int pci_enable_msi_range(struct pci_dev *dev, int minvec,
>> int maxvec)
>>  	} while (rc);
>>
>>  	do {
>> -		rc = msi_capability_init(dev, nvec);
>> +		rc = msi_capability_init(dev->msi, nvec);
>>  		if (rc < 0) {
>>  			return rc;
>>  		} else if (rc > 0) {
>> @@ -1107,14 +1172,14 @@ int pci_enable_msi_range(struct pci_dev *dev, int
>> minvec, int maxvec)
>>
>>  	rc = populate_msi_sysfs(dev);
>>  	if (rc) {
>> -		msi_set_enable(dev, 0);
>> -		pci_intx_for_msi(dev, 1);
>> -		dev->msi_enabled = 0;
>> -		free_msi_irqs(dev);
>> +		msi_set_enable(dev->msi, 0, MSI_TYPE);
>> +		pci_intx_for_msi(dev->msi, 1);
>> +		dev->msi->msi_enabled = 0;
>> +		free_msi_irqs(dev->msi);
>>  		return rc;
>>  	}
>>
>> -	entry = list_entry(dev->msi_list.next, struct msi_desc, list);
>> +	entry = list_entry(dev->msi->msi_list.next, struct msi_desc, list);
>>  	dev->irq = entry->irq;
>>  	return nvec;
>>  }
>> @@ -1158,3 +1223,5 @@ int pci_enable_msix_range(struct pci_dev *dev, struct
>> msix_entry *entries,
>>  	return nvec;
>>  }
>>  EXPORT_SYMBOL(pci_enable_msix_range);
>> +
>> +
>> diff --git a/include/linux/msi.h b/include/linux/msi.h
>> index 5a672d3..fc8f3e8 100644
>> --- a/include/linux/msi.h
>> +++ b/include/linux/msi.h
>> @@ -83,15 +83,15 @@ struct msi_desc {
>>   * implemented as weak symbols so that they /can/ be overriden by
>>   * architecture specific code if needed.
>>   */
>> -int arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc);
>> +int arch_setup_msi_irq(struct msi_irqs *msi, struct msi_desc *desc);
>>  void arch_teardown_msi_irq(unsigned int irq);
>> -int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type);
>> -void arch_teardown_msi_irqs(struct pci_dev *dev);
>> -int arch_msi_check_device(struct pci_dev* dev, int nvec, int type);
>> -void arch_restore_msi_irqs(struct pci_dev *dev);
>> +int arch_setup_msi_irqs(struct msi_irqs *msi, int nvec, int type);
>> +void arch_teardown_msi_irqs(struct msi_irqs *msi);
>> +int arch_msi_check_device(struct msi_irqs *msi, int nvec, int type);
>> +void arch_restore_msi_irqs(struct msi_irqs *msi);
>>
>> -void default_teardown_msi_irqs(struct pci_dev *dev);
>> -void default_restore_msi_irqs(struct pci_dev *dev);
>> +void default_teardown_msi_irqs(struct msi_irqs *msi);
>> +void default_restore_msi_irqs(struct msi_irqs *msi);
>>  u32 default_msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag);
>>  u32 default_msix_mask_irq(struct msi_desc *desc, u32 flag);
>>
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index c7bca1c..d7126fc 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -334,8 +334,6 @@ struct pci_dev {
>>  	unsigned int	block_cfg_access:1;	/* config space access is blocked */
>>  	unsigned int	broken_parity_status:1;	/* Device generates false positive
>> parity */
>>  	unsigned int	irq_reroute_variant:2;	/* device needs IRQ rerouting
>> variant */
>> -	unsigned int	msi_enabled:1;
>> -	unsigned int	msix_enabled:1;
>>  	unsigned int	ari_enabled:1;	/* ARI forwarding */
>>  	unsigned int	is_managed:1;
>>  	unsigned int    needs_freset:1; /* Dev requires fundamental reset */
>> @@ -358,7 +356,7 @@ struct pci_dev {
>>  	struct bin_attribute *res_attr[DEVICE_COUNT_RESOURCE]; /* sysfs file for
>> resources */
>>  	struct bin_attribute *res_attr_wc[DEVICE_COUNT_RESOURCE]; /* sysfs file
>> for WC mapping of resources */
>>  #ifdef CONFIG_PCI_MSI
>> -	struct list_head msi_list;
>> +	struct msi_irqs *msi;
>>  	const struct attribute_group **msi_irq_groups;
>>  #endif
>>  	struct pci_vpd *vpd;
>> @@ -510,11 +508,14 @@ static inline struct pci_dev *pci_upstream_bridge(struct
>> pci_dev *dev)
>>  static inline bool pci_dev_msi_enabled(struct pci_dev *pci_dev, int type)
>>  {
>>  	bool enabled = 0;
>> +
>> +	if (!pci_dev->msi)
>> +		return false;
>>
>>  	if (type & MSI_TYPE)
>> -		enabled |= pci_dev->msi_enabled;
>> +		enabled |= pci_dev->msi->msi_enabled;
>>  	if (type & MSIX_TYPE)
>> -		enabled |= pci_dev->msix_enabled;
>> +		enabled |= pci_dev->msi->msix_enabled;
>>
>>  	return enabled;
>>  }
>> --
>> 1.7.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to majordomo@...r.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> .
> 


-- 
Thanks!
Yijing

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ