From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:52780) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R9jNa-0000fG-30 for qemu-devel@nongnu.org; Fri, 30 Sep 2011 16:08:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R9jNY-0002Xq-3K for qemu-devel@nongnu.org; Fri, 30 Sep 2011 16:08:18 -0400 Received: from mail-qw0-f45.google.com ([209.85.216.45]:42441) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R9jNY-0002Xj-17 for qemu-devel@nongnu.org; Fri, 30 Sep 2011 16:08:16 -0400 Received: by qadb10 with SMTP id b10so810514qad.4 for ; Fri, 30 Sep 2011 13:08:15 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <87y5x76mtk.fsf@ginnungagap.bsc.es> References: <20110929134727.19559.54734.stgit@ginnungagap.bsc.es> <20110929134749.19559.26774.stgit@ginnungagap.bsc.es> <87y5x76mtk.fsf@ginnungagap.bsc.es> From: Blue Swirl Date: Fri, 30 Sep 2011 20:07:54 +0000 Message-ID: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 4/5] backdoor: [softmmu] Add QEMU-side proxy to "libbackdoor.a" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl , qemu-devel@nongnu.org, Zhi Yong Wu 2011/9/29 Llu=C3=ADs Vilanova : > Blue Swirl writes: > >> 2011/9/29 Llu=C3=ADs Vilanova : >>> +static uint64_t control_io_read(void *opaque, target_phys_addr_t addr,= unsigned size) >>> +{ >>> + =C2=A0 =C2=A0State *s =3D opaque; >>> + >>> + =C2=A0 =C2=A0uint64_t res =3D ldq_p(&s->size); >>> + =C2=A0 =C2=A0uint8_t *resb =3D (uint8_t*)&res; >>> + =C2=A0 =C2=A0return resb[addr % CTRL_BYTES]; > >> I don't think these lines do what you mean, but I'm also not sure what >> it is supposed to mean. > > Pre: only can read on a byte-per-byte basis (as stated in control_ops.imp= l), > just because the code looks less ugly, and host performance should not be= an > issue here. > > The device is treated as a circular buffer of length CTRL_BYTES > > Reads are only used to get the size of the data channel. > > First line should handle guest/host endianess swapping, although I'm not = sure if > that's the API I'm supposed to use. > > Then return the N'th byte of the uint64_t variable holding the (endianess= -aware) > result. That may be the intention, but the first line will load res from guest memory using an address (&s->size) in host memory. I think the next two lines are equal to return res >> (addr % CTRL_BYTES); but with some obfuscation. It would be much clearer if the registers were byte arrays so you could read and write the data directly without pointer arithmetic. Byte accesses will be slower than larger word size accesses, I thought performance was one of the goals with this? >>> +} >>> + >>> +static void control_io_write(void *opaque, target_phys_addr_t addr, ui= nt64_t data, unsigned size) >>> +{ >>> + =C2=A0 =C2=A0State *s =3D opaque; >>> + >>> + =C2=A0 =C2=A0uint8_t *cmdb =3D (uint8_t*)&s->cmd; >>> + =C2=A0 =C2=A0cmdb[addr % CTRL_BYTES] =3D (uint8_t)data; >>> + >>> + =C2=A0 =C2=A0if ((addr + size) % CTRL_BYTES =3D=3D 0) { >>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0qemu_backdoor(ldq_p(&s->cmd), s->data_ptr)= ; >>> + =C2=A0 =C2=A0} > >> Same here. > > Pre: same as during reads. > > Accumulates writes into s->cmd to build the command the guest is sending = us (in > guest endianess). > > When CTRL_BYTES bytes have been written into the device, get the command = value > into host endianess and invoke the user-provided backdoor callback. > > This assumes that when executing in KVM, the device handling infrastructu= re will > get a lock and only one CPU will be sending a backdoor command until comp= letion. > > > I'll add some comments there and prefix all structs and functions with > "backdoor_", as otherwise debugging could get harder if everyone started > avoiding the prefixes. > > > Lluis > > -- > =C2=A0"And it's much the same thing with knowledge, for whenever you lear= n > =C2=A0something new, the whole world becomes that much richer." > =C2=A0-- The Princess of Pure Reason, as told by Norton Juster in The Pha= ntom > =C2=A0Tollbooth >