Re: [Qemu-devel] [PATCH v3 06/16] piix4: add Reset Control Register

[Marcel Apfelbaum <marcel@redhat.com>]

On 29/12/2017 16:29, Hervé Poussineau wrote:
> The RCR I/O port (0xcf9) is used to generate a hard reset or a soft reset.
> 
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>   hw/isa/piix4.c | 35 +++++++++++++++++++++++++++++++++++
>   1 file changed, 35 insertions(+)
> 
> diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
> index 4f476dc7e6..7c83e7c23d 100644
> --- a/hw/isa/piix4.c
> +++ b/hw/isa/piix4.c
> @@ -2,6 +2,7 @@
>    * QEMU PIIX4 PCI Bridge Emulation
>    *
>    * Copyright (c) 2006 Fabrice Bellard
> + * Copyright (c) 2016 Hervé Poussineau

I think 2018 :)

>    *
>    * Permission is hereby granted, free of charge, to any person obtaining a copy
>    * of this software and associated documentation files (the "Software"), to deal
> @@ -33,6 +34,10 @@ PCIDevice *piix4_dev;
>   
>   typedef struct PIIX4State {
>       PCIDevice dev;
> +
> +    /* Reset Control Register */
> +    MemoryRegion rcr_mem;
> +    uint8_t rcr;
>   } PIIX4State;
>   
>   #define TYPE_PIIX4_PCI_DEVICE "PIIX4"
> @@ -87,6 +92,30 @@ static const VMStateDescription vmstate_piix4 = {
>       }
>   };
>   
> +static void piix4_rcr_write(void *opaque, hwaddr addr, uint64_t val,
> +                            unsigned int len)
> +{
> +    PIIX4State *s = opaque;
> +
> +    if (val & 4) {
> +        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
> +        return;
> +    }
> +    s->rcr = val & 2; /* keep System Reset type only */

How does it work? When do we reset the value?

> +}
> +
> +static uint64_t piix4_rcr_read(void *opaque, hwaddr addr, unsigned int len)
> +{
> +    PIIX4State *s = opaque;
> +    return s->rcr;
> +}
> +
> +static const MemoryRegionOps piix4_rcr_ops = {
> +    .read = piix4_rcr_read,
> +    .write = piix4_rcr_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN

Maybe we need to specify the access_size.
Something like:
.impl = {
         .min_access_size = 1,
         .max_access_size = 1,
     },

Thanks,
Marcel

> +};
> +
>   static void piix4_realize(PCIDevice *pci, Error **errp)
>   {
>       DeviceState *dev = DEVICE(pci);
> @@ -96,6 +125,12 @@ static void piix4_realize(PCIDevice *pci, Error **errp)
>                        pci_address_space_io(pci), errp)) {
>           return;
>       }
> +
> +    memory_region_init_io(&s->rcr_mem, OBJECT(dev), &piix4_rcr_ops, s,
> +                          "reset-control", 1);
> +    memory_region_add_subregion_overlap(pci_address_space_io(pci), 0xcf9,
> +                                        &s->rcr_mem, 1);
> +
>       piix4_dev = pci;
>       qemu_register_reset(piix4_reset, s);
>   }
>