From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KwhU0-00005C-13 for qemu-devel@nongnu.org; Sun, 02 Nov 2008 13:15:28 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KwhTy-0008WL-Be for qemu-devel@nongnu.org; Sun, 02 Nov 2008 13:15:27 -0500 Received: from [199.232.76.173] (port=51663 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KwhTy-0008WI-5f for qemu-devel@nongnu.org; Sun, 02 Nov 2008 13:15:26 -0500 Received: from mail.gmx.net ([213.165.64.20]:50338) by monty-python.gnu.org with smtp (Exim 4.60) (envelope-from ) id 1KwhTv-0006X7-3I for qemu-devel@nongnu.org; Sun, 02 Nov 2008 13:15:25 -0500 Message-ID: From: "Sebastian Herbszt" References: <20081027101249.21464.57377.stgit@gleb-debian.qumranet.com.qumranet.com> <20081027101259.21464.36016.stgit@gleb-debian.qumranet.com.qumranet.com> <20081102100432.GB16809@redhat.com> In-Reply-To: <20081102100432.GB16809@redhat.com> Date: Sun, 2 Nov 2008 19:13:37 +0100 MIME-Version: 1.0 Content-Type: text/plain; format=flowed; charset="iso-8859-1"; reply-type=original Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [Bochs-developers] [PATCH 2/6] Add S3 state to DSDT. Handleresumeevent in the BIOS. Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gleb Natapov Cc: bochs-developers@lists.sourceforge.net, qemu-devel@nongnu.org Gleb Natapov wrote: > On Thu, Oct 30, 2008 at 11:41:28PM +0100, Sebastian Herbszt wrote: >> Gleb Natapov wrote: >>> diff --git a/bios/rombios.c b/bios/rombios.c >>> index 88eac04..46996fa 100644 >>> --- a/bios/rombios.c >>> +++ b/bios/rombios.c >>> @@ -2198,6 +2198,31 @@ debugger_off() >>> outb(0xfedc, 0x00); >>> } >>> >>> +void >>> +s3_resume() >>> +{ >>> + Bit32u s3_wakeup_vector; >>> + Bit8u s3_resume_flag; >>> + >>> + s3_resume_flag = read_byte(0x40, 0xb0); >>> + s3_wakeup_vector = read_dword(0x40, 0xb2); >>> + >>> + BX_INFO("S3 resume called %x 0x%lx\n", s3_resume_flag, s3_wakeup_vector); >>> + if (s3_resume_flag != 0xFE || !s3_wakeup_vector) >>> + return; >>> + >>> + write_byte(0x40, 0xb0, 0); >>> + >>> + /* setup wakeup vector */ >>> + write_word(0x40, 0xb6, (s3_wakeup_vector & 0xF)); /* IP */ >>> + write_word(0x40, 0xb8, (s3_wakeup_vector >> 4)); /* CS */ >> >> Any reason not to use 0040h:0067h instead? >> > It is OS visible. OS can assume that a value written there will stay > there till overwritten by the OS itself. 0040h:00B6h is as much OS visiable as the "RESET RESTART ADDRESS". The former has no standard definition tho and could or not be used by the OS or BIOS. The "RESET RESTART ADDRESS" is used by the (commercial) BIOS together with the CMOS "Shutdown Status Byte" for internal purpose. I am not sure whether the OS expectation is valid. This however is a minor detail which can be changed later if needed. >>> + >>> + if (*shutdown_flag == 0xfe) { >>> + *s3_resume_vector = find_resume_vector(); >>> + if (!*s3_resume_vector) { >>> + BX_INFO("This is S3 resume but wakeup vector is NULL\n"); >>> + } else { >>> + BX_INFO("S3 resume vector %p\n", *s3_resume_vector); >>> + /* redirect bios read access to RAM */ >>> + pci_for_each_device(find_440fx); >>> + bios_lock_shadow_ram(); /* bios is already copied */ >> >> bios_shadow_init() is called in pci_bios_init(). Why do we need to lock it here? >> > Because we don't call pci_bios_init() if it is S3 resume. If pci_bios_init() is not called on S3 resume, pci_bios_init_bridges() is not called either. This way bios_shadow_init() is never called and the bios is never shadowed. Since it's not shadowed there is no need to lock it. What do i miss here? >>> + return; >>> + } >>> + } >>> + >>> + uuid_probe(); >>> + >>> pci_bios_init(); >>> >>> if (bios_table_cur_addr != 0) { >>> >> >> rombios32.c r1.32 got: >> >> smp_probe(); >> >> pci_bios_init(); >> >> if (bios_table_cur_addr != 0) { >> >> mptable_init(); >> >> uuid_probe(); >> >> The patch should remove uuid_probe() from the if-case. >> > Why? uuid_probe() is needed only if SMBIOS tables are built. And > smbios_init() call is in the same if. Your patch does also add a uuid_probe() before the if-case. Since it does not remove the one in the if-case, uuid_probe() will be called twice. > BTW judging by this if it > looks like BX_USE_EBDA_TABLES is not supported today too since > bios_table_cur_addr is always zero in this case. find_bios_table_area() is supposed to also succeed in the BX_USE_EBDA_TABLES case because bios_table_area_start in rombios.c is unconditional. In the BX_USE_EBDA_TABLES case mptable_init() does use mp_config_table and smbios_init() does use ebda_cur_addr which is set in ram_probe(). - Sebastian