Do we need acpiphp_sanitize_bus() in acpiphp?
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;
}
}
}
}Bjorn Helgaas
maintainerQuoted 8 lines · Yijing Wang
Hi all,
I have some confusion onacpiphp_sanitize_bus()functions.If I understand wrong about this, correct me,thanks your help. I think there are three problems in this function:
1、Uselist_for_each_entryhere is not safe, becausepci_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, inreset_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.
Quoted 23 lines
/*
* Remove devices for which we could not assign resources, call
* arch specific code to fix-up the bus
*/
static voidacpiphp_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; } } } }
Quoted 10 lines · Yijing Wang
Quoted 8 lines
Hi all,
I have some confusion onacpiphp_sanitize_bus()functions.If I understand wrong about this, correct me,thanks your help. I think there are three problems in this function:
1、Uselist_for_each_entryhere is not safe, becausepci_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, inreset_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?
Quoted 4 lines
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.