Re: [Qemu-devel] [PATCH] target/i386: enable A20 automatically in system management mode

[Xu, Anthony]

wrote:
> > On 12/05/2017 20:55, Xu, Anthony wrote:
> > > If that's the case,  QEMU/TCG should work with SeaBios even with
> ignoring A20.
> > >
> > > During SeaBios boot, there are >350 port 92 access, if we don't need to
> handle A20,
> > > we can make A20 configurable in Seabios, It may reduce SeaBios boot
> time.
> >
> > Yes, that's a good idea.
> 
> SeaBIOS defaults to enabling A20 and it's a rare beast that disables
> it.  One could change x86.h:set_a20 and romlayout.S:transition32 to
> only issue the outb() if the inb() indicates a change is needed.  That
> would likely eliminate half the accesses.

The 350 port 92 access is for write operation only.
If include the inb(), it would be 700, and every time it actually has a change
To be precise, It is about 175 switches from 32 bit to 16 bit, then back to 32 
bit.
call16 is called 175 times during Seabios boot without any option rom,
It would be more if some option roms are included.


I think A20 is disabled by default in SeaBios.
Call16Data.a20 is initialized to 0,
call16_override may set Call16Data.a20 to 1,
call16 called before call16_override would disable and enable A20.

BTW, A20 is enabled in QEMU by default.



> 
> I'd be surprised if it would impact the overall boot time though.
> SeaBIOS only touches the port on a cpu mode switch and I would have
> thought that was heavier than an IO port access.  Maybe that is skewed
> on KVM though.

Maybe not, but if we have more these kind  of optimizations , we may see the 
impact.
A20 is already configurable in SeaBios,  CONFIG_DISABLE_A20.
Then the change is very small. No PORT_A20 access after the change.


-Anthony


diff --git a/src/x86.h b/src/x86.h
index a770e6f..8efb94a 100644
--- a/src/x86.h
+++ b/src/x86.h
@@ -21,6 +21,7 @@
 #ifndef __ASSEMBLY__

 #include "types.h" // u32
+#include "../out/autoconf.h"

 static inline void irq_disable(void)
 {
@@ -254,13 +255,19 @@ static inline void lgdt(struct descloc_s *desc) {
 }

 static inline u8 get_a20(void) {
-    return (inb(PORT_A20) & A20_ENABLE_BIT) != 0;
+    if (CONFIG_DISABLE_A20) {
+        return (inb(PORT_A20) & A20_ENABLE_BIT) != 0;
+    }
+    return 1;
 }

 static inline u8 set_a20(u8 cond) {
-    u8 val = inb(PORT_A20);
-    outb((val & ~A20_ENABLE_BIT) | (cond ? A20_ENABLE_BIT : 0), PORT_A20);
-    return (val & A20_ENABLE_BIT) != 0;
+    if (CONFIG_DISABLE_A20) {
+        u8 val = inb(PORT_A20);
+        outb((val & ~A20_ENABLE_BIT) | (cond ? A20_ENABLE_BIT : 0), PORT_A20);
+        return (val & A20_ENABLE_BIT) != 0;
+    }
+    return 1;
 }