Re: [Qemu-devel] [PATCH] piix: do not reset APIC base address (0x80) on piix4_reset.

[Laszlo Ersek]

On 12/18/13 15:19, Paolo Bonzini wrote:
> Il 11/12/2013 12:04, Gal Hammer ha scritto:
>> Michael,
>>
>> True, I haven't figure it out yet, but the current status is that recover 
>> from sleep doesn't work.
>>
>> As far as I can tell it could be either:
>>
>> 1. piix4_reset shouldn't be call on resume.
>> 2. memory_region_set_enabled (called in pm_io_space_update) shouldn't use 
>> config[0x80].
>> 3. the config[0x80] shouldn't be zero in piix4_reset (current solution).
>> 4. something else?
>>
>> I'm not well familiar with the PIIX4 emulation and your help will be 
>> appreciated.
> 
> The datasheet says that config[0x80] is reset to 0.
> 
> The PIIX spec says that during S3 the chipset provides "Shadow registers
> for standard AT write only registers to save and restore system state
> information"  These are just for the 825x (DMA controller, PIC, PIT).
> We do not emulate them and our BIOS does not support them.
> 
> I was told that a few memory controller registers survive S3, which in
> our case would be the i440FX's PAM registers, but I don't think this
> register should be one of them.
> 
> What guest is breaking and how?  Does the guest usually initialize this
> register, or does the firmware (SeaBIOS) do that?  If the latter, this
> could be a SeaBIOS bug instead.

I tend to agree. The commit Gal identified (c046e8c4) is part of v1.7.0,
and Fedora 19 correctly resumes from S3 when using OVMF and running on
qemu v1.7.0 (an out-of-tree qemu fix is needed for setting the 32-bit
PCI hole, but that's irrelevant now.)

OVMF has a bit of code that runs after both cold boot and during S3
resume:

InitializePlatform() [OvmfPkg/PlatformPei/Platform.c]
  MiscInitialization()

It reads:

  //
  // If PMREGMISC/PMIOSE is set, assume the ACPI PMBA has been configured (for
  // example by Xen) and skip the setup here. This matches the logic in
  // AcpiTimerLibConstructor ().
  //
  if ((PciRead8 (PCI_LIB_ADDRESS (0, 1, 3, 0x80)) & 0x01) == 0) {
    //
    // The PEI phase should be exited with fully accessibe PIIX4 IO space:
    // 1. set PMBA
    //
    PciAndThenOr32 (
      PCI_LIB_ADDRESS (0, 1, 3, 0x40),
      (UINT32) ~0xFFC0,
      PcdGet16 (PcdAcpiPmBaseAddress)
      );

    //
    // 2. set PCICMD/IOSE
    //
    PciOr8 (
      PCI_LIB_ADDRESS (0, 1, 3, PCI_COMMAND_OFFSET),
      EFI_PCI_COMMAND_IO_SPACE
      );

    //
    // 3. set PMREGMISC/PMIOSE
    //
    PciOr8 (PCI_LIB_ADDRESS (0, 1, 3, 0x80), 0x01);
  }

>From edk2 SVN revisions 13722 and 13723.

https://github.com/tianocore/edk2/commit/931a0c7
https://github.com/tianocore/edk2/commit/0e20a18

Laszlo