From: Avi Kivity Subject: Re: [Qemu-devel] [PATCH v4 26/39] pcnet: convert to memory API Date: Tue, 09 Aug 2011 15:44:35 +0300 Message-ID: <4E412BB3.9070703@redhat.com> References: <1312808972-1718-1-git-send-email-avi@redhat.com> <1312808972-1718-27-git-send-email-avi@redhat.com> <4E40D9F3.5080309@mc.net> <4E40D921.9080700@redhat.com> <20110809124246.GF524@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Bob Breuer , Anthony Liguori , qemu-devel@nongnu.org, kvm@vger.kernel.org, Blue Swirl To: "Michael S. Tsirkin" Return-path: Received: from mx1.redhat.com ([209.132.183.28]:12832 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753281Ab1HIMot (ORCPT ); Tue, 9 Aug 2011 08:44:49 -0400 In-Reply-To: <20110809124246.GF524@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 08/09/2011 03:42 PM, Michael S. Tsirkin wrote: > On Tue, Aug 09, 2011 at 09:52:17AM +0300, Avi Kivity wrote: > > On 08/09/2011 09:55 AM, Bob Breuer wrote: > > >> static void lance_cleanup(VLANClientState *nc) > > >> @@ -117,13 +116,11 @@ static int lance_init(SysBusDevice *dev) > > >> SysBusPCNetState *d = FROM_SYSBUS(SysBusPCNetState, dev); > > >> PCNetState *s =&d->state; > > >> > > >> - s->mmio_index = > > >> - cpu_register_io_memory(lance_mem_read, lance_mem_write, d, > > >> - DEVICE_NATIVE_ENDIAN); > > >> + memory_region_init_io(&s->mmio,&lance_mem_ops, s, "lance-mmio", 4); > > > > > >You've switched up d and s here, so anything that tries to talk to the > > >ethernet, such as a sparc32 guest, will now cause Qemu to segfault. > > > > > > > > > > Good catch; will post a fix. > > > > Maybe keeping the opaque wasn't such a good idea. > > Yes, we typically can get from the mmio to the device state > using container_of. > > But in some cases, we can't, and the it's a pain having to wrap MemoryRegion in another structure containing an opaque. Maybe a good compromise is to: - keep MemoryRegion::opaque - pass a MemoryRegion *mr to callbacks instead of opaque - use container_of() when possible - use mr->opaque otherwise -- error compiling committee.c: too many arguments to function