[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3] PIIX3: reset the VM when the Reset Control R
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH v3] PIIX3: reset the VM when the Reset Control Register's RCPU bit gets set |
Date: |
Fri, 25 Jan 2013 08:55:12 +0100 |
On Thu, Jan 24, 2013 at 10:31 AM, Laszlo Ersek <address@hidden> 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 <address@hidden>
> ---
> 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 <address@hidden>