[PATCH] pci: Return/dmesg errors when reading PCI ROMs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This patch makes the ROM reading code return an error to user space if the size of the ROM read is equal to 0.

The patch also emits dmesg-type warnings if the contents of the ROM are invalid, and documents the effects of the "enable" file on ROM reading.

The Acked-by indicates that Alex indicated to Timothy by private e-mail that his contribution to the patch below is released under the appropriate license.

Kernel version: 2.6.27
Signed-off-by: Timothy S. Nelson <wayland@xxxxxxxxxxxxx>
Acked-by: Alex Villacis-Lasso <a_villacis@xxxxxxxxxxxxx>

---

	Take 2.

This is my first kernel patch. Hopefully it's fine. Many thanks to those who helped with it (including answering my stupid questions, and pointing out problems). I'm hoping that Jesse, as maintainer, will pick it up from this list.

Since it seems I had trouble with word wrapping, and I can't see how it happened (it doesn't appear to have been word wrapped when it left my end), this time I've included it both inline and as an attachment; hopefully this is deemed acceptable.

diff -uprN linux-2.6.27/arch/ia64/sn/kernel/io_acpi_init.c linux-2.6.27.rom/arch/ia64/sn/kernel/io_acpi_init.c
--- linux-2.6.27/arch/ia64/sn/kernel/io_acpi_init.c	2008-10-10 09:13:53.000000000 +1100
+++ linux-2.6.27.rom/arch/ia64/sn/kernel/io_acpi_init.c	2009-01-29 23:28:48.000000000 +1100
@@ -434,7 +434,7 @@ sn_acpi_slot_fixup(struct pci_dev *dev)
 		size = pci_resource_len(dev, PCI_ROM_RESOURCE);
 		addr = ioremap(pcidev_info->pdi_pio_mapped_addr[PCI_ROM_RESOURCE],
 			       size);
-		image_size = pci_get_rom_size(addr, size);
+		image_size = pci_get_rom_size(dev, addr, size);
 		dev->resource[PCI_ROM_RESOURCE].start = (unsigned long) addr;
 		dev->resource[PCI_ROM_RESOURCE].end =
 					(unsigned long) addr + image_size - 1;
diff -uprN linux-2.6.27/arch/ia64/sn/kernel/io_init.c linux-2.6.27.rom/arch/ia64/sn/kernel/io_init.c
--- linux-2.6.27/arch/ia64/sn/kernel/io_init.c	2008-10-10 09:13:53.000000000 +1100
+++ linux-2.6.27.rom/arch/ia64/sn/kernel/io_init.c	2009-01-29 23:29:12.000000000 +1100
@@ -269,7 +269,7 @@ sn_io_slot_fixup(struct pci_dev *dev)

 			rom = ioremap(pci_resource_start(dev, PCI_ROM_RESOURCE),
 				      size + 1);
-			image_size = pci_get_rom_size(rom, size + 1);
+			image_size = pci_get_rom_size(dev, rom, size + 1);
 			dev->resource[PCI_ROM_RESOURCE].end =
 				dev->resource[PCI_ROM_RESOURCE].start +
 				image_size - 1;
diff -uprN linux-2.6.27/Documentation/filesystems/sysfs-pci.txt linux-2.6.27.rom/Documentation/filesystems/sysfs-pci.txt
--- linux-2.6.27/Documentation/filesystems/sysfs-pci.txt	2008-10-10 09:13:53.000000000 +1100
+++ linux-2.6.27.rom/Documentation/filesystems/sysfs-pci.txt	2009-01-29 23:11:31.000000000 +1100
@@ -9,6 +9,7 @@ that support it.  For example, a given b
      |   |-- class
      |   |-- config
      |   |-- device
+     |   |-- enable
      |   |-- irq
      |   |-- local_cpus
      |   |-- resource
@@ -32,6 +33,7 @@ files, each with their own function.
        class		   PCI class (ascii, ro)
        config		   PCI config space (binary, rw)
        device		   PCI device (ascii, ro)
+       enable              Whether the device is enabled (ascii, rw)
        irq		   IRQ number (ascii, ro)
        local_cpus	   nearby CPU mask (cpumask, ro)
        resource		   PCI resource host addresses (ascii, ro)
@@ -57,10 +59,18 @@ used to do actual device programming fro
 don't support mmapping of certain resources, so be sure to check the return
 value from any attempted mmap.

+The 'enable' file provides a counter that indicates how many times the device +has been enabled. If the 'enable' file currently returns '4', and a '1' is
+echoed into it, it will then return '5'.  Echoing a '0' into it will decrease
+the count.  Even when it returns to 0, though, some of the initialisation
+may not be reversed. +
 The 'rom' file is special in that it provides read-only access to the device's
 ROM file, if available.  It's disabled by default, however, so applications
 should write the string "1" to the file to enable it before attempting a read
-call, and disable it following the access by writing "0" to the file.
+call, and disable it following the access by writing "0" to the file. Even +when enabled, it won't return any sensible result unless the "enable" file has
+also had a "1" echoed into it.

 Accessing legacy resources through sysfs
 ----------------------------------------
diff -uprN linux-2.6.27/drivers/pci/pci-sysfs.c linux-2.6.27.rom/drivers/pci/pci-sysfs.c
--- linux-2.6.27/drivers/pci/pci-sysfs.c	2008-10-10 09:13:53.000000000 +1100
+++ linux-2.6.27.rom/drivers/pci/pci-sysfs.c	2009-01-29 23:25:38.000000000 +1100
@@ -694,8 +694,8 @@ pci_read_rom(struct kobject *kobj, struc
 		return -EINVAL;

 	rom = pci_map_rom(pdev, &size);	/* size starts out as PCI window size */
-	if (!rom)
-		return 0;
+	if (!rom || !size)
+		return -EIO;

 	if (off >= size)
 		count = 0;
diff -uprN linux-2.6.27/drivers/pci/rom.c linux-2.6.27.rom/drivers/pci/rom.c
--- linux-2.6.27/drivers/pci/rom.c	2008-10-10 09:13:53.000000000 +1100
+++ linux-2.6.27.rom/drivers/pci/rom.c	2009-01-30 00:33:01.000000000 +1100
@@ -63,7 +63,7 @@ static void pci_disable_rom(struct pci_d
  * The PCI window size could be much larger than the
  * actual image size.
  */
-size_t pci_get_rom_size(void __iomem *rom, size_t size)
+size_t pci_get_rom_size(struct pci_dev *pdev, void __iomem *rom, size_t size)
 {
 	void __iomem *image;
 	int last_image;
@@ -72,8 +72,10 @@ size_t pci_get_rom_size(void __iomem *ro
 	do {
 		void __iomem *pds;
 		/* Standard PCI ROMs start out with these bytes 55 AA */
-		if (readb(image) != 0x55)
+		if (readb(image) != 0x55) {
+			dev_err(&pdev->dev, "Invalid ROM contents\n");
 			break;
+		}
 		if (readb(image + 1) != 0xAA)
 			break;
 		/* get the PCI data structure and check its signature */
@@ -158,7 +160,7 @@ void __iomem *pci_map_rom(struct pci_dev
 	 * size is much larger than the actual size of the ROM.
 	 * True size is important if the ROM is going to be copied.
 	 */
-	*size = pci_get_rom_size(rom, *size);
+	*size = pci_get_rom_size(pdev, rom, *size);
 	return rom;
 }

diff -uprN linux-2.6.27/include/linux/pci.h linux-2.6.27.rom/include/linux/pci.h
--- linux-2.6.27/include/linux/pci.h	2008-10-10 09:13:53.000000000 +1100
+++ linux-2.6.27.rom/include/linux/pci.h	2009-01-29 23:28:13.000000000 +1100
@@ -633,7 +633,7 @@ int pci_select_bars(struct pci_dev *dev,
 /* ROM control related routines */
 void __iomem __must_check *pci_map_rom(struct pci_dev *pdev, size_t *size);
 void pci_unmap_rom(struct pci_dev *pdev, void __iomem *rom);
-size_t pci_get_rom_size(void __iomem *rom, size_t size);
+size_t pci_get_rom_size(struct pci_dev *pdev, void __iomem *rom, size_t size);

 /* Power management related routines */
 int pci_save_state(struct pci_dev *dev);

---------------------------------------------------------------------
| Name: Tim Nelson                 | Because the Creator is,        |
| E-mail: wayland@xxxxxxxxxxxxx    | I am                           |
---------------------------------------------------------------------

----BEGIN GEEK CODE BLOCK----
Version 3.12
GCS d+++ s+: a- C++$ U+++$ P+++$ L+++ E- W+ N+ w--- V- PE(+) Y+>++ PGP->+++ R(+) !tv b++ DI++++ D G+ e++>++++ h! y-
-----END GEEK CODE BLOCK-----
diff -uprN linux-2.6.27/arch/ia64/sn/kernel/io_acpi_init.c linux-2.6.27.rom/arch/ia64/sn/kernel/io_acpi_init.c
--- linux-2.6.27/arch/ia64/sn/kernel/io_acpi_init.c	2008-10-10 09:13:53.000000000 +1100
+++ linux-2.6.27.rom/arch/ia64/sn/kernel/io_acpi_init.c	2009-01-29 23:28:48.000000000 +1100
@@ -434,7 +434,7 @@ sn_acpi_slot_fixup(struct pci_dev *dev)
 		size = pci_resource_len(dev, PCI_ROM_RESOURCE);
 		addr = ioremap(pcidev_info->pdi_pio_mapped_addr[PCI_ROM_RESOURCE],
 			       size);
-		image_size = pci_get_rom_size(addr, size);
+		image_size = pci_get_rom_size(dev, addr, size);
 		dev->resource[PCI_ROM_RESOURCE].start = (unsigned long) addr;
 		dev->resource[PCI_ROM_RESOURCE].end =
 					(unsigned long) addr + image_size - 1;
diff -uprN linux-2.6.27/arch/ia64/sn/kernel/io_init.c linux-2.6.27.rom/arch/ia64/sn/kernel/io_init.c
--- linux-2.6.27/arch/ia64/sn/kernel/io_init.c	2008-10-10 09:13:53.000000000 +1100
+++ linux-2.6.27.rom/arch/ia64/sn/kernel/io_init.c	2009-01-29 23:29:12.000000000 +1100
@@ -269,7 +269,7 @@ sn_io_slot_fixup(struct pci_dev *dev)
 
 			rom = ioremap(pci_resource_start(dev, PCI_ROM_RESOURCE),
 				      size + 1);
-			image_size = pci_get_rom_size(rom, size + 1);
+			image_size = pci_get_rom_size(dev, rom, size + 1);
 			dev->resource[PCI_ROM_RESOURCE].end =
 				dev->resource[PCI_ROM_RESOURCE].start +
 				image_size - 1;
diff -uprN linux-2.6.27/Documentation/filesystems/sysfs-pci.txt linux-2.6.27.rom/Documentation/filesystems/sysfs-pci.txt
--- linux-2.6.27/Documentation/filesystems/sysfs-pci.txt	2008-10-10 09:13:53.000000000 +1100
+++ linux-2.6.27.rom/Documentation/filesystems/sysfs-pci.txt	2009-01-29 23:11:31.000000000 +1100
@@ -9,6 +9,7 @@ that support it.  For example, a given b
      |   |-- class
      |   |-- config
      |   |-- device
+     |   |-- enable
      |   |-- irq
      |   |-- local_cpus
      |   |-- resource
@@ -32,6 +33,7 @@ files, each with their own function.
        class		   PCI class (ascii, ro)
        config		   PCI config space (binary, rw)
        device		   PCI device (ascii, ro)
+       enable              Whether the device is enabled (ascii, rw)
        irq		   IRQ number (ascii, ro)
        local_cpus	   nearby CPU mask (cpumask, ro)
        resource		   PCI resource host addresses (ascii, ro)
@@ -57,10 +59,18 @@ used to do actual device programming fro
 don't support mmapping of certain resources, so be sure to check the return
 value from any attempted mmap.
 
+The 'enable' file provides a counter that indicates how many times the device 
+has been enabled.  If the 'enable' file currently returns '4', and a '1' is
+echoed into it, it will then return '5'.  Echoing a '0' into it will decrease
+the count.  Even when it returns to 0, though, some of the initialisation
+may not be reversed.  
+
 The 'rom' file is special in that it provides read-only access to the device's
 ROM file, if available.  It's disabled by default, however, so applications
 should write the string "1" to the file to enable it before attempting a read
-call, and disable it following the access by writing "0" to the file.
+call, and disable it following the access by writing "0" to the file.  Even 
+when enabled, it won't return any sensible result unless the "enable" file has
+also had a "1" echoed into it.  
 
 Accessing legacy resources through sysfs
 ----------------------------------------
diff -uprN linux-2.6.27/drivers/pci/pci-sysfs.c linux-2.6.27.rom/drivers/pci/pci-sysfs.c
--- linux-2.6.27/drivers/pci/pci-sysfs.c	2008-10-10 09:13:53.000000000 +1100
+++ linux-2.6.27.rom/drivers/pci/pci-sysfs.c	2009-01-29 23:25:38.000000000 +1100
@@ -694,8 +694,8 @@ pci_read_rom(struct kobject *kobj, struc
 		return -EINVAL;
 	
 	rom = pci_map_rom(pdev, &size);	/* size starts out as PCI window size */
-	if (!rom)
-		return 0;
+	if (!rom || !size)
+		return -EIO;
 		
 	if (off >= size)
 		count = 0;
diff -uprN linux-2.6.27/drivers/pci/rom.c linux-2.6.27.rom/drivers/pci/rom.c
--- linux-2.6.27/drivers/pci/rom.c	2008-10-10 09:13:53.000000000 +1100
+++ linux-2.6.27.rom/drivers/pci/rom.c	2009-01-30 00:33:01.000000000 +1100
@@ -63,7 +63,7 @@ static void pci_disable_rom(struct pci_d
  * The PCI window size could be much larger than the
  * actual image size.
  */
-size_t pci_get_rom_size(void __iomem *rom, size_t size)
+size_t pci_get_rom_size(struct pci_dev *pdev, void __iomem *rom, size_t size)
 {
 	void __iomem *image;
 	int last_image;
@@ -72,8 +72,10 @@ size_t pci_get_rom_size(void __iomem *ro
 	do {
 		void __iomem *pds;
 		/* Standard PCI ROMs start out with these bytes 55 AA */
-		if (readb(image) != 0x55)
+		if (readb(image) != 0x55) {
+			dev_err(&pdev->dev, "Invalid ROM contents\n");
 			break;
+		}
 		if (readb(image + 1) != 0xAA)
 			break;
 		/* get the PCI data structure and check its signature */
@@ -158,7 +160,7 @@ void __iomem *pci_map_rom(struct pci_dev
 	 * size is much larger than the actual size of the ROM.
 	 * True size is important if the ROM is going to be copied.
 	 */
-	*size = pci_get_rom_size(rom, *size);
+	*size = pci_get_rom_size(pdev, rom, *size);
 	return rom;
 }
 
diff -uprN linux-2.6.27/include/linux/pci.h linux-2.6.27.rom/include/linux/pci.h
--- linux-2.6.27/include/linux/pci.h	2008-10-10 09:13:53.000000000 +1100
+++ linux-2.6.27.rom/include/linux/pci.h	2009-01-29 23:28:13.000000000 +1100
@@ -633,7 +633,7 @@ int pci_select_bars(struct pci_dev *dev,
 /* ROM control related routines */
 void __iomem __must_check *pci_map_rom(struct pci_dev *pdev, size_t *size);
 void pci_unmap_rom(struct pci_dev *pdev, void __iomem *rom);
-size_t pci_get_rom_size(void __iomem *rom, size_t size);
+size_t pci_get_rom_size(struct pci_dev *pdev, void __iomem *rom, size_t size);
 
 /* Power management related routines */
 int pci_save_state(struct pci_dev *dev);

[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux