From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:33150) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TIh3J-0007WW-Vn for qemu-devel@nongnu.org; Mon, 01 Oct 2012 10:33:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TIh3D-0002tt-DN for qemu-devel@nongnu.org; Mon, 01 Oct 2012 10:32:57 -0400 Received: from e6.ny.us.ibm.com ([32.97.182.146]:54581) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TIh3D-0002tS-8h for qemu-devel@nongnu.org; Mon, 01 Oct 2012 10:32:51 -0400 Received: from /spool/local by e6.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 1 Oct 2012 10:32:48 -0400 Received: from d01av03.pok.ibm.com (d01av03.pok.ibm.com [9.56.224.217]) by d01relay01.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q91ESr1P140474 for ; Mon, 1 Oct 2012 10:28:53 -0400 Received: from d01av03.pok.ibm.com (loopback [127.0.0.1]) by d01av03.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q91ESqmr006059 for ; Mon, 1 Oct 2012 11:28:52 -0300 From: Anthony Liguori In-Reply-To: <50699E11.8020203@redhat.com> References: <1347244257-15586-1-git-send-email-david@gibson.dropbear.id.au> <1347244257-15586-3-git-send-email-david@gibson.dropbear.id.au> <504DEAD1.3060908@suse.de> <20120912055706.GX12554@truffula.fritz.box> <50699E11.8020203@redhat.com> Date: Mon, 01 Oct 2012 09:28:48 -0500 Message-ID: <871uhigtb3.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 2/2] cpu_physical_memory_write_rom() needs to do TB invalidates List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Pavel Hrdina , David Gibson Cc: Andreas =?utf-8?Q?F=C3=A4rber?= , qemu-devel@nongnu.org Pavel Hrdina writes: > On 09/12/2012 07:57 AM, David Gibson wrote: >> On Mon, Sep 10, 2012 at 03:27:45PM +0200, Andreas F=C3=A4rber wrote: >>> Am 10.09.2012 04:30, schrieb David Gibson: >>>> cpu_physical_memory_write_rom(), despite the name, can also be used to >>>> write images into RAM - and will often be used that way if the machine >>>> uses load_image_targphys() into RAM addresses. >>>> >>>> However, cpu_physical_memory_write_rom(), unlike cpu_physical_memory_r= w() >>>> does invalidate any cached TBs which might be affected by the region >>> "doesn't"? >>> >>> Otherwise doesn't look wrong. >> Oops, comment updated. >> >> From 6b913afaf83f52ee787271827c84b492e8ac5895 Mon Sep 17 00:00:00 2001 >> From: David Gibson >> Date: Wed, 22 Aug 2012 14:58:04 +1000 >> Subject: [PATCH] cpu_physical_memory_write_rom() needs to do TB invalida= tes >> >> cpu_physical_memory_write_rom(), despite the name, can also be used to >> write images into RAM - and will often be used that way if the machine >> uses load_image_targphys() into RAM addresses. >> >> However, cpu_physical_memory_write_rom(), unlike >> cpu_physical_memory_rw() doesn't invalidate any cached TBs which might >> be affected by the region written. >> >> This was breaking reset (under full emu) on the pseries machine - we loa= ded >> our firmware image into RAM, and while executing it rewrite the code at >> the entry point (correctly causing a TB invalidate/refresh). When we >> reset the firmware image was reloaded, but the TB from the rewrite was >> still active and caused us to get an illegal instruction trap. >> >> This patch fixes the bug by duplicating the tb invalidate code from >> cpu_physical_memory_rw() in cpu_physical_memory_write_rom(). >> >> Signed-off-by: David Gibson >> --- >> exec.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/exec.c b/exec.c >> index 5834766..eff40d7 100644 >> --- a/exec.c >> +++ b/exec.c >> @@ -3523,6 +3523,13 @@ void cpu_physical_memory_write_rom(target_phys_ad= dr_t addr, >> /* ROM/RAM case */ >> ptr =3D qemu_get_ram_ptr(addr1); >> memcpy(ptr, buf, l); >> + if (!cpu_physical_memory_is_dirty(addr1)) { >> + /* invalidate code */ >> + tb_invalidate_phys_page_range(addr1, addr1 + l, 0); >> + /* set dirty bit */ >> + cpu_physical_memory_set_dirty_flags( >> + addr1, (0xff & ~CODE_DIRTY_FLAG)); >> + } >> qemu_put_ram_ptr(ptr); >> } >> len -=3D l; > Hi, > > this patch breaks Windows XP guest at all. Windows XP boot ends in loob=20 > by restarting itself after time-out expires in windows advanced boot=20 > options. > > I started the guest using this command-line: > > ./x86_64-softmmu/qemu-system-x86_64 -m 2G -drive=20 > file=3D/data/data-shared/images/winxp-test.img -vnc 0.0.0.0:0 Does changing the tb_invalidate_phys_page_range() call to: tb_invalidate_phys_page_range(addr1, addr1 + MAX(l, TARGET_PAGE_SIZE), 0); The dirty flag is being reset for the full page but we're potentially only invalidating a subset of TBs that occur on the page. Regards, Anthony Liguori > > Pavel