From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57370) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bN8QN-0003Vi-S9 for qemu-devel@nongnu.org; Tue, 12 Jul 2016 20:53:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bN8QL-0007YP-Cj for qemu-devel@nongnu.org; Tue, 12 Jul 2016 20:53:14 -0400 Received: from ozlabs.org ([2401:3900:2:1::2]:50974) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bN8QK-0007YJ-VS for qemu-devel@nongnu.org; Tue, 12 Jul 2016 20:53:13 -0400 Date: Wed, 13 Jul 2016 10:54:59 +1000 From: David Gibson Message-ID: <20160713005459.GA14615@voom.fritz.box> References: <1467934423-5997-1-git-send-email-andrew.smirnov@gmail.com> <1467934423-5997-7-git-send-email-andrew.smirnov@gmail.com> <20160708034259.GC14675@voom.fritz.box> <20160711022401.GI16355@voom.fritz.box> <20160712052750.GY16355@voom.fritz.box> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="9jxsPFA5p3P2qPhR" Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH 6/9] Convert cpu_memory_rw_debug to use MMUAccessType List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andrey Smirnov Cc: Peter Maydell , QEMU Developers --9jxsPFA5p3P2qPhR Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Jul 12, 2016 at 09:05:24AM -0700, Andrey Smirnov wrote: > On Mon, Jul 11, 2016 at 10:27 PM, David Gibson > wrote: > > On Mon, Jul 11, 2016 at 05:27:50PM +0100, Peter Maydell wrote: > >> On 11 July 2016 at 03:24, David Gibson w= rote: > >> > On Sun, Jul 10, 2016 at 08:32:32PM +0100, Peter Maydell wrote: > >> >> On 8 July 2016 at 04:42, David Gibson = wrote: > >> >> > My only concern here is that the constants are named > >> >> > *MMU*_DATA_... whereas these are physical memory accesses not > >> >> > involving the MMU. I can't actually see any current users of > >> >> > MMUAccessType which makes me a bit confused as to what it's inten= ded > >> >> > meaning was > >> >> > >> >> If you grep for MMU_DATA_LOAD/MMU_DATA_STORE/MMU_INST_FETCH > >> >> you'll see the uses. A lot of the softmmu code uses the > >> >> convention of 0=3Dread,1=3Dwrite,2=3Dinsn (which developed I > >> >> think historically from a bool "is_write", which you'll > >> >> still see in some function argument names, that was > >> >> augmented to handle insn-fetch separately). The enum > >> >> gives us some symbolic names for the constant values. > >> >> (There's a proposed patch somewhere to change the > >> >> 'int is_write' arguments to actually use the enum type.) > >> > > >> > Ah, yes, I see. Still surprisingly few, actually. > >> > >> Yeah, we didn't go through (yet) and update the legacy code, > >> just provided the common type so new code could use it. > > > > Ah, yes, I see. > > > >> > My concern about the potentially misleading name still stands. > >> > >> I don't mind if we want to rename it, but I don't think we want > >> to have two types. This is all in the softmmu code, whether it's > >> in the physical-address parts or the virtual-address parts. > > > > Right, I agree we shouldn't have two types. > > > > I think we should rename the existing constants, though, since it > > doesn't really have anything to do with the MMU. > > >=20 > I agree with your concern about confusing naming, David, and think > those should be renamed as well. In the original version of this patch > I introduced my own enumeration which was duplication MMUAccessType, > so in his feedback Peter asked me to use MMUAccessType instead. >=20 > Please let me know how I should proceed with this series. My preference would be to start the series with something which renames MMUAccessType and its individual values to something more generic, including changing the existing users. But I can't speak to what Peter would prefer. --=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 --9jxsPFA5p3P2qPhR Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXhZFiAAoJEGw4ysog2bOSI2gP/0/AygeWsnm/R1TY7pZoCYzX Ckw8HxkYUC8vuzF7NuEh5FY1t5RdX3ROW2UCZysborfrt4eggD24/vHk2asmEmzR FGR8CVFRJZZCR8Kpp/X9AcjaaP3NMWrgzqzQUdsY5DDGYxt2ML4DN6yZ5XUC0rCC UL9EookdvLWXSAryhNAJWUspFFWaoAcMSQgcaKaYVid3DkvBQPq7cSADVu7LZf81 ZmXp6dQJ1OReI+l2nUB5kjULNy0ytARNs43bKuHTRS/vm70XFEKe57YpGRL04c17 5Pyx5GVLVfj7KuPwtYmrT85Q+BXFUSt7NpB/qh/RuozmdUz/yE4y66+QFJCQ1Xvk UHP/K28Ri0ZNbT8NqMz+ibLmYisnwSvHHC9L9t8USHedE2t0tHY+RKMP6BMqXPPt SGMKRAlXNIMjZ6eiNGEVQqx64FHWR6a0M4gQoTJ9YtQfl2wgBMKJh0MBpTkV3G5r bQu36VOri7maQTBvIJOwnQYygU2aDBjsFvC7uYZ+LdeMzuRyatiW0O/HdyMZrxQx wF9S0t4EDwDncxZLghprduK7R5gl77XokHyon0w+/Dl8tLpYWM5gJwzUmZBpmfM8 Y9dR+YeB0JHqiLbyT7+mC60SZVaHGuJTwfPakOIIl4knUbWI1pwGL1bFB1k/LRsC mJlPgAe0dDAJnx2kZr7W =8iAR -----END PGP SIGNATURE----- --9jxsPFA5p3P2qPhR--