On Tuesday 21 October 2008 15:16:06 Matthew Wilcox wrote:
> On Fri, Oct 17, 2008 at 09:39:53AM +0800, Sheng Yang wrote:
> > Can you check this in? I have been waiting for more than 3 weeks without
> > any response from Matthew. I have indeed run out of patience and got no
> > idea what happened.
>
> I was busy with other things. I can't spend all my time reviewing
> patches; I have to spend some time working on my own projects.
> Here's a version of the patch done the way I think it needs to be done.
> Compared to the version you posted:
>
> - Renamed functions to change pcie_ into pci_
> - Changed the export to be _GPL only
> - Switched the functions around so that pci_execute_reset_function() is
> defined first
> - Removed all references to 'FLR' in documentation
> - Moved the disabling of port / memory / interrupts to
> pci_reset_function() - Changed the error messages
> - Generally improved the kerneldoc
>
Thanks again for being so detail... I will update the patch follow this
_exactly_.
> I think it would be a good idea to add a 'reset' method to the pci_driver
> struct. Certainly the sym2 driver could implement it to do a reset of
> the device. This can be a later addition though.
I will take a look at this.
> More importantly,
> we need users for this -- I'm very reluctant to see this merged without
> some assurance that we'll see users in the very near term. Do you have
> any patches to existing drivers to use this functionality?
Yes, of course. KVM will use this as soon as it was checked in. A very simple
patch can help KVM handle device moving to another partition(guest in kvm)
well, otherwise some behavior like kill guest directly may result in device's
transcation didn't stop, and cause trouble in host.
And I will continue to work on this for emulate FLR by using D0/D3, which is
more common feature in the current device.
Well, I also want users other than KVM, and it's a good chance to get involved
in PCI subsystem for me. I will also take a look at AER part as well.
Thanks.
--
regards
Yang, Sheng
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 4db261e..3bdaed6 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -18,6 +18,7 @@
> #include <linux/log2.h>
> #include <linux/pci-aspm.h>
> #include <linux/pm_wakeup.h>
> +#include <linux/interrupt.h>
> #include <asm/dma.h> /* isa_dma_bridge_buggy */
> #include "pci.h"
>
> @@ -1746,6 +1747,103 @@ EXPORT_SYMBOL(pci_set_dma_seg_boundary);
> #endif
>
> /**
> + * pci_execute_reset_function() - Reset a PCI device function
> + * @dev: Device function to reset
> + *
> + * Some devices allow an individual function to be reset without affecting
> + * other functions in the same device. The PCI device must be responsive
> + * to PCI config space in order to use this function.
> + *
> + * The device function is presumed to be unused when this function is
> called. + * Resetting the device will make the contents of PCI
> configuration space + * random, so any caller of this must be prepared to
> reinitialise the + * device including MSI, bus mastering, BARs, decoding IO
> and memory spaces, + * etc.
> + *
> + * Returns 0 if the device function was successfully reset or -ENOTTY if
> the + * device doesn't support resetting a single function.
> + */
> +int pci_execute_reset_function(struct pci_dev *dev)
> +{
> + u16 status;
> + u32 cap;
> + int exppos = pci_find_capability(dev, PCI_CAP_ID_EXP);
> +
> + if (!exppos)
> + return -ENOTTY;
> + pci_read_config_dword(dev, exppos + PCI_EXP_DEVCAP, &cap);
> + if (!(cap & PCI_EXP_DEVCAP_FLR))
> + return -ENOTTY;
> +
> + pci_block_user_cfg_access(dev);
> +
> + /* Wait for Transaction Pending bit clean */
> + msleep(100);
> + pci_read_config_word(dev, exppos + PCI_EXP_DEVSTA, &status);
> + if (status & PCI_EXP_DEVSTA_TRPND) {
> + dev_info(&dev->dev, "Busy after 100ms while trying to reset; "
> + "sleeping for 1 second\n");
> + ssleep(1);
> + pci_read_config_word(dev, exppos + PCI_EXP_DEVSTA, &status);
> + if (status & PCI_EXP_DEVSTA_TRPND)
> + dev_info(&dev->dev, "Still busy after 1s; "
> + "proceeding with reset anyway\n");
> + }
> +
> + pci_write_config_word(dev, exppos + PCI_EXP_DEVCTL,
> + PCI_EXP_DEVCTL_BCR_FLR);
> + mdelay(100);
> +
> + pci_unblock_user_cfg_access(dev);
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(pci_execute_reset_function);
> +
> +/**
> + * pci_reset_function() - quiesce and reset a PCI device function
> + * @dev: Device function to reset
> + *
> + * Some devices allow an individual function to be reset without affecting
> + * other functions in the same device. The PCI device must be responsive
> + * to PCI config space in order to use this function.
> + *
> + * This function does not just reset the PCI portion of a device, but
> + * clears all the state associated with the device. This function differs
> + * from pci_execute_reset_function in that it saves and restores device
> state + * over the reset.
> + *
> + * Returns 0 if the device function was successfully reset or -ENOTTY if
> the + * device doesn't support resetting a single function.
> + */
> +int pci_reset_function(struct pci_dev *dev)
> +{
> + u32 cap;
> + int exppos = pci_find_capability(dev, PCI_CAP_ID_EXP);
> + int r;
> +
> + if (!exppos)
> + return -ENOTTY;
> + pci_read_config_dword(dev, exppos + PCI_EXP_DEVCAP, &cap);
> + if (!(cap & PCI_EXP_DEVCAP_FLR))
> + return -ENOTTY;
> +
> + if (!dev->msi_enabled && !dev->msix_enabled)
> + disable_irq(dev->irq);
> + pci_save_state(dev);
> +
> + pci_write_config_word(dev, PCI_COMMAND, PCI_COMMAND_INTX_DISABLE);
> +
> + r = pci_execute_reset_function(dev);
> +
> + pci_restore_state(dev);
> + if (!dev->msi_enabled && !dev->msix_enabled)
> + enable_irq(dev->irq);
> +
> + return r;
> +}
> +EXPORT_SYMBOL_GPL(pci_reset_function);
> +
> +/**
> * pcix_get_max_mmrbc - get PCI-X maximum designed memory read byte count
> * @dev: PCI device to query
> *
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 085187b..f6f6810 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -626,6 +626,8 @@ int pcix_get_mmrbc(struct pci_dev *dev);
> int pcix_set_mmrbc(struct pci_dev *dev, int mmrbc);
> int pcie_get_readrq(struct pci_dev *dev);
> int pcie_set_readrq(struct pci_dev *dev, int rq);
> +int pci_reset_function(struct pci_dev *dev);
> +int pci_execute_reset_function(struct pci_dev *dev);
> void pci_update_resource(struct pci_dev *dev, struct resource *res, int
> resno); int __must_check pci_assign_resource(struct pci_dev *dev, int i);
> int pci_select_bars(struct pci_dev *dev, unsigned long flags);
> diff --git a/include/linux/pci_regs.h b/include/linux/pci_regs.h
> index eb6686b..e5effd4 100644
> --- a/include/linux/pci_regs.h
> +++ b/include/linux/pci_regs.h
> @@ -377,6 +377,7 @@
> #define PCI_EXP_DEVCAP_RBER 0x8000 /* Role-Based Error Reporting */
> #define PCI_EXP_DEVCAP_PWR_VAL 0x3fc0000 /* Slot Power Limit Value */
> #define PCI_EXP_DEVCAP_PWR_SCL 0xc000000 /* Slot Power Limit Scale */
> +#define PCI_EXP_DEVCAP_FLR 0x10000000 /* Function Level Reset */
> #define PCI_EXP_DEVCTL 8 /* Device Control */
> #define PCI_EXP_DEVCTL_CERE 0x0001 /* Correctable Error Reporting En. */
> #define PCI_EXP_DEVCTL_NFERE 0x0002 /* Non-Fatal Error Reporting Enable
> */ @@ -389,6 +390,7 @@
> #define PCI_EXP_DEVCTL_AUX_PME 0x0400 /* Auxiliary Power PM Enable */
> #define PCI_EXP_DEVCTL_NOSNOOP_EN 0x0800 /* Enable No Snoop */
> #define PCI_EXP_DEVCTL_READRQ 0x7000 /* Max_Read_Request_Size */
> +#define PCI_EXP_DEVCTL_BCR_FLR 0x8000 /* Bridge Configuration Retry /
> FLR */ #define PCI_EXP_DEVSTA 10 /* Device Status */
> #define PCI_EXP_DEVSTA_CED 0x01 /* Correctable Error Detected */
> #define PCI_EXP_DEVSTA_NFED 0x02 /* Non-Fatal Error Detected */
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
