Do we need acpiphp_sanitize_bus() in acpiphp?

Do we need acpiphp_sanitize_bus() in acpiphp?

[Yijing Wang]

Hi all,
   I have some confusion on acpiphp_sanitize_bus() functions.If I understand wrong about this, correct me,thanks your help.
   I think there are three problems in this function:
   1、Use list_for_each_entry here is not safe, because pci_stop_and_remove_bus_device() will remove the device from bus.
   2、If assign resources fail after hot-add a device, the resource will reset to zero, in reset_resource(), res->start = 0,
      res->end =0, res->end = 0.
   3、I think leave the fail pci devices in the system is better rather than remove them, so we can see all pci devices
      by lspci after hot-add, what about your opinion?

thanks!
Yijing.

/*
 * Remove devices for which we could not assign resources, call
 * arch specific code to fix-up the bus
 */
static void acpiphp_sanitize_bus(struct pci_bus *bus)
{
	struct pci_dev *dev;
	int i;
	unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM;

	list_for_each_entry(dev, &bus->devices, bus_list) {
		for (i=0; i<PCI_BRIDGE_RESOURCES; i++) {
			struct resource *res = &dev->resource[i];
			if ((res->flags & type_mask) && !res->start &&
					res->end) {
				/* Could not assign a required resources
				 * for this device, remove it */
				pci_stop_and_remove_bus_device(dev);
				break;
			}
		}
	}
}

Re: Do we need acpiphp_sanitize_bus() in acpiphp?

[Bjorn Helgaas]

On Wed, Aug 15, 2012 at 7:13 AM, Yijing Wang <wangyijing0307@gmail.com> wrote:
> Hi all,
>    I have some confusion on acpiphp_sanitize_bus() functions.If I understand wrong about this, correct me,thanks your help.
>    I think there are three problems in this function:
>    1、Use list_for_each_entry here is not safe, because pci_stop_and_remove_bus_device() will remove the device from bus.
>    2、If assign resources fail after hot-add a device, the resource will reset to zero, in reset_resource(), res->start = 0,
>       res->end =0, res->end = 0.
>    3、I think leave the fail pci devices in the system is better rather than remove them, so we can see all pci devices
>       by lspci after hot-add, what about your opinion?

These sound like reasonable concerns.  The best thing to do is to
propose patches (separate ones for each logical issue).  Then we can
review a concrete change and apply it if appropriate.

> /*
>  * Remove devices for which we could not assign resources, call
>  * arch specific code to fix-up the bus
>  */
> static void acpiphp_sanitize_bus(struct pci_bus *bus)
> {
>         struct pci_dev *dev;
>         int i;
>         unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM;
>
>         list_for_each_entry(dev, &bus->devices, bus_list) {
>                 for (i=0; i<PCI_BRIDGE_RESOURCES; i++) {
>                         struct resource *res = &dev->resource[i];
>                         if ((res->flags & type_mask) && !res->start &&
>                                         res->end) {
>                                 /* Could not assign a required resources
>                                  * for this device, remove it */
>                                 pci_stop_and_remove_bus_device(dev);
>                                 break;
>                         }
>                 }
>         }
> }

Re: Do we need acpiphp_sanitize_bus() in acpiphp?

[Yijing Wang]

> On Wed, Aug 15, 2012 at 7:13 AM, Yijing Wang <wangyijing0307@gmail.com> wrote:
>> Hi all,
>>    I have some confusion on acpiphp_sanitize_bus() functions.If I understand wrong about this, correct me,thanks your help.
>>    I think there are three problems in this function:
>>    1、Use list_for_each_entry here is not safe, because pci_stop_and_remove_bus_device() will remove the device from bus.
>>    2、If assign resources fail after hot-add a device, the resource will reset to zero, in reset_resource(), res->start = 0,
>>       res->end =0, res->end = 0.
>>    3、I think leave the fail pci devices in the system is better rather than remove them, so we can see all pci devices
>>       by lspci after hot-add, what about your opinion?
> 
> These sound like reasonable concerns.  The best thing to do is to
> propose patches (separate ones for each logical issue).  Then we can
> review a concrete change and apply it if appropriate.
> 
Hi Bjorn,
   Thanks for your comments!
   You are right, I will provide some patches as soon.