Re: [SeaBIOS] [PATCH 8/8] Convert some QEMU cmos config variables to the romfile interface.

[Kevin O'Connor]

On Mon, Feb 11, 2013 at 09:21:29AM +0100, Paolo Bonzini wrote:
> Il 10/02/2013 05:48, Kevin O'Connor ha scritto:
> > @@ -300,9 +300,8 @@ timer_setup(void)
> >      SET_BDA(timer_counter, ticks);
> >  // Setup Century storage
> > if (CONFIG_QEMU) {
> >    Century = inb_cmos(CMOS_CENTURY);
> > } else {
> > Century = romfile_loadint("etc/century", 0);
> > if (!Century) {
> >     // Infer current century from the year.
> >     u8 year = inb_cmos(CMOS_RTC_YEAR);
> >     if (year > 0x80)
>
> The right thing to would be to use the FADT century field.  In fact we
> should also set the century field to 0x32 under QEMU.

I'm not much of a fan of storing content in the CMOS.  It's fine on
QEMU, but on real-hardware it tends to cause conflicts when switching
between the factory bios and coreboot.  Since the century field isn't
going to change for another 87 years I think it's safe to avoid cmos
in this case.

-Kevin

[PATCH 8/8] Convert some QEMU cmos config variables to the romfile interface.

[Paolo Bonzini]

Il 13/02/2013 02:57, Kevin O'Connor ha scritto:
> > > > > @@ -300,9 +300,8 @@ timer_setup(void)
> > > > >      SET_BDA(timer_counter, ticks);
> > > > >  // Setup Century storage
> > > > > if (CONFIG_QEMU) {
> > > > >    Century = inb_cmos(CMOS_CENTURY);
> > > > > } else {
> > > > > Century = romfile_loadint("etc/century", 0);
> > > > > if (!Century) {
> > > > >     // Infer current century from the year.
> > > > >     u8 year = inb_cmos(CMOS_RTC_YEAR);
> > > > >     if (year > 0x80)
> > >
> > > The right thing to would be to use the FADT century field.  In fact we
> > > should also set the century field to 0x32 under QEMU.
>
> I'm not much of a fan of storing content in the CMOS.

This is time data, where does it belong if not in the CMOS?
> It's fine on
> QEMU, but on real-hardware it tends to cause conflicts when switching
> between the factory bios and coreboot.  Since the century field isn't
> going to change for another 87 years I think it's safe to avoid cmos
> in this case.

I don't know what happens in real hardware if one uses INT 1Ah after
setting the RTC to binary mode, but the code above is wrong in this
case.  Also, there is an off-by-one error because 0x80 should be parsed
as 1980.  (For the same reason it's really 67 years not 87. :))

But yeah, nobody uses INT 1Ah probably, so it's not a big deal.

Paolo

[PATCH 8/8] Convert some QEMU cmos config variables to the romfile interface.

[Kevin O'Connor]

On Wed, Feb 13, 2013 at 12:33:20PM +0100, Paolo Bonzini wrote:
> Il 13/02/2013 02:57, Kevin O'Connor ha scritto:
> > > > The right thing to would be to use the FADT century field.  In fact we
> > > > should also set the century field to 0x32 under QEMU.
> >
> > I'm not much of a fan of storing content in the CMOS.
>
> This is time data, where does it belong if not in the CMOS?

It still has the challenge of knowing where in the CMOS to put it, and
where in the CMOS to update the checksum if it needs to change.

That aside, though, I'm not really happy about patch 8 anyway -
additional complexity and indirection for no real gain, so I'll
withdraw patch 8 for consideration.  The first 7 patches I think are
still a simplification.

Thanks.
-Kevin