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

[Stefan Hajnoczi]

Hi Kevin,
I think you might have some insight from SeaBIOS into this issue.

Bernhard reports that under KVM PCI device passthrough it is necessary
to save the contents of PCI_COMMAND and restore it after sizing a PCI
BAR.  Otherwise PCI config space seems to contain junk after BAR
sizing.  gPXE currently has this code change applied, but I don't
think it is the root cause of this issue:

http://git.etherboot.org/?p=gpxe.git;a=blob;f=src/drivers/bus/pciextra.c;h=7...

Note that gPXE is not disabling memory decode in PCI_COMMAND during
BAR sizing (and Linux doesn't either AFAIK).  I'm surprised that
restoring an unchanged PCI_COMMAND would have any effect at all.

When gPXE is built to do Type 1 accesses
(gpxe/src/arch/x86/include/gpxe/pcidirect.h) instead of PCI BIOS then
this issue does not appear.

Any idea what is happening?

Full email thread here:
http://etherboot.org/pipermail/gpxe/2010-January/000287.html

Stefan

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

[Kevin O'Connor]

On Wed, Feb 10, 2010 at 09:57:52AM +0000, Stefan Hajnoczi wrote:
> I think you might have some insight from SeaBIOS into this issue.
>
> Bernhard reports that under KVM PCI device passthrough it is necessary
> to save the contents of PCI_COMMAND and restore it after sizing a PCI
> BAR.  Otherwise PCI config space seems to contain junk after BAR
> sizing.  gPXE currently has this code change applied, but I don't
> think it is the root cause of this issue:

Hi Stefan,

I don't know why that would be.  My first guess would be something
that kvm is doing (if I understand correctly, kvm has to remap the
physical address in a BAR to the guest's physical memory location).

The patch below to SeaBIOS could help track down the issue.  Apply the
patch, edit SeaBIOS' src/config.h and set CONFIG_SERIAL_DEBUG to 1,
and then run kvm with "-serial file:foo".  SeaBIOS' debugging output
will be in the file.

-Kevin

diff --git a/src/pcibios.c b/src/pcibios.c
index a23248b..3069151 100644
--- a/src/pcibios.c
+++ b/src/pcibios.c
@@ -82,48 +82,60 @@ handle_1ab103(struct bregs *regs)
 static void
 handle_1ab108(struct bregs *regs)
 {
+    dprintf(1, "val=%x\n", pci_config_readw(regs->bx, PCI_COMMAND));
     regs->cl = pci_config_readb(regs->bx, regs->di);
     set_code_success(regs);
+    dprintf(1, "val=%x\n", pci_config_readw(regs->bx, PCI_COMMAND));
 }

// read configuration word
 static void
 handle_1ab109(struct bregs *regs)
 {
+    dprintf(1, "val=%x\n", pci_config_readw(regs->bx, PCI_COMMAND));
     regs->cx = pci_config_readw(regs->bx, regs->di);
     set_code_success(regs);
+    dprintf(1, "val=%x\n", pci_config_readw(regs->bx, PCI_COMMAND));
 }

// read configuration dword
 static void
 handle_1ab10a(struct bregs *regs)
 {
+    dprintf(1, "val=%x\n", pci_config_readw(regs->bx, PCI_COMMAND));
     regs->ecx = pci_config_readl(regs->bx, regs->di);
     set_code_success(regs);
+    dprintf(1, "val=%x\n", pci_config_readw(regs->bx, PCI_COMMAND));
 }

// write configuration byte
 static void
 handle_1ab10b(struct bregs *regs)
 {
+    dprintf(1, "val=%x\n", pci_config_readw(regs->bx, PCI_COMMAND));
     pci_config_writeb(regs->bx, regs->di, regs->cl);
     set_code_success(regs);
+    dprintf(1, "val=%x\n", pci_config_readw(regs->bx, PCI_COMMAND));
 }

// write configuration word
 static void
 handle_1ab10c(struct bregs *regs)
 {
+    dprintf(1, "val=%x\n", pci_config_readw(regs->bx, PCI_COMMAND));
     pci_config_writew(regs->bx, regs->di, regs->cx);
     set_code_success(regs);
+    dprintf(1, "val=%x\n", pci_config_readw(regs->bx, PCI_COMMAND));
 }

// write configuration dword
 static void
 handle_1ab10d(struct bregs *regs)
 {
+    dprintf(1, "val=%x\n", pci_config_readw(regs->bx, PCI_COMMAND));
     pci_config_writel(regs->bx, regs->di, regs->ecx);
     set_code_success(regs);
+    dprintf(1, "val=%x\n", pci_config_readw(regs->bx, PCI_COMMAND));
 }

// get irq routing options
@@ -175,7 +187,7 @@ handle_1ab1XX(struct bregs *regs)
 void
 handle_1ab1(struct bregs *regs)
 {
-    //debug_stub(regs);
+    debug_stub(regs);

if (! CONFIG_PCIBIOS) {
         set_invalid(regs);