From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:56459) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TunRj-0008MH-Dc for qemu-devel@nongnu.org; Mon, 14 Jan 2013 12:03:47 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TunRb-0003y6-BI for qemu-devel@nongnu.org; Mon, 14 Jan 2013 12:03:39 -0500 Received: from mx1.redhat.com ([209.132.183.28]:33225) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TunRb-0003xz-3i for qemu-devel@nongnu.org; Mon, 14 Jan 2013 12:03:31 -0500 Message-ID: <50F43AD3.5030604@redhat.com> Date: Mon, 14 Jan 2013 18:05:23 +0100 From: Laszlo Ersek MIME-Version: 1.0 References: <1357681452-24963-1-git-send-email-lersek@redhat.com> <50F00384.10907@redhat.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH] PIIX: reset the VM when the Reset Control Register's RCPU bit gets set List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: Paolo Bonzini , akong@redhat.com, qemu-devel@nongnu.org, Stefan Hajnoczi On 01/12/13 13:13, Blue Swirl wrote: > On Fri, Jan 11, 2013 at 12:20 PM, Laszlo Ersek wrote: >> On 01/09/13 22:01, Blue Swirl wrote: >>> On Tue, Jan 8, 2013 at 9:44 PM, Laszlo Ersek wrote: >> >>>> static int i440fx_pcihost_initfn(SysBusDevice *dev) >>>> { >>>> PCIHostState *s = PCI_HOST_BRIDGE(dev); >>>> >>>> - memory_region_init_io(&s->conf_mem, &pci_host_conf_le_ops, s, >>>> + i440fx_host_conf_ops.read = pci_host_conf_le_ops.read; >>> >>> It would be cleaner to introduce a new memory region (without this >>> copying) which passes 0xcf8 and 0xcfc to standard PCI host but catches >>> accesses to 0xcf9. This may mean that pci_host_config_{read,write} >>> will need to be exposed. >> >> (3) Do you have a single region in mind that covers [ 0xcf8 ..0xcff ] >> contiguously? That would require exposing pci_host_data_{read,write} >> too, not only pci_host_config_{read,write}. > > Actually I was thinking about a new region for 0xcf9 only, but perhaps > that is not possible without the higher priority and overlap. I'm also > not familiar with overlapping regions, could you try if it works? I'll come through as my own detractor, but I don't feel confident experimenting with this. It might work, but that would not be convincing enough; I don't want to violate any (undocumented) rules of the memory region framework. The region with name "pci-conf-idx" is registered like this, for [0xcf8..0xcfb]: i440fx_pcihost_initfn() memory_region_init_io() mr->terminates = true; sysbus_add_io() memory_region_add_subregion() subregion->may_overlap = false; memory_region_add_subregion_common() The points that speak against turning 0xcf9 into a subregion are: - in the memory_region_init_io() primitive (it says terminates=true, apparently affecting the traversal in render_memory_region()), and - in the memory_region_add_subregion() primitive (which says may_overlap=false, which doesn't look very promising, because 0xcf9 would do exactly that). I guess I could try to implement the subregion by open-coding the memory_region_init_io() and sysbus_add_io() functions and overriding as needed, but it seems like a mess. (Which is of course caused by this impossible hack of PIIX4 that places the reset control register inside the PCI config index register! The IO port 0xcf9 actually has double purpose, and it doesn't depend on any kind of "mapping".) Annotating "memory.c", most of it seems to have come from Avi. Can someone else closely familiar with this code advise me? I maintain that 0xcf9 it's going to be a hack, and that my RFC (or the followup to that) keeps the damage reasonably contained. Thanks! Laszlo