Re: [SeaBIOS] [PATCH] ps2: disable the keyboard and mouse before flushing the queue

2013-04-25 Thread Kevin O'Connor
On Thu, Apr 25, 2013 at 07:08:26PM -0400, Kevin O'Connor wrote:
> My understanding is that the i8042 has two queues - one for the
> results of i8042 commands and one for the keyboard.  Upon sending an
> i8042 command any reads from PORT_PS2_DATA will return data from the
> i8042 queue until that queue is drained.  Only after that would a read
> from PORT_PS2_DATA return data from the keyboard queue.  Otherwise,
> I'm not sure how one could ever reliably read the results from an
> i8042 command.
> 
> The above aside, we can't disable the keyboard before flushing,
> because we don't know the i8042 exists at all.  Also, we can't send an
> i8042 command without verifying the i8042 is ready for a command (ie,
> using i8042_command(I8042_CMD_KBD_DISABLE)).  However, we can't really
> do that until we know the i8042 exists - so it's a bit circular.

Thinking about this further, I guess one could move up the existing
keyboard disable command with the patch below.  That should be safe
even if there is no i8042.  I'd still like to further understand why
keyboard data would be misinterpreted as i8042 data though.

-Kevin


--- a/src/ps2port.c
+++ b/src/ps2port.c
@@ -424,6 +424,16 @@ ps2_keyboard_setup(void *data)
 if (ret)
 return;
 
+// Disable keyboard and mouse events.
+Ps2ctr = I8042_CTR_KBDDIS | I8042_CTR_AUXDIS;
+ret = i8042_command(I8042_CMD_CTL_WCTR, &Ps2ctr);
+if (ret)
+return;
+
+ret = i8042_flush();
+if (ret)
+return;
+
 // Controller self-test.
 u8 param[2];
 ret = i8042_command(I8042_CMD_CTL_TEST, param);
@@ -443,9 +453,6 @@ ps2_keyboard_setup(void *data)
 return;
 }
 
-// Disable keyboard and mouse events.
-SET_LOW(Ps2ctr, I8042_CTR_KBDDIS | I8042_CTR_AUXDIS);
-
 
 /* --- keyboard side */
 /* reset keyboard and self test  (keyboard side) */
@@ -479,7 +486,7 @@ ps2_keyboard_setup(void *data)
 return;
 
 // Keyboard Mode: disable mouse, scan code convert, enable kbd IRQ
-SET_LOW(Ps2ctr, I8042_CTR_AUXDIS | I8042_CTR_XLATE | I8042_CTR_KBDINT);
+Ps2ctr = I8042_CTR_AUXDIS | I8042_CTR_XLATE | I8042_CTR_KBDINT;
 
 /* Enable keyboard */
 ret = ps2_kbd_command(ATKBD_CMD_ENABLE, NULL);

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


Re: [SeaBIOS] [PATCH] ps2: disable the keyboard and mouse before flushing the queue

2013-04-25 Thread Kevin O'Connor
On Thu, Apr 25, 2013 at 10:33:57AM -0500, Anthony Liguori wrote:
> Kevin O'Connor  writes:
> > Thanks.  I don't understand why keyboard/mouse events would be a
> > problem.  Those events should get discarded with the "Discarding ps2
> > data %02x (status=%02x)" message.  (The role of the flush is to make
> > sure there are no i8042 command responses pending - I don't think
> > keyboard/mouse events are a problem.)
> 
> This is much too late.  I think what's happening is:
> 
> >/* flush incoming keys */
> >int ret = i8042_flush();
> >if (ret)
> >return;
> 
> This drains the PS/2 queue but if the keyboard is enabled, then more
> data can appear in the queue.
> 
> >// Controller self-test.
> >u8 param[2];
> >ret = i8042_command(I8042_CMD_CTL_TEST, param);
> >if (ret)
> >return;
> >if (param[0] != 0x55) {
> >dprintf(1, "i8042 self test failed (got %x not 0x55)\n", param[0]);
> >return;
> >}
> 
> The patch below in QEMU will effectively queue the injected queue
> keycode before the 0x55 that self-test returns.  The command queue is
> shared with the keyboard and the i8042 so if the keyboard is active, you
> may get keyboard data before getting the command responses.

My understanding is that the i8042 has two queues - one for the
results of i8042 commands and one for the keyboard.  Upon sending an
i8042 command any reads from PORT_PS2_DATA will return data from the
i8042 queue until that queue is drained.  Only after that would a read
from PORT_PS2_DATA return data from the keyboard queue.  Otherwise,
I'm not sure how one could ever reliably read the results from an
i8042 command.

The above aside, we can't disable the keyboard before flushing,
because we don't know the i8042 exists at all.  Also, we can't send an
i8042 command without verifying the i8042 is ready for a command (ie,
using i8042_command(I8042_CMD_KBD_DISABLE)).  However, we can't really
do that until we know the i8042 exists - so it's a bit circular.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


[SeaBIOS] [PATCH] ps2: disable the keyboard and mouse before flushing the queue

2013-04-25 Thread Anthony Liguori
If SeaBIOS is run as a payload via coreboot (and presumably as a
CSM), then it's possible the keyboard or mouse will still be
enabled.  This can lead to data being queued even after the flush
function attempts to clear the queue.

Disabling the keyboard/mouse prior to flushing is pretty standard
in DOS programming so it's not surprising that it's needed here.

I believe this problem manifests with the Chromebook Pixel.  People
have reported that sometimes the 'ESC to Select Boot Devices'
doesn't work.  I can reproduce this faithfully by holding 'Ctrl-L'
in the firmware screen during SeaBIOS initialization.

I can't test this fix on an actual Pixel because I don't know how
to update SeaBIOS but I have tested the patch under QEMU.

Signed-off-by: Anthony Liguori 
---
 src/ps2port.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/ps2port.c b/src/ps2port.c
index 9b760fd..2169171 100644
--- a/src/ps2port.c
+++ b/src/ps2port.c
@@ -55,6 +55,12 @@ static int
 i8042_flush(void)
 {
 dprintf(7, "i8042_flush\n");
+
+/* Disable the keyboard and mouse to prevent additional data from
+ * being queued. */
+outb(0xad, PORT_PS2_STATUS);
+outb(0xa7, PORT_PS2_STATUS);
+
 int i;
 for (i=0; ihttp://www.seabios.org/mailman/listinfo/seabios


Re: [SeaBIOS] [Qemu-devel] [PATCH RFC 0/3] seabios: move acpi table formatting out of bios

2013-04-25 Thread Kevin O'Connor
On Fri, Apr 26, 2013 at 12:11:24AM +0300, Michael S. Tsirkin wrote:
> On Thu, Apr 25, 2013 at 08:19:48PM +0200, Fred . wrote:
> > With ACPI moved out of SeaBIOS to QEMU, how will ACPI work when using 
> > SeaBIOS
> > without QEMU?
> > 
> > Like if using SeaBIOS with Boch, KVM or Coreboot?
> 
> KVM merged with QEMU, so it will use romfiles too.
> Others will have two options:
> 
> - keep using that tables in seabios. With time we will be able
>   to drop qemu-specific stuff in the tables which should make life
>   easier for Bochs/coreboot.

FYI - coreboot does not and never has used the acpi tables in seabios.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


Re: [SeaBIOS] [PATCH RFC 0/3] seabios: move acpi table formatting out of bios

2013-04-25 Thread Michael S. Tsirkin
On Thu, Apr 25, 2013 at 08:19:48PM +0200, Fred . wrote:
> With ACPI moved out of SeaBIOS to QEMU, how will ACPI work when using SeaBIOS
> without QEMU?
> 
> Like if using SeaBIOS with Boch, KVM or Coreboot?

KVM merged with QEMU, so it will use romfiles too.
Others will have two options:

- keep using that tables in seabios. With time we will be able
  to drop qemu-specific stuff in the tables which should make life
  easier for Bochs/coreboot.
- add ACPI through romfiles like QEMU will do.

Which one will be chosen will be up to relevant projects,
but note how they benefit in any case.

> 
> On Thu, Apr 25, 2013 at 11:02 AM, Michael S. Tsirkin  wrote:
> 
> Untested yet, but I thought I'd share the
> BIOS bits so we can agree on direction.
> 
> In particular check out ROM sizes:
> - Before patchset with DSDT enabled
>     Total size: 127880  Fixed: 59060  Free: 3192 (used 97.6% of 128KiB 
> rom)
> - Before patchset with DSDT disabled
>     Total size: 122844  Fixed: 58884  Free: 8228 (used 93.7% of 128KiB 
> rom)
> - After patchset:
>     Total size: 128776  Fixed: 59100  Free: 2296 (used 98.2% of 128KiB 
> rom)
> - Legacy disabled at build time:
>     Total size: 119836  Fixed: 58996  Free: 11236 (used 91.4% of 128KiB
> rom)
> 
> As can be seen from this, most size savings come
> from dropping DSDT, but we do save a bit by removing
> other tables. Of course the real reason to move tables to QEMU
> is so that ACPI can better match hardware.
> 
> This patchset adds an option to move all code for formatting acpi tables
> out of BIOS. With this, QEMU has full control over the table layout.
> All tables are loaded from the new "/etc/acpi/" directory.
> Any entries in this directory cause BIOS to disable
> ACPI table generation completely.
> A generic linker script, controlled by QEMU, is
> loaded from "/etc/linker-script". It is used to
> patch in table pointers and checksums.
> 
> BIOS still has limited ability to parse the tables,
> for the following purposes:
>         - locate resume vector
>         - allocate RSDP in FSEG
>         - allocate FACS at an aligned address
> 
> --
> MST
> 
> 
> Michael S. Tsirkin (3):
>   linker: utility to patch in-memory ROM files
>   acpi: load and link tables from /etc/acpi/
>   acpi: add an option to disable builtin tables
> 
>  Makefile     |  2 +-
>  src/Kconfig  | 12 +++-
>  src/acpi.c   | 67 +++-
>  src/linker.c | 90
> 
>  src/linker.h | 50 +
>  src/util.h   |  1 +
>  6 files changed, 219 insertions(+), 3 deletions(-)
>  create mode 100644 src/linker.c
>  create mode 100644 src/linker.h
>
> --
> MST
> 
> ___
> SeaBIOS mailing list
> SeaBIOS@seabios.org
> http://www.seabios.org/mailman/listinfo/seabios
> 
> 

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


Re: [SeaBIOS] [PATCH] ps2: disable the keyboard and mouse before flushing the queue

2013-04-25 Thread Anthony Liguori
Kevin O'Connor  writes:

> On Wed, Apr 24, 2013 at 08:32:09PM -0500, Anthony Liguori wrote:
>> If SeaBIOS is run as a payload via coreboot (and presumably as a
>> CSM), then it's possible the keyboard or mouse will still be
>> enabled.  This can lead to data being queued even after the flush
>> function attempts to clear the queue.
>> 
>> Disabling the keyboard/mouse prior to flushing is pretty standard
>> in DOS programming so it's not surprising that it's needed here.
>> 
>> I believe this problem manifests with the Chromebook Pixel.  People
>> have reported that sometimes the 'ESC to Select Boot Devices'
>> doesn't work.  I can reproduce this faithfully by holding 'Ctrl-L'
>> in the firmware screen during SeaBIOS initialization.
>
> Thanks.  I don't understand why keyboard/mouse events would be a
> problem.  Those events should get discarded with the "Discarding ps2
> data %02x (status=%02x)" message.  (The role of the flush is to make
> sure there are no i8042 command responses pending - I don't think
> keyboard/mouse events are a problem.)

This is much too late.  I think what's happening is:

>/* flush incoming keys */
>int ret = i8042_flush();
>if (ret)
>return;

This drains the PS/2 queue but if the keyboard is enabled, then more
data can appear in the queue.

>// Controller self-test.
>u8 param[2];
>ret = i8042_command(I8042_CMD_CTL_TEST, param);
>if (ret)
>return;
>if (param[0] != 0x55) {
>dprintf(1, "i8042 self test failed (got %x not 0x55)\n", param[0]);
>return;
>}

The patch below in QEMU will effectively queue the injected queue
keycode before the 0x55 that self-test returns.  The command queue is
shared with the keyboard and the i8042 so if the keyboard is active, you
may get keyboard data before getting the command responses.

>> I can't test this fix on an actual Pixel because I don't know how
>> to update SeaBIOS but I have tested the patch under QEMU.
>
> Are you able to reproduce the problem under QEMU?

I can with the following patch.  It's very hard to get the right timing
manually because IO writes are much faster in QEMU.  We also don't
emulate key repeat correctly in QEMU so just holding a key won't do it.

But the following patch is a reasonable approximation.  kbd_put_keycode()
can happen at any time in QEMU (this is what VNC/GTK will call to inject
a keycode).  This patch just makes it happen at exactly the right time
to reproduce the bug.

This replicates the same behavior in QEMU as I see on the Pixel.

(And the patch I sent fixes the problem with my hacked QEMU).

diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
index 08ceb9f..bc35dd6 100644
--- a/hw/input/pckbd.c
+++ b/hw/input/pckbd.c
@@ -26,6 +26,7 @@
 #include "hw/i386/pc.h"
 #include "hw/input/ps2.h"
 #include "sysemu/sysemu.h"
+#include "ui/console.h"
 
 /* debug PC keyboard */
 //#define DEBUG_KBD
@@ -267,6 +268,7 @@ static void kbd_write_command(void *opaque, hwaddr addr,
 break;
 case KBD_CCMD_SELF_TEST:
 s->status |= KBD_STAT_SELFTEST;
+kbd_put_keycode(0x1d);
 kbd_queue(s, 0x55, 0);
 break;
 case KBD_CCMD_KBD_TEST:

Regards,

Anthony Liguori

> I'd guess someone on the coreboot list could describe how to reflash
> the pixel.  I don't own one myself.
>
> -Kevin


___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


Re: [SeaBIOS] Updating SeaBIOS on a Chromebook (Was: [PATCH] ps2: disable the keyboard and mouse before flushing the queue)

2013-04-25 Thread Anthony Liguori

Thanks Stefan!

I'll give it a try this evening.

Regards,

Anthony Liguori

Stefan Reinauer  writes:

> Hi Anthony,
>
> In order to build a working SeaBIOS for the Pixel, you should use our
> ChromeOS tree because unfortunately we have not been able to upstream all
> the patches we carry around yet.
>
> $ git clone
> https://gerrit.chromium.org/gerrit/p/chromiumos/third_party/seabios.git
> $ cd seabios
> $ git checkout origin/chromeos-2012.05.12
>
> Build it with:
> $ cp default.config .config
> $ make
>
> Then create a CBFS
>
> # get the PCI option rom
> $ wget http://www.coreboot.org/~stepan/pci8086,0166.rom
> # Create a dummy bootblock to make cbfstool happy
> $ dd if=/dev/zero of=bootblock count=1 bs=64
> # Create empty CBFS
> $ cbfstool seabios.cbfs create -s $(( 2*1024*1024 )) -B bootblock -m x86
> # Add SeaBIOS binary to CBFS
> $ cbfstool seabios.cbfs add-payload -f out/bios.bin.elf -n payload -c lzma
> # Add VGA option rom to CBFS
> $ cbfstool seabios.cbfs add -f pci8086,0166.rom -n pci8086,0166.rom -t
> optionrom
> # Add additional configuration
> $ cbfstool seabios.cbfs add -f bootorder -n bootorder -t raw
> $ cbfstool seabios.cbfs add -f boot-menu-wait -n boot-menu-wait -t raw
> # Print CBFS inventory
> $ cbfstool seabios.cbfs print
> # Fix up CBFS to live at 0xffc0. The last four bytes of a CBFS
> # image are a pointer to the CBFS master header. Per default a CBFS
> # lives at 4G - rom size, and the CBFS master header ends up at
> # 0xffa0. However our CBFS lives at 4G-4M and is 2M in size, so
> # the CBFS master header is at 0xffdfffa0 instead. The two lines
> # below correct the according byte in that pointer to make all CBFS
> # parsing code happy. In the long run we should fix cbfstool and
> # remove this workaround.
> /bin/echo -ne \\0737 | dd of=seabios.cbfs seek=$(( (2*1024*1024) - 2 ))
> bs=1 conv=notrunc
>
> That's it. :-)
>
> I also uploaded an image with your change to
>
> http://www.coreboot.org/~stepan/seabios.cbfs.bz2
>
>
> Flash it on the ChromeBook with:
>
> # cd /tmp
> # flashrom -r image.rom
> # dd if=seabios.cbfs of=image.rom seek=2 bs=2M conv=notrunc
> # flashrom -w image.rom -i RW_LEGACY
>
> On Thu, Apr 25, 2013 at 9:15 AM, ron minnich  wrote:
>
>> stefan, how do we fix this?
>>
>> On Wed, Apr 24, 2013 at 6:36 PM, Anthony Liguori 
>> wrote:
>> >
>> > Hi Ron,
>> >
>> > I just got a lovely new Pixel and noticed the following problem with
>> > SeaBIOS.  Any chance you know how I can update the SeaBIOS firmware to
>> > test this?  Or perhaps the person to poke to get fixes included in the
>> > shipped SeaBIOS?
>> >
>> > Regards,
>> >
>> > Anthony Liguori
>> >
>> >
>> >
>> > -- Forwarded message --
>> > From: "Anthony Liguori" 
>> > To: "" 
>> > Cc: "Kevin O'Connor"  , "Anthony Liguori" <
>> aligu...@us.ibm.com>
>> > Date: Wed, 24 Apr 2013 20:32:09 -0500
>> > Subject: [PATCH] ps2: disable the keyboard and mouse before flushing the
>> queue
>> > If SeaBIOS is run as a payload via coreboot (and presumably as a
>> > CSM), then it's possible the keyboard or mouse will still be
>> > enabled.  This can lead to data being queued even after the flush
>> > function attempts to clear the queue.
>> >
>> > Disabling the keyboard/mouse prior to flushing is pretty standard
>> > in DOS programming so it's not surprising that it's needed here.
>> >
>> > I believe this problem manifests with the Chromebook Pixel.  People
>> > have reported that sometimes the 'ESC to Select Boot Devices'
>> > doesn't work.  I can reproduce this faithfully by holding 'Ctrl-L'
>> > in the firmware screen during SeaBIOS initialization.
>> >
>> > I can't test this fix on an actual Pixel because I don't know how
>> > to update SeaBIOS but I have tested the patch under QEMU.
>> >
>> > Signed-off-by: Anthony Liguori 
>> > ---
>> >  src/ps2port.c | 6 ++
>> >  1 file changed, 6 insertions(+)
>> >
>> > diff --git a/src/ps2port.c b/src/ps2port.c
>> > index 9b760fd..2169171 100644
>> > --- a/src/ps2port.c
>> > +++ b/src/ps2port.c
>> > @@ -55,6 +55,12 @@ static int
>> >  i8042_flush(void)
>> >  {
>> >  dprintf(7, "i8042_flush\n");
>> > +
>> > +/* Disable the keyboard and mouse to prevent additional data from
>> > + * being queued. */
>> > +outb(0xad, PORT_PS2_STATUS);
>> > +outb(0xa7, PORT_PS2_STATUS);
>> > +
>> >  int i;
>> >  for (i=0; i> >  u8 status = inb(PORT_PS2_STATUS);
>> > --
>> > 1.8.0
>> >
>> >
>>
>>
>
>
> -- 
> Stefan Reinauer
> Google Inc.
> ___
> SeaBIOS mailing list
> SeaBIOS@seabios.org
> http://www.seabios.org/mailman/listinfo/seabios


___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


Re: [SeaBIOS] Updating SeaBIOS on a Chromebook (Was: [PATCH] ps2: disable the keyboard and mouse before flushing the queue)

2013-04-25 Thread Stefan Reinauer
Hi Anthony,

In order to build a working SeaBIOS for the Pixel, you should use our
ChromeOS tree because unfortunately we have not been able to upstream all
the patches we carry around yet.

$ git clone
https://gerrit.chromium.org/gerrit/p/chromiumos/third_party/seabios.git
$ cd seabios
$ git checkout origin/chromeos-2012.05.12

Build it with:
$ cp default.config .config
$ make

Then create a CBFS

# get the PCI option rom
$ wget http://www.coreboot.org/~stepan/pci8086,0166.rom
# Create a dummy bootblock to make cbfstool happy
$ dd if=/dev/zero of=bootblock count=1 bs=64
# Create empty CBFS
$ cbfstool seabios.cbfs create -s $(( 2*1024*1024 )) -B bootblock -m x86
# Add SeaBIOS binary to CBFS
$ cbfstool seabios.cbfs add-payload -f out/bios.bin.elf -n payload -c lzma
# Add VGA option rom to CBFS
$ cbfstool seabios.cbfs add -f pci8086,0166.rom -n pci8086,0166.rom -t
optionrom
# Add additional configuration
$ cbfstool seabios.cbfs add -f bootorder -n bootorder -t raw
$ cbfstool seabios.cbfs add -f boot-menu-wait -n boot-menu-wait -t raw
# Print CBFS inventory
$ cbfstool seabios.cbfs print
# Fix up CBFS to live at 0xffc0. The last four bytes of a CBFS
# image are a pointer to the CBFS master header. Per default a CBFS
# lives at 4G - rom size, and the CBFS master header ends up at
# 0xffa0. However our CBFS lives at 4G-4M and is 2M in size, so
# the CBFS master header is at 0xffdfffa0 instead. The two lines
# below correct the according byte in that pointer to make all CBFS
# parsing code happy. In the long run we should fix cbfstool and
# remove this workaround.
/bin/echo -ne \\0737 | dd of=seabios.cbfs seek=$(( (2*1024*1024) - 2 ))
bs=1 conv=notrunc

That's it. :-)

I also uploaded an image with your change to

http://www.coreboot.org/~stepan/seabios.cbfs.bz2


Flash it on the ChromeBook with:

# cd /tmp
# flashrom -r image.rom
# dd if=seabios.cbfs of=image.rom seek=2 bs=2M conv=notrunc
# flashrom -w image.rom -i RW_LEGACY

On Thu, Apr 25, 2013 at 9:15 AM, ron minnich  wrote:

> stefan, how do we fix this?
>
> On Wed, Apr 24, 2013 at 6:36 PM, Anthony Liguori 
> wrote:
> >
> > Hi Ron,
> >
> > I just got a lovely new Pixel and noticed the following problem with
> > SeaBIOS.  Any chance you know how I can update the SeaBIOS firmware to
> > test this?  Or perhaps the person to poke to get fixes included in the
> > shipped SeaBIOS?
> >
> > Regards,
> >
> > Anthony Liguori
> >
> >
> >
> > -- Forwarded message --
> > From: "Anthony Liguori" 
> > To: "" 
> > Cc: "Kevin O'Connor"  , "Anthony Liguori" <
> aligu...@us.ibm.com>
> > Date: Wed, 24 Apr 2013 20:32:09 -0500
> > Subject: [PATCH] ps2: disable the keyboard and mouse before flushing the
> queue
> > If SeaBIOS is run as a payload via coreboot (and presumably as a
> > CSM), then it's possible the keyboard or mouse will still be
> > enabled.  This can lead to data being queued even after the flush
> > function attempts to clear the queue.
> >
> > Disabling the keyboard/mouse prior to flushing is pretty standard
> > in DOS programming so it's not surprising that it's needed here.
> >
> > I believe this problem manifests with the Chromebook Pixel.  People
> > have reported that sometimes the 'ESC to Select Boot Devices'
> > doesn't work.  I can reproduce this faithfully by holding 'Ctrl-L'
> > in the firmware screen during SeaBIOS initialization.
> >
> > I can't test this fix on an actual Pixel because I don't know how
> > to update SeaBIOS but I have tested the patch under QEMU.
> >
> > Signed-off-by: Anthony Liguori 
> > ---
> >  src/ps2port.c | 6 ++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/src/ps2port.c b/src/ps2port.c
> > index 9b760fd..2169171 100644
> > --- a/src/ps2port.c
> > +++ b/src/ps2port.c
> > @@ -55,6 +55,12 @@ static int
> >  i8042_flush(void)
> >  {
> >  dprintf(7, "i8042_flush\n");
> > +
> > +/* Disable the keyboard and mouse to prevent additional data from
> > + * being queued. */
> > +outb(0xad, PORT_PS2_STATUS);
> > +outb(0xa7, PORT_PS2_STATUS);
> > +
> >  int i;
> >  for (i=0; i >  u8 status = inb(PORT_PS2_STATUS);
> > --
> > 1.8.0
> >
> >
>
>


-- 
Stefan Reinauer
Google Inc.
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


Re: [SeaBIOS] [PATCH] ps2: disable the keyboard and mouse before flushing the queue

2013-04-25 Thread Kevin O'Connor
On Wed, Apr 24, 2013 at 08:32:09PM -0500, Anthony Liguori wrote:
> If SeaBIOS is run as a payload via coreboot (and presumably as a
> CSM), then it's possible the keyboard or mouse will still be
> enabled.  This can lead to data being queued even after the flush
> function attempts to clear the queue.
> 
> Disabling the keyboard/mouse prior to flushing is pretty standard
> in DOS programming so it's not surprising that it's needed here.
> 
> I believe this problem manifests with the Chromebook Pixel.  People
> have reported that sometimes the 'ESC to Select Boot Devices'
> doesn't work.  I can reproduce this faithfully by holding 'Ctrl-L'
> in the firmware screen during SeaBIOS initialization.

Thanks.  I don't understand why keyboard/mouse events would be a
problem.  Those events should get discarded with the "Discarding ps2
data %02x (status=%02x)" message.  (The role of the flush is to make
sure there are no i8042 command responses pending - I don't think
keyboard/mouse events are a problem.)

> I can't test this fix on an actual Pixel because I don't know how
> to update SeaBIOS but I have tested the patch under QEMU.

Are you able to reproduce the problem under QEMU?

I'd guess someone on the coreboot list could describe how to reflash
the pixel.  I don't own one myself.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


Re: [SeaBIOS] [PATCH RFC 0/3] seabios: move acpi table formatting out of bios

2013-04-25 Thread Fred .
With ACPI moved out of SeaBIOS to QEMU, how will ACPI work when using
SeaBIOS without QEMU?

Like if using SeaBIOS with Boch, KVM or Coreboot?


On Thu, Apr 25, 2013 at 11:02 AM, Michael S. Tsirkin  wrote:

> Untested yet, but I thought I'd share the
> BIOS bits so we can agree on direction.
>
> In particular check out ROM sizes:
> - Before patchset with DSDT enabled
> Total size: 127880  Fixed: 59060  Free: 3192 (used 97.6% of 128KiB rom)
> - Before patchset with DSDT disabled
> Total size: 122844  Fixed: 58884  Free: 8228 (used 93.7% of 128KiB rom)
> - After patchset:
> Total size: 128776  Fixed: 59100  Free: 2296 (used 98.2% of 128KiB rom)
> - Legacy disabled at build time:
> Total size: 119836  Fixed: 58996  Free: 11236 (used 91.4% of 128KiB
> rom)
>
> As can be seen from this, most size savings come
> from dropping DSDT, but we do save a bit by removing
> other tables. Of course the real reason to move tables to QEMU
> is so that ACPI can better match hardware.
>
> This patchset adds an option to move all code for formatting acpi tables
> out of BIOS. With this, QEMU has full control over the table layout.
> All tables are loaded from the new "/etc/acpi/" directory.
> Any entries in this directory cause BIOS to disable
> ACPI table generation completely.
> A generic linker script, controlled by QEMU, is
> loaded from "/etc/linker-script". It is used to
> patch in table pointers and checksums.
>
> BIOS still has limited ability to parse the tables,
> for the following purposes:
> - locate resume vector
> - allocate RSDP in FSEG
> - allocate FACS at an aligned address
>
> --
> MST
>
>
> Michael S. Tsirkin (3):
>   linker: utility to patch in-memory ROM files
>   acpi: load and link tables from /etc/acpi/
>   acpi: add an option to disable builtin tables
>
>  Makefile |  2 +-
>  src/Kconfig  | 12 +++-
>  src/acpi.c   | 67 +++-
>  src/linker.c | 90
> 
>  src/linker.h | 50 +
>  src/util.h   |  1 +
>  6 files changed, 219 insertions(+), 3 deletions(-)
>  create mode 100644 src/linker.c
>  create mode 100644 src/linker.h
>
> --
> MST
>
> ___
> SeaBIOS mailing list
> SeaBIOS@seabios.org
> http://www.seabios.org/mailman/listinfo/seabios
>
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


[SeaBIOS] [PATCH RFC 0/3] seabios: move acpi table formatting out of bios

2013-04-25 Thread Michael S. Tsirkin
Untested yet, but I thought I'd share the
BIOS bits so we can agree on direction.

In particular check out ROM sizes:
- Before patchset with DSDT enabled
Total size: 127880  Fixed: 59060  Free: 3192 (used 97.6% of 128KiB rom)
- Before patchset with DSDT disabled
Total size: 122844  Fixed: 58884  Free: 8228 (used 93.7% of 128KiB rom)
- After patchset:
Total size: 128776  Fixed: 59100  Free: 2296 (used 98.2% of 128KiB rom)
- Legacy disabled at build time:
Total size: 119836  Fixed: 58996  Free: 11236 (used 91.4% of 128KiB rom)

As can be seen from this, most size savings come
from dropping DSDT, but we do save a bit by removing
other tables. Of course the real reason to move tables to QEMU
is so that ACPI can better match hardware.

This patchset adds an option to move all code for formatting acpi tables
out of BIOS. With this, QEMU has full control over the table layout.
All tables are loaded from the new "/etc/acpi/" directory.
Any entries in this directory cause BIOS to disable
ACPI table generation completely.
A generic linker script, controlled by QEMU, is
loaded from "/etc/linker-script". It is used to
patch in table pointers and checksums.

BIOS still has limited ability to parse the tables,
for the following purposes:
- locate resume vector
- allocate RSDP in FSEG
- allocate FACS at an aligned address

-- 
MST


Michael S. Tsirkin (3):
  linker: utility to patch in-memory ROM files
  acpi: load and link tables from /etc/acpi/
  acpi: add an option to disable builtin tables

 Makefile |  2 +-
 src/Kconfig  | 12 +++-
 src/acpi.c   | 67 +++-
 src/linker.c | 90 
 src/linker.h | 50 +
 src/util.h   |  1 +
 6 files changed, 219 insertions(+), 3 deletions(-)
 create mode 100644 src/linker.c
 create mode 100644 src/linker.h

-- 
MST

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


[SeaBIOS] [PATCH RFC 1/3] linker: utility to patch in-memory ROM files

2013-04-25 Thread Michael S. Tsirkin
Add ability for a ROM file to point to
it's image in memory. When file is in memory,
add utility that can patch it, storing
pointers to one file within another file.

This is not a lot of code: together with the follow-up patch to load
ACPI tables from ROM, we get:
Before:
Total size: 127880  Fixed: 59060  Free: 3192 (used 97.6% of 128KiB rom)
After:
Total size: 128776  Fixed: 59100  Free: 2296 (used 98.2% of 128KiB rom)

Signed-off-by: Michael S. Tsirkin 
---
 Makefile |  2 +-
 src/linker.c | 90 
 src/linker.h | 50 +
 src/util.h   |  1 +
 4 files changed, 142 insertions(+), 1 deletion(-)
 create mode 100644 src/linker.c
 create mode 100644 src/linker.h

diff --git a/Makefile b/Makefile
index 759..020fb2f 100644
--- a/Makefile
+++ b/Makefile
@@ -18,7 +18,7 @@ SRC16=$(SRCBOTH) system.c disk.c font.c
 SRC32FLAT=$(SRCBOTH) post.c shadow.c memmap.c pmm.c coreboot.c boot.c \
 acpi.c smm.c mptable.c pirtable.c smbios.c pciinit.c optionroms.c mtrr.c \
 lzmadecode.c bootsplash.c jpeg.c usb-hub.c paravirt.c \
-biostables.c xen.c bmp.c romfile.c csm.c
+biostables.c xen.c bmp.c romfile.c csm.c linker.c
 SRC32SEG=util.c output.c pci.c pcibios.c apm.c stacks.c
 
 # Default compiler flags
diff --git a/src/linker.c b/src/linker.c
new file mode 100644
index 000..223d2db
--- /dev/null
+++ b/src/linker.c
@@ -0,0 +1,90 @@
+#include "linker.h"
+#include "byteorder.h" // le64_to_cpu
+
+void linker_link(const char *name)
+{
+struct linker_entry_s *entry;
+int offset = 0;
+int size, lsrc;
+void *data = romfile_loadfile(name, &size);
+struct romfile_s *src, *dst;
+u32 dst_offset;
+u64 val, buf;
+if (!data)
+return;
+
+for (offset = 0; offset < size; offset += entry->size) {
+entry = data + offset;
+/* Entry must have some data. If not - skip it. */
+if (entry->size <= sizeof *entry)
+continue;
+/* Check that entry fits in buffer. */
+if (offset + entry->size > size) {
+warn_internalerror();
+break;
+}
+/* Skip any command that we don't recognize. */
+if (entry->reserved1 || entry->reserved2)
+continue;
+if (entry->bytes != 1 &&
+entry->bytes != 2 &&
+entry->bytes != 4 &&
+entry->bytes != 8)
+continue;
+if (entry->shift > 63)
+continue;
+if (entry->type >= LINKER_ENTRY_TYPE_MAX)
+continue;
+if (entry->format != LINKER_ENTRY_FORMAT_LE)
+continue;
+/* Last byte must be 0 */
+if (entry->src_dst[entry->size - 1]) {
+warn_internalerror();
+continue;
+}
+lsrc = strlen(entry->src_dst);
+if (!lsrc) {
+warn_internalerror();
+continue;
+}
+src = romfile_find(entry->src_dst);
+if (!src) {
+warn_internalerror();
+continue;
+}
+if (!src->data) {
+warn_internalerror();
+continue;
+}
+if (lsrc + 1 +  sizeof *entry >= entry->size) {
+warn_internalerror();
+continue;
+}
+dst = romfile_find(entry->src_dst + lsrc + 1);
+if (!dst) {
+warn_internalerror();
+continue;
+}
+if (!dst->data) {
+warn_internalerror();
+continue;
+}
+dst_offset = le32_to_cpu(entry->dst_offset);
+if (dst->size < dst_offset + entry->bytes) {
+warn_internalerror();
+continue;
+}
+buf = 0;
+memcpy(&buf, dst->data + dst_offset, entry->bytes);
+val = ((u64)(unsigned long)src->data) >> entry->shift;
+buf = le64_to_cpu(buf);
+if (entry->type == LINKER_ENTRY_TYPE_ADD)
+buf += val;
+else
+buf -= val;
+buf = cpu_to_le64(val);
+memcpy(dst->data + dst_offset, &buf, entry->bytes);
+}
+
+free(data);
+}
diff --git a/src/linker.h b/src/linker.h
new file mode 100644
index 000..d8f4a79
--- /dev/null
+++ b/src/linker.h
@@ -0,0 +1,50 @@
+#ifndef __LINKER_H
+#define __LINKER_H
+
+#include "types.h" // u8
+#include "util.h" // romfile_s
+
+/* ROM file linker interface. Linker uses little endian format */
+struct linker_entry_s {
+u8 size; /* size in bytes including the header */
+u8 bytes; /* How many bytes to change. Can be 1,2,4 or 8. */
+u8 shift; /* Shift source address right by this many bits. 0-63. */
+u8 type;
+u8 format;
+u8 reserved1;
+u16 reserved2;
+u64 dst_offset; /* Offset in destination. Little endian format. */
+/* Followed by source and destination, optionally padded by
+ * 0, up to the total of entry_len - 4 bytes.
+ * Strings are 0-terminated. */
+char src_dst[];
+} PACKED;
+
+/* Note: align types 

[SeaBIOS] [PATCH RFC 3/3] acpi: add an option to disable builtin tables

2013-04-25 Thread Michael S. Tsirkin
Serves to save a bit of memory, and is helpful
for debugging (making sure tables come from qemu).

Memory stats:
Enabled:
Total size: 128776  Fixed: 59100  Free: 2296 (used 98.2% of 128KiB rom)
Disabled:
Total size: 119836  Fixed: 58996  Free: 11236 (used 91.4% of 128KiB rom)

Signed-off-by: Michael S. Tsirkin 
---
 src/Kconfig | 12 +++-
 src/acpi.c  |  2 +-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/Kconfig b/src/Kconfig
index 3c80132..1b54b83 100644
--- a/src/Kconfig
+++ b/src/Kconfig
@@ -387,10 +387,20 @@ menu "BIOS Tables"
 default y
 help
 Support generation of ACPI tables.
+config ACPI_BUILTIN
+bool "Include built-in ACPI tables"
+default y
+depends on ACPI
+help
+Include built-in ACPI tables in BIOS.
+Required for QEMU 1.5 and older.
+This option can be disabled for QEMU 1.6 and newer
+to save some space in the ROM file.
+If unsure, say Y.
 config ACPI_DSDT
 bool "Include default ACPI DSDT"
 default y
-depends on ACPI
+depends on ACPI && ACPI_BUILTIN
 help
 Include default DSDT ACPI table in BIOS.
 Required for QEMU 1.3 and older.
diff --git a/src/acpi.c b/src/acpi.c
index 16ea9f4..b03b2ba 100644
--- a/src/acpi.c
+++ b/src/acpi.c
@@ -664,7 +664,7 @@ acpi_setup(void)
 
 linker_link("/etc/linker-script");
 
-if (!acpi_generate) {
+if (!CONFIG_ACPI_BUILTIN || !acpi_generate) {
 return;
 }
 
-- 
MST

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


[SeaBIOS] [PATCH RFC 2/3] acpi: load and link tables from /etc/acpi/

2013-04-25 Thread Michael S. Tsirkin
Load files in /etc/acpi/ and use for acpi tables.
Any files in this directory completely disable
generating and loading legacy acpi tables.

Signed-off-by: Michael S. Tsirkin 
---
 src/acpi.c | 67 +-
 1 file changed, 66 insertions(+), 1 deletion(-)

diff --git a/src/acpi.c b/src/acpi.c
index 58cd6d7..16ea9f4 100644
--- a/src/acpi.c
+++ b/src/acpi.c
@@ -27,6 +27,7 @@
 #include "config.h" // CONFIG_*
 #include "paravirt.h" // RamSize
 #include "dev-q35.h"
+#include "linker.h"
 
 #include "acpi-dsdt.hex"
 
@@ -603,8 +604,72 @@ acpi_setup(void)
 if (! CONFIG_ACPI)
 return;
 
+int acpi_generate = 1;
+
 dprintf(3, "init ACPI tables\n");
 
+struct romfile_s *file = NULL;
+for (;;) {
+file = romfile_findprefix("/etc/acpi/", file);
+if (!file)
+break;
+
+/*
+ * Disable ACPI table generation. All ACPI tables must come from
+ * etc/acpi/ romfile entries.
+ */
+acpi_generate = 0;
+
+if (!file->size)
+continue;
+
+void *data = malloc_high(file->size);
+if (!data) {
+warn_noalloc();
+break;
+}
+int ret = file->copy(file, data, file->size);
+if (ret < file->size) {
+free(data);
+continue;
+}
+/* Handle RSDP and FACS quirks */
+if (file->size >= 8) {
+struct rsdp_descriptor *rsdp = data;
+struct acpi_table_header *hdr = data;
+/* RSDP is in FSEG memory. */
+if (rsdp->signature == cpu_to_le64(RSDP_SIGNATURE)) {
+data = malloc_fseg(file->size);
+if (!data) {
+warn_noalloc();
+break;
+}
+memcpy(data, rsdp, file->size);
+free(rsdp);
+/* Store Rsdp pointer for use by find_fadt. */
+RsdpAddr = data;
+} else if (hdr->signature == FACS_SIGNATURE) {
+/* FACS is aligned to a 64 bit boundary. */
+data = memalign_high(64, file->size);
+if (!data) {
+warn_noalloc();
+break;
+}
+memcpy(data, hdr, file->size);
+free(hdr);
+}
+}
+file->data = data;
+}
+
+linker_link("/etc/linker-script");
+
+if (!acpi_generate) {
+return;
+}
+
+dprintf(3, "generate ACPI tables\n");
+
 // This code is hardcoded for PIIX4 Power Management device.
 struct pci_device *pci = pci_find_init_device(acpi_find_tbl, NULL);
 if (!pci)
@@ -630,7 +695,7 @@ acpi_setup(void)
 if (pci->device == PCI_DEVICE_ID_INTEL_ICH9_LPC)
 ACPI_INIT_TABLE(build_mcfg_q35());
 
-struct romfile_s *file = NULL;
+file = NULL;
 for (;;) {
 file = romfile_findprefix("acpi/", file);
 if (!file)
-- 
MST


___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios