On Mon, Mar 02, 2009 at 03:08:15PM +0800, Yuji Shimada wrote:
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index b182626..290e701 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -1756,6 +1756,15 @@ and is between 256 and 4096 characters. It is defined in the file
> cbmemsize=nn[KMG] The fixed amount of bus space which is
> reserved for the CardBus bridge's memory
> window. The default value is 64 megabytes.
> + resource_alignment=
> + Format:
> + [<order of align>@][<domain>:]<bus>:<slot>.<func>[; ...]
> + Specifies alignment and device to reassign
> + aligned memory resources.
> + If <order of align> is not specified,
> + PAGE_SIZE is used as alignment.
> + PCI-PCI bridge can be specified, if resource
> + windows need to be expanded.
>
> pcie_aspm= [PCIE] Forcibly enable or disable PCIe Active State Power
> Management.
Please remove trailing spaces in the patch.
> +/**
> + * pci_specified_resource_alignemnt - get resource alignment specified by user.
> + * @dev: the PCI device to get
> + *
> + * RETURNS: Resource alignment if it is specified.
> + * Zero if it is not specified.
> + */
> +resource_size_t pci_specified_resource_alignment(struct pci_dev *dev)
> +{
> + int seg, bus, slot, func, align_order, count;
> + resource_size_t align = 0;
> + char *p;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&resource_alignment_lock, flags);
These functions won't be called in interrupt handler, right? So it doesn't
need to disable interrupt.
> + p = resource_alignment_param;
> + while (*p) {
> + count = 0;
> + if (sscanf(p, "%d%n", &align_order, &count) == 1 &&
> + p[count] == '@') {
> + p += count + 1;
> + } else {
> + align_order = -1;
> + }
> + if (sscanf(p, "%x:%x:%x.%x%n",
> + &seg, &bus, &slot, &func, &count) != 4) {
> + seg = 0;
> + if (sscanf(p, "%x:%x.%x%n",
> + &bus, &slot, &func, &count) != 3) {
> + /* Invalid format */
> + printk(KERN_ERR "PCI: Can't parse resource_alignment parameter: %s\n",
> + p);
> + break;
> + }
> + }
> + p += count;
> + if (seg == pci_domain_nr(dev->bus) &&
> + bus == dev->bus->number &&
> + slot == PCI_SLOT(dev->devfn) &&
> + func == PCI_FUNC(dev->devfn)) {
> + if (align_order == -1) {
> + align = PAGE_SIZE;
> + } else {
> + align = 1 << align_order;
> + }
> + /* Found */
> + break;
> + }
> + if (*p != ';' && *p != ',') {
> + /* End of param or invalid format */
> + break;
> + }
> + p++;
> + }
> + spin_unlock_irqrestore(&resource_alignment_lock, flags);
> + return align;
> +}
> +
> +/**
> + * pci_is_reassigndev - check if specified PCI is target device to reassign
> + * @dev: the PCI device to check
> + *
> + * RETURNS: non-zero for PCI device is a target device to reassign,
> + * or zero is not.
> + */
> +int pci_is_reassigndev(struct pci_dev *dev)
> +{
> + return (pci_specified_resource_alignment(dev) != 0);
> +}
Looks like this wrapper is unnecessary.
> +
> +ssize_t pci_set_resource_alignment_param(const char *buf, size_t count)
> +{
> + unsigned long flags;
> + if (count > RESOURCE_ALIGNMENT_PARAM_SIZE - 1)
> + count = RESOURCE_ALIGNMENT_PARAM_SIZE - 1;
> + spin_lock_irqsave(&resource_alignment_lock, flags);
> + strncpy(resource_alignment_param, buf, count);
> + resource_alignment_param[count] = '\0';
> + spin_unlock_irqrestore(&resource_alignment_lock, flags);
> + return count;
> +}
> +
> +ssize_t pci_get_resource_alignment_param(char *buf, size_t size)
> +{
> + unsigned long flags;
> + size_t count;
> + spin_lock_irqsave(&resource_alignment_lock, flags);
> + count = snprintf(buf, size, "%s", resource_alignment_param);
> + spin_unlock_irqrestore(&resource_alignment_lock, flags);
> + return count;
> +}
> +
> +static void __devinit pci_resource_alignment_setup(char *str)
> +{
> + pci_set_resource_alignment_param(str, strlen(str));
> +}
Ditto.
> +
> +static ssize_t pci_resource_alignment_show(struct bus_type *bus, char *buf)
> +{
> + return pci_get_resource_alignment_param(buf, PAGE_SIZE);
> +}
> +
> +static ssize_t pci_resource_alignment_store(struct bus_type *bus,
> + const char *buf, size_t count)
> +{
> + return pci_set_resource_alignment_param(buf, count);
Check user input here?
> +}
> +
> +BUS_ATTR(resource_alignment, 0644, pci_resource_alignment_show,
> + pci_resource_alignment_store);
> +
> index f20d553..b17f1aa 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -33,6 +33,63 @@ int pcie_mch_quirk;
> EXPORT_SYMBOL(pcie_mch_quirk);
>
> #ifdef CONFIG_PCI_QUIRKS
> +/*
> + * This quirk function disables the device and releases resources
> + * which is specified by kernel's boot parameter 'pci=resource_alignment='.
> + * It also rounds up size to specified alignment.
> + * Later on, the kernel will assign page-aligned memory resource back
> + * to that device.
> + */
> +static void __devinit quirk_resource_alignment(struct pci_dev *dev)
> +{
> + int i;
> + struct resource *r;
> + resource_size_t align, size;
> +
> + if (!pci_is_reassigndev(dev))
> + return;
Use pci_specified_resource_alignment() directly here so it doesn't need
to call it again below?
> +
> + if (dev->hdr_type == PCI_HEADER_TYPE_NORMAL &&
> + (dev->class >> 8) == PCI_CLASS_BRIDGE_HOST) {
> + /* PCI host bridge isn't a target device */
> + dev_warn(&dev->dev,
> + "Can't reassign resources to host bridge.\n");
> + return;
> + }
> +
> + dev_info(&dev->dev, "Disabling device and release resources.\n");
> + pci_disable_device(dev);
> +
> + align = pci_specified_resource_alignment(dev);
> + for (i=0; i < PCI_NUM_RESOURCES; i++) {
PCI_NUM_RESOURCES includes bus resources, it should be PCI_BRIDGE_RESOURCES.
> + r = &dev->resource[i];
> + if (!(r->flags & IORESOURCE_MEM))
> + continue;
> + size = r->end - r->start + 1;
Please use resource_size().
> + if (size < align) {
> + size = align;
> + dev_info(&dev->dev,
> + "Rounding up size of resource #%d to %#llx.\n",
> + i, (unsigned long long)size);
> + }
> + r->end = size - 1;
> + r->start = 0;
> +
> + if (i < PCI_BRIDGE_RESOURCES) {
> + pci_update_resource(dev, i);
> + }
The resource is not allocated, so it shouldn't call pci_update_resource(),
especially with start=0.
> + }
> + /* need to disable bridge's resource window,
> + * to enable the kernel to reassign new resource
> + * window later on.
> + */
> + if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE &&
> + (dev->class >> 8) == PCI_CLASS_BRIDGE_PCI) {
> + pci_disable_bridge_window(dev);
> + }
> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, quirk_resource_alignment);
--
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
