Re: [Qemu-devel] [RFC PATCH] PIIX: reset the VM when the Reset Control Register's RCPU bit gets set

[Laszlo Ersek]

On 01/12/13 13:13, Blue Swirl wrote:
> On Fri, Jan 11, 2013 at 12:20 PM, Laszlo Ersek <address@hidden> wrote:
>> On 01/09/13 22:01, Blue Swirl wrote:
>>> On Tue, Jan 8, 2013 at 9:44 PM, Laszlo Ersek <address@hidden> 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