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

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

On 1/29/10 8:39 AM, Marty Connor wrote:
> 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 /

I messed up the first patch.  Please try the attached one instead.

Thanks,

/ Marty /



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

diff --git a/src/drivers/bus/pciextra.c b/src/drivers/bus/pciextra.c
index 74c4099..fb48cde 100644
--- a/src/drivers/bus/pciextra.c
+++ b/src/drivers/bus/pciextra.c
@@ -50,37 +50,48 @@ 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_size & 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:
+ */

--------------060302090901020208010207
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

--------------060302090901020208010207--