On Tue, Jan 17, 2012 at 12:06:54AM +0100, Nils wrote:
> Op maandag 16-01-2012 om 00:23 uur [tijdzone -0500], schreef Kevin
> O'Connor:
> > Oh - it was on top of the recent patches I sent to the mailing list.
> OK sorry!
>
> Now that i used that patch set also everything compiles.
>
> First i used your instructions and named the vgarom: "pci100b,0030.rom"
> but then the vgabios is not run. (See attachment v475.log)
Make sure you have CONFIG_VGA_BRIDGE_SETUP set in coreboot.
Otherwise, I don't understand why it wouldn't run. (Your logs seem to
indicate that optionroms.c:is_pci_vga() is failing - which I think
would only happen if CONFIG_VGA_BRIDGE_SETUP isn't set.)
> Second i used vgaroms/vgabios.bin as the vgabios name.
> The vgabios gets run but i get no display output and SeaBIOS reboots
> over and over. (see attachment v475-2.log)
Yeah - it will break if it gets run via "vgaroms/" - I'll change the
vgabios code to check for this.
> This could have to do with vga region setup.
> I wil investigate in the next day's (when i find time) how to initialize
> MSR's to get pci access set up.
>
> Should the E820 table also reflect the vga range?
> (I have no clue how to fix that)
The e820 shouldn't have anything to do with this.
-Kevin
On Thu, Jan 19, 2012 at 03:02:30PM +0100, Vasilis Liaskovitis wrote:
> On Fri, Jan 13, 2012 at 07:27:01PM -0500, Kevin O'Connor wrote:
> >
> > [...]
> > > Method (CPEJ, 2, NotSerialized) {
> > > // _EJ0 method - eject callback
> > > + Store(ShiftLeft(1, Arg0), PRE)
> > > Sleep(200)
> > > }
> >
> I have another question here: the PCI _EJO callback seems to return 0x0, but
> the CPU _EJ0 doesn't return anything. THe ACPIspec4.0a draft section 6.3.3
> mentions that _EJx methods have no return value. Is the above difference
> intentional?
If the spec says it doesn't return anything, but the acpi code is,
it's probably just an error.
-Kevin
On Fri, Jan 13, 2012 at 12:11:30PM +0100, Vasilis Liaskovitis wrote:
>
> Signed-off-by: Vasilis Liaskovitis <vasilis.liaskovitis(a)profitbricks.com>
The SeaBIOS change is okay with me, but the qemu/kvm change needs to
be accepted first.
[...]
> Method (CPEJ, 2, NotSerialized) {
> // _EJ0 method - eject callback
> + Store(ShiftLeft(1, Arg0), PRE)
> Sleep(200)
> }
Is the Sleep() still needed?
-Kevin
S3 is broken in qemu in the following ways:
1. S3 is immediately followed by a resume (or it doesn't even
take place)
2. The screen goes black after S3
3. Some people reported a hang (I couldn't reproduce it myself)
Chances are we can get these bugs fixed for 1.1, until there let's
not advertise S3 to prevent things like qemu-ga from breaking qemu.
Signed-off-by: Luiz Capitulino <lcapitulino(a)redhat.com>
---
src/acpi-dsdt.dsl | 3 +++
src/acpi-dsdt.hex | 16 ++--------------
2 files changed, 5 insertions(+), 14 deletions(-)
diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl
index 7082b65..2bd8d95 100644
--- a/src/acpi-dsdt.dsl
+++ b/src/acpi-dsdt.dsl
@@ -599,6 +599,8 @@ DefinitionBlock (
* S3 (suspend-to-ram), S4 (suspend-to-disk) and S5 (power-off) type codes:
* must match piix4 emulation.
*/
+#if 0
+ S3 is br0ken in qemu, do not advertise it
Name (\_S3, Package (0x04)
{
0x01, /* PM1a_CNT.SLP_TYP */
@@ -606,6 +608,7 @@ DefinitionBlock (
Zero, /* reserved */
Zero /* reserved */
})
+#endif
Name (\_S4, Package (0x04)
{
Zero, /* PM1a_CNT.SLP_TYP */
diff --git a/src/acpi-dsdt.hex b/src/acpi-dsdt.hex
index 5dc7bb4..9dcef05 100644
--- a/src/acpi-dsdt.hex
+++ b/src/acpi-dsdt.hex
@@ -3,12 +3,12 @@ static unsigned char AmlCode[] = {
0x53,
0x44,
0x54,
-0xd3,
+0xc7,
0x10,
0x0,
0x0,
0x1,
-0x2d,
+0xa3,
0x42,
0x58,
0x50,
@@ -3850,18 +3850,6 @@ static unsigned char AmlCode[] = {
0x8,
0x5f,
0x53,
-0x33,
-0x5f,
-0x12,
-0x6,
-0x4,
-0x1,
-0x1,
-0x0,
-0x0,
-0x8,
-0x5f,
-0x53,
0x34,
0x5f,
0x12,
--
1.7.9.rc0.dirty
On 01/16/2012 01:32 PM, Vasilis Liaskovitis wrote:
> On Sun, Jan 15, 2012 at 02:38:52PM +0200, Avi Kivity wrote:
> > On 01/13/2012 01:11 PM, Vasilis Liaskovitis wrote:
> > > Signed-off-by: Vasilis Liaskovitis <vasilis.liaskovitis(a)profitbricks.com>
> > > ---
> > > hw/acpi_piix4.c | 15 +++++++++++++++
> > > 1 files changed, 15 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
> > > index d5743b6..8bf30dd 100644
> > > --- a/hw/acpi_piix4.c
> > > +++ b/hw/acpi_piix4.c
> > > @@ -37,6 +37,7 @@
> > >
> > > #define GPE_BASE 0xafe0
> > > #define PROC_BASE 0xaf00
> > > +#define PROC_EJ_BASE 0xaf20
> > >
> >
> > We're adding stuff to piix4 which was never there. At a minimum this
> > needs to be documented. Also needs to be -M pc-1.1 and later only.
>
> Where should this be documented? PCI/ACPI hotplug addresses are documented in
> docs/specs/acpi_pci_hotplug.txt
A pleasant surprise
> but for CPU hotplug documentation (i.e.
> for the existing PROC_BASE) I don't see relevant documentation. I will
> create a docs/specs/acpi_cpu_hotplug.txt if that sounds reasonable.
I suggest renaming it to acpi_hotplug.txt, so it covers both cases.
> For pc-1.1, a new QEMUmachine type will be needed I assume. Should a check be
> made against the machine version in the piix4 code? any relevant examples?
>
The standard practice is to set a property. See for example
pc_machine_v0_14 in hw/pc_piix.c, it autosets properties for devices
(erroneously called "drivers" in the code).
btw, I notice the I/O ports are write only and don't remember their
state. I can't think offhand if there's anything bad about it (in fact
not having state makes live migration more robust), but perhaps someone
else will.
--
error compiling committee.c: too many arguments to function
On 01/13/2012 01:11 PM, Vasilis Liaskovitis wrote:
> Signed-off-by: Vasilis Liaskovitis <vasilis.liaskovitis(a)profitbricks.com>
> ---
> hw/acpi_piix4.c | 15 +++++++++++++++
> 1 files changed, 15 insertions(+), 0 deletions(-)
>
> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
> index d5743b6..8bf30dd 100644
> --- a/hw/acpi_piix4.c
> +++ b/hw/acpi_piix4.c
> @@ -37,6 +37,7 @@
>
> #define GPE_BASE 0xafe0
> #define PROC_BASE 0xaf00
> +#define PROC_EJ_BASE 0xaf20
>
We're adding stuff to piix4 which was never there. At a minimum this
needs to be documented. Also needs to be -M pc-1.1 and later only.
--
error compiling committee.c: too many arguments to function
Improvements to the Kconfig system. Namely, this series allows the
main makefile target to build the vgabios if selected. (Thanks Nils
for pointing this out.) It also fixes an issue causing the PCI vendor
id and device id to often not be populated.
-Kevin
Kevin O'Connor (4):
vgabios: Improve vgabios Kconfig menu.
vgabios: Build vgabios by default if enabled in Kconfig.
vgabios: Move vgabios Kconfig definitions to vgasrc/Kconfig.
vgabios: Make VBE code depend on a config setting.
Makefile | 42 ++++++++++++++-----------
src/Kconfig | 55 +---------------------------------
vgasrc/Kconfig | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++++
vgasrc/bochsvga.c | 1 -
vgasrc/clext.c | 1 -
vgasrc/vbe.c | 3 +-
vgasrc/vgabios.h | 1 -
7 files changed, 112 insertions(+), 78 deletions(-)
create mode 100644 vgasrc/Kconfig
--
1.7.6.4
Hi Kevin,
You have been busy with patches, nice! :)
You wrote:
>I noticed that the Geode VGA code seems to be doing manual PCI reads -
>which is kinda odd. Does the patch below work for you?
>
>You'll need to register the rom in cbfs as "pci100b,0030.rom", you'll
>need to make sure coreboot is enabling the vga memory ranges, and
>you'll need to make sure out/autoconf.h really has CONFIG_VGA_DID/VID
>defined corectly - but otherwise it should work.
I tried to compile your patch but it gave the following error:
Linking out/vgarom.o
out/vgalayout16.o: In function `vp_setup':
/home/nils/seabios/seabios_test4/vgasrc/geodevga.c:267: undefined
reference to `pci_config_readl'
out/vgalayout16.o: In function `dc_setup':
/home/nils/seabios/seabios_test4/vgasrc/geodevga.c:235: undefined
reference to `pci_config_readl'
make: *** [out/vgarom.o] Error 1
I don't have more time to investigate this further at the moment.
(I did the #include "pci.h")
Thanks, Nils.
