Re: [Xen-devel] [PATCH v2 02/20] piix4: Add the Reset Control Register

[Philippe Mathieu-Daudé <philmd@xxxxxxxxxx>]

On 10/21/19 3:25 AM, Li Qiang wrote:
> Philippe Mathieu-Daudé <philmd@xxxxxxxxxx <mailto:philmd@xxxxxxxxxx>> 于 
> 2019年10月18日周五 下午9:50写道：
>     From: Hervé Poussineau <hpoussin@xxxxxxxxxxx
>     <mailto:hpoussin@xxxxxxxxxxx>>
>
>     The RCR I/O port (0xcf9) is used to generate a hard reset or a soft
>     reset.
>
>     Acked-by: Michael S. Tsirkin <mst@xxxxxxxxxx <mailto:mst@xxxxxxxxxx>>
>     Acked-by: Paolo Bonzini <pbonzini@xxxxxxxxxx
>     <mailto:pbonzini@xxxxxxxxxx>>
>     Signed-off-by: Hervé Poussineau <hpoussin@xxxxxxxxxxx
>     <mailto:hpoussin@xxxxxxxxxxx>>
>     Message-Id: <20171216090228.28505-7-hpoussin@xxxxxxxxxxx
>     <mailto:20171216090228.28505-7-hpoussin@xxxxxxxxxxx>>
>     Reviewed-by: Aleksandar Markovic <amarkovic@xxxxxxxxxxxx
>     <mailto:amarkovic@xxxxxxxxxxxx>>
>     [PMD: rebased, updated includes]
>     Signed-off-by: Philippe Mathieu-Daudé <philmd@xxxxxxxxxx
>     <mailto:philmd@xxxxxxxxxx>>
>     ---
>       hw/isa/piix4.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++---
>       1 file changed, 46 insertions(+), 3 deletions(-)
>
>     diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
>     index 890d999abf..d0b18e0586 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) 2018 Hervé Poussineau
>        *
>        * Permission is hereby granted, free of charge, to any person
>     obtaining a copy
>        * of this software and associated documentation files (the
>     "Software"), to deal
>     @@ -28,11 +29,17 @@
>       #include "hw/isa/isa.h"
>       #include "hw/sysbus.h"
>       #include "migration/vmstate.h"
>     +#include "sysemu/reset.h"
>     +#include "sysemu/runstate.h"
>
>       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,15 +94,51 @@ 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 */
>     +}
>     +
>     +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,
>     +    .impl = {
>     +        .min_access_size = 1,
>     +        .max_access_size = 1,
>     +    },
>     +};
>     +
>       static void piix4_realize(PCIDevice *dev, Error **errp)
>       {
>     -    PIIX4State *d = PIIX4_PCI_DEVICE(dev);
>     +    PIIX4State *s = PIIX4_PCI_DEVICE(dev);
>
>     -    if (!isa_bus_new(DEVICE(d), pci_address_space(dev),
>     +    if (!isa_bus_new(DEVICE(dev), pci_address_space(dev),
>                            pci_address_space_io(dev), errp)) {
>               return;
>           }
>     -    piix4_dev = &d->dev;
>     +
>     +    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(dev),
>     0xcf9,
>
>
>
> Can we use 'RCR_IOPORT' instead of constant value here? Also don't see 
> this change
> in later patches of this seirals.
Good idea, I missed this one :)

> Anyway
>
> Reviewed-by: Li Qiang <liq3ea@xxxxxxxxx <mailto:liq3ea@xxxxxxxxx>>
Thanks!

> Thanks,
> Li Qiang
>
>     +                                        &s->rcr_mem, 1);
>     +
>     +    piix4_dev = dev;
>       }
>
>       int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn)
>     -- 
>     2.21.0
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel