From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:44134) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T49Gd-00059k-5v for qemu-devel@nongnu.org; Wed, 22 Aug 2012 07:38:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T49GZ-00050e-0Z for qemu-devel@nongnu.org; Wed, 22 Aug 2012 07:38:35 -0400 Received: from e23smtp09.au.ibm.com ([202.81.31.142]:34045) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T49GY-000506-FE for qemu-devel@nongnu.org; Wed, 22 Aug 2012 07:38:30 -0400 Received: from /spool/local by e23smtp09.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 22 Aug 2012 21:37:18 +1000 Received: from d23av04.au.ibm.com (d23av04.au.ibm.com [9.190.235.139]) by d23relay03.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q7MBcOkk19857518 for ; Wed, 22 Aug 2012 21:38:25 +1000 Received: from d23av04.au.ibm.com (loopback [127.0.0.1]) by d23av04.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q7MBcOPD032541 for ; Wed, 22 Aug 2012 21:38:24 +1000 Date: Wed, 22 Aug 2012 21:38:24 +1000 From: David Gibson Message-ID: <20120822113824.GA15822@truffula.fritz.box> References: <1345611560-8290-1-git-send-email-david@gibson.dropbear.id.au> <48B2B165-8770-4BD9-9D9A-2D089A9A9208@suse.de> <20120822055725.GZ29724@truffula.fritz.box> <5034807C.8010004@web.de> <503484D0.6090100@web.de> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="45Z9DzgjV8m4Oswq" Content-Disposition: inline In-Reply-To: <503484D0.6090100@web.de> Subject: Re: [Qemu-devel] [PATCH] cpu_physical_memory_write_rom() needs to do TB invalidates List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: aliguori@us.ibm.com, Alexander Graf , qemu-devel@nongnu.org --45Z9DzgjV8m4Oswq Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Aug 22, 2012 at 09:05:52AM +0200, Jan Kiszka wrote: > On 2012-08-22 08:47, Jan Kiszka wrote: > > On 2012-08-22 07:57, David Gibson wrote: > >> On Wed, Aug 22, 2012 at 07:55:31AM +0200, Alexander Graf wrote: > >>> > >>> On 22.08.2012, at 06:59, David Gibson wrote: > >>> > >>>> 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 machi= ne > >>>> uses load_image_targphys() into RAM addresses. > >>>> > >>>> However, cpu_physical_memory_write_rom(), unlike cpu_physical_memory= _rw() > >>>> does invalidate any cached TBs which might be affected by the region > >>>> written. > >>>> > >>>> This was breaking reset (under full emu) on the pseries machine - we= loaded > >>>> 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 w= as > >>>> 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_phy= s_addr_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)); > >>>> + } > >>> > >>> Can't we just call cpu_physical_memory_rw in the RAM case? The > >>> function only tries to not do MMIO accesses on ROM pages, right? > >> > >> Maybe. It's not clear at all to me what cases > >> cpu_physical_memory_write_rom() is supposed to be for, as opposed to > >> just using cpu_physical_memory_rw(). > >=20 > > write_rom ignores write protection - that you usually find on ROMs. That > > makes no difference under KVM so far as there we lack read-only > > sections. But that will be fixed soon, patches are on the list. >=20 > In fact, it does make a difference also for KVM mode as > cpu_physical_memory_rw works from userspace while the limitation only > affects guest code running under KVM control. Ok, so IIUC, that means we do need the cpu_physical_memory_write_rom() version for load_image_targphys(), and so my original patch is basically the right fix. > Jan >=20 > PS: I'm still facing a bogus Mail-Followup-To tag in your postings, > David, thus you easily fall from the To list on reply. Ah, yes, thanks for the reminder. I think I finally found the right option to stop mutt from doing that. --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --45Z9DzgjV8m4Oswq Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) iEYEARECAAYFAlA0xLAACgkQaILKxv3ab8bUIgCeJMo+ttIht9rmq1VgkAgTFp8j Yt4An0qlS23izHdK2M/sXiKT1fQTz5MJ =wiW1 -----END PGP SIGNATURE----- --45Z9DzgjV8m4Oswq--