From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48537) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bLMjq-0003Ej-MX for qemu-devel@nongnu.org; Thu, 07 Jul 2016 23:46:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bLMjp-0006sE-FY for qemu-devel@nongnu.org; Thu, 07 Jul 2016 23:46:02 -0400 Received: from ozlabs.org ([2401:3900:2:1::2]:60129) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bLMjo-0006ru-TK for qemu-devel@nongnu.org; Thu, 07 Jul 2016 23:46:01 -0400 Date: Fri, 8 Jul 2016 13:45:39 +1000 From: David Gibson Message-ID: <20160708034539.GD14675@voom.fritz.box> References: <1467934423-5997-1-git-send-email-andrew.smirnov@gmail.com> <1467934423-5997-8-git-send-email-andrew.smirnov@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="HjNkcEWJ4DMx36DP" Content-Disposition: inline In-Reply-To: <1467934423-5997-8-git-send-email-andrew.smirnov@gmail.com> Subject: Re: [Qemu-devel] [PATCH 7/9] Convert address_space_rw to use MMUAccessType List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andrey Smirnov Cc: qemu-devel@nongnu.org --HjNkcEWJ4DMx36DP Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jul 07, 2016 at 04:33:41PM -0700, Andrey Smirnov wrote: > Convert address_space_rw() to use MMUAccessType following the conversion > of cpu_memory_rw_debug(). Same concerns as the previous patch. These paths don't actually have anything to do with the MMU, which makes the constants oddly named. >=20 > Signed-off-by: Andrey Smirnov > --- > exec.c | 13 ++++++++----- > include/exec/memory.h | 7 +++++-- > kvm-all.c | 8 +++++--- > scripts/coverity-model.c | 7 +++++-- > 4 files changed, 23 insertions(+), 12 deletions(-) >=20 > diff --git a/exec.c b/exec.c > index 36a66e6..1fc6726 100644 > --- a/exec.c > +++ b/exec.c > @@ -2718,12 +2718,15 @@ MemTxResult address_space_read_full(AddressSpace = *as, hwaddr addr, > } > =20 > MemTxResult address_space_rw(AddressSpace *as, hwaddr addr, MemTxAttrs a= ttrs, > - uint8_t *buf, int len, bool is_write) > + uint8_t *buf, int len, MMUAccessType access= _type) > { > - if (is_write) { > + switch (access_type) { > + case MMU_DATA_STORE: > return address_space_write(as, addr, attrs, buf, len); > - } else { > + case MMU_DATA_LOAD: > return address_space_read(as, addr, attrs, buf, len); > + default: > + abort(); > } > } > =20 > @@ -2731,7 +2734,7 @@ void cpu_physical_memory_rw(hwaddr addr, uint8_t *b= uf, > int len, int is_write) > { > address_space_rw(&address_space_memory, addr, MEMTXATTRS_UNSPECIFIED, > - buf, len, is_write); > + buf, len, is_write ? MMU_DATA_STORE : MMU_DATA_LOAD= ); > } > =20 > enum write_rom_type { > @@ -3643,7 +3646,7 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong= addr, > } else { > address_space_rw(cpu->cpu_ases[asidx].as, phys_addr, > MEMTXATTRS_UNSPECIFIED, > - buf, l, 0); > + buf, l, access_type); Replacing a fixed 0 with access_type looks wrong here, but I don't have the context readily to hand to be sure. > } > len -=3D l; > buf +=3D l; > diff --git a/include/exec/memory.h b/include/exec/memory.h > index 51e4c2f..368263c 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -22,6 +22,7 @@ > #define DIRTY_MEMORY_NUM 3 /* num of dirty bits */ > =20 > #include "exec/cpu-common.h" > +#include "qom/cpu.h" > #ifndef CONFIG_USER_ONLY > #include "exec/hwaddr.h" > #endif > @@ -1251,11 +1252,13 @@ void address_space_destroy(AddressSpace *as); > * @addr: address within that address space > * @attrs: memory transaction attributes > * @buf: buffer with the data transferred > - * @is_write: indicates the transfer direction > + * @access_type: indicates the transfer direction (only valid values > + * are MMU_DATA_LOAD for data reads and MMU_DATA_STORE for data > + * writes) > */ > MemTxResult address_space_rw(AddressSpace *as, hwaddr addr, > MemTxAttrs attrs, uint8_t *buf, > - int len, bool is_write); > + int len, MMUAccessType access_type); > =20 > /** > * address_space_write: write to address space. > diff --git a/kvm-all.c b/kvm-all.c > index a88f917..7582275 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -1767,11 +1767,12 @@ static void kvm_handle_io(uint16_t port, MemTxAtt= rs attrs, void *data, int direc > { > int i; > uint8_t *ptr =3D data; > + MMUAccessType access_type =3D > + (direction =3D=3D KVM_EXIT_IO_OUT) ? MMU_DATA_STORE : MMU_DATA_L= OAD; > =20 > for (i =3D 0; i < count; i++) { > address_space_rw(&address_space_io, port, attrs, > - ptr, size, > - direction =3D=3D KVM_EXIT_IO_OUT); > + ptr, size, access_type); > ptr +=3D size; > } > } > @@ -1947,7 +1948,8 @@ int kvm_cpu_exec(CPUState *cpu) > run->mmio.phys_addr, attrs, > run->mmio.data, > run->mmio.len, > - run->mmio.is_write); > + run->mmio.is_write ? > + MMU_DATA_STORE : MMU_DATA_LOAD); > ret =3D 0; > break; > case KVM_EXIT_IRQ_WINDOW_OPEN: > diff --git a/scripts/coverity-model.c b/scripts/coverity-model.c > index ee5bf9d..be3418d 100644 > --- a/scripts/coverity-model.c > +++ b/scripts/coverity-model.c > @@ -68,13 +68,16 @@ static void __bufread(uint8_t *buf, ssize_t len) > } > =20 > MemTxResult address_space_rw(AddressSpace *as, hwaddr addr, MemTxAttrs a= ttrs, > - uint8_t *buf, int len, bool is_write) > + uint8_t *buf, int len, MMUAccessType access= _type) > { > MemTxResult result; > =20 > // TODO: investigate impact of treating reads as producing > // tainted data, with __coverity_tainted_data_argument__(buf). > - if (is_write) __bufread(buf, len); else __bufwrite(buf, len); > + if (access_type =3D=3D MMU_DATA_STORE) > + __bufread(buf, len); > + else > + __bufwrite(buf, len); > =20 > return result; > } --=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 --HjNkcEWJ4DMx36DP Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXfyHjAAoJEGw4ysog2bOSn2wP/0qFupTt1o8UVdqpEGjWHqYb ty1kJAwVPGKYk6FbC9LEwG63XIy5RucEirnZ7LVveHxvvnDXFBEJASmCdOtHyXQx XwGXKb5WFzPql33ugXWs7NAqIKpv0TGC/YBe3r68ddjEmuCpscqdDq/zQMn0xmlp 8AGg9Xho12eGKEOy5bVH49JJChcum8F2xuFuAtZNYJpWkYdBC2XumrFnkalNTh16 5GwApVUin3Vz0aZ1jg6rEK5Sjn6IiHZSGyn7CA9oCCUdViSMN7ADrwzDqNDOZYKO OkbQcAp1uqbAn5//0DQzoZGpWnsV8o5qkIjviiF+pV8JI9Rxw43K6XLrbzPVxISa eouDYmSyJq+WisjeU0SYWn4zdWpObPkFnBKa0i7okiBBjqHJh/BiKCHFh01tLqVp MqTXLLnISnwqyzpWMfRMRgIDE4HECda5h8jvUNu8CfYB6TJmluroMy93g+HKC6PY 0O7yr8UhoUq7jeXfcpRx9SwVVCOA2gwX0L7vhOhWjQYfrxLanjFts4WlabRxg1iy zp6tNdKOqZVNLLNEL1e/yUSgwKKRaW96IDdLHSECZA92GgGviG08qIUBKZ0tRCFs inRU/fW8wfP0TWjpnmsCXYYtcUHVSGIXoueZfgW0r/2g1qI7J7wbECXhidUC5coW 1tRBfnWhXd0IjlVa04uC =amfR -----END PGP SIGNATURE----- --HjNkcEWJ4DMx36DP--