Re: [PATCH] [pciextra] Restore the PCI COMMAND register after writing to the BARs

This is a multi-part message in MIME format.
--------------060201080506010708030808
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

On 1/22/10 10:25 AM, Bernhard Kohl wrote:
> By the way, I saw that my patch was already applied on the master 
> branch. There is some disordering of the comment lines. The line /* Find 
> the significant bits */ must go 2 lines lower.
> Thank You
> Bernhard

Hi Bernhard,

I rewrote the pci_bar_size function for clarity and to make clear that
we (gPXE developers) are not convinced that the saving of the
PCI_COMMAND word is needed.

Our larger concern is that we are masking a KVM PCI bug here, and we
would prefer not to add this work-around to our code if we can avoid it.
 As far as I can tell, the saving and restoring of that word is a NOOP.

Please test the attached patch and let us know if it works.

Thanks for your help with this,

/ Marty /



--------------060201080506010708030808
Content-Type: text/x-patch;
 name="pci_bar_size_patch.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="pci_bar_size_patch.patch"

diff --git a/src/drivers/bus/pciextra.c b/src/drivers/bus/pciextra.c
index 74c4099..6a330c4 100644
--- a/src/drivers/bus/pciextra.c
+++ b/src/drivers/bus/pciextra.c
@@ -50,37 +50,52 @@ int pci_find_capability ( struct pci_device *pci, int cap ) {
 }
 
 /**
- * Find the size of a PCI BAR
+ * Find the size of a PCI BAR (Base Address Register)
  *
- * @v pci		PCI device
- * @v reg		PCI register number
- * @ret size		BAR size
- *
- * It should not be necessary for any Etherboot code to call this
- * function.
+ * @v pci               PCI device
+ * @v bar               PCI BAR
+ * @ret bar_size        BAR size
  */
-unsigned long pci_bar_size ( struct pci_device *pci, unsigned int reg ) {
-	uint16_t cmd;
-	uint32_t start, size;
+unsigned long pci_bar_size ( struct pci_device *pci, unsigned int bar )
+{
+	uint16_t cmd_save;
+	uint32_t bar_save;
+	uint32_t bar_size;
 
-	/* Save the original command register */
-	pci_read_config_word ( pci, PCI_COMMAND, &cmd );
-	/* Save the original bar */
-	pci_read_config_dword ( pci, reg, &start );
-	/* Compute which bits can be set */
-	pci_write_config_dword ( pci, reg, ~0 );
-	pci_read_config_dword ( pci, reg, &size );
-	/* Restore the original size */
-	pci_write_config_dword ( pci, reg, start );
-	/* Find the significant bits */
-	/* Restore the original command register. This reenables decoding. */
-	pci_write_config_word ( pci, PCI_COMMAND, cmd );
-	if ( start & PCI_BASE_ADDRESS_SPACE_IO ) {
-		size &= PCI_BASE_ADDRESS_IO_MASK;
-	} else {
-		size &= PCI_BASE_ADDRESS_MEM_MASK;
-	}
-	/* Find the lowest bit set */
-	size = size & ~( size - 1 );
-	return size;
+	/* Save Command Word and BAR values */
+
+	/** FIXME: Is saving of PCI_COMMAND word really needed?
+	    Should memory (and I/O) decode be disabled as well? 
+	    The save and restore of PCI_COMMAND is reported to 
+	    fix a problem when running in KVM virtual machines, but
+	    it is possible the bug is in KVM's PCI code.
+	**/
+	pci_read_config_word   ( pci, PCI_COMMAND, &cmd_save );
+
+	pci_read_config_dword  ( pci, bar, &bar_save );
+
+	/* Get raw BAR size bits */
+	pci_write_config_dword ( pci, bar, ~0 );
+	pci_read_config_dword  ( pci, bar, &bar_size );
+
+	/* Restore Command Word and BAR values */
+	pci_write_config_dword ( pci, bar, bar_save );
+	pci_write_config_word  ( pci, PCI_COMMAND, cmd_save );
+
+	/* Complete BAR size calculation */
+	bar_size &= ( bar_save & PCI_BASE_ADDRESS_SPACE_IO ) ?
+		PCI_BASE_ADDRESS_IO_MASK :
+		PCI_BASE_ADDRESS_MEM_MASK;
+
+	bar_size = bar_size & ~( bar_size - 1 );
+
+	return bar_size;
 }
+
+/*
+ * Local variables:
+ *  c-basic-offset: 8
+ *  c-indent-level: 8
+ *  tab-width: 8
+ * End:
+ */

--------------060201080506010708030808
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
gPXE mailing list
[email protected]
http://etherboot.org/mailman/listinfo/gpxe

--------------060201080506010708030808--