[Qemu-devel] Re: [Bochs-developers] [PATCH v4 2/5] Preserve memory content duringSMM init.

["Sebastian Herbszt" <herbszt@gmx.de>]

Gleb Natapov wrote:
> Signed-off-by: Gleb Natapov <gleb@redhat.com>
> ---
> 
> bios/rombios32.c |   10 ++++++++--
> 1 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/bios/rombios32.c b/bios/rombios32.c
> index 7b354a8..7075aa9 100644
> --- a/bios/rombios32.c
> +++ b/bios/rombios32.c
> @@ -731,6 +731,12 @@ static void smm_init(PCIDevice *d)
>     value = pci_config_readl(d, 0x58);
>     if ((value & (1 << 25)) == 0) {
> 
> +        /* enable the SMM memory window */
> +        pci_config_writeb(&i440_pcidev, 0x72, 0x02 | 0x48);
> +
> +        /* save original memory content */
> +        memcpy((void *)0xa8000, (void *)0x38000, 0x8000);
> +
>         /* copy the SMM relocation code */
>         memcpy((void *)0x38000, &smm_relocation_start,
>                &smm_relocation_end - &smm_relocation_start);
> @@ -747,8 +753,8 @@ static void smm_init(PCIDevice *d)
>         /* wait until SMM code executed */
>         while (inb(0xb3) != 0x00);
> 
> -        /* enable the SMM memory window */
> -        pci_config_writeb(&i440_pcidev, 0x72, 0x02 | 0x48);
> +        /* restore original memory content */
> +        memcpy((void *)0x38000, (void *)0xa8000, 0x8000);
> 
>         /* copy the SMM code */
>         memcpy((void *)0xa8000, &smm_code_start,

Gleb,
can you please explain why this is needed?

smm_init() is currently only run on VM start and overwriting
memory is not an issue there. This would matter on S3 resume,
but smm_init() is not run there. It is only run from pci_bios_init(),
but on S3 resume we bail out from rombios32.c before this call.
Your patch to handle S3 in qemu does use qemu_system_reset_request()
which does reset some emulated devices. Since piix4_pm does not
register a reset handler it is not reset. I assume the reset on S3 resume
is a hardware reset and the state of piix4_pm should be reset too, right?
cpu_reset() in target-i386/helper.c does set smbase to 0x30000. Since the
piix4_pm state is not lost, APMC_EN is still set and even on a normal reset
SMBASE is not relocated. If i don't mistake smm_init() should be run on S3
resume and we would need something like

pci_for_each_device(find_piix4pm)
if (piix4pm_pcidev)
    smm_init();

in the resume path in rombios32.c.

- Sebastian