diff mbox series

[v2,08/10] arm: allwinner-h3: add Security Identifier device

Message ID 20191216233519.29030-9-nieklinnenbank@gmail.com
State New
Headers show
Series Add Allwinner H3 SoC and Orange Pi PC Machine | expand

Commit Message

Niek Linnenbank Dec. 16, 2019, 11:35 p.m. UTC
The Security Identifier device in Allwinner H3 System on Chip
gives applications a per-board unique identifier. This commit
adds support for the Allwinner H3 Security Identifier using
a 128-bit UUID value as input.

Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com>
---
 include/hw/arm/allwinner-h3.h      |   2 +
 include/hw/misc/allwinner-h3-sid.h |  40 +++++++
 hw/arm/allwinner-h3.c              |   7 ++
 hw/arm/orangepi.c                  |   4 +
 hw/misc/allwinner-h3-sid.c         | 179 +++++++++++++++++++++++++++++
 hw/misc/Makefile.objs              |   1 +
 hw/misc/trace-events               |   4 +
 7 files changed, 237 insertions(+)
 create mode 100644 include/hw/misc/allwinner-h3-sid.h
 create mode 100644 hw/misc/allwinner-h3-sid.c

Comments

Philippe Mathieu-Daudé Dec. 17, 2019, 7:45 a.m. UTC | #1
Hi Niek,

On 12/17/19 12:35 AM, Niek Linnenbank wrote:
> The Security Identifier device in Allwinner H3 System on Chip
> gives applications a per-board unique identifier. This commit
> adds support for the Allwinner H3 Security Identifier using
> a 128-bit UUID value as input.
> 
> Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com>
> ---
>   include/hw/arm/allwinner-h3.h      |   2 +
>   include/hw/misc/allwinner-h3-sid.h |  40 +++++++
>   hw/arm/allwinner-h3.c              |   7 ++
>   hw/arm/orangepi.c                  |   4 +
>   hw/misc/allwinner-h3-sid.c         | 179 +++++++++++++++++++++++++++++
>   hw/misc/Makefile.objs              |   1 +
>   hw/misc/trace-events               |   4 +
>   7 files changed, 237 insertions(+)
>   create mode 100644 include/hw/misc/allwinner-h3-sid.h
>   create mode 100644 hw/misc/allwinner-h3-sid.c
> 
> diff --git a/include/hw/arm/allwinner-h3.h b/include/hw/arm/allwinner-h3.h
> index 8128ae6131..c98c1972a6 100644
> --- a/include/hw/arm/allwinner-h3.h
> +++ b/include/hw/arm/allwinner-h3.h
> @@ -29,6 +29,7 @@
>   #include "hw/misc/allwinner-h3-clk.h"
>   #include "hw/misc/allwinner-h3-cpucfg.h"
>   #include "hw/misc/allwinner-h3-syscon.h"
> +#include "hw/misc/allwinner-h3-sid.h"
>   #include "target/arm/cpu.h"
>   
>   enum {
> @@ -77,6 +78,7 @@ typedef struct AwH3State {
>       AwH3ClockState ccu;
>       AwH3CpuCfgState cpucfg;
>       AwH3SysconState syscon;
> +    AwH3SidState sid;
>       GICState gic;
>       MemoryRegion sram_a1;
>       MemoryRegion sram_a2;
> diff --git a/include/hw/misc/allwinner-h3-sid.h b/include/hw/misc/allwinner-h3-sid.h
> new file mode 100644
> index 0000000000..79c9a24459
> --- /dev/null
> +++ b/include/hw/misc/allwinner-h3-sid.h
> @@ -0,0 +1,40 @@
> +/*
> + * Allwinner H3 Security ID emulation
> + *
> + * Copyright (C) 2019 Niek Linnenbank <nieklinnenbank@gmail.com>
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef HW_MISC_ALLWINNER_H3_SID_H
> +#define HW_MISC_ALLWINNER_H3_SID_H
> +
> +#include "hw/sysbus.h"
> +#include "qemu/uuid.h"
> +
> +#define TYPE_AW_H3_SID    "allwinner-h3-sid"
> +#define AW_H3_SID(obj)    OBJECT_CHECK(AwH3SidState, (obj), TYPE_AW_H3_SID)
> +
> +typedef struct AwH3SidState {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +    /*< public >*/
> +
> +    MemoryRegion iomem;
> +    uint32_t control;
> +    uint32_t rdkey;
> +    QemuUUID identifier;
> +} AwH3SidState;
> +
> +#endif
> diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c
> index 1a9748ab2e..ba34f905cd 100644
> --- a/hw/arm/allwinner-h3.c
> +++ b/hw/arm/allwinner-h3.c
> @@ -196,6 +196,9 @@ static void aw_h3_init(Object *obj)
>   
>       sysbus_init_child_obj(obj, "cpucfg", &s->cpucfg, sizeof(s->cpucfg),
>                             TYPE_AW_H3_CPUCFG);
> +
> +    sysbus_init_child_obj(obj, "sid", &s->sid, sizeof(s->sid),
> +                          TYPE_AW_H3_SID);

Here add a property alias:

        object_property_add_alias(obj, "identifier", OBJECT(&s->sid),
                                  "identifier", &error_abort);

>   }
>   
>   static void aw_h3_realize(DeviceState *dev, Error **errp)
> @@ -332,6 +335,10 @@ static void aw_h3_realize(DeviceState *dev, Error **errp)
>       qdev_init_nofail(DEVICE(&s->cpucfg));
>       sysbus_mmio_map(SYS_BUS_DEVICE(&s->cpucfg), 0, s->memmap[AW_H3_CPUCFG]);
>   
> +    /* Security Identifier */
> +    qdev_init_nofail(DEVICE(&s->sid));
> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->sid), 0, s->memmap[AW_H3_SID]);
> +
>       /* Universal Serial Bus */
>       sysbus_create_simple(TYPE_AW_H3_EHCI, s->memmap[AW_H3_EHCI0],
>                            qdev_get_gpio_in(DEVICE(&s->gic),
> diff --git a/hw/arm/orangepi.c b/hw/arm/orangepi.c
> index 62cefc8c06..b01c4b4f01 100644
> --- a/hw/arm/orangepi.c
> +++ b/hw/arm/orangepi.c
> @@ -62,6 +62,10 @@ static void orangepi_init(MachineState *machine)
>           exit(1);
>       }
>   
> +    /* Setup SID properties */
> +    qdev_prop_set_string(DEVICE(&s->h3->sid), "identifier",
> +                         "8100c002-0001-0002-0003-000044556677");

And here use the alias:

        qdev_prop_set_string(DEVICE(&s->h3), "identifier",
                             "8100c002-0001-0002-0003-000044556677");

What means this value? Don't you want to be able to set it from command 
line?

>       /* Mark H3 object realized */
>       object_property_set_bool(OBJECT(s->h3), true, "realized", &error_abort);
>       if (error_abort != NULL) {
> diff --git a/hw/misc/allwinner-h3-sid.c b/hw/misc/allwinner-h3-sid.c
> new file mode 100644
> index 0000000000..c472f2bcc6
> --- /dev/null
> +++ b/hw/misc/allwinner-h3-sid.c
> @@ -0,0 +1,179 @@
> +/*
> + * Allwinner H3 Security ID emulation
> + *
> + * Copyright (C) 2019 Niek Linnenbank <nieklinnenbank@gmail.com>
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/units.h"
> +#include "hw/sysbus.h"
> +#include "migration/vmstate.h"
> +#include "qemu/log.h"
> +#include "qemu/module.h"
> +#include "qemu/guest-random.h"
> +#include "qapi/error.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/misc/allwinner-h3-sid.h"
> +#include "trace.h"
> +
> +/* SID register offsets */
> +enum {
> +    REG_PRCTL = 0x40,   /* Control */
> +    REG_RDKEY = 0x60,   /* Read Key */
> +};
> +
> +/* SID register flags */
> +enum {
> +    REG_PRCTL_WRITE   = 0x0002, /* Unknown write flag */
> +    REG_PRCTL_OP_LOCK = 0xAC00, /* Lock operation */
> +};
> +
> +static uint64_t allwinner_h3_sid_read(void *opaque, hwaddr offset,
> +                                      unsigned size)
> +{
> +    const AwH3SidState *s = (AwH3SidState *)opaque;
> +    uint64_t val = 0;
> +
> +    switch (offset) {
> +    case REG_PRCTL:    /* Control */
> +        val = s->control;
> +        break;
> +    case REG_RDKEY:    /* Read Key */
> +        val = s->rdkey;
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: bad read offset 0x%04x\n",
> +                      __func__, (uint32_t)offset);
> +        return 0;
> +    }
> +
> +    trace_allwinner_h3_sid_read(offset, val, size);
> +
> +    return val;
> +}
> +
> +static void allwinner_h3_sid_write(void *opaque, hwaddr offset,
> +                                   uint64_t val, unsigned size)
> +{
> +    AwH3SidState *s = (AwH3SidState *)opaque;
> +
> +    trace_allwinner_h3_sid_write(offset, val, size);
> +
> +    switch (offset) {
> +    case REG_PRCTL:    /* Control */
> +        s->control = val;
> +
> +        if ((s->control & REG_PRCTL_OP_LOCK) &&
> +            (s->control & REG_PRCTL_WRITE)) {
> +            uint32_t id = s->control >> 16;
> +
> +            if (id < sizeof(QemuUUID)) {
> +                s->rdkey = (s->identifier.data[id]) |
> +                           (s->identifier.data[id + 1] << 8) |
> +                           (s->identifier.data[id + 2] << 16) |
> +                           (s->identifier.data[id + 3] << 24);

This is:

                    s->rdkey = ldl_le_p(&s->identifier.data[id]);

> +            }
> +        }
> +        s->control &= ~REG_PRCTL_WRITE;
> +        break;
> +    case REG_RDKEY:    /* Read Key */

Read in a write()?

Maybe we can simply /* fall through */ LOG_GUEST_ERROR?

> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: bad write offset 0x%04x\n",
> +                      __func__, (uint32_t)offset);
> +        break;
> +    }
> +}
> +
> +static const MemoryRegionOps allwinner_h3_sid_ops = {
> +    .read = allwinner_h3_sid_read,
> +    .write = allwinner_h3_sid_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .valid = {
> +        .min_access_size = 4,
> +        .max_access_size = 4,
> +        .unaligned = false

'false' is the default value, maybe we can omit it?

> +    },
> +    .impl.min_access_size = 4,
> +};
> +
> +static void allwinner_h3_sid_reset(DeviceState *dev)
> +{
> +    AwH3SidState *s = AW_H3_SID(dev);
> +
> +    /* Set default values for registers */
> +    s->control = 0;
> +    s->rdkey = 0;
> +}
> +
> +static void allwinner_h3_sid_realize(DeviceState *dev, Error **errp)
> +{
> +}

If you don't need realize(), just remove it. However maybe we want to 
check if the identifier is null, either warn/abort or generate a random one?

> +
> +static void allwinner_h3_sid_init(Object *obj)
> +{
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> +    AwH3SidState *s = AW_H3_SID(obj);
> +
> +    /* Fill UUID with zeroes by default */
> +    qemu_uuid_parse(UUID_NONE, &s->identifier);

AwH3SidState is zeroed just before this init() call. I think we don't 
need to zeroes the UUID again.

> +
> +    /* Memory mapping */
> +    memory_region_init_io(&s->iomem, OBJECT(s), &allwinner_h3_sid_ops, s,
> +                          TYPE_AW_H3_SID, 1 * KiB);
> +    sysbus_init_mmio(sbd, &s->iomem);
> +}
> +
> +static Property allwinner_h3_sid_properties[] = {
> +    DEFINE_PROP_UUID_NODEFAULT("identifier", AwH3SidState, identifier),
> +    DEFINE_PROP_END_OF_LIST()
> +};
> +
> +static const VMStateDescription allwinner_h3_sid_vmstate = {
> +    .name = "allwinner-h3-sid",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32(control, AwH3SidState),
> +        VMSTATE_UINT32(rdkey, AwH3SidState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static void allwinner_h3_sid_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->reset = allwinner_h3_sid_reset;
> +    dc->realize = allwinner_h3_sid_realize;
> +    dc->vmsd = &allwinner_h3_sid_vmstate;
> +    dc->props = allwinner_h3_sid_properties;
> +}
> +
> +static const TypeInfo allwinner_h3_sid_info = {
> +    .name          = TYPE_AW_H3_SID,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_init = allwinner_h3_sid_init,
> +    .instance_size = sizeof(AwH3SidState),
> +    .class_init    = allwinner_h3_sid_class_init,
> +};
> +
> +static void allwinner_h3_sid_register(void)
> +{
> +    type_register_static(&allwinner_h3_sid_info);
> +}
> +
> +type_init(allwinner_h3_sid_register)
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index c4ca2ed689..f3620eee4e 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -31,6 +31,7 @@ common-obj-$(CONFIG_IVSHMEM_DEVICE) += ivshmem.o
>   common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3-clk.o
>   obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3-cpucfg.o
>   common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3-syscon.o
> +common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3-sid.o
>   common-obj-$(CONFIG_REALVIEW) += arm_sysctl.o
>   common-obj-$(CONFIG_NSERIES) += cbus.o
>   common-obj-$(CONFIG_ECCMEMCTL) += eccmemctl.o
> diff --git a/hw/misc/trace-events b/hw/misc/trace-events
> index b93089d010..a777844ca3 100644
> --- a/hw/misc/trace-events
> +++ b/hw/misc/trace-events
> @@ -5,6 +5,10 @@ allwinner_h3_cpucfg_cpu_reset(uint8_t cpu_id, uint32_t reset_addr) "H3-CPUCFG: c
>   allwinner_h3_cpucfg_read(uint64_t offset, uint64_t data, unsigned size) "H3-CPUCFG: read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32
>   allwinner_h3_cpucfg_write(uint64_t offset, uint64_t data, unsigned size) "H3-CPUCFG: write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32
>   
> +# allwinner-h3-sid.c
> +allwinner_h3_sid_read(uint64_t offset, uint64_t data, unsigned size) "H3-SID: read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32
> +allwinner_h3_sid_write(uint64_t offset, uint64_t data, unsigned size) "H3-SID: write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32
> +
>   # eccmemctl.c
>   ecc_mem_writel_mer(uint32_t val) "Write memory enable 0x%08x"
>   ecc_mem_writel_mdr(uint32_t val) "Write memory delay 0x%08x"
>
Niek Linnenbank Dec. 18, 2019, 8:49 p.m. UTC | #2
Hi Philippe,

On Tue, Dec 17, 2019 at 8:45 AM Philippe Mathieu-Daudé <philmd@redhat.com>
wrote:

> Hi Niek,
>
> On 12/17/19 12:35 AM, Niek Linnenbank wrote:
> > The Security Identifier device in Allwinner H3 System on Chip
> > gives applications a per-board unique identifier. This commit
> > adds support for the Allwinner H3 Security Identifier using
> > a 128-bit UUID value as input.
> >
> > Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com>
> > ---
> >   include/hw/arm/allwinner-h3.h      |   2 +
> >   include/hw/misc/allwinner-h3-sid.h |  40 +++++++
> >   hw/arm/allwinner-h3.c              |   7 ++
> >   hw/arm/orangepi.c                  |   4 +
> >   hw/misc/allwinner-h3-sid.c         | 179 +++++++++++++++++++++++++++++
> >   hw/misc/Makefile.objs              |   1 +
> >   hw/misc/trace-events               |   4 +
> >   7 files changed, 237 insertions(+)
> >   create mode 100644 include/hw/misc/allwinner-h3-sid.h
> >   create mode 100644 hw/misc/allwinner-h3-sid.c
> >
> > diff --git a/include/hw/arm/allwinner-h3.h
> b/include/hw/arm/allwinner-h3.h
> > index 8128ae6131..c98c1972a6 100644
> > --- a/include/hw/arm/allwinner-h3.h
> > +++ b/include/hw/arm/allwinner-h3.h
> > @@ -29,6 +29,7 @@
> >   #include "hw/misc/allwinner-h3-clk.h"
> >   #include "hw/misc/allwinner-h3-cpucfg.h"
> >   #include "hw/misc/allwinner-h3-syscon.h"
> > +#include "hw/misc/allwinner-h3-sid.h"
> >   #include "target/arm/cpu.h"
> >
> >   enum {
> > @@ -77,6 +78,7 @@ typedef struct AwH3State {
> >       AwH3ClockState ccu;
> >       AwH3CpuCfgState cpucfg;
> >       AwH3SysconState syscon;
> > +    AwH3SidState sid;
> >       GICState gic;
> >       MemoryRegion sram_a1;
> >       MemoryRegion sram_a2;
> > diff --git a/include/hw/misc/allwinner-h3-sid.h
> b/include/hw/misc/allwinner-h3-sid.h
> > new file mode 100644
> > index 0000000000..79c9a24459
> > --- /dev/null
> > +++ b/include/hw/misc/allwinner-h3-sid.h
> > @@ -0,0 +1,40 @@
> > +/*
> > + * Allwinner H3 Security ID emulation
> > + *
> > + * Copyright (C) 2019 Niek Linnenbank <nieklinnenbank@gmail.com>
> > + *
> > + * This program is free software: you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation, either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program.  If not, see <http://www.gnu.org/licenses/
> >.
> > + */
> > +
> > +#ifndef HW_MISC_ALLWINNER_H3_SID_H
> > +#define HW_MISC_ALLWINNER_H3_SID_H
> > +
> > +#include "hw/sysbus.h"
> > +#include "qemu/uuid.h"
> > +
> > +#define TYPE_AW_H3_SID    "allwinner-h3-sid"
> > +#define AW_H3_SID(obj)    OBJECT_CHECK(AwH3SidState, (obj),
> TYPE_AW_H3_SID)
> > +
> > +typedef struct AwH3SidState {
> > +    /*< private >*/
> > +    SysBusDevice parent_obj;
> > +    /*< public >*/
> > +
> > +    MemoryRegion iomem;
> > +    uint32_t control;
> > +    uint32_t rdkey;
> > +    QemuUUID identifier;
> > +} AwH3SidState;
> > +
> > +#endif
> > diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c
> > index 1a9748ab2e..ba34f905cd 100644
> > --- a/hw/arm/allwinner-h3.c
> > +++ b/hw/arm/allwinner-h3.c
> > @@ -196,6 +196,9 @@ static void aw_h3_init(Object *obj)
> >
> >       sysbus_init_child_obj(obj, "cpucfg", &s->cpucfg, sizeof(s->cpucfg),
> >                             TYPE_AW_H3_CPUCFG);
> > +
> > +    sysbus_init_child_obj(obj, "sid", &s->sid, sizeof(s->sid),
> > +                          TYPE_AW_H3_SID);
>
> Here add a property alias:
>
>         object_property_add_alias(obj, "identifier", OBJECT(&s->sid),
>                                   "identifier", &error_abort);
>
> >   }
> >
> >   static void aw_h3_realize(DeviceState *dev, Error **errp)
> > @@ -332,6 +335,10 @@ static void aw_h3_realize(DeviceState *dev, Error
> **errp)
> >       qdev_init_nofail(DEVICE(&s->cpucfg));
> >       sysbus_mmio_map(SYS_BUS_DEVICE(&s->cpucfg), 0,
> s->memmap[AW_H3_CPUCFG]);
> >
> > +    /* Security Identifier */
> > +    qdev_init_nofail(DEVICE(&s->sid));
> > +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->sid), 0, s->memmap[AW_H3_SID]);
> > +
> >       /* Universal Serial Bus */
> >       sysbus_create_simple(TYPE_AW_H3_EHCI, s->memmap[AW_H3_EHCI0],
> >                            qdev_get_gpio_in(DEVICE(&s->gic),
> > diff --git a/hw/arm/orangepi.c b/hw/arm/orangepi.c
> > index 62cefc8c06..b01c4b4f01 100644
> > --- a/hw/arm/orangepi.c
> > +++ b/hw/arm/orangepi.c
> > @@ -62,6 +62,10 @@ static void orangepi_init(MachineState *machine)
> >           exit(1);
> >       }
> >
> > +    /* Setup SID properties */
> > +    qdev_prop_set_string(DEVICE(&s->h3->sid), "identifier",
> > +                         "8100c002-0001-0002-0003-000044556677");
>
> And here use the alias:
>
>         qdev_prop_set_string(DEVICE(&s->h3), "identifier",
>                              "8100c002-0001-0002-0003-000044556677");
>

Ah OK, I see what you mean. The boards should be using the SoC object only
and
not directly any of its sub devices, correct?



>
> What means this value? Don't you want to be able to set it from command
> line?
>
The first word 0x02c00081 is the identifying word for the H3 SoC in the SID
data.
After that come the per-device unique specific bytes. This is documented at
the end of this page in 'Currently known SID's' on the linux-sunxi.org Wiki:
  https://linux-sunxi.org/SID_Register_Guide

The remaining parts of this value I simply made up without any real meaning.
And yes, it would in fact make sense to have the user be able to override
it from the command line.
It is used by U-boot as an input for generating the MAC address. Linux also
reads it, but I did not investigate
how it us used there. I think I did make a TODO of using a cmdline
argument, but later forgot to actually implement it.

Do you have a suggestion how to best provide the command line argument? I
do see '-device driver[,prop=value]'
is there in the --help for qemu-system-arm, but it looks like that should
be used by the user for adding PCI / USB devices?


> >       /* Mark H3 object realized */
> >       object_property_set_bool(OBJECT(s->h3), true, "realized",
> &error_abort);
> >       if (error_abort != NULL) {
> > diff --git a/hw/misc/allwinner-h3-sid.c b/hw/misc/allwinner-h3-sid.c
> > new file mode 100644
> > index 0000000000..c472f2bcc6
> > --- /dev/null
> > +++ b/hw/misc/allwinner-h3-sid.c
> > @@ -0,0 +1,179 @@
> > +/*
> > + * Allwinner H3 Security ID emulation
> > + *
> > + * Copyright (C) 2019 Niek Linnenbank <nieklinnenbank@gmail.com>
> > + *
> > + * This program is free software: you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation, either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program.  If not, see <http://www.gnu.org/licenses/
> >.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qemu/units.h"
> > +#include "hw/sysbus.h"
> > +#include "migration/vmstate.h"
> > +#include "qemu/log.h"
> > +#include "qemu/module.h"
> > +#include "qemu/guest-random.h"
> > +#include "qapi/error.h"
> > +#include "hw/qdev-properties.h"
> > +#include "hw/misc/allwinner-h3-sid.h"
> > +#include "trace.h"
> > +
> > +/* SID register offsets */
> > +enum {
> > +    REG_PRCTL = 0x40,   /* Control */
> > +    REG_RDKEY = 0x60,   /* Read Key */
> > +};
> > +
> > +/* SID register flags */
> > +enum {
> > +    REG_PRCTL_WRITE   = 0x0002, /* Unknown write flag */
> > +    REG_PRCTL_OP_LOCK = 0xAC00, /* Lock operation */
> > +};
> > +
> > +static uint64_t allwinner_h3_sid_read(void *opaque, hwaddr offset,
> > +                                      unsigned size)
> > +{
> > +    const AwH3SidState *s = (AwH3SidState *)opaque;
> > +    uint64_t val = 0;
> > +
> > +    switch (offset) {
> > +    case REG_PRCTL:    /* Control */
> > +        val = s->control;
> > +        break;
> > +    case REG_RDKEY:    /* Read Key */
> > +        val = s->rdkey;
> > +        break;
> > +    default:
> > +        qemu_log_mask(LOG_GUEST_ERROR, "%s: bad read offset 0x%04x\n",
> > +                      __func__, (uint32_t)offset);
> > +        return 0;
> > +    }
> > +
> > +    trace_allwinner_h3_sid_read(offset, val, size);
> > +
> > +    return val;
> > +}
> > +
> > +static void allwinner_h3_sid_write(void *opaque, hwaddr offset,
> > +                                   uint64_t val, unsigned size)
> > +{
> > +    AwH3SidState *s = (AwH3SidState *)opaque;
> > +
> > +    trace_allwinner_h3_sid_write(offset, val, size);
> > +
> > +    switch (offset) {
> > +    case REG_PRCTL:    /* Control */
> > +        s->control = val;
> > +
> > +        if ((s->control & REG_PRCTL_OP_LOCK) &&
> > +            (s->control & REG_PRCTL_WRITE)) {
> > +            uint32_t id = s->control >> 16;
> > +
> > +            if (id < sizeof(QemuUUID)) {
> > +                s->rdkey = (s->identifier.data[id]) |
> > +                           (s->identifier.data[id + 1] << 8) |
> > +                           (s->identifier.data[id + 2] << 16) |
> > +                           (s->identifier.data[id + 3] << 24);
>
> This is:
>
>                     s->rdkey = ldl_le_p(&s->identifier.data[id]);
>
> > +            }
> > +        }
> > +        s->control &= ~REG_PRCTL_WRITE;
> > +        break;
> > +    case REG_RDKEY:    /* Read Key */
>
> Read in a write()?
>
> Maybe we can simply /* fall through */ LOG_GUEST_ERROR?
>

When writing this module, I looked at how U-Boot is using the SID registers
and simply
named the registers after the names used by U-Boot. You can find this part
in arch/arm/mach-sunxi/cpu_info.c:111,
functions sun8i_efuse_read() and sunxi_get_sid(). U-Boot defines
SIDC_RDKEY, so I named the register also rdkey.
I used the U-Boot source because the Allwinner H3 datasheet does not
document the registers. Later I
found the SID page on the linux-sunxi wiki that I mentioned earlier, and
they also describe the same register names:

   https://linux-sunxi.org/SID_Register_Guide

I suspect the information on this page is written based on the source code
from the original SDK (which I did not study btw)


>
> > +        break;
> > +    default:
> > +        qemu_log_mask(LOG_GUEST_ERROR, "%s: bad write offset 0x%04x\n",
> > +                      __func__, (uint32_t)offset);
> > +        break;
> > +    }
> > +}
> > +
> > +static const MemoryRegionOps allwinner_h3_sid_ops = {
> > +    .read = allwinner_h3_sid_read,
> > +    .write = allwinner_h3_sid_write,
> > +    .endianness = DEVICE_NATIVE_ENDIAN,
> > +    .valid = {
> > +        .min_access_size = 4,
> > +        .max_access_size = 4,
> > +        .unaligned = false
>
> 'false' is the default value, maybe we can omit it?
>

Sure, I'll remove it.


>
> > +    },
> > +    .impl.min_access_size = 4,
> > +};
> > +
> > +static void allwinner_h3_sid_reset(DeviceState *dev)
> > +{
> > +    AwH3SidState *s = AW_H3_SID(dev);
> > +
> > +    /* Set default values for registers */
> > +    s->control = 0;
> > +    s->rdkey = 0;
> > +}
> > +
> > +static void allwinner_h3_sid_realize(DeviceState *dev, Error **errp)
> > +{
> > +}
>
> If you don't need realize(), just remove it. However maybe we want to
> check if the identifier is null, either warn/abort or generate a random
> one?
>
OK, removing it!


>
> > +
> > +static void allwinner_h3_sid_init(Object *obj)
> > +{
> > +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> > +    AwH3SidState *s = AW_H3_SID(obj);
> > +
> > +    /* Fill UUID with zeroes by default */
> > +    qemu_uuid_parse(UUID_NONE, &s->identifier);
>
> AwH3SidState is zeroed just before this init() call. I think we don't
> need to zeroes the UUID again.
>

Ah OK, so you mean new objects are always zeroed. That makes it
much easier indeed. Thanks, I'll remove those lines.


>
> > +
> > +    /* Memory mapping */
> > +    memory_region_init_io(&s->iomem, OBJECT(s), &allwinner_h3_sid_ops,
> s,
> > +                          TYPE_AW_H3_SID, 1 * KiB);
> > +    sysbus_init_mmio(sbd, &s->iomem);
> > +}
> > +
> > +static Property allwinner_h3_sid_properties[] = {
> > +    DEFINE_PROP_UUID_NODEFAULT("identifier", AwH3SidState, identifier),
> > +    DEFINE_PROP_END_OF_LIST()
> > +};
> > +
> > +static const VMStateDescription allwinner_h3_sid_vmstate = {
> > +    .name = "allwinner-h3-sid",
> > +    .version_id = 1,
> > +    .minimum_version_id = 1,
> > +    .fields = (VMStateField[]) {
> > +        VMSTATE_UINT32(control, AwH3SidState),
> > +        VMSTATE_UINT32(rdkey, AwH3SidState),
> > +        VMSTATE_END_OF_LIST()
> > +    }
> > +};
> > +
> > +static void allwinner_h3_sid_class_init(ObjectClass *klass, void *data)
> > +{
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > +
> > +    dc->reset = allwinner_h3_sid_reset;
> > +    dc->realize = allwinner_h3_sid_realize;
> > +    dc->vmsd = &allwinner_h3_sid_vmstate;
> > +    dc->props = allwinner_h3_sid_properties;
> > +}
> > +
> > +static const TypeInfo allwinner_h3_sid_info = {
> > +    .name          = TYPE_AW_H3_SID,
> > +    .parent        = TYPE_SYS_BUS_DEVICE,
> > +    .instance_init = allwinner_h3_sid_init,
> > +    .instance_size = sizeof(AwH3SidState),
> > +    .class_init    = allwinner_h3_sid_class_init,
> > +};
> > +
> > +static void allwinner_h3_sid_register(void)
> > +{
> > +    type_register_static(&allwinner_h3_sid_info);
> > +}
> > +
> > +type_init(allwinner_h3_sid_register)
> > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> > index c4ca2ed689..f3620eee4e 100644
> > --- a/hw/misc/Makefile.objs
> > +++ b/hw/misc/Makefile.objs
> > @@ -31,6 +31,7 @@ common-obj-$(CONFIG_IVSHMEM_DEVICE) += ivshmem.o
> >   common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3-clk.o
> >   obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3-cpucfg.o
> >   common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3-syscon.o
> > +common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3-sid.o
> >   common-obj-$(CONFIG_REALVIEW) += arm_sysctl.o
> >   common-obj-$(CONFIG_NSERIES) += cbus.o
> >   common-obj-$(CONFIG_ECCMEMCTL) += eccmemctl.o
> > diff --git a/hw/misc/trace-events b/hw/misc/trace-events
> > index b93089d010..a777844ca3 100644
> > --- a/hw/misc/trace-events
> > +++ b/hw/misc/trace-events
> > @@ -5,6 +5,10 @@ allwinner_h3_cpucfg_cpu_reset(uint8_t cpu_id, uint32_t
> reset_addr) "H3-CPUCFG: c
> >   allwinner_h3_cpucfg_read(uint64_t offset, uint64_t data, unsigned
> size) "H3-CPUCFG: read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %"
> PRIu32
> >   allwinner_h3_cpucfg_write(uint64_t offset, uint64_t data, unsigned
> size) "H3-CPUCFG: write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %"
> PRIu32
> >
> > +# allwinner-h3-sid.c
> > +allwinner_h3_sid_read(uint64_t offset, uint64_t data, unsigned size)
> "H3-SID: read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32
> > +allwinner_h3_sid_write(uint64_t offset, uint64_t data, unsigned size)
> "H3-SID: write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32
> > +
> >   # eccmemctl.c
> >   ecc_mem_writel_mer(uint32_t val) "Write memory enable 0x%08x"
> >   ecc_mem_writel_mdr(uint32_t val) "Write memory delay 0x%08x"
> >
>
>
Regards,
Niek
Philippe Mathieu-Daudé Dec. 30, 2019, 2:49 p.m. UTC | #3
On 12/18/19 9:49 PM, Niek Linnenbank wrote:
> Hi Philippe,
> 
> On Tue, Dec 17, 2019 at 8:45 AM Philippe Mathieu-Daudé 
> <philmd@redhat.com <mailto:philmd@redhat.com>> wrote:
> 
>     Hi Niek,
> 
>     On 12/17/19 12:35 AM, Niek Linnenbank wrote:
>      > The Security Identifier device in Allwinner H3 System on Chip
>      > gives applications a per-board unique identifier. This commit
>      > adds support for the Allwinner H3 Security Identifier using
>      > a 128-bit UUID value as input.
>      >
>      > Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com
>     <mailto:nieklinnenbank@gmail.com>>
>      > ---
>      >   include/hw/arm/allwinner-h3.h      |   2 +
>      >   include/hw/misc/allwinner-h3-sid.h |  40 +++++++
>      >   hw/arm/allwinner-h3.c              |   7 ++
>      >   hw/arm/orangepi.c                  |   4 +
>      >   hw/misc/allwinner-h3-sid.c         | 179
>     +++++++++++++++++++++++++++++
>      >   hw/misc/Makefile.objs              |   1 +
>      >   hw/misc/trace-events               |   4 +
>      >   7 files changed, 237 insertions(+)
>      >   create mode 100644 include/hw/misc/allwinner-h3-sid.h
>      >   create mode 100644 hw/misc/allwinner-h3-sid.c
>      >
>      > diff --git a/include/hw/arm/allwinner-h3.h
>     b/include/hw/arm/allwinner-h3.h
>      > index 8128ae6131..c98c1972a6 100644
>      > --- a/include/hw/arm/allwinner-h3.h
>      > +++ b/include/hw/arm/allwinner-h3.h
>      > @@ -29,6 +29,7 @@
>      >   #include "hw/misc/allwinner-h3-clk.h"
>      >   #include "hw/misc/allwinner-h3-cpucfg.h"
>      >   #include "hw/misc/allwinner-h3-syscon.h"
>      > +#include "hw/misc/allwinner-h3-sid.h"
>      >   #include "target/arm/cpu.h"
>      >
>      >   enum {
>      > @@ -77,6 +78,7 @@ typedef struct AwH3State {
>      >       AwH3ClockState ccu;
>      >       AwH3CpuCfgState cpucfg;
>      >       AwH3SysconState syscon;
>      > +    AwH3SidState sid;
>      >       GICState gic;
>      >       MemoryRegion sram_a1;
>      >       MemoryRegion sram_a2;
>      > diff --git a/include/hw/misc/allwinner-h3-sid.h
>     b/include/hw/misc/allwinner-h3-sid.h
>      > new file mode 100644
>      > index 0000000000..79c9a24459
>      > --- /dev/null
>      > +++ b/include/hw/misc/allwinner-h3-sid.h
>      > @@ -0,0 +1,40 @@
>      > +/*
>      > + * Allwinner H3 Security ID emulation
>      > + *
>      > + * Copyright (C) 2019 Niek Linnenbank <nieklinnenbank@gmail.com
>     <mailto:nieklinnenbank@gmail.com>>
>      > + *
>      > + * This program is free software: you can redistribute it and/or
>     modify
>      > + * it under the terms of the GNU General Public License as
>     published by
>      > + * the Free Software Foundation, either version 2 of the License, or
>      > + * (at your option) any later version.
>      > + *
>      > + * This program is distributed in the hope that it will be useful,
>      > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>      > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>      > + * GNU General Public License for more details.
>      > + *
>      > + * You should have received a copy of the GNU General Public License
>      > + * along with this program.  If not, see
>     <http://www.gnu.org/licenses/>.
>      > + */
>      > +
>      > +#ifndef HW_MISC_ALLWINNER_H3_SID_H
>      > +#define HW_MISC_ALLWINNER_H3_SID_H
>      > +
>      > +#include "hw/sysbus.h"
>      > +#include "qemu/uuid.h"
>      > +
>      > +#define TYPE_AW_H3_SID    "allwinner-h3-sid"
>      > +#define AW_H3_SID(obj)    OBJECT_CHECK(AwH3SidState, (obj),
>     TYPE_AW_H3_SID)
>      > +
>      > +typedef struct AwH3SidState {
>      > +    /*< private >*/
>      > +    SysBusDevice parent_obj;
>      > +    /*< public >*/
>      > +
>      > +    MemoryRegion iomem;
>      > +    uint32_t control;
>      > +    uint32_t rdkey;
>      > +    QemuUUID identifier;
>      > +} AwH3SidState;
>      > +
>      > +#endif
>      > diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c
>      > index 1a9748ab2e..ba34f905cd 100644
>      > --- a/hw/arm/allwinner-h3.c
>      > +++ b/hw/arm/allwinner-h3.c
>      > @@ -196,6 +196,9 @@ static void aw_h3_init(Object *obj)
>      >
>      >       sysbus_init_child_obj(obj, "cpucfg", &s->cpucfg,
>     sizeof(s->cpucfg),
>      >                             TYPE_AW_H3_CPUCFG);
>      > +
>      > +    sysbus_init_child_obj(obj, "sid", &s->sid, sizeof(s->sid),
>      > +                          TYPE_AW_H3_SID);
> 
>     Here add a property alias:
> 
>              object_property_add_alias(obj, "identifier", OBJECT(&s->sid),
>                                        "identifier", &error_abort);
> 
>      >   }
>      >
>      >   static void aw_h3_realize(DeviceState *dev, Error **errp)
>      > @@ -332,6 +335,10 @@ static void aw_h3_realize(DeviceState *dev,
>     Error **errp)
>      >       qdev_init_nofail(DEVICE(&s->cpucfg));
>      >       sysbus_mmio_map(SYS_BUS_DEVICE(&s->cpucfg), 0,
>     s->memmap[AW_H3_CPUCFG]);
>      >
>      > +    /* Security Identifier */
>      > +    qdev_init_nofail(DEVICE(&s->sid));
>      > +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->sid), 0,
>     s->memmap[AW_H3_SID]);
>      > +
>      >       /* Universal Serial Bus */
>      >       sysbus_create_simple(TYPE_AW_H3_EHCI, s->memmap[AW_H3_EHCI0],
>      >                            qdev_get_gpio_in(DEVICE(&s->gic),
>      > diff --git a/hw/arm/orangepi.c b/hw/arm/orangepi.c
>      > index 62cefc8c06..b01c4b4f01 100644
>      > --- a/hw/arm/orangepi.c
>      > +++ b/hw/arm/orangepi.c
>      > @@ -62,6 +62,10 @@ static void orangepi_init(MachineState *machine)
>      >           exit(1);
>      >       }
>      >
>      > +    /* Setup SID properties */
>      > +    qdev_prop_set_string(DEVICE(&s->h3->sid), "identifier",
>      > +                         "8100c002-0001-0002-0003-000044556677");
> 
>     And here use the alias:
> 
>              qdev_prop_set_string(DEVICE(&s->h3), "identifier",
>                                   "8100c002-0001-0002-0003-000044556677");
> 
> 
> Ah OK, I see what you mean. The boards should be using the SoC object 
> only and
> not directly any of its sub devices, correct?
> 
> 
>     What means this value? Don't you want to be able to set it from command
>     line?
> 
> The first word 0x02c00081 is the identifying word for the H3 SoC in the 
> SID data.
> After that come the per-device unique specific bytes. This is documented 
> at the end of this page in 'Currently known SID's' on the 
> linux-sunxi.org <http://linux-sunxi.org> Wiki:
> https://linux-sunxi.org/SID_Register_Guide
> 
> The remaining parts of this value I simply made up without any real meaning.
> And yes, it would in fact make sense to have the user be able to 
> override it from the command line.
> It is used by U-boot as an input for generating the MAC address. Linux 
> also reads it, but I did not investigate
> how it us used there. I think I did make a TODO of using a cmdline 
> argument, but later forgot to actually implement it.
> 
> Do you have a suggestion how to best provide the command line argument? 
> I do see '-device driver[,prop=value]'
> is there in the --help for qemu-system-arm, but it looks like that 
> should be used by the user for adding PCI / USB devices?

Look for '-global' in the manpage:


   -global driver.prop=value
   -global driver=driver,property=property,value=value

     Set default value of driver's property prop to value.

     In particular, you can use this to set driver properties
     for devices which are created automatically by the machine
     model. To create a device which is not created automatically
     and set properties on it, use -device.

     -global driver.prop=value is shorthand for
     -global driver=driver,property=prop,value=value.

So this should work for your device:

     -global 
allwinner-h3-sid.identifier=8100c002-0001-0002-0003-000044556677

> 
> 
>      >       /* Mark H3 object realized */
>      >       object_property_set_bool(OBJECT(s->h3), true, "realized",
>     &error_abort);
>      >       if (error_abort != NULL) {
>      > diff --git a/hw/misc/allwinner-h3-sid.c b/hw/misc/allwinner-h3-sid.c
>      > new file mode 100644
>      > index 0000000000..c472f2bcc6
>      > --- /dev/null
>      > +++ b/hw/misc/allwinner-h3-sid.c
>      > @@ -0,0 +1,179 @@
>      > +/*
>      > + * Allwinner H3 Security ID emulation
>      > + *
>      > + * Copyright (C) 2019 Niek Linnenbank <nieklinnenbank@gmail.com
>     <mailto:nieklinnenbank@gmail.com>>
>      > + *
>      > + * This program is free software: you can redistribute it and/or
>     modify
>      > + * it under the terms of the GNU General Public License as
>     published by
>      > + * the Free Software Foundation, either version 2 of the License, or
>      > + * (at your option) any later version.
>      > + *
>      > + * This program is distributed in the hope that it will be useful,
>      > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>      > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>      > + * GNU General Public License for more details.
>      > + *
>      > + * You should have received a copy of the GNU General Public License
>      > + * along with this program.  If not, see
>     <http://www.gnu.org/licenses/>.
>      > + */
>      > +
>      > +#include "qemu/osdep.h"
>      > +#include "qemu/units.h"
>      > +#include "hw/sysbus.h"
>      > +#include "migration/vmstate.h"
>      > +#include "qemu/log.h"
>      > +#include "qemu/module.h"
>      > +#include "qemu/guest-random.h"
>      > +#include "qapi/error.h"
>      > +#include "hw/qdev-properties.h"
>      > +#include "hw/misc/allwinner-h3-sid.h"
>      > +#include "trace.h"
>      > +
>      > +/* SID register offsets */
>      > +enum {
>      > +    REG_PRCTL = 0x40,   /* Control */
>      > +    REG_RDKEY = 0x60,   /* Read Key */
>      > +};
>      > +
>      > +/* SID register flags */
>      > +enum {
>      > +    REG_PRCTL_WRITE   = 0x0002, /* Unknown write flag */
>      > +    REG_PRCTL_OP_LOCK = 0xAC00, /* Lock operation */
>      > +};
>      > +
>      > +static uint64_t allwinner_h3_sid_read(void *opaque, hwaddr offset,
>      > +                                      unsigned size)
>      > +{
>      > +    const AwH3SidState *s = (AwH3SidState *)opaque;
>      > +    uint64_t val = 0;
>      > +
>      > +    switch (offset) {
>      > +    case REG_PRCTL:    /* Control */
>      > +        val = s->control;
>      > +        break;
>      > +    case REG_RDKEY:    /* Read Key */
>      > +        val = s->rdkey;
>      > +        break;
>      > +    default:
>      > +        qemu_log_mask(LOG_GUEST_ERROR, "%s: bad read offset
>     0x%04x\n",
>      > +                      __func__, (uint32_t)offset);
>      > +        return 0;
>      > +    }
>      > +
>      > +    trace_allwinner_h3_sid_read(offset, val, size);
>      > +
>      > +    return val;
>      > +}
>      > +
>      > +static void allwinner_h3_sid_write(void *opaque, hwaddr offset,
>      > +                                   uint64_t val, unsigned size)
>      > +{
>      > +    AwH3SidState *s = (AwH3SidState *)opaque;
>      > +
>      > +    trace_allwinner_h3_sid_write(offset, val, size);
>      > +
>      > +    switch (offset) {
>      > +    case REG_PRCTL:    /* Control */
>      > +        s->control = val;
>      > +
>      > +        if ((s->control & REG_PRCTL_OP_LOCK) &&
>      > +            (s->control & REG_PRCTL_WRITE)) {
>      > +            uint32_t id = s->control >> 16;
>      > +
>      > +            if (id < sizeof(QemuUUID)) {
>      > +                s->rdkey = (s->identifier.data[id]) |
>      > +                           (s->identifier.data[id + 1] << 8) |
>      > +                           (s->identifier.data[id + 2] << 16) |
>      > +                           (s->identifier.data[id + 3] << 24);
> 
>     This is:
> 
>                          s->rdkey = ldl_le_p(&s->identifier.data[id]);
> 
>      > +            }
>      > +        }
>      > +        s->control &= ~REG_PRCTL_WRITE;
>      > +        break;
>      > +    case REG_RDKEY:    /* Read Key */
> 
>     Read in a write()?
> 
>     Maybe we can simply /* fall through */ LOG_GUEST_ERROR?
> 
> 
> When writing this module, I looked at how U-Boot is using the SID 
> registers and simply
> named the registers after the names used by U-Boot. You can find this 
> part in arch/arm/mach-sunxi/cpu_info.c:111,
> functions sun8i_efuse_read() and sunxi_get_sid(). U-Boot defines 
> SIDC_RDKEY, so I named the register also rdkey.
> I used the U-Boot source because the Allwinner H3 datasheet does not 
> document the registers. Later I
> found the SID page on the linux-sunxi wiki that I mentioned earlier, and 
> they also describe the same register names:
> 
> https://linux-sunxi.org/SID_Register_Guide

Hmm this page describe this register as RW, odd. I think this is wrong 
because we deal with a fuse, and we program it via the REG_PRKEY 
register. Does Linux/U-Boot do write access to this register?
We can start logging LOG_GUEST_ERROR, and correct it if we notice this 
register is really writable (which I doubt).

> 
> I suspect the information on this page is written based on the source 
> code from the original SDK (which I did not study btw)
[...]
Niek Linnenbank Jan. 7, 2020, 9:53 p.m. UTC | #4
On Mon, Dec 30, 2019 at 3:49 PM Philippe Mathieu-Daudé <philmd@redhat.com>
wrote:

> On 12/18/19 9:49 PM, Niek Linnenbank wrote:
> > Hi Philippe,
> >
> > On Tue, Dec 17, 2019 at 8:45 AM Philippe Mathieu-Daudé
> > <philmd@redhat.com <mailto:philmd@redhat.com>> wrote:
> >
> >     Hi Niek,
> >
> >     On 12/17/19 12:35 AM, Niek Linnenbank wrote:
> >      > The Security Identifier device in Allwinner H3 System on Chip
> >      > gives applications a per-board unique identifier. This commit
> >      > adds support for the Allwinner H3 Security Identifier using
> >      > a 128-bit UUID value as input.
> >      >
> >      > Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com
> >     <mailto:nieklinnenbank@gmail.com>>
> >      > ---
> >      >   include/hw/arm/allwinner-h3.h      |   2 +
> >      >   include/hw/misc/allwinner-h3-sid.h |  40 +++++++
> >      >   hw/arm/allwinner-h3.c              |   7 ++
> >      >   hw/arm/orangepi.c                  |   4 +
> >      >   hw/misc/allwinner-h3-sid.c         | 179
> >     +++++++++++++++++++++++++++++
> >      >   hw/misc/Makefile.objs              |   1 +
> >      >   hw/misc/trace-events               |   4 +
> >      >   7 files changed, 237 insertions(+)
> >      >   create mode 100644 include/hw/misc/allwinner-h3-sid.h
> >      >   create mode 100644 hw/misc/allwinner-h3-sid.c
> >      >
> >      > diff --git a/include/hw/arm/allwinner-h3.h
> >     b/include/hw/arm/allwinner-h3.h
> >      > index 8128ae6131..c98c1972a6 100644
> >      > --- a/include/hw/arm/allwinner-h3.h
> >      > +++ b/include/hw/arm/allwinner-h3.h
> >      > @@ -29,6 +29,7 @@
> >      >   #include "hw/misc/allwinner-h3-clk.h"
> >      >   #include "hw/misc/allwinner-h3-cpucfg.h"
> >      >   #include "hw/misc/allwinner-h3-syscon.h"
> >      > +#include "hw/misc/allwinner-h3-sid.h"
> >      >   #include "target/arm/cpu.h"
> >      >
> >      >   enum {
> >      > @@ -77,6 +78,7 @@ typedef struct AwH3State {
> >      >       AwH3ClockState ccu;
> >      >       AwH3CpuCfgState cpucfg;
> >      >       AwH3SysconState syscon;
> >      > +    AwH3SidState sid;
> >      >       GICState gic;
> >      >       MemoryRegion sram_a1;
> >      >       MemoryRegion sram_a2;
> >      > diff --git a/include/hw/misc/allwinner-h3-sid.h
> >     b/include/hw/misc/allwinner-h3-sid.h
> >      > new file mode 100644
> >      > index 0000000000..79c9a24459
> >      > --- /dev/null
> >      > +++ b/include/hw/misc/allwinner-h3-sid.h
> >      > @@ -0,0 +1,40 @@
> >      > +/*
> >      > + * Allwinner H3 Security ID emulation
> >      > + *
> >      > + * Copyright (C) 2019 Niek Linnenbank <nieklinnenbank@gmail.com
> >     <mailto:nieklinnenbank@gmail.com>>
> >      > + *
> >      > + * This program is free software: you can redistribute it and/or
> >     modify
> >      > + * it under the terms of the GNU General Public License as
> >     published by
> >      > + * the Free Software Foundation, either version 2 of the
> License, or
> >      > + * (at your option) any later version.
> >      > + *
> >      > + * This program is distributed in the hope that it will be
> useful,
> >      > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >      > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >      > + * GNU General Public License for more details.
> >      > + *
> >      > + * You should have received a copy of the GNU General Public
> License
> >      > + * along with this program.  If not, see
> >     <http://www.gnu.org/licenses/>.
> >      > + */
> >      > +
> >      > +#ifndef HW_MISC_ALLWINNER_H3_SID_H
> >      > +#define HW_MISC_ALLWINNER_H3_SID_H
> >      > +
> >      > +#include "hw/sysbus.h"
> >      > +#include "qemu/uuid.h"
> >      > +
> >      > +#define TYPE_AW_H3_SID    "allwinner-h3-sid"
> >      > +#define AW_H3_SID(obj)    OBJECT_CHECK(AwH3SidState, (obj),
> >     TYPE_AW_H3_SID)
> >      > +
> >      > +typedef struct AwH3SidState {
> >      > +    /*< private >*/
> >      > +    SysBusDevice parent_obj;
> >      > +    /*< public >*/
> >      > +
> >      > +    MemoryRegion iomem;
> >      > +    uint32_t control;
> >      > +    uint32_t rdkey;
> >      > +    QemuUUID identifier;
> >      > +} AwH3SidState;
> >      > +
> >      > +#endif
> >      > diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c
> >      > index 1a9748ab2e..ba34f905cd 100644
> >      > --- a/hw/arm/allwinner-h3.c
> >      > +++ b/hw/arm/allwinner-h3.c
> >      > @@ -196,6 +196,9 @@ static void aw_h3_init(Object *obj)
> >      >
> >      >       sysbus_init_child_obj(obj, "cpucfg", &s->cpucfg,
> >     sizeof(s->cpucfg),
> >      >                             TYPE_AW_H3_CPUCFG);
> >      > +
> >      > +    sysbus_init_child_obj(obj, "sid", &s->sid, sizeof(s->sid),
> >      > +                          TYPE_AW_H3_SID);
> >
> >     Here add a property alias:
> >
> >              object_property_add_alias(obj, "identifier",
> OBJECT(&s->sid),
> >                                        "identifier", &error_abort);
> >
> >      >   }
> >      >
> >      >   static void aw_h3_realize(DeviceState *dev, Error **errp)
> >      > @@ -332,6 +335,10 @@ static void aw_h3_realize(DeviceState *dev,
> >     Error **errp)
> >      >       qdev_init_nofail(DEVICE(&s->cpucfg));
> >      >       sysbus_mmio_map(SYS_BUS_DEVICE(&s->cpucfg), 0,
> >     s->memmap[AW_H3_CPUCFG]);
> >      >
> >      > +    /* Security Identifier */
> >      > +    qdev_init_nofail(DEVICE(&s->sid));
> >      > +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->sid), 0,
> >     s->memmap[AW_H3_SID]);
> >      > +
> >      >       /* Universal Serial Bus */
> >      >       sysbus_create_simple(TYPE_AW_H3_EHCI,
> s->memmap[AW_H3_EHCI0],
> >      >                            qdev_get_gpio_in(DEVICE(&s->gic),
> >      > diff --git a/hw/arm/orangepi.c b/hw/arm/orangepi.c
> >      > index 62cefc8c06..b01c4b4f01 100644
> >      > --- a/hw/arm/orangepi.c
> >      > +++ b/hw/arm/orangepi.c
> >      > @@ -62,6 +62,10 @@ static void orangepi_init(MachineState
> *machine)
> >      >           exit(1);
> >      >       }
> >      >
> >      > +    /* Setup SID properties */
> >      > +    qdev_prop_set_string(DEVICE(&s->h3->sid), "identifier",
> >      > +                         "8100c002-0001-0002-0003-000044556677");
> >
> >     And here use the alias:
> >
> >              qdev_prop_set_string(DEVICE(&s->h3), "identifier",
> >
>  "8100c002-0001-0002-0003-000044556677");
> >
> >
> > Ah OK, I see what you mean. The boards should be using the SoC object
> > only and
> > not directly any of its sub devices, correct?
> >
> >
> >     What means this value? Don't you want to be able to set it from
> command
> >     line?
> >
> > The first word 0x02c00081 is the identifying word for the H3 SoC in the
> > SID data.
> > After that come the per-device unique specific bytes. This is documented
> > at the end of this page in 'Currently known SID's' on the
> > linux-sunxi.org <http://linux-sunxi.org> Wiki:
> > https://linux-sunxi.org/SID_Register_Guide
> >
> > The remaining parts of this value I simply made up without any real
> meaning.
> > And yes, it would in fact make sense to have the user be able to
> > override it from the command line.
> > It is used by U-boot as an input for generating the MAC address. Linux
> > also reads it, but I did not investigate
> > how it us used there. I think I did make a TODO of using a cmdline
> > argument, but later forgot to actually implement it.
> >
> > Do you have a suggestion how to best provide the command line argument?
> > I do see '-device driver[,prop=value]'
> > is there in the --help for qemu-system-arm, but it looks like that
> > should be used by the user for adding PCI / USB devices?
>
> Look for '-global' in the manpage:
>
>
>    -global driver.prop=value
>    -global driver=driver,property=property,value=value
>
>      Set default value of driver's property prop to value.
>
>      In particular, you can use this to set driver properties
>      for devices which are created automatically by the machine
>      model. To create a device which is not created automatically
>      and set properties on it, use -device.
>
>      -global driver.prop=value is shorthand for
>      -global driver=driver,property=prop,value=value.
>
> So this should work for your device:
>
>      -global
> allwinner-h3-sid.identifier=8100c002-0001-0002-0003-000044556677
>

Thanks, I tried this, however currently its not applied when used.
I think its because the orangepi.c file sets the property as well.

Its not a blocking problem if the user can't override the SID value, but it
would be a nice-to-have.
U-Boot uses its value for MAC generation and that can be overridden as well
by U-Boot itself.



> >
> >
> >      >       /* Mark H3 object realized */
> >      >       object_property_set_bool(OBJECT(s->h3), true, "realized",
> >     &error_abort);
> >      >       if (error_abort != NULL) {
> >      > diff --git a/hw/misc/allwinner-h3-sid.c
> b/hw/misc/allwinner-h3-sid.c
> >      > new file mode 100644
> >      > index 0000000000..c472f2bcc6
> >      > --- /dev/null
> >      > +++ b/hw/misc/allwinner-h3-sid.c
> >      > @@ -0,0 +1,179 @@
> >      > +/*
> >      > + * Allwinner H3 Security ID emulation
> >      > + *
> >      > + * Copyright (C) 2019 Niek Linnenbank <nieklinnenbank@gmail.com
> >     <mailto:nieklinnenbank@gmail.com>>
> >      > + *
> >      > + * This program is free software: you can redistribute it and/or
> >     modify
> >      > + * it under the terms of the GNU General Public License as
> >     published by
> >      > + * the Free Software Foundation, either version 2 of the
> License, or
> >      > + * (at your option) any later version.
> >      > + *
> >      > + * This program is distributed in the hope that it will be
> useful,
> >      > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >      > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >      > + * GNU General Public License for more details.
> >      > + *
> >      > + * You should have received a copy of the GNU General Public
> License
> >      > + * along with this program.  If not, see
> >     <http://www.gnu.org/licenses/>.
> >      > + */
> >      > +
> >      > +#include "qemu/osdep.h"
> >      > +#include "qemu/units.h"
> >      > +#include "hw/sysbus.h"
> >      > +#include "migration/vmstate.h"
> >      > +#include "qemu/log.h"
> >      > +#include "qemu/module.h"
> >      > +#include "qemu/guest-random.h"
> >      > +#include "qapi/error.h"
> >      > +#include "hw/qdev-properties.h"
> >      > +#include "hw/misc/allwinner-h3-sid.h"
> >      > +#include "trace.h"
> >      > +
> >      > +/* SID register offsets */
> >      > +enum {
> >      > +    REG_PRCTL = 0x40,   /* Control */
> >      > +    REG_RDKEY = 0x60,   /* Read Key */
> >      > +};
> >      > +
> >      > +/* SID register flags */
> >      > +enum {
> >      > +    REG_PRCTL_WRITE   = 0x0002, /* Unknown write flag */
> >      > +    REG_PRCTL_OP_LOCK = 0xAC00, /* Lock operation */
> >      > +};
> >      > +
> >      > +static uint64_t allwinner_h3_sid_read(void *opaque, hwaddr
> offset,
> >      > +                                      unsigned size)
> >      > +{
> >      > +    const AwH3SidState *s = (AwH3SidState *)opaque;
> >      > +    uint64_t val = 0;
> >      > +
> >      > +    switch (offset) {
> >      > +    case REG_PRCTL:    /* Control */
> >      > +        val = s->control;
> >      > +        break;
> >      > +    case REG_RDKEY:    /* Read Key */
> >      > +        val = s->rdkey;
> >      > +        break;
> >      > +    default:
> >      > +        qemu_log_mask(LOG_GUEST_ERROR, "%s: bad read offset
> >     0x%04x\n",
> >      > +                      __func__, (uint32_t)offset);
> >      > +        return 0;
> >      > +    }
> >      > +
> >      > +    trace_allwinner_h3_sid_read(offset, val, size);
> >      > +
> >      > +    return val;
> >      > +}
> >      > +
> >      > +static void allwinner_h3_sid_write(void *opaque, hwaddr offset,
> >      > +                                   uint64_t val, unsigned size)
> >      > +{
> >      > +    AwH3SidState *s = (AwH3SidState *)opaque;
> >      > +
> >      > +    trace_allwinner_h3_sid_write(offset, val, size);
> >      > +
> >      > +    switch (offset) {
> >      > +    case REG_PRCTL:    /* Control */
> >      > +        s->control = val;
> >      > +
> >      > +        if ((s->control & REG_PRCTL_OP_LOCK) &&
> >      > +            (s->control & REG_PRCTL_WRITE)) {
> >      > +            uint32_t id = s->control >> 16;
> >      > +
> >      > +            if (id < sizeof(QemuUUID)) {
> >      > +                s->rdkey = (s->identifier.data[id]) |
> >      > +                           (s->identifier.data[id + 1] << 8) |
> >      > +                           (s->identifier.data[id + 2] << 16) |
> >      > +                           (s->identifier.data[id + 3] << 24);
> >
> >     This is:
> >
> >                          s->rdkey = ldl_le_p(&s->identifier.data[id]);
> >
> >      > +            }
> >      > +        }
> >      > +        s->control &= ~REG_PRCTL_WRITE;
> >      > +        break;
> >      > +    case REG_RDKEY:    /* Read Key */
> >
> >     Read in a write()?
> >
> >     Maybe we can simply /* fall through */ LOG_GUEST_ERROR?
> >
> >
> > When writing this module, I looked at how U-Boot is using the SID
> > registers and simply
> > named the registers after the names used by U-Boot. You can find this
> > part in arch/arm/mach-sunxi/cpu_info.c:111,
> > functions sun8i_efuse_read() and sunxi_get_sid(). U-Boot defines
> > SIDC_RDKEY, so I named the register also rdkey.
> > I used the U-Boot source because the Allwinner H3 datasheet does not
> > document the registers. Later I
> > found the SID page on the linux-sunxi wiki that I mentioned earlier, and
> > they also describe the same register names:
> >
> > https://linux-sunxi.org/SID_Register_Guide
>
> Hmm this page describe this register as RW, odd. I think this is wrong
> because we deal with a fuse, and we program it via the REG_PRKEY
> register. Does Linux/U-Boot do write access to this register?
> We can start logging LOG_GUEST_ERROR, and correct it if we notice this
> register is really writable (which I doubt).
>
>
Yes it seems so. The PRCTL is the "Control" register (R/W) and the RDKEY
the "Data" register (RO).
You can see the Linux implementation in drivers/nvmem/sunxi_sid.c
For U-Boot the file is in arch/arm/mach-sunxi/cpu_info.c, functions
sunxi_get_sid and sun8i_efuse_read.

Regards,
Niek


> >
> > I suspect the information on this page is written based on the source
> > code from the original SDK (which I did not study btw)
> [...]
>
>
diff mbox series

Patch

diff --git a/include/hw/arm/allwinner-h3.h b/include/hw/arm/allwinner-h3.h
index 8128ae6131..c98c1972a6 100644
--- a/include/hw/arm/allwinner-h3.h
+++ b/include/hw/arm/allwinner-h3.h
@@ -29,6 +29,7 @@ 
 #include "hw/misc/allwinner-h3-clk.h"
 #include "hw/misc/allwinner-h3-cpucfg.h"
 #include "hw/misc/allwinner-h3-syscon.h"
+#include "hw/misc/allwinner-h3-sid.h"
 #include "target/arm/cpu.h"
 
 enum {
@@ -77,6 +78,7 @@  typedef struct AwH3State {
     AwH3ClockState ccu;
     AwH3CpuCfgState cpucfg;
     AwH3SysconState syscon;
+    AwH3SidState sid;
     GICState gic;
     MemoryRegion sram_a1;
     MemoryRegion sram_a2;
diff --git a/include/hw/misc/allwinner-h3-sid.h b/include/hw/misc/allwinner-h3-sid.h
new file mode 100644
index 0000000000..79c9a24459
--- /dev/null
+++ b/include/hw/misc/allwinner-h3-sid.h
@@ -0,0 +1,40 @@ 
+/*
+ * Allwinner H3 Security ID emulation
+ *
+ * Copyright (C) 2019 Niek Linnenbank <nieklinnenbank@gmail.com>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef HW_MISC_ALLWINNER_H3_SID_H
+#define HW_MISC_ALLWINNER_H3_SID_H
+
+#include "hw/sysbus.h"
+#include "qemu/uuid.h"
+
+#define TYPE_AW_H3_SID    "allwinner-h3-sid"
+#define AW_H3_SID(obj)    OBJECT_CHECK(AwH3SidState, (obj), TYPE_AW_H3_SID)
+
+typedef struct AwH3SidState {
+    /*< private >*/
+    SysBusDevice parent_obj;
+    /*< public >*/
+
+    MemoryRegion iomem;
+    uint32_t control;
+    uint32_t rdkey;
+    QemuUUID identifier;
+} AwH3SidState;
+
+#endif
diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c
index 1a9748ab2e..ba34f905cd 100644
--- a/hw/arm/allwinner-h3.c
+++ b/hw/arm/allwinner-h3.c
@@ -196,6 +196,9 @@  static void aw_h3_init(Object *obj)
 
     sysbus_init_child_obj(obj, "cpucfg", &s->cpucfg, sizeof(s->cpucfg),
                           TYPE_AW_H3_CPUCFG);
+
+    sysbus_init_child_obj(obj, "sid", &s->sid, sizeof(s->sid),
+                          TYPE_AW_H3_SID);
 }
 
 static void aw_h3_realize(DeviceState *dev, Error **errp)
@@ -332,6 +335,10 @@  static void aw_h3_realize(DeviceState *dev, Error **errp)
     qdev_init_nofail(DEVICE(&s->cpucfg));
     sysbus_mmio_map(SYS_BUS_DEVICE(&s->cpucfg), 0, s->memmap[AW_H3_CPUCFG]);
 
+    /* Security Identifier */
+    qdev_init_nofail(DEVICE(&s->sid));
+    sysbus_mmio_map(SYS_BUS_DEVICE(&s->sid), 0, s->memmap[AW_H3_SID]);
+
     /* Universal Serial Bus */
     sysbus_create_simple(TYPE_AW_H3_EHCI, s->memmap[AW_H3_EHCI0],
                          qdev_get_gpio_in(DEVICE(&s->gic),
diff --git a/hw/arm/orangepi.c b/hw/arm/orangepi.c
index 62cefc8c06..b01c4b4f01 100644
--- a/hw/arm/orangepi.c
+++ b/hw/arm/orangepi.c
@@ -62,6 +62,10 @@  static void orangepi_init(MachineState *machine)
         exit(1);
     }
 
+    /* Setup SID properties */
+    qdev_prop_set_string(DEVICE(&s->h3->sid), "identifier",
+                         "8100c002-0001-0002-0003-000044556677");
+
     /* Mark H3 object realized */
     object_property_set_bool(OBJECT(s->h3), true, "realized", &error_abort);
     if (error_abort != NULL) {
diff --git a/hw/misc/allwinner-h3-sid.c b/hw/misc/allwinner-h3-sid.c
new file mode 100644
index 0000000000..c472f2bcc6
--- /dev/null
+++ b/hw/misc/allwinner-h3-sid.c
@@ -0,0 +1,179 @@ 
+/*
+ * Allwinner H3 Security ID emulation
+ *
+ * Copyright (C) 2019 Niek Linnenbank <nieklinnenbank@gmail.com>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/units.h"
+#include "hw/sysbus.h"
+#include "migration/vmstate.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "qemu/guest-random.h"
+#include "qapi/error.h"
+#include "hw/qdev-properties.h"
+#include "hw/misc/allwinner-h3-sid.h"
+#include "trace.h"
+
+/* SID register offsets */
+enum {
+    REG_PRCTL = 0x40,   /* Control */
+    REG_RDKEY = 0x60,   /* Read Key */
+};
+
+/* SID register flags */
+enum {
+    REG_PRCTL_WRITE   = 0x0002, /* Unknown write flag */
+    REG_PRCTL_OP_LOCK = 0xAC00, /* Lock operation */
+};
+
+static uint64_t allwinner_h3_sid_read(void *opaque, hwaddr offset,
+                                      unsigned size)
+{
+    const AwH3SidState *s = (AwH3SidState *)opaque;
+    uint64_t val = 0;
+
+    switch (offset) {
+    case REG_PRCTL:    /* Control */
+        val = s->control;
+        break;
+    case REG_RDKEY:    /* Read Key */
+        val = s->rdkey;
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: bad read offset 0x%04x\n",
+                      __func__, (uint32_t)offset);
+        return 0;
+    }
+
+    trace_allwinner_h3_sid_read(offset, val, size);
+
+    return val;
+}
+
+static void allwinner_h3_sid_write(void *opaque, hwaddr offset,
+                                   uint64_t val, unsigned size)
+{
+    AwH3SidState *s = (AwH3SidState *)opaque;
+
+    trace_allwinner_h3_sid_write(offset, val, size);
+
+    switch (offset) {
+    case REG_PRCTL:    /* Control */
+        s->control = val;
+
+        if ((s->control & REG_PRCTL_OP_LOCK) &&
+            (s->control & REG_PRCTL_WRITE)) {
+            uint32_t id = s->control >> 16;
+
+            if (id < sizeof(QemuUUID)) {
+                s->rdkey = (s->identifier.data[id]) |
+                           (s->identifier.data[id + 1] << 8) |
+                           (s->identifier.data[id + 2] << 16) |
+                           (s->identifier.data[id + 3] << 24);
+            }
+        }
+        s->control &= ~REG_PRCTL_WRITE;
+        break;
+    case REG_RDKEY:    /* Read Key */
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: bad write offset 0x%04x\n",
+                      __func__, (uint32_t)offset);
+        break;
+    }
+}
+
+static const MemoryRegionOps allwinner_h3_sid_ops = {
+    .read = allwinner_h3_sid_read,
+    .write = allwinner_h3_sid_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+        .unaligned = false
+    },
+    .impl.min_access_size = 4,
+};
+
+static void allwinner_h3_sid_reset(DeviceState *dev)
+{
+    AwH3SidState *s = AW_H3_SID(dev);
+
+    /* Set default values for registers */
+    s->control = 0;
+    s->rdkey = 0;
+}
+
+static void allwinner_h3_sid_realize(DeviceState *dev, Error **errp)
+{
+}
+
+static void allwinner_h3_sid_init(Object *obj)
+{
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+    AwH3SidState *s = AW_H3_SID(obj);
+
+    /* Fill UUID with zeroes by default */
+    qemu_uuid_parse(UUID_NONE, &s->identifier);
+
+    /* Memory mapping */
+    memory_region_init_io(&s->iomem, OBJECT(s), &allwinner_h3_sid_ops, s,
+                          TYPE_AW_H3_SID, 1 * KiB);
+    sysbus_init_mmio(sbd, &s->iomem);
+}
+
+static Property allwinner_h3_sid_properties[] = {
+    DEFINE_PROP_UUID_NODEFAULT("identifier", AwH3SidState, identifier),
+    DEFINE_PROP_END_OF_LIST()
+};
+
+static const VMStateDescription allwinner_h3_sid_vmstate = {
+    .name = "allwinner-h3-sid",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(control, AwH3SidState),
+        VMSTATE_UINT32(rdkey, AwH3SidState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void allwinner_h3_sid_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->reset = allwinner_h3_sid_reset;
+    dc->realize = allwinner_h3_sid_realize;
+    dc->vmsd = &allwinner_h3_sid_vmstate;
+    dc->props = allwinner_h3_sid_properties;
+}
+
+static const TypeInfo allwinner_h3_sid_info = {
+    .name          = TYPE_AW_H3_SID,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_init = allwinner_h3_sid_init,
+    .instance_size = sizeof(AwH3SidState),
+    .class_init    = allwinner_h3_sid_class_init,
+};
+
+static void allwinner_h3_sid_register(void)
+{
+    type_register_static(&allwinner_h3_sid_info);
+}
+
+type_init(allwinner_h3_sid_register)
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index c4ca2ed689..f3620eee4e 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -31,6 +31,7 @@  common-obj-$(CONFIG_IVSHMEM_DEVICE) += ivshmem.o
 common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3-clk.o
 obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3-cpucfg.o
 common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3-syscon.o
+common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3-sid.o
 common-obj-$(CONFIG_REALVIEW) += arm_sysctl.o
 common-obj-$(CONFIG_NSERIES) += cbus.o
 common-obj-$(CONFIG_ECCMEMCTL) += eccmemctl.o
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index b93089d010..a777844ca3 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -5,6 +5,10 @@  allwinner_h3_cpucfg_cpu_reset(uint8_t cpu_id, uint32_t reset_addr) "H3-CPUCFG: c
 allwinner_h3_cpucfg_read(uint64_t offset, uint64_t data, unsigned size) "H3-CPUCFG: read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32
 allwinner_h3_cpucfg_write(uint64_t offset, uint64_t data, unsigned size) "H3-CPUCFG: write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32
 
+# allwinner-h3-sid.c
+allwinner_h3_sid_read(uint64_t offset, uint64_t data, unsigned size) "H3-SID: read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32
+allwinner_h3_sid_write(uint64_t offset, uint64_t data, unsigned size) "H3-SID: write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32
+
 # eccmemctl.c
 ecc_mem_writel_mer(uint32_t val) "Write memory enable 0x%08x"
 ecc_mem_writel_mdr(uint32_t val) "Write memory delay 0x%08x"