[SeaBIOS] [Bochs] [PATCH RFC] shadow: Change order of PCI writes

[petr.berky at email.cz]

I am working on an issue with SeaBIOS running in Bochs emulator. I found
I need to revert SeaBIOS patch d449a11, otherwise Bochs freezes. After
the further investigation, I narrowed down the problem to those lines:

File: src/fw/shadow.c
Function: __make_bios_writable_intel
 48  // Write PAM settings back to pci config space
 49  pci_config_writel(bdf, ALIGN_DOWN(pam0, 4), pamdata.data32[0]);
 50  pci_config_writel(bdf, ALIGN_DOWN(pam0, 4) + 4, pamdata.data32[1]);

File: src/fw/shadow.c
Function: make_bios_readonly_intel
109  // Write PAM settings back to pci config space
110  pci_config_writel(bdf, ALIGN_DOWN(pam0, 4), pamdata.data32[0]);
111  pci_config_writel(bdf, ALIGN_DOWN(pam0, 4) + 4, pamdata.data32[1]);

What has changed, comparing to the code before patch d449a11 is the
order of writes to PAM registers.

Before d449a11 the order was: 0x5a 0x5b 0x5c 0x5d 0x5e 0x5f 0x59
After  d449a11 the order is : 0x59 0x5a 0x5b 0x5c 0x5d 0x5e 0x5f

I believe also the 0x58 (DRAMT?) register is written after the patch
d449a11 and was not before.

I checked, and the registers values after write are exactly the same
in both cases (before/after patch). This lead me to think the order
of writes matters.

So I prepared the patch that restores the old order. I works well and
fixes the issue with Bochs. But there is still one think I cannot
explain and I hope I can find some help here. If you look on the patch
below, you will see I had to use different pci write functions in
__make_bios_writable_intel and make_bios_readonly_intel. The only
difference in those functions (I think) is the size of data that
is written at one call. Still It does not work for me when written
some other way. Again, I would appreciate if someone could explain
it to me or point me some documentation.

Just to summarize, I have two questions:

  1.  Why the order of pci writes makes a difference here?
  2.  Why the size of pci writes matters?


>From 953c133a6ade49e9148a0734ffefc0fb27c116fb Mon Sep 17 00:00:00 2001
From: Petr Berky <petr.berky at email.cz>
Date: Sun, 19 Mar 2017 10:14:49 +0100
Subject: [PATCH RFC] shadow: Change order of PCI writes

This change solves the problem with Bochs freeze
---
 src/fw/shadow.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/fw/shadow.c b/src/fw/shadow.c
index c80b266..508af20 100644
--- a/src/fw/shadow.c
+++ b/src/fw/shadow.c
@@ -23,6 +23,7 @@
 
 union pamdata_u {
     u8 data8[8];
+    u16 data16[4];
     u32 data32[2];
 };
 
@@ -46,8 +47,9 @@ __make_bios_writable_intel(u16 bdf, u32 pam0)
     pam[0] = 0x30;
 
     // Write PAM settings back to pci config space
-    pci_config_writel(bdf, ALIGN_DOWN(pam0, 4), pamdata.data32[0]);
+    pci_config_writew(bdf, ALIGN_DOWN(pam0, 4) + 2, pamdata.data16[1]);
     pci_config_writel(bdf, ALIGN_DOWN(pam0, 4) + 4, pamdata.data32[1]);
+    pci_config_writeb(bdf, ALIGN_DOWN(pam0, 4) + 1, pamdata.data8[1]);
 
     if (!ram_present)
         // Copy bios.
@@ -107,8 +109,11 @@ make_bios_readonly_intel(u16 bdf, u32 pam0)
     pam[0] = 0x10;
 
     // Write PAM settings back to pci config space
-    pci_config_writel(bdf, ALIGN_DOWN(pam0, 4), pamdata.data32[0]);
-    pci_config_writel(bdf, ALIGN_DOWN(pam0, 4) + 4, pamdata.data32[1]);
+    pci_config_writew(bdf, ALIGN_DOWN(pam0, 4) + 2, pamdata.data16[1]);
+    pci_config_writew(bdf, ALIGN_DOWN(pam0, 4) + 4, pamdata.data16[2]);
+    pci_config_writeb(bdf, ALIGN_DOWN(pam0, 4) + 6, pamdata.data8[6]);
+    pci_config_writeb(bdf, ALIGN_DOWN(pam0, 4) + 7, pamdata.data8[7]);
+    pci_config_writeb(bdf, ALIGN_DOWN(pam0, 4) + 1, pamdata.data8[1]);
 }
 
 static int ShadowBDF = -1;
-- 
2.11.0