On Thu, Jan 24, 2013 at 10:31 AM, Laszlo Ersek <ler...@redhat.com> wrote: > From <http://mjg59.dreamwidth.org/3561.html>: > > Traditional PCI config space access is achieved by writing a 32 bit > value to io port 0xcf8 to identify the bus, device, function and config > register. Port 0xcfc then contains the register in question. But if you > write the appropriate pair of magic values to 0xcf9, the machine will > reboot. Spectacular! And not standardised in any way (certainly not part > of the PCI spec), so different chipsets may have different requirements. > Booo. > > In the PIIX3 spec, IO port 0xcf9 is specified as the Reset Control > Register. Bit 1 (System Reset, SRST) would normally differentiate between > soft reset and hard reset, but we ignore the difference beyond allowing > the guest to read it back. > > RHBZ reference: 890459 > > This patch introduces the following overlap between the preexistent > "pci-conf-idx" region and the "piix3-reset-control" region just being > added. Partial output from "info mtree": > > I/O > 0000000000000000-000000000000ffff (prio 0, RW): io > 0000000000000cf8-0000000000000cfb (prio 0, RW): pci-conf-idx > 0000000000000cf9-0000000000000cf9 (prio 1, RW): piix3-reset-control > > I sanity-checked the patch by booting a RHEL-6.3 guest and found no > problems. I summoned gdb and set a breakpoint on rcr_write() in order to > gather a bit more confidence. Relevant frames of the stack: > > kvm_handle_io (port=3321, data=0x7f3f5f3de000, direction=1, size=1, > count=1) [kvm-all.c:1422] > cpu_outb (addr=3321, val=6 '\006') [ioport.c:289] > ioport_write (index=0, address=3321, data=6) [ioport.c:83] > ioport_writeb_thunk (opaque=0x7f3f622c4680, addr=3321, data=6) > [ioport.c:212] > memory_region_iorange_write (iorange=0x7f3f622c4680, offset=0, > width=1, data=6) [memory.c:439] > access_with_adjusted_size (addr=0, value=0x7f3f531fbac0, > size=1, access_size_min=1, > access_size_max=4, > access=0x7f3f5f6e0f90 > <memory_region_write_accessor>, > opaque=0x7f3f6227b668) > [memory.c:364] > memory_region_write_accessor (opaque=0x7f3f6227b668, addr=0, > value=0x7f3f531fbac0, size=1, > shift=0, mask=255) > [memory.c:334] > rcr_write (opaque=0x7f3f6227afb0, addr=0, val=6, len=1) > [hw/piix_pci.c:498] > > The dispatch happens in ioport_write(); "index=0" means byte-wide access: > > static void ioport_write(int index, uint32_t address, uint32_t data) > { > static IOPortWriteFunc * const default_func[3] = { > default_ioport_writeb, > default_ioport_writew, > default_ioport_writel > }; > IOPortWriteFunc *func = ioport_write_table[index][address]; > if (!func) > func = default_func[index]; > func(ioport_opaque[address], address, data); > } > > The "ioport_write_table" and "ioport_opaque" arrays describe the flattened > IO port space. The first array is less interesting (it selects a thunk > function). The "ioport_opaque" array is interesting because it decides how > writing to the port is implemented ultimately. > > 4-byte wide access to 0xcf8 (pci-conf-idx): > > (gdb) print ioport_write_table[2][0xcf8] > $1 = (IOPortWriteFunc *) 0x7f3f5f6d99ba <ioport_writel_thunk> > > (gdb) print \ > ((struct MemoryRegionIORange*)ioport_opaque[0xcf8])->mr->ops.write > $2 = (void (*)(void *, hwaddr, uint64_t, unsigned int)) > 0x7f3f5f5575cb <pci_host_config_write> > > 1-byte wide access to 0xcf9 (piix3-reset-control): > > (gdb) print ioport_write_table[0][0xcf9] > $3 = (IOPortWriteFunc *) 0x7f3f5f6d98d0 <ioport_writeb_thunk> > > (gdb) print \ > ((struct MemoryRegionIORange*)ioport_opaque[0xcf9])->mr->ops.write > $4 = (void (*)(void *, hwaddr, uint64_t, unsigned int)) > 0x7f3f5f6b42f1 <rcr_write> > > The higher priority of "piix3-reset-control" ensures that the 0xcf9 > entries in ioport_write_table / ioport_opaque will always belong to it, > independently of its relative registration order versus "pci-conf-idx". > > Signed-off-by: Laszlo Ersek <ler...@redhat.com> > --- > v2->v3: > - don't touch piix3_post_load(); take the RCR as it comes (Stefan). > Diff against v2: > > diff --git a/hw/piix_pci.c b/hw/piix_pci.c > index 38a1027..4c97a84 100644 > --- a/hw/piix_pci.c > +++ b/hw/piix_pci.c > @@ -462,7 +462,6 @@ static int piix3_post_load(void *opaque, int version_id) > { > PIIX3State *piix3 = opaque; > piix3_update_irq_levels(piix3); > - piix3->rcr &= 2; /* keep System Reset type only */ > return 0; > } > > v1->v2: > - move the RCR into a subsection for migration (Paolo) > > hw/piix_pci.c | 68 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 68 insertions(+), 0 deletions(-)
Thanks Laszlo! Reviewed-by: Stefan Hajnoczi <stefa...@gmail.com>
