target/i386: enable A20 automatically in system management mode

On Fri, May 12, 2017 at 11:19:00PM +0000, Xu, Anthony wrote:
> > 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.

I don't know why you think that.  One can check with:


What OS / bootloader are you running?

-Kevin

> -----Original Message-----
> From: Kevin O'Connor [mailto:kevin@koconnor.net]
> Sent: Friday, May 12, 2017 5:02 PM
> To: Xu, Anthony <anthony.xu@intel.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>; qemu-devel@nongnu.org
> Subject: Re: [PATCH] target/i386: enable A20 automatically in system
> management mode
> 
> On Fri, May 12, 2017 at 11:19:00PM +0000, Xu, Anthony wrote:
> > > 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.
> 
> I don't know why you think that.  One can check with:
> 
> --- a/src/stacks.c
> +++ b/src/stacks.c
> @@ -99,6 +99,8 @@ call32_post(void)
>          if (cr0_caching)
>              cr0_mask(CR0_CD|CR0_NW, cr0_caching);
>      }
> +    if (!get_a20())
> +        dprintf(1, "a20=0\n");
> 
>      // Restore cmos index register
>      outb(GET_LOW(Call16Data.cmosindex), PORT_CMOS_INDEX);
> 
> With the above I only see a handful of cases where SeaBIOS has to
> restore a20 to a disabled state.

I think it is related to accel and platform, the result I gave before is for q35 tcg,

With the above change,   I got below data

Platform	accel		count of restoring A20 to 0
Q35		kvm		96
Q35		tcg		271
PC		kvm		3
PC		tcg		3

A lot of A20 restoring happen when SeaBIOS scans AHCI links.


> 
> The handful I do see are due to cases where yield() is called prior to
> option rom initialization.  Those handful are eliminated for me with
> the following fix:
> 
> --- a/src/stacks.c
> +++ b/src/stacks.c
> @@ -496,6 +496,7 @@ void
>  thread_setup(void)
>  {
>      CanInterrupt = 1;
> +    call16_override(1);
>      if (! CONFIG_THREADS)
>          return;
>      ThreadControl = romfile_loadint("etc/threads", 1);

But I still see a lot of PORT_A20 accesses in QEMU as I expected


> 
> What OS / bootloader are you running?

/x86_64-softmmu/qemu-system-x86_64 -bios /home/root/git/seabios/out/bios.bin -smp 1 
-machine q35,accel=tcg -m 1G -drive format=raw,file=/home/root/images/centos7.2.img,if=ide,index=0 
-nographic  -nodefaults  -serial stdio -monitor pty

-Anthony

On Sat, May 13, 2017 at 01:24:30AM +0000, Xu, Anthony wrote:
> I think it is related to accel and platform, the result I gave before is for q35 tcg,
> 
> With the above change,   I got below data
> 
> Platform	accel		count of restoring A20 to 0
> Q35		kvm		96
> Q35		tcg		271
> PC		kvm		3
> PC		tcg		3

Okay, thanks.  I think the number of a20 switches is due to
differences in option rom execution interacting with the fact that
some mode switches were occurring before SeaBIOS set
call16_override().

> But I still see a lot of PORT_A20 accesses in QEMU as I expected

Yes, but it should be possible to significantly reduce the number of
outb() calls by limiting them to when A20 changes.  This should also
be useful to reduce the number of outb() calls needed to disable NMIs.
I sent a patch series to the seabios mailing list to demonstrate the
idea.

-Kevin

> On Sat, May 13, 2017 at 01:24:30AM +0000, Xu, Anthony wrote:
> > I think it is related to accel and platform, the result I gave before is for q35
> tcg,
> >
> > With the above change,   I got below data
> >
> > Platform	accel		count of restoring A20 to 0
> > Q35		kvm		96
> > Q35		tcg		271
> > PC		kvm		3
> > PC		tcg		3
> 
> Okay, thanks.  I think the number of a20 switches is due to
> differences in option rom execution interacting with the fact that
> some mode switches were occurring before SeaBIOS set
> call16_override().
> 
> > But I still see a lot of PORT_A20 accesses in QEMU as I expected
> 
> Yes, but it should be possible to significantly reduce the number of
> outb() calls by limiting them to when A20 changes.  This should also
> be useful to reduce the number of outb() calls needed to disable NMIs.
> I sent a patch series to the seabios mailing list to demonstrate the
> idea.

If both TCG and KVM work by ignoring A20,  why not remove all PORT_A20
access in SeaBios when CONFIG_DISABLE_A20 is not defined?
Do you see any impact?


-Anthony

On Tue, May 16, 2017 at 08:00:28PM +0000, Xu, Anthony wrote:
> > On Sat, May 13, 2017 at 01:24:30AM +0000, Xu, Anthony wrote:
> > > I think it is related to accel and platform, the result I gave before is for q35
> > tcg,
> > >
> > > With the above change,   I got below data
> > >
> > > Platform	accel		count of restoring A20 to 0
> > > Q35		kvm		96
> > > Q35		tcg		271
> > > PC		kvm		3
> > > PC		tcg		3
> > 
> > Okay, thanks.  I think the number of a20 switches is due to
> > differences in option rom execution interacting with the fact that
> > some mode switches were occurring before SeaBIOS set
> > call16_override().
> > 
> > > But I still see a lot of PORT_A20 accesses in QEMU as I expected
> > 
> > Yes, but it should be possible to significantly reduce the number of
> > outb() calls by limiting them to when A20 changes.  This should also
> > be useful to reduce the number of outb() calls needed to disable NMIs.
> > I sent a patch series to the seabios mailing list to demonstrate the
> > idea.
> 
> If both TCG and KVM work by ignoring A20,  why not remove all PORT_A20
> access in SeaBios when CONFIG_DISABLE_A20 is not defined?
> Do you see any impact?

The SeaBIOS CONFIG_DISABLE_A20 build option does not mean "disable
support for A20"; it means "start the initial operating system
bootloader with A20 disabled".  CONFIG_DISABLE_A20=y is a
pessimization, not an optimization.

As for adding a new SeaBIOS build option to compile out support for
A20 - that seems like a very small optimization that would risk memory
corruption and hard to diagnose crashes.  SeaBIOS runs natively on
real hardware (with coreboot and as a CSM on UEFI) as well as on
QEMU/KVM.

-Kevin

> > > > With the above change,   I got below data
> > > >
> > > > Platform	accel		count of restoring A20 to 0
> > > > Q35		kvm		96
> > > > Q35		tcg		271
> > > > PC		kvm		3
> > > > PC		tcg		3
> > >
> > > Okay, thanks.  I think the number of a20 switches is due to
> > > differences in option rom execution interacting with the fact that
> > > some mode switches were occurring before SeaBIOS set
> > > call16_override().
> > >
> > > > But I still see a lot of PORT_A20 accesses in QEMU as I expected
> > >
> > > Yes, but it should be possible to significantly reduce the number of
> > > outb() calls by limiting them to when A20 changes.  This should also
> > > be useful to reduce the number of outb() calls needed to disable NMIs.
> > > I sent a patch series to the seabios mailing list to demonstrate the
> > > idea.
> >
> > If both TCG and KVM work by ignoring A20,  why not remove all PORT_A20
> > access in SeaBios when CONFIG_DISABLE_A20 is not defined?
> > Do you see any impact?
> 
> The SeaBIOS CONFIG_DISABLE_A20 build option does not mean "disable
> support for A20"; it means "start the initial operating system
> bootloader with A20 disabled".  CONFIG_DISABLE_A20=y is a
> pessimization, not an optimization.
Make sense, Thanks for explanation, 

> 
> As for adding a new SeaBIOS build option to compile out support for
> A20 - that seems like a very small optimization that would risk memory
> corruption and hard to diagnose crashes.  SeaBIOS runs natively on
> real hardware (with coreboot and as a CSM on UEFI) as well as on
> QEMU/KVM.
I heard new platform doesn't support A20.
What's the hardware SeaBIOS runs natively on needs A20 support?

It is just a build option, we can disable A20 support for QEMU/KVM and
enable A20 support for real hardware. Any concerns here?

BTW QEMU/KVM ignores A20 even SeaBIOS supports A20.

Or, we can add some logic in SeaBIOS to check if the platform supports A20,
if it doesn't support A20, SeaBIOS won't access PORT_A20 anymore.

The check logic is like,
write 0x55 to 0x00:0xeff0 (or other unused address)
disable A20
write 0xaa to 0xffff:0xf000
read from 0x00:0xeff0,
If the return value is 0x55, A20 is not supported by this platform.


Anthony

On 16/05/2017 23:42, Kevin O'Connor wrote:
> 
> As for adding a new SeaBIOS build option to compile out support for
> A20 - that seems like a very small optimization that would risk memory
> corruption and hard to diagnose crashes.  SeaBIOS runs natively on
> real hardware (with coreboot and as a CSM on UEFI) as well as on
> QEMU/KVM.

Haswell (2013) no longer has A20 support (and hypervisors never had it,
not just KVM).

Thanks,

Paolo