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

[Blue Swirl]

On Tue, Jan 8, 2013 at 9:44 PM, 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 PIIX4 spec, IO port 0xcf9 is specified as the Reset Control
> Register. Therefore let's handle single byte writes to offset 1 in the
> [0xcf8, 0xcfb] range separately, and interpret the RCPU bit in the value.
> (Based on "docs/memory.txt", overlapping regions seem to serve a different
> purpose.)
>
> The SRST bit alone could be stateful. However we don't distinguish between
> soft & hard reset, hence the SRST bit is neither checked nor saved.
>
> RHBZ reference: 890459
>
> Signed-off-by: Laszlo Ersek <address@hidden>
> ---
>  hw/piix_pci.c |   22 +++++++++++++++++++++-
>  1 files changed, 21 insertions(+), 1 deletions(-)
>
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index 3d79c73..89d694c 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -31,6 +31,7 @@
>  #include "qemu/range.h"
>  #include "xen.h"
>  #include "pam.h"
> +#include "sysemu/sysemu.h"
>
>  /*
>   * I440FX chipset data sheet.
> @@ -180,11 +181,30 @@ static const VMStateDescription vmstate_i440fx = {
>      }
>  };
>
> +static void i440fx_host_config_write(void *opaque, hwaddr addr,
> +                                     uint64_t val, unsigned len)
> +{
> +    if (addr == 1 && len == 1) {
> +        if (val & 4) {
> +            qemu_system_reset_request();
> +        }
> +        return;
> +    }
> +    pci_host_conf_le_ops.write(opaque, addr, val, len);
> +}
> +
> +static MemoryRegionOps i440fx_host_conf_ops = {
> +    .read       = NULL,
> +    .write      = i440fx_host_config_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN
> +};
> +
>  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.

> +    memory_region_init_io(&s->conf_mem, &i440fx_host_conf_ops, s,
>                            "pci-conf-idx", 4);
>      sysbus_add_io(dev, 0xcf8, &s->conf_mem);
>      sysbus_init_ioports(&s->busdev, 0xcf8, 4);
> --
> 1.7.1
>
>