| Message ID | 20200108200020.4745-8-nieklinnenbank@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add Allwinner H3 SoC and Orange Pi PC Machine | expand |
Cc'ing Corey/David for good advices about using UUID. On 1/8/20 9:00 PM, Niek Linnenbank wrote: > The Security Identifier device found in various Allwinner System on Chip > designs gives applications a per-board unique identifier. This commit > adds support for the Allwinner Security Identifier using a 128-bit > UUID value as input. > > Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com> > --- > include/hw/arm/allwinner-h3.h | 3 + > include/hw/misc/allwinner-sid.h | 61 ++++++++++++ > hw/arm/allwinner-h3.c | 11 ++- > hw/arm/orangepi.c | 4 + > hw/misc/allwinner-sid.c | 170 ++++++++++++++++++++++++++++++++ > hw/misc/Makefile.objs | 1 + > hw/misc/trace-events | 4 + > 7 files changed, 253 insertions(+), 1 deletion(-) > create mode 100644 include/hw/misc/allwinner-sid.h > create mode 100644 hw/misc/allwinner-sid.c > > diff --git a/include/hw/arm/allwinner-h3.h b/include/hw/arm/allwinner-h3.h > index 5a25a92eae..9ed365507c 100644 > --- a/include/hw/arm/allwinner-h3.h > +++ b/include/hw/arm/allwinner-h3.h > @@ -46,6 +46,7 @@ > #include "hw/misc/allwinner-h3-ccu.h" > #include "hw/misc/allwinner-cpucfg.h" > #include "hw/misc/allwinner-h3-sysctrl.h" > +#include "hw/misc/allwinner-sid.h" > #include "target/arm/cpu.h" > > /** > @@ -63,6 +64,7 @@ enum { > AW_H3_SRAM_A2, > AW_H3_SRAM_C, > AW_H3_SYSCTRL, > + AW_H3_SID, > AW_H3_EHCI0, > AW_H3_OHCI0, > AW_H3_EHCI1, > @@ -115,6 +117,7 @@ typedef struct AwH3State { > AwH3ClockCtlState ccu; > AwCpuCfgState cpucfg; > AwH3SysCtrlState sysctrl; > + AwSidState sid; > GICState gic; > MemoryRegion sram_a1; > MemoryRegion sram_a2; > diff --git a/include/hw/misc/allwinner-sid.h b/include/hw/misc/allwinner-sid.h > new file mode 100644 > index 0000000000..41189967e2 > --- /dev/null > +++ b/include/hw/misc/allwinner-sid.h > @@ -0,0 +1,61 @@ > +/* > + * Allwinner 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_SID_H > +#define HW_MISC_ALLWINNER_SID_H > + > +#include "qemu/osdep.h" > +#include "qom/object.h" > +#include "hw/sysbus.h" > +#include "qemu/uuid.h" > + > +/** > + * Object model > + * @{ > + */ > + > +#define TYPE_AW_SID "allwinner-sid" > +#define AW_SID(obj) \ > + OBJECT_CHECK(AwSidState, (obj), TYPE_AW_SID) > + > +/** @} */ > + > +/** > + * Allwinner Security ID object instance state > + */ > +typedef struct AwSidState { > + /*< private >*/ > + SysBusDevice parent_obj; > + /*< public >*/ > + > + /** Maps I/O registers in physical memory */ > + MemoryRegion iomem; > + > + /** Control register defines how and what to read */ > + uint32_t control; > + > + /** RdKey register contains the data retrieved by the device */ > + uint32_t rdkey; > + > + /** Stores the emulated device identifier */ > + QemuUUID identifier; > + > +} AwSidState; > + > +#endif /* HW_MISC_ALLWINNER_SID_H */ > diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c > index e9ad6d23df..af7317e86a 100644 > --- a/hw/arm/allwinner-h3.c > +++ b/hw/arm/allwinner-h3.c > @@ -36,6 +36,7 @@ const hwaddr allwinner_h3_memmap[] = { > [AW_H3_SRAM_A2] = 0x00044000, > [AW_H3_SRAM_C] = 0x00010000, > [AW_H3_SYSCTRL] = 0x01c00000, > + [AW_H3_SID] = 0x01c14000, > [AW_H3_EHCI0] = 0x01c1a000, > [AW_H3_OHCI0] = 0x01c1a400, > [AW_H3_EHCI1] = 0x01c1b000, > @@ -76,7 +77,6 @@ struct AwH3Unimplemented { > { "mmc0", 0x01c0f000, 4 * KiB }, > { "mmc1", 0x01c10000, 4 * KiB }, > { "mmc2", 0x01c11000, 4 * KiB }, > - { "sid", 0x01c14000, 1 * KiB }, > { "crypto", 0x01c15000, 4 * KiB }, > { "msgbox", 0x01c17000, 4 * KiB }, > { "spinlock", 0x01c18000, 4 * KiB }, > @@ -196,6 +196,11 @@ static void allwinner_h3_init(Object *obj) > > sysbus_init_child_obj(obj, "cpucfg", &s->cpucfg, sizeof(s->cpucfg), > TYPE_AW_CPUCFG); > + > + sysbus_init_child_obj(obj, "sid", &s->sid, sizeof(s->sid), > + TYPE_AW_SID); > + object_property_add_alias(obj, "identifier", OBJECT(&s->sid), > + "identifier", &error_abort); > } > > static void allwinner_h3_realize(DeviceState *dev, Error **errp) > @@ -316,6 +321,10 @@ static void allwinner_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 051184f14f..a7f870c88b 100644 > --- a/hw/arm/orangepi.c > +++ b/hw/arm/orangepi.c > @@ -54,6 +54,10 @@ static void orangepi_init(MachineState *machine) > object_property_set_int(OBJECT(s->h3), 24000000, "clk1-freq", > &error_abort); > > + /* Setup SID properties */ > + qdev_prop_set_string(DEVICE(s->h3), "identifier", > + "8100c002-0001-0002-0003-000044556677"); > + > /* Mark H3 object realized */ > object_property_set_bool(OBJECT(s->h3), true, "realized", &error_abort); > > diff --git a/hw/misc/allwinner-sid.c b/hw/misc/allwinner-sid.c > new file mode 100644 > index 0000000000..954de935bc > --- /dev/null > +++ b/hw/misc/allwinner-sid.c > @@ -0,0 +1,170 @@ > +/* > + * Allwinner 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-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_sid_read(void *opaque, hwaddr offset, > + unsigned size) > +{ > + const AwSidState *s = AW_SID(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: out-of-bounds offset 0x%04x\n", > + __func__, (uint32_t)offset); > + return 0; > + } > + > + trace_allwinner_sid_read(offset, val, size); > + > + return val; > +} > + > +static void allwinner_sid_write(void *opaque, hwaddr offset, > + uint64_t val, unsigned size) > +{ > + AwSidState *s = AW_SID(opaque); > + > + trace_allwinner_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: out-of-bounds offset 0x%04x\n", > + __func__, (uint32_t)offset); > + break; > + } > +} > + > +static const MemoryRegionOps allwinner_sid_ops = { > + .read = allwinner_sid_read, > + .write = allwinner_sid_write, > + .endianness = DEVICE_NATIVE_ENDIAN, > + .valid = { > + .min_access_size = 4, > + .max_access_size = 4, > + }, > + .impl.min_access_size = 4, > +}; > + > +static void allwinner_sid_reset(DeviceState *dev) > +{ > + AwSidState *s = AW_SID(dev); > + > + /* Set default values for registers */ > + s->control = 0; > + s->rdkey = 0; > +} > + > +static void allwinner_sid_init(Object *obj) > +{ > + SysBusDevice *sbd = SYS_BUS_DEVICE(obj); > + AwSidState *s = AW_SID(obj); > + > + /* Memory mapping */ > + memory_region_init_io(&s->iomem, OBJECT(s), &allwinner_sid_ops, s, > + TYPE_AW_SID, 1 * KiB); > + sysbus_init_mmio(sbd, &s->iomem); > +} > + > +static Property allwinner_sid_properties[] = { > + DEFINE_PROP_UUID_NODEFAULT("identifier", AwSidState, identifier), > + DEFINE_PROP_END_OF_LIST() > +}; > + > +static const VMStateDescription allwinner_sid_vmstate = { > + .name = "allwinner-sid", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_UINT32(control, AwSidState), > + VMSTATE_UINT32(rdkey, AwSidState), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static void allwinner_sid_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + > + dc->reset = allwinner_sid_reset; > + dc->vmsd = &allwinner_sid_vmstate; > + dc->props = allwinner_sid_properties; > +} > + > +static const TypeInfo allwinner_sid_info = { > + .name = TYPE_AW_SID, > + .parent = TYPE_SYS_BUS_DEVICE, > + .instance_init = allwinner_sid_init, > + .instance_size = sizeof(AwSidState), > + .class_init = allwinner_sid_class_init, > +}; > + > +static void allwinner_sid_register(void) > +{ > + type_register_static(&allwinner_sid_info); > +} > + > +type_init(allwinner_sid_register) > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs > index 12c2c306b5..59500d5681 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-ccu.o > obj-$(CONFIG_ALLWINNER_H3) += allwinner-cpucfg.o > common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3-sysctrl.o > +common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-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 d3e0952429..67d8bf493c 100644 > --- a/hw/misc/trace-events > +++ b/hw/misc/trace-events > @@ -5,6 +5,10 @@ allwinner_cpucfg_cpu_reset(uint8_t cpu_id, uint32_t reset_addr) "id %u, reset_ad > allwinner_cpucfg_read(uint64_t offset, uint64_t data, unsigned size) "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32 > allwinner_cpucfg_write(uint64_t offset, uint64_t data, unsigned size) "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32 > > +# allwinner-sid.c > +allwinner_sid_read(uint64_t offset, uint64_t data, unsigned size) "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32 > +allwinner_sid_write(uint64_t offset, uint64_t data, unsigned size) "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" >
On Sat, Jan 18, 2020 at 04:25:08PM +0100, Philippe Mathieu-Daudé wrote: > Cc'ing Corey/David for good advices about using UUID. Is there any reason you didn't use the built-in qemu UUID for this? It would simplify things in general. Also, in case no one else say, you have tabs in your code that you need to get rid of. -corey > > On 1/8/20 9:00 PM, Niek Linnenbank wrote: > > The Security Identifier device found in various Allwinner System on Chip > > designs gives applications a per-board unique identifier. This commit > > adds support for the Allwinner Security Identifier using a 128-bit > > UUID value as input. > > > > Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com> > > --- > > include/hw/arm/allwinner-h3.h | 3 + > > include/hw/misc/allwinner-sid.h | 61 ++++++++++++ > > hw/arm/allwinner-h3.c | 11 ++- > > hw/arm/orangepi.c | 4 + > > hw/misc/allwinner-sid.c | 170 ++++++++++++++++++++++++++++++++ > > hw/misc/Makefile.objs | 1 + > > hw/misc/trace-events | 4 + > > 7 files changed, 253 insertions(+), 1 deletion(-) > > create mode 100644 include/hw/misc/allwinner-sid.h > > create mode 100644 hw/misc/allwinner-sid.c > > > > diff --git a/include/hw/arm/allwinner-h3.h b/include/hw/arm/allwinner-h3.h > > index 5a25a92eae..9ed365507c 100644 > > --- a/include/hw/arm/allwinner-h3.h > > +++ b/include/hw/arm/allwinner-h3.h > > @@ -46,6 +46,7 @@ > > #include "hw/misc/allwinner-h3-ccu.h" > > #include "hw/misc/allwinner-cpucfg.h" > > #include "hw/misc/allwinner-h3-sysctrl.h" > > +#include "hw/misc/allwinner-sid.h" > > #include "target/arm/cpu.h" > > /** > > @@ -63,6 +64,7 @@ enum { > > AW_H3_SRAM_A2, > > AW_H3_SRAM_C, > > AW_H3_SYSCTRL, > > + AW_H3_SID, > > AW_H3_EHCI0, > > AW_H3_OHCI0, > > AW_H3_EHCI1, > > @@ -115,6 +117,7 @@ typedef struct AwH3State { > > AwH3ClockCtlState ccu; > > AwCpuCfgState cpucfg; > > AwH3SysCtrlState sysctrl; > > + AwSidState sid; > > GICState gic; > > MemoryRegion sram_a1; > > MemoryRegion sram_a2; > > diff --git a/include/hw/misc/allwinner-sid.h b/include/hw/misc/allwinner-sid.h > > new file mode 100644 > > index 0000000000..41189967e2 > > --- /dev/null > > +++ b/include/hw/misc/allwinner-sid.h > > @@ -0,0 +1,61 @@ > > +/* > > + * Allwinner 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_SID_H > > +#define HW_MISC_ALLWINNER_SID_H > > + > > +#include "qemu/osdep.h" > > +#include "qom/object.h" > > +#include "hw/sysbus.h" > > +#include "qemu/uuid.h" > > + > > +/** > > + * Object model > > + * @{ > > + */ > > + > > +#define TYPE_AW_SID "allwinner-sid" > > +#define AW_SID(obj) \ > > + OBJECT_CHECK(AwSidState, (obj), TYPE_AW_SID) > > + > > +/** @} */ > > + > > +/** > > + * Allwinner Security ID object instance state > > + */ > > +typedef struct AwSidState { > > + /*< private >*/ > > + SysBusDevice parent_obj; > > + /*< public >*/ > > + > > + /** Maps I/O registers in physical memory */ > > + MemoryRegion iomem; > > + > > + /** Control register defines how and what to read */ > > + uint32_t control; > > + > > + /** RdKey register contains the data retrieved by the device */ > > + uint32_t rdkey; > > + > > + /** Stores the emulated device identifier */ > > + QemuUUID identifier; > > + > > +} AwSidState; > > + > > +#endif /* HW_MISC_ALLWINNER_SID_H */ > > diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c > > index e9ad6d23df..af7317e86a 100644 > > --- a/hw/arm/allwinner-h3.c > > +++ b/hw/arm/allwinner-h3.c > > @@ -36,6 +36,7 @@ const hwaddr allwinner_h3_memmap[] = { > > [AW_H3_SRAM_A2] = 0x00044000, > > [AW_H3_SRAM_C] = 0x00010000, > > [AW_H3_SYSCTRL] = 0x01c00000, > > + [AW_H3_SID] = 0x01c14000, > > [AW_H3_EHCI0] = 0x01c1a000, > > [AW_H3_OHCI0] = 0x01c1a400, > > [AW_H3_EHCI1] = 0x01c1b000, > > @@ -76,7 +77,6 @@ struct AwH3Unimplemented { > > { "mmc0", 0x01c0f000, 4 * KiB }, > > { "mmc1", 0x01c10000, 4 * KiB }, > > { "mmc2", 0x01c11000, 4 * KiB }, > > - { "sid", 0x01c14000, 1 * KiB }, > > { "crypto", 0x01c15000, 4 * KiB }, > > { "msgbox", 0x01c17000, 4 * KiB }, > > { "spinlock", 0x01c18000, 4 * KiB }, > > @@ -196,6 +196,11 @@ static void allwinner_h3_init(Object *obj) > > sysbus_init_child_obj(obj, "cpucfg", &s->cpucfg, sizeof(s->cpucfg), > > TYPE_AW_CPUCFG); > > + > > + sysbus_init_child_obj(obj, "sid", &s->sid, sizeof(s->sid), > > + TYPE_AW_SID); > > + object_property_add_alias(obj, "identifier", OBJECT(&s->sid), > > + "identifier", &error_abort); > > } > > static void allwinner_h3_realize(DeviceState *dev, Error **errp) > > @@ -316,6 +321,10 @@ static void allwinner_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 051184f14f..a7f870c88b 100644 > > --- a/hw/arm/orangepi.c > > +++ b/hw/arm/orangepi.c > > @@ -54,6 +54,10 @@ static void orangepi_init(MachineState *machine) > > object_property_set_int(OBJECT(s->h3), 24000000, "clk1-freq", > > &error_abort); > > + /* Setup SID properties */ > > + qdev_prop_set_string(DEVICE(s->h3), "identifier", > > + "8100c002-0001-0002-0003-000044556677"); > > + > > /* Mark H3 object realized */ > > object_property_set_bool(OBJECT(s->h3), true, "realized", &error_abort); > > diff --git a/hw/misc/allwinner-sid.c b/hw/misc/allwinner-sid.c > > new file mode 100644 > > index 0000000000..954de935bc > > --- /dev/null > > +++ b/hw/misc/allwinner-sid.c > > @@ -0,0 +1,170 @@ > > +/* > > + * Allwinner 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-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_sid_read(void *opaque, hwaddr offset, > > + unsigned size) > > +{ > > + const AwSidState *s = AW_SID(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: out-of-bounds offset 0x%04x\n", > > + __func__, (uint32_t)offset); > > + return 0; > > + } > > + > > + trace_allwinner_sid_read(offset, val, size); > > + > > + return val; > > +} > > + > > +static void allwinner_sid_write(void *opaque, hwaddr offset, > > + uint64_t val, unsigned size) > > +{ > > + AwSidState *s = AW_SID(opaque); > > + > > + trace_allwinner_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: out-of-bounds offset 0x%04x\n", > > + __func__, (uint32_t)offset); > > + break; > > + } > > +} > > + > > +static const MemoryRegionOps allwinner_sid_ops = { > > + .read = allwinner_sid_read, > > + .write = allwinner_sid_write, > > + .endianness = DEVICE_NATIVE_ENDIAN, > > + .valid = { > > + .min_access_size = 4, > > + .max_access_size = 4, > > + }, > > + .impl.min_access_size = 4, > > +}; > > + > > +static void allwinner_sid_reset(DeviceState *dev) > > +{ > > + AwSidState *s = AW_SID(dev); > > + > > + /* Set default values for registers */ > > + s->control = 0; > > + s->rdkey = 0; > > +} > > + > > +static void allwinner_sid_init(Object *obj) > > +{ > > + SysBusDevice *sbd = SYS_BUS_DEVICE(obj); > > + AwSidState *s = AW_SID(obj); > > + > > + /* Memory mapping */ > > + memory_region_init_io(&s->iomem, OBJECT(s), &allwinner_sid_ops, s, > > + TYPE_AW_SID, 1 * KiB); > > + sysbus_init_mmio(sbd, &s->iomem); > > +} > > + > > +static Property allwinner_sid_properties[] = { > > + DEFINE_PROP_UUID_NODEFAULT("identifier", AwSidState, identifier), > > + DEFINE_PROP_END_OF_LIST() > > +}; > > + > > +static const VMStateDescription allwinner_sid_vmstate = { > > + .name = "allwinner-sid", > > + .version_id = 1, > > + .minimum_version_id = 1, > > + .fields = (VMStateField[]) { > > + VMSTATE_UINT32(control, AwSidState), > > + VMSTATE_UINT32(rdkey, AwSidState), > > + VMSTATE_END_OF_LIST() > > + } > > +}; > > + > > +static void allwinner_sid_class_init(ObjectClass *klass, void *data) > > +{ > > + DeviceClass *dc = DEVICE_CLASS(klass); > > + > > + dc->reset = allwinner_sid_reset; > > + dc->vmsd = &allwinner_sid_vmstate; > > + dc->props = allwinner_sid_properties; > > +} > > + > > +static const TypeInfo allwinner_sid_info = { > > + .name = TYPE_AW_SID, > > + .parent = TYPE_SYS_BUS_DEVICE, > > + .instance_init = allwinner_sid_init, > > + .instance_size = sizeof(AwSidState), > > + .class_init = allwinner_sid_class_init, > > +}; > > + > > +static void allwinner_sid_register(void) > > +{ > > + type_register_static(&allwinner_sid_info); > > +} > > + > > +type_init(allwinner_sid_register) > > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs > > index 12c2c306b5..59500d5681 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-ccu.o > > obj-$(CONFIG_ALLWINNER_H3) += allwinner-cpucfg.o > > common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3-sysctrl.o > > +common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-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 d3e0952429..67d8bf493c 100644 > > --- a/hw/misc/trace-events > > +++ b/hw/misc/trace-events > > @@ -5,6 +5,10 @@ allwinner_cpucfg_cpu_reset(uint8_t cpu_id, uint32_t reset_addr) "id %u, reset_ad > > allwinner_cpucfg_read(uint64_t offset, uint64_t data, unsigned size) "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32 > > allwinner_cpucfg_write(uint64_t offset, uint64_t data, unsigned size) "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32 > > +# allwinner-sid.c > > +allwinner_sid_read(uint64_t offset, uint64_t data, unsigned size) "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32 > > +allwinner_sid_write(uint64_t offset, uint64_t data, unsigned size) "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" > > >
Hi Corey, Thanks for reviewing! On Mon, Jan 20, 2020 at 6:59 PM Corey Minyard <cminyard@mvista.com> wrote: > On Sat, Jan 18, 2020 at 04:25:08PM +0100, Philippe Mathieu-Daudé wrote: > > Cc'ing Corey/David for good advices about using UUID. > > Is there any reason you didn't use the built-in qemu UUID for this? It > would simplify things in general. > Currently the Allwinner SID device is using the QemuUUID type from include/qemu/uuid.h. Is that the build-in UUID you are referring to or should I use something else? > Also, in case no one else say, you have tabs in your code that you need > to get rid of. > > If there are any tabs in the code, it was not intended. I re-checked this patch and others again but found no tabs in the code. Could you please point out where you found the extra tabs? Regards, Niek > -corey > > > > > On 1/8/20 9:00 PM, Niek Linnenbank wrote: > > > The Security Identifier device found in various Allwinner System on > Chip > > > designs gives applications a per-board unique identifier. This commit > > > adds support for the Allwinner Security Identifier using a 128-bit > > > UUID value as input. > > > > > > Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com> > > > --- > > > include/hw/arm/allwinner-h3.h | 3 + > > > include/hw/misc/allwinner-sid.h | 61 ++++++++++++ > > > hw/arm/allwinner-h3.c | 11 ++- > > > hw/arm/orangepi.c | 4 + > > > hw/misc/allwinner-sid.c | 170 > ++++++++++++++++++++++++++++++++ > > > hw/misc/Makefile.objs | 1 + > > > hw/misc/trace-events | 4 + > > > 7 files changed, 253 insertions(+), 1 deletion(-) > > > create mode 100644 include/hw/misc/allwinner-sid.h > > > create mode 100644 hw/misc/allwinner-sid.c > > > > > > diff --git a/include/hw/arm/allwinner-h3.h > b/include/hw/arm/allwinner-h3.h > > > index 5a25a92eae..9ed365507c 100644 > > > --- a/include/hw/arm/allwinner-h3.h > > > +++ b/include/hw/arm/allwinner-h3.h > > > @@ -46,6 +46,7 @@ > > > #include "hw/misc/allwinner-h3-ccu.h" > > > #include "hw/misc/allwinner-cpucfg.h" > > > #include "hw/misc/allwinner-h3-sysctrl.h" > > > +#include "hw/misc/allwinner-sid.h" > > > #include "target/arm/cpu.h" > > > /** > > > @@ -63,6 +64,7 @@ enum { > > > AW_H3_SRAM_A2, > > > AW_H3_SRAM_C, > > > AW_H3_SYSCTRL, > > > + AW_H3_SID, > > > AW_H3_EHCI0, > > > AW_H3_OHCI0, > > > AW_H3_EHCI1, > > > @@ -115,6 +117,7 @@ typedef struct AwH3State { > > > AwH3ClockCtlState ccu; > > > AwCpuCfgState cpucfg; > > > AwH3SysCtrlState sysctrl; > > > + AwSidState sid; > > > GICState gic; > > > MemoryRegion sram_a1; > > > MemoryRegion sram_a2; > > > diff --git a/include/hw/misc/allwinner-sid.h > b/include/hw/misc/allwinner-sid.h > > > new file mode 100644 > > > index 0000000000..41189967e2 > > > --- /dev/null > > > +++ b/include/hw/misc/allwinner-sid.h > > > @@ -0,0 +1,61 @@ > > > +/* > > > + * Allwinner 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_SID_H > > > +#define HW_MISC_ALLWINNER_SID_H > > > + > > > +#include "qemu/osdep.h" > > > +#include "qom/object.h" > > > +#include "hw/sysbus.h" > > > +#include "qemu/uuid.h" > > > + > > > +/** > > > + * Object model > > > + * @{ > > > + */ > > > + > > > +#define TYPE_AW_SID "allwinner-sid" > > > +#define AW_SID(obj) \ > > > + OBJECT_CHECK(AwSidState, (obj), TYPE_AW_SID) > > > + > > > +/** @} */ > > > + > > > +/** > > > + * Allwinner Security ID object instance state > > > + */ > > > +typedef struct AwSidState { > > > + /*< private >*/ > > > + SysBusDevice parent_obj; > > > + /*< public >*/ > > > + > > > + /** Maps I/O registers in physical memory */ > > > + MemoryRegion iomem; > > > + > > > + /** Control register defines how and what to read */ > > > + uint32_t control; > > > + > > > + /** RdKey register contains the data retrieved by the device */ > > > + uint32_t rdkey; > > > + > > > + /** Stores the emulated device identifier */ > > > + QemuUUID identifier; > > > + > > > +} AwSidState; > > > + > > > +#endif /* HW_MISC_ALLWINNER_SID_H */ > > > diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c > > > index e9ad6d23df..af7317e86a 100644 > > > --- a/hw/arm/allwinner-h3.c > > > +++ b/hw/arm/allwinner-h3.c > > > @@ -36,6 +36,7 @@ const hwaddr allwinner_h3_memmap[] = { > > > [AW_H3_SRAM_A2] = 0x00044000, > > > [AW_H3_SRAM_C] = 0x00010000, > > > [AW_H3_SYSCTRL] = 0x01c00000, > > > + [AW_H3_SID] = 0x01c14000, > > > [AW_H3_EHCI0] = 0x01c1a000, > > > [AW_H3_OHCI0] = 0x01c1a400, > > > [AW_H3_EHCI1] = 0x01c1b000, > > > @@ -76,7 +77,6 @@ struct AwH3Unimplemented { > > > { "mmc0", 0x01c0f000, 4 * KiB }, > > > { "mmc1", 0x01c10000, 4 * KiB }, > > > { "mmc2", 0x01c11000, 4 * KiB }, > > > - { "sid", 0x01c14000, 1 * KiB }, > > > { "crypto", 0x01c15000, 4 * KiB }, > > > { "msgbox", 0x01c17000, 4 * KiB }, > > > { "spinlock", 0x01c18000, 4 * KiB }, > > > @@ -196,6 +196,11 @@ static void allwinner_h3_init(Object *obj) > > > sysbus_init_child_obj(obj, "cpucfg", &s->cpucfg, > sizeof(s->cpucfg), > > > TYPE_AW_CPUCFG); > > > + > > > + sysbus_init_child_obj(obj, "sid", &s->sid, sizeof(s->sid), > > > + TYPE_AW_SID); > > > + object_property_add_alias(obj, "identifier", OBJECT(&s->sid), > > > + "identifier", &error_abort); > > > } > > > static void allwinner_h3_realize(DeviceState *dev, Error **errp) > > > @@ -316,6 +321,10 @@ static void allwinner_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 051184f14f..a7f870c88b 100644 > > > --- a/hw/arm/orangepi.c > > > +++ b/hw/arm/orangepi.c > > > @@ -54,6 +54,10 @@ static void orangepi_init(MachineState *machine) > > > object_property_set_int(OBJECT(s->h3), 24000000, "clk1-freq", > > > &error_abort); > > > + /* Setup SID properties */ > > > + qdev_prop_set_string(DEVICE(s->h3), "identifier", > > > + "8100c002-0001-0002-0003-000044556677"); > > > + > > > /* Mark H3 object realized */ > > > object_property_set_bool(OBJECT(s->h3), true, "realized", > &error_abort); > > > diff --git a/hw/misc/allwinner-sid.c b/hw/misc/allwinner-sid.c > > > new file mode 100644 > > > index 0000000000..954de935bc > > > --- /dev/null > > > +++ b/hw/misc/allwinner-sid.c > > > @@ -0,0 +1,170 @@ > > > +/* > > > + * Allwinner 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-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_sid_read(void *opaque, hwaddr offset, > > > + unsigned size) > > > +{ > > > + const AwSidState *s = AW_SID(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: out-of-bounds offset > 0x%04x\n", > > > + __func__, (uint32_t)offset); > > > + return 0; > > > + } > > > + > > > + trace_allwinner_sid_read(offset, val, size); > > > + > > > + return val; > > > +} > > > + > > > +static void allwinner_sid_write(void *opaque, hwaddr offset, > > > + uint64_t val, unsigned size) > > > +{ > > > + AwSidState *s = AW_SID(opaque); > > > + > > > + trace_allwinner_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: out-of-bounds offset > 0x%04x\n", > > > + __func__, (uint32_t)offset); > > > + break; > > > + } > > > +} > > > + > > > +static const MemoryRegionOps allwinner_sid_ops = { > > > + .read = allwinner_sid_read, > > > + .write = allwinner_sid_write, > > > + .endianness = DEVICE_NATIVE_ENDIAN, > > > + .valid = { > > > + .min_access_size = 4, > > > + .max_access_size = 4, > > > + }, > > > + .impl.min_access_size = 4, > > > +}; > > > + > > > +static void allwinner_sid_reset(DeviceState *dev) > > > +{ > > > + AwSidState *s = AW_SID(dev); > > > + > > > + /* Set default values for registers */ > > > + s->control = 0; > > > + s->rdkey = 0; > > > +} > > > + > > > +static void allwinner_sid_init(Object *obj) > > > +{ > > > + SysBusDevice *sbd = SYS_BUS_DEVICE(obj); > > > + AwSidState *s = AW_SID(obj); > > > + > > > + /* Memory mapping */ > > > + memory_region_init_io(&s->iomem, OBJECT(s), &allwinner_sid_ops, s, > > > + TYPE_AW_SID, 1 * KiB); > > > + sysbus_init_mmio(sbd, &s->iomem); > > > +} > > > + > > > +static Property allwinner_sid_properties[] = { > > > + DEFINE_PROP_UUID_NODEFAULT("identifier", AwSidState, identifier), > > > + DEFINE_PROP_END_OF_LIST() > > > +}; > > > + > > > +static const VMStateDescription allwinner_sid_vmstate = { > > > + .name = "allwinner-sid", > > > + .version_id = 1, > > > + .minimum_version_id = 1, > > > + .fields = (VMStateField[]) { > > > + VMSTATE_UINT32(control, AwSidState), > > > + VMSTATE_UINT32(rdkey, AwSidState), > > > + VMSTATE_END_OF_LIST() > > > + } > > > +}; > > > + > > > +static void allwinner_sid_class_init(ObjectClass *klass, void *data) > > > +{ > > > + DeviceClass *dc = DEVICE_CLASS(klass); > > > + > > > + dc->reset = allwinner_sid_reset; > > > + dc->vmsd = &allwinner_sid_vmstate; > > > + dc->props = allwinner_sid_properties; > > > +} > > > + > > > +static const TypeInfo allwinner_sid_info = { > > > + .name = TYPE_AW_SID, > > > + .parent = TYPE_SYS_BUS_DEVICE, > > > + .instance_init = allwinner_sid_init, > > > + .instance_size = sizeof(AwSidState), > > > + .class_init = allwinner_sid_class_init, > > > +}; > > > + > > > +static void allwinner_sid_register(void) > > > +{ > > > + type_register_static(&allwinner_sid_info); > > > +} > > > + > > > +type_init(allwinner_sid_register) > > > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs > > > index 12c2c306b5..59500d5681 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-ccu.o > > > obj-$(CONFIG_ALLWINNER_H3) += allwinner-cpucfg.o > > > common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3-sysctrl.o > > > +common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-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 d3e0952429..67d8bf493c 100644 > > > --- a/hw/misc/trace-events > > > +++ b/hw/misc/trace-events > > > @@ -5,6 +5,10 @@ allwinner_cpucfg_cpu_reset(uint8_t cpu_id, uint32_t > reset_addr) "id %u, reset_ad > > > allwinner_cpucfg_read(uint64_t offset, uint64_t data, unsigned size) > "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32 > > > allwinner_cpucfg_write(uint64_t offset, uint64_t data, unsigned > size) "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32 > > > +# allwinner-sid.c > > > +allwinner_sid_read(uint64_t offset, uint64_t data, unsigned size) > "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32 > > > +allwinner_sid_write(uint64_t offset, uint64_t data, unsigned size) > "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" > > > > > >
On Sun, Feb 02, 2020 at 10:27:49PM +0100, Niek Linnenbank wrote: > Hi Corey, > > Thanks for reviewing! > > On Mon, Jan 20, 2020 at 6:59 PM Corey Minyard <cminyard@mvista.com> wrote: > > > On Sat, Jan 18, 2020 at 04:25:08PM +0100, Philippe Mathieu-Daudé wrote: > > > Cc'ing Corey/David for good advices about using UUID. > > > > Is there any reason you didn't use the built-in qemu UUID for this? It > > would simplify things in general. > > > > Currently the Allwinner SID device is using the QemuUUID type from > include/qemu/uuid.h. > Is that the build-in UUID you are referring to or should I use something > else? You are using the QemuUUID type, which is of course what you should do, but you aren't using the UUID generated by qemu (at least that I can find). That in include/sysemu/sysemu.h and is named qemu_uuid. Whether you should use that or not depends on your needs. If you just need some uuid, then that's what you should probably use. If you need something the user can individually control for this device, for instance, then that probably won't do. > > > > Also, in case no one else say, you have tabs in your code that you need > > to get rid of. > > > > > If there are any tabs in the code, it was not intended. I re-checked this > patch and others > again but found no tabs in the code. > Could you please point out where you found the extra tabs? My apologies, I saw 1-character misalignments, and that usually means that there's a tab. But it looks like it has something to do with the way it was forwarded. I didn't get the original email. -corey > > Regards, > Niek > > > > -corey > > > > > > > > On 1/8/20 9:00 PM, Niek Linnenbank wrote: > > > > The Security Identifier device found in various Allwinner System on > > Chip > > > > designs gives applications a per-board unique identifier. This commit > > > > adds support for the Allwinner Security Identifier using a 128-bit > > > > UUID value as input. > > > > > > > > Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com> > > > > --- > > > > include/hw/arm/allwinner-h3.h | 3 + > > > > include/hw/misc/allwinner-sid.h | 61 ++++++++++++ > > > > hw/arm/allwinner-h3.c | 11 ++- > > > > hw/arm/orangepi.c | 4 + > > > > hw/misc/allwinner-sid.c | 170 > > ++++++++++++++++++++++++++++++++ > > > > hw/misc/Makefile.objs | 1 + > > > > hw/misc/trace-events | 4 + > > > > 7 files changed, 253 insertions(+), 1 deletion(-) > > > > create mode 100644 include/hw/misc/allwinner-sid.h > > > > create mode 100644 hw/misc/allwinner-sid.c > > > > > > > > diff --git a/include/hw/arm/allwinner-h3.h > > b/include/hw/arm/allwinner-h3.h > > > > index 5a25a92eae..9ed365507c 100644 > > > > --- a/include/hw/arm/allwinner-h3.h > > > > +++ b/include/hw/arm/allwinner-h3.h > > > > @@ -46,6 +46,7 @@ > > > > #include "hw/misc/allwinner-h3-ccu.h" > > > > #include "hw/misc/allwinner-cpucfg.h" > > > > #include "hw/misc/allwinner-h3-sysctrl.h" > > > > +#include "hw/misc/allwinner-sid.h" > > > > #include "target/arm/cpu.h" > > > > /** > > > > @@ -63,6 +64,7 @@ enum { > > > > AW_H3_SRAM_A2, > > > > AW_H3_SRAM_C, > > > > AW_H3_SYSCTRL, > > > > + AW_H3_SID, > > > > AW_H3_EHCI0, > > > > AW_H3_OHCI0, > > > > AW_H3_EHCI1, > > > > @@ -115,6 +117,7 @@ typedef struct AwH3State { > > > > AwH3ClockCtlState ccu; > > > > AwCpuCfgState cpucfg; > > > > AwH3SysCtrlState sysctrl; > > > > + AwSidState sid; > > > > GICState gic; > > > > MemoryRegion sram_a1; > > > > MemoryRegion sram_a2; > > > > diff --git a/include/hw/misc/allwinner-sid.h > > b/include/hw/misc/allwinner-sid.h > > > > new file mode 100644 > > > > index 0000000000..41189967e2 > > > > --- /dev/null > > > > +++ b/include/hw/misc/allwinner-sid.h > > > > @@ -0,0 +1,61 @@ > > > > +/* > > > > + * Allwinner 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_SID_H > > > > +#define HW_MISC_ALLWINNER_SID_H > > > > + > > > > +#include "qemu/osdep.h" > > > > +#include "qom/object.h" > > > > +#include "hw/sysbus.h" > > > > +#include "qemu/uuid.h" > > > > + > > > > +/** > > > > + * Object model > > > > + * @{ > > > > + */ > > > > + > > > > +#define TYPE_AW_SID "allwinner-sid" > > > > +#define AW_SID(obj) \ > > > > + OBJECT_CHECK(AwSidState, (obj), TYPE_AW_SID) > > > > + > > > > +/** @} */ > > > > + > > > > +/** > > > > + * Allwinner Security ID object instance state > > > > + */ > > > > +typedef struct AwSidState { > > > > + /*< private >*/ > > > > + SysBusDevice parent_obj; > > > > + /*< public >*/ > > > > + > > > > + /** Maps I/O registers in physical memory */ > > > > + MemoryRegion iomem; > > > > + > > > > + /** Control register defines how and what to read */ > > > > + uint32_t control; > > > > + > > > > + /** RdKey register contains the data retrieved by the device */ > > > > + uint32_t rdkey; > > > > + > > > > + /** Stores the emulated device identifier */ > > > > + QemuUUID identifier; > > > > + > > > > +} AwSidState; > > > > + > > > > +#endif /* HW_MISC_ALLWINNER_SID_H */ > > > > diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c > > > > index e9ad6d23df..af7317e86a 100644 > > > > --- a/hw/arm/allwinner-h3.c > > > > +++ b/hw/arm/allwinner-h3.c > > > > @@ -36,6 +36,7 @@ const hwaddr allwinner_h3_memmap[] = { > > > > [AW_H3_SRAM_A2] = 0x00044000, > > > > [AW_H3_SRAM_C] = 0x00010000, > > > > [AW_H3_SYSCTRL] = 0x01c00000, > > > > + [AW_H3_SID] = 0x01c14000, > > > > [AW_H3_EHCI0] = 0x01c1a000, > > > > [AW_H3_OHCI0] = 0x01c1a400, > > > > [AW_H3_EHCI1] = 0x01c1b000, > > > > @@ -76,7 +77,6 @@ struct AwH3Unimplemented { > > > > { "mmc0", 0x01c0f000, 4 * KiB }, > > > > { "mmc1", 0x01c10000, 4 * KiB }, > > > > { "mmc2", 0x01c11000, 4 * KiB }, > > > > - { "sid", 0x01c14000, 1 * KiB }, > > > > { "crypto", 0x01c15000, 4 * KiB }, > > > > { "msgbox", 0x01c17000, 4 * KiB }, > > > > { "spinlock", 0x01c18000, 4 * KiB }, > > > > @@ -196,6 +196,11 @@ static void allwinner_h3_init(Object *obj) > > > > sysbus_init_child_obj(obj, "cpucfg", &s->cpucfg, > > sizeof(s->cpucfg), > > > > TYPE_AW_CPUCFG); > > > > + > > > > + sysbus_init_child_obj(obj, "sid", &s->sid, sizeof(s->sid), > > > > + TYPE_AW_SID); > > > > + object_property_add_alias(obj, "identifier", OBJECT(&s->sid), > > > > + "identifier", &error_abort); > > > > } > > > > static void allwinner_h3_realize(DeviceState *dev, Error **errp) > > > > @@ -316,6 +321,10 @@ static void allwinner_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 051184f14f..a7f870c88b 100644 > > > > --- a/hw/arm/orangepi.c > > > > +++ b/hw/arm/orangepi.c > > > > @@ -54,6 +54,10 @@ static void orangepi_init(MachineState *machine) > > > > object_property_set_int(OBJECT(s->h3), 24000000, "clk1-freq", > > > > &error_abort); > > > > + /* Setup SID properties */ > > > > + qdev_prop_set_string(DEVICE(s->h3), "identifier", > > > > + "8100c002-0001-0002-0003-000044556677"); > > > > + > > > > /* Mark H3 object realized */ > > > > object_property_set_bool(OBJECT(s->h3), true, "realized", > > &error_abort); > > > > diff --git a/hw/misc/allwinner-sid.c b/hw/misc/allwinner-sid.c > > > > new file mode 100644 > > > > index 0000000000..954de935bc > > > > --- /dev/null > > > > +++ b/hw/misc/allwinner-sid.c > > > > @@ -0,0 +1,170 @@ > > > > +/* > > > > + * Allwinner 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-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_sid_read(void *opaque, hwaddr offset, > > > > + unsigned size) > > > > +{ > > > > + const AwSidState *s = AW_SID(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: out-of-bounds offset > > 0x%04x\n", > > > > + __func__, (uint32_t)offset); > > > > + return 0; > > > > + } > > > > + > > > > + trace_allwinner_sid_read(offset, val, size); > > > > + > > > > + return val; > > > > +} > > > > + > > > > +static void allwinner_sid_write(void *opaque, hwaddr offset, > > > > + uint64_t val, unsigned size) > > > > +{ > > > > + AwSidState *s = AW_SID(opaque); > > > > + > > > > + trace_allwinner_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: out-of-bounds offset > > 0x%04x\n", > > > > + __func__, (uint32_t)offset); > > > > + break; > > > > + } > > > > +} > > > > + > > > > +static const MemoryRegionOps allwinner_sid_ops = { > > > > + .read = allwinner_sid_read, > > > > + .write = allwinner_sid_write, > > > > + .endianness = DEVICE_NATIVE_ENDIAN, > > > > + .valid = { > > > > + .min_access_size = 4, > > > > + .max_access_size = 4, > > > > + }, > > > > + .impl.min_access_size = 4, > > > > +}; > > > > + > > > > +static void allwinner_sid_reset(DeviceState *dev) > > > > +{ > > > > + AwSidState *s = AW_SID(dev); > > > > + > > > > + /* Set default values for registers */ > > > > + s->control = 0; > > > > + s->rdkey = 0; > > > > +} > > > > + > > > > +static void allwinner_sid_init(Object *obj) > > > > +{ > > > > + SysBusDevice *sbd = SYS_BUS_DEVICE(obj); > > > > + AwSidState *s = AW_SID(obj); > > > > + > > > > + /* Memory mapping */ > > > > + memory_region_init_io(&s->iomem, OBJECT(s), &allwinner_sid_ops, s, > > > > + TYPE_AW_SID, 1 * KiB); > > > > + sysbus_init_mmio(sbd, &s->iomem); > > > > +} > > > > + > > > > +static Property allwinner_sid_properties[] = { > > > > + DEFINE_PROP_UUID_NODEFAULT("identifier", AwSidState, identifier), > > > > + DEFINE_PROP_END_OF_LIST() > > > > +}; > > > > + > > > > +static const VMStateDescription allwinner_sid_vmstate = { > > > > + .name = "allwinner-sid", > > > > + .version_id = 1, > > > > + .minimum_version_id = 1, > > > > + .fields = (VMStateField[]) { > > > > + VMSTATE_UINT32(control, AwSidState), > > > > + VMSTATE_UINT32(rdkey, AwSidState), > > > > + VMSTATE_END_OF_LIST() > > > > + } > > > > +}; > > > > + > > > > +static void allwinner_sid_class_init(ObjectClass *klass, void *data) > > > > +{ > > > > + DeviceClass *dc = DEVICE_CLASS(klass); > > > > + > > > > + dc->reset = allwinner_sid_reset; > > > > + dc->vmsd = &allwinner_sid_vmstate; > > > > + dc->props = allwinner_sid_properties; > > > > +} > > > > + > > > > +static const TypeInfo allwinner_sid_info = { > > > > + .name = TYPE_AW_SID, > > > > + .parent = TYPE_SYS_BUS_DEVICE, > > > > + .instance_init = allwinner_sid_init, > > > > + .instance_size = sizeof(AwSidState), > > > > + .class_init = allwinner_sid_class_init, > > > > +}; > > > > + > > > > +static void allwinner_sid_register(void) > > > > +{ > > > > + type_register_static(&allwinner_sid_info); > > > > +} > > > > + > > > > +type_init(allwinner_sid_register) > > > > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs > > > > index 12c2c306b5..59500d5681 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-ccu.o > > > > obj-$(CONFIG_ALLWINNER_H3) += allwinner-cpucfg.o > > > > common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3-sysctrl.o > > > > +common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-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 d3e0952429..67d8bf493c 100644 > > > > --- a/hw/misc/trace-events > > > > +++ b/hw/misc/trace-events > > > > @@ -5,6 +5,10 @@ allwinner_cpucfg_cpu_reset(uint8_t cpu_id, uint32_t > > reset_addr) "id %u, reset_ad > > > > allwinner_cpucfg_read(uint64_t offset, uint64_t data, unsigned size) > > "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32 > > > > allwinner_cpucfg_write(uint64_t offset, uint64_t data, unsigned > > size) "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32 > > > > +# allwinner-sid.c > > > > +allwinner_sid_read(uint64_t offset, uint64_t data, unsigned size) > > "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32 > > > > +allwinner_sid_write(uint64_t offset, uint64_t data, unsigned size) > > "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
Hi Corey, On Mon, Feb 3, 2020 at 2:10 PM Corey Minyard <cminyard@mvista.com> wrote: > On Sun, Feb 02, 2020 at 10:27:49PM +0100, Niek Linnenbank wrote: > > Hi Corey, > > > > Thanks for reviewing! > > > > On Mon, Jan 20, 2020 at 6:59 PM Corey Minyard <cminyard@mvista.com> > wrote: > > > > > On Sat, Jan 18, 2020 at 04:25:08PM +0100, Philippe Mathieu-Daudé wrote: > > > > Cc'ing Corey/David for good advices about using UUID. > > > > > > Is there any reason you didn't use the built-in qemu UUID for this? It > > > would simplify things in general. > > > > > > > Currently the Allwinner SID device is using the QemuUUID type from > > include/qemu/uuid.h. > > Is that the build-in UUID you are referring to or should I use something > > else? > > You are using the QemuUUID type, which is of course what you should do, > but you aren't using the UUID generated by qemu (at least that I can find). > That in include/sysemu/sysemu.h and is named qemu_uuid. Whether you > should use that or not depends on your needs. If you just need some > uuid, then that's what you should probably use. If you need something > the user can individually control for this device, for instance, then > that probably won't do. > Ah I did not know about the qemu_uuid variable, thanks for pointing that out. Basically the SID identifier is a number that is unique for each board that comes out of the factory. It is currently used by U-Boot to as input to generate a MAC address. If I understand correctly, qemu_uuid is optional and by default zero. However the SID value should not be zero, as otherwise U-Boot can't pick a MAC address resulting in a non-working ethernet device. Currently the hw/arm/orangepi.c machine specifies a fixed SID to be used for the emulated board, also containing a prefix (8100c002) that indicates the H3 chipset. One thing that I am strugling with is that I'm not able to override the property using '-global', if hw/arm/orangepi.c initializes the property with qdev_prop_set_string: $ qemu-system-arm -M orangepi-pc -kernel u-boot -nographic -nic user \ -global allwinner-sid.identifier=8100c002-0001-0002-0003-000044556688 If I don't set the property in hw/arm/orangepi.c, I can set it with '-global'. Do you perhaps have a recommendation how to improve that? Basically what is needed is that the machine sets the default property including the chip prefix, and that the user can override it. Although it is not required for a working emulated board, it would be a nice-to-have that the user can set it. > > > > > > > Also, in case no one else say, you have tabs in your code that you need > > > to get rid of. > > > > > > > > If there are any tabs in the code, it was not intended. I re-checked this > > patch and others > > again but found no tabs in the code. > > Could you please point out where you found the extra tabs? > > My apologies, I saw 1-character misalignments, and that usually means > that there's a tab. But it looks like it has something to do with the > way it was forwarded. I didn't get the original email. > > Ah yes that is possible indeed. I'll add you to the CC list for the next version of this patch series. Regards, Niek > -corey > > > > > Regards, > > Niek > > > > > > > -corey > > > > > > > > > > > On 1/8/20 9:00 PM, Niek Linnenbank wrote: > > > > > The Security Identifier device found in various Allwinner System on > > > Chip > > > > > designs gives applications a per-board unique identifier. This > commit > > > > > adds support for the Allwinner Security Identifier using a 128-bit > > > > > UUID value as input. > > > > > > > > > > Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com> > > > > > --- > > > > > include/hw/arm/allwinner-h3.h | 3 + > > > > > include/hw/misc/allwinner-sid.h | 61 ++++++++++++ > > > > > hw/arm/allwinner-h3.c | 11 ++- > > > > > hw/arm/orangepi.c | 4 + > > > > > hw/misc/allwinner-sid.c | 170 > > > ++++++++++++++++++++++++++++++++ > > > > > hw/misc/Makefile.objs | 1 + > > > > > hw/misc/trace-events | 4 + > > > > > 7 files changed, 253 insertions(+), 1 deletion(-) > > > > > create mode 100644 include/hw/misc/allwinner-sid.h > > > > > create mode 100644 hw/misc/allwinner-sid.c > > > > > > > > > > diff --git a/include/hw/arm/allwinner-h3.h > > > b/include/hw/arm/allwinner-h3.h > > > > > index 5a25a92eae..9ed365507c 100644 > > > > > --- a/include/hw/arm/allwinner-h3.h > > > > > +++ b/include/hw/arm/allwinner-h3.h > > > > > @@ -46,6 +46,7 @@ > > > > > #include "hw/misc/allwinner-h3-ccu.h" > > > > > #include "hw/misc/allwinner-cpucfg.h" > > > > > #include "hw/misc/allwinner-h3-sysctrl.h" > > > > > +#include "hw/misc/allwinner-sid.h" > > > > > #include "target/arm/cpu.h" > > > > > /** > > > > > @@ -63,6 +64,7 @@ enum { > > > > > AW_H3_SRAM_A2, > > > > > AW_H3_SRAM_C, > > > > > AW_H3_SYSCTRL, > > > > > + AW_H3_SID, > > > > > AW_H3_EHCI0, > > > > > AW_H3_OHCI0, > > > > > AW_H3_EHCI1, > > > > > @@ -115,6 +117,7 @@ typedef struct AwH3State { > > > > > AwH3ClockCtlState ccu; > > > > > AwCpuCfgState cpucfg; > > > > > AwH3SysCtrlState sysctrl; > > > > > + AwSidState sid; > > > > > GICState gic; > > > > > MemoryRegion sram_a1; > > > > > MemoryRegion sram_a2; > > > > > diff --git a/include/hw/misc/allwinner-sid.h > > > b/include/hw/misc/allwinner-sid.h > > > > > new file mode 100644 > > > > > index 0000000000..41189967e2 > > > > > --- /dev/null > > > > > +++ b/include/hw/misc/allwinner-sid.h > > > > > @@ -0,0 +1,61 @@ > > > > > +/* > > > > > + * Allwinner 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_SID_H > > > > > +#define HW_MISC_ALLWINNER_SID_H > > > > > + > > > > > +#include "qemu/osdep.h" > > > > > +#include "qom/object.h" > > > > > +#include "hw/sysbus.h" > > > > > +#include "qemu/uuid.h" > > > > > + > > > > > +/** > > > > > + * Object model > > > > > + * @{ > > > > > + */ > > > > > + > > > > > +#define TYPE_AW_SID "allwinner-sid" > > > > > +#define AW_SID(obj) \ > > > > > + OBJECT_CHECK(AwSidState, (obj), TYPE_AW_SID) > > > > > + > > > > > +/** @} */ > > > > > + > > > > > +/** > > > > > + * Allwinner Security ID object instance state > > > > > + */ > > > > > +typedef struct AwSidState { > > > > > + /*< private >*/ > > > > > + SysBusDevice parent_obj; > > > > > + /*< public >*/ > > > > > + > > > > > + /** Maps I/O registers in physical memory */ > > > > > + MemoryRegion iomem; > > > > > + > > > > > + /** Control register defines how and what to read */ > > > > > + uint32_t control; > > > > > + > > > > > + /** RdKey register contains the data retrieved by the device > */ > > > > > + uint32_t rdkey; > > > > > + > > > > > + /** Stores the emulated device identifier */ > > > > > + QemuUUID identifier; > > > > > + > > > > > +} AwSidState; > > > > > + > > > > > +#endif /* HW_MISC_ALLWINNER_SID_H */ > > > > > diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c > > > > > index e9ad6d23df..af7317e86a 100644 > > > > > --- a/hw/arm/allwinner-h3.c > > > > > +++ b/hw/arm/allwinner-h3.c > > > > > @@ -36,6 +36,7 @@ const hwaddr allwinner_h3_memmap[] = { > > > > > [AW_H3_SRAM_A2] = 0x00044000, > > > > > [AW_H3_SRAM_C] = 0x00010000, > > > > > [AW_H3_SYSCTRL] = 0x01c00000, > > > > > + [AW_H3_SID] = 0x01c14000, > > > > > [AW_H3_EHCI0] = 0x01c1a000, > > > > > [AW_H3_OHCI0] = 0x01c1a400, > > > > > [AW_H3_EHCI1] = 0x01c1b000, > > > > > @@ -76,7 +77,6 @@ struct AwH3Unimplemented { > > > > > { "mmc0", 0x01c0f000, 4 * KiB }, > > > > > { "mmc1", 0x01c10000, 4 * KiB }, > > > > > { "mmc2", 0x01c11000, 4 * KiB }, > > > > > - { "sid", 0x01c14000, 1 * KiB }, > > > > > { "crypto", 0x01c15000, 4 * KiB }, > > > > > { "msgbox", 0x01c17000, 4 * KiB }, > > > > > { "spinlock", 0x01c18000, 4 * KiB }, > > > > > @@ -196,6 +196,11 @@ static void allwinner_h3_init(Object *obj) > > > > > sysbus_init_child_obj(obj, "cpucfg", &s->cpucfg, > > > sizeof(s->cpucfg), > > > > > TYPE_AW_CPUCFG); > > > > > + > > > > > + sysbus_init_child_obj(obj, "sid", &s->sid, sizeof(s->sid), > > > > > + TYPE_AW_SID); > > > > > + object_property_add_alias(obj, "identifier", OBJECT(&s->sid), > > > > > + "identifier", &error_abort); > > > > > } > > > > > static void allwinner_h3_realize(DeviceState *dev, Error **errp) > > > > > @@ -316,6 +321,10 @@ static void allwinner_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 051184f14f..a7f870c88b 100644 > > > > > --- a/hw/arm/orangepi.c > > > > > +++ b/hw/arm/orangepi.c > > > > > @@ -54,6 +54,10 @@ static void orangepi_init(MachineState *machine) > > > > > object_property_set_int(OBJECT(s->h3), 24000000, "clk1-freq", > > > > > &error_abort); > > > > > + /* Setup SID properties */ > > > > > + qdev_prop_set_string(DEVICE(s->h3), "identifier", > > > > > + "8100c002-0001-0002-0003-000044556677"); > > > > > + > > > > > /* Mark H3 object realized */ > > > > > object_property_set_bool(OBJECT(s->h3), true, "realized", > > > &error_abort); > > > > > diff --git a/hw/misc/allwinner-sid.c b/hw/misc/allwinner-sid.c > > > > > new file mode 100644 > > > > > index 0000000000..954de935bc > > > > > --- /dev/null > > > > > +++ b/hw/misc/allwinner-sid.c > > > > > @@ -0,0 +1,170 @@ > > > > > +/* > > > > > + * Allwinner 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-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_sid_read(void *opaque, hwaddr offset, > > > > > + unsigned size) > > > > > +{ > > > > > + const AwSidState *s = AW_SID(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: out-of-bounds offset > > > 0x%04x\n", > > > > > + __func__, (uint32_t)offset); > > > > > + return 0; > > > > > + } > > > > > + > > > > > + trace_allwinner_sid_read(offset, val, size); > > > > > + > > > > > + return val; > > > > > +} > > > > > + > > > > > +static void allwinner_sid_write(void *opaque, hwaddr offset, > > > > > + uint64_t val, unsigned size) > > > > > +{ > > > > > + AwSidState *s = AW_SID(opaque); > > > > > + > > > > > + trace_allwinner_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: out-of-bounds offset > > > 0x%04x\n", > > > > > + __func__, (uint32_t)offset); > > > > > + break; > > > > > + } > > > > > +} > > > > > + > > > > > +static const MemoryRegionOps allwinner_sid_ops = { > > > > > + .read = allwinner_sid_read, > > > > > + .write = allwinner_sid_write, > > > > > + .endianness = DEVICE_NATIVE_ENDIAN, > > > > > + .valid = { > > > > > + .min_access_size = 4, > > > > > + .max_access_size = 4, > > > > > + }, > > > > > + .impl.min_access_size = 4, > > > > > +}; > > > > > + > > > > > +static void allwinner_sid_reset(DeviceState *dev) > > > > > +{ > > > > > + AwSidState *s = AW_SID(dev); > > > > > + > > > > > + /* Set default values for registers */ > > > > > + s->control = 0; > > > > > + s->rdkey = 0; > > > > > +} > > > > > + > > > > > +static void allwinner_sid_init(Object *obj) > > > > > +{ > > > > > + SysBusDevice *sbd = SYS_BUS_DEVICE(obj); > > > > > + AwSidState *s = AW_SID(obj); > > > > > + > > > > > + /* Memory mapping */ > > > > > + memory_region_init_io(&s->iomem, OBJECT(s), > &allwinner_sid_ops, s, > > > > > + TYPE_AW_SID, 1 * KiB); > > > > > + sysbus_init_mmio(sbd, &s->iomem); > > > > > +} > > > > > + > > > > > +static Property allwinner_sid_properties[] = { > > > > > + DEFINE_PROP_UUID_NODEFAULT("identifier", AwSidState, > identifier), > > > > > + DEFINE_PROP_END_OF_LIST() > > > > > +}; > > > > > + > > > > > +static const VMStateDescription allwinner_sid_vmstate = { > > > > > + .name = "allwinner-sid", > > > > > + .version_id = 1, > > > > > + .minimum_version_id = 1, > > > > > + .fields = (VMStateField[]) { > > > > > + VMSTATE_UINT32(control, AwSidState), > > > > > + VMSTATE_UINT32(rdkey, AwSidState), > > > > > + VMSTATE_END_OF_LIST() > > > > > + } > > > > > +}; > > > > > + > > > > > +static void allwinner_sid_class_init(ObjectClass *klass, void > *data) > > > > > +{ > > > > > + DeviceClass *dc = DEVICE_CLASS(klass); > > > > > + > > > > > + dc->reset = allwinner_sid_reset; > > > > > + dc->vmsd = &allwinner_sid_vmstate; > > > > > + dc->props = allwinner_sid_properties; > > > > > +} > > > > > + > > > > > +static const TypeInfo allwinner_sid_info = { > > > > > + .name = TYPE_AW_SID, > > > > > + .parent = TYPE_SYS_BUS_DEVICE, > > > > > + .instance_init = allwinner_sid_init, > > > > > + .instance_size = sizeof(AwSidState), > > > > > + .class_init = allwinner_sid_class_init, > > > > > +}; > > > > > + > > > > > +static void allwinner_sid_register(void) > > > > > +{ > > > > > + type_register_static(&allwinner_sid_info); > > > > > +} > > > > > + > > > > > +type_init(allwinner_sid_register) > > > > > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs > > > > > index 12c2c306b5..59500d5681 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-ccu.o > > > > > obj-$(CONFIG_ALLWINNER_H3) += allwinner-cpucfg.o > > > > > common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3-sysctrl.o > > > > > +common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-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 d3e0952429..67d8bf493c 100644 > > > > > --- a/hw/misc/trace-events > > > > > +++ b/hw/misc/trace-events > > > > > @@ -5,6 +5,10 @@ allwinner_cpucfg_cpu_reset(uint8_t cpu_id, > uint32_t > > > reset_addr) "id %u, reset_ad > > > > > allwinner_cpucfg_read(uint64_t offset, uint64_t data, unsigned > size) > > > "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32 > > > > > allwinner_cpucfg_write(uint64_t offset, uint64_t data, unsigned > > > size) "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32 > > > > > +# allwinner-sid.c > > > > > +allwinner_sid_read(uint64_t offset, uint64_t data, unsigned size) > > > "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32 > > > > > +allwinner_sid_write(uint64_t offset, uint64_t data, unsigned size) > > > "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 >
Hi Corey, On Thu, Feb 6, 2020 at 10:09 PM Niek Linnenbank <nieklinnenbank@gmail.com> wrote: > Hi Corey, > > On Mon, Feb 3, 2020 at 2:10 PM Corey Minyard <cminyard@mvista.com> wrote: > >> On Sun, Feb 02, 2020 at 10:27:49PM +0100, Niek Linnenbank wrote: >> > Hi Corey, >> > >> > Thanks for reviewing! >> > >> > On Mon, Jan 20, 2020 at 6:59 PM Corey Minyard <cminyard@mvista.com> >> wrote: >> > >> > > On Sat, Jan 18, 2020 at 04:25:08PM +0100, Philippe Mathieu-Daudé >> wrote: >> > > > Cc'ing Corey/David for good advices about using UUID. >> > > >> > > Is there any reason you didn't use the built-in qemu UUID for this? >> It >> > > would simplify things in general. >> > > >> > >> > Currently the Allwinner SID device is using the QemuUUID type from >> > include/qemu/uuid.h. >> > Is that the build-in UUID you are referring to or should I use something >> > else? >> >> You are using the QemuUUID type, which is of course what you should do, >> but you aren't using the UUID generated by qemu (at least that I can >> find). >> That in include/sysemu/sysemu.h and is named qemu_uuid. Whether you >> should use that or not depends on your needs. If you just need some >> uuid, then that's what you should probably use. If you need something >> the user can individually control for this device, for instance, then >> that probably won't do. >> > > Ah I did not know about the qemu_uuid variable, thanks for pointing that > out. > Basically the SID identifier is a number that is unique for each board > that comes > out of the factory. It is currently used by U-Boot to as input to generate > a MAC address. > > If I understand correctly, qemu_uuid is optional and by default zero. > However the SID value should not be zero, as otherwise U-Boot can't pick a > MAC address > resulting in a non-working ethernet device. > > Currently the hw/arm/orangepi.c machine specifies a fixed SID to be used > for the emulated board, > also containing a prefix (8100c002) that indicates the H3 chipset. One > thing that I am strugling with is that > I'm not able to override the property using '-global', if > hw/arm/orangepi.c initializes the property with qdev_prop_set_string: > > $ qemu-system-arm -M orangepi-pc -kernel u-boot -nographic -nic user \ > -global allwinner-sid.identifier=8100c002-0001-0002-0003-000044556688 > > If I don't set the property in hw/arm/orangepi.c, I can set it with > '-global'. Do you perhaps have a > recommendation how to improve that? Basically what is needed is that the > machine sets the default > property including the chip prefix, and that the user can override it. > Although it is not required for a > working emulated board, it would be a nice-to-have that the user can set > it. > FYI and possibly others who have a similar usecase, I figured out how to do this. In the machine init function, after creating the new SoC object, simply check if the identifier has a value: + if (qemu_uuid_is_null(&s->h3->sid.identifier)) { + qdev_prop_set_string(DEVICE(s->h3), "identifier", + "8100c002-0001-0002-0003-000044556677"); + } That way, if the user passed -global to override it, the machine will not overrule the user's value and by default the machine sets an identifier containing the H3 specific chip prefix. Regards, Niek > >> > >> > >> > > Also, in case no one else say, you have tabs in your code that you >> need >> > > to get rid of. >> > > >> > > >> > If there are any tabs in the code, it was not intended. I re-checked >> this >> > patch and others >> > again but found no tabs in the code. >> > Could you please point out where you found the extra tabs? >> >> My apologies, I saw 1-character misalignments, and that usually means >> that there's a tab. But it looks like it has something to do with the >> way it was forwarded. I didn't get the original email. >> >> Ah yes that is possible indeed. I'll add you to the CC list for the next > version of this patch series. > > Regards, > Niek > > >> -corey >> >> > >> > Regards, >> > Niek >> > >> > >> > > -corey >> > > >> > > > >> > > > On 1/8/20 9:00 PM, Niek Linnenbank wrote: >> > > > > The Security Identifier device found in various Allwinner System >> on >> > > Chip >> > > > > designs gives applications a per-board unique identifier. This >> commit >> > > > > adds support for the Allwinner Security Identifier using a 128-bit >> > > > > UUID value as input. >> > > > > >> > > > > Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com> >> > > > > --- >> > > > > include/hw/arm/allwinner-h3.h | 3 + >> > > > > include/hw/misc/allwinner-sid.h | 61 ++++++++++++ >> > > > > hw/arm/allwinner-h3.c | 11 ++- >> > > > > hw/arm/orangepi.c | 4 + >> > > > > hw/misc/allwinner-sid.c | 170 >> > > ++++++++++++++++++++++++++++++++ >> > > > > hw/misc/Makefile.objs | 1 + >> > > > > hw/misc/trace-events | 4 + >> > > > > 7 files changed, 253 insertions(+), 1 deletion(-) >> > > > > create mode 100644 include/hw/misc/allwinner-sid.h >> > > > > create mode 100644 hw/misc/allwinner-sid.c >> > > > > >> > > > > diff --git a/include/hw/arm/allwinner-h3.h >> > > b/include/hw/arm/allwinner-h3.h >> > > > > index 5a25a92eae..9ed365507c 100644 >> > > > > --- a/include/hw/arm/allwinner-h3.h >> > > > > +++ b/include/hw/arm/allwinner-h3.h >> > > > > @@ -46,6 +46,7 @@ >> > > > > #include "hw/misc/allwinner-h3-ccu.h" >> > > > > #include "hw/misc/allwinner-cpucfg.h" >> > > > > #include "hw/misc/allwinner-h3-sysctrl.h" >> > > > > +#include "hw/misc/allwinner-sid.h" >> > > > > #include "target/arm/cpu.h" >> > > > > /** >> > > > > @@ -63,6 +64,7 @@ enum { >> > > > > AW_H3_SRAM_A2, >> > > > > AW_H3_SRAM_C, >> > > > > AW_H3_SYSCTRL, >> > > > > + AW_H3_SID, >> > > > > AW_H3_EHCI0, >> > > > > AW_H3_OHCI0, >> > > > > AW_H3_EHCI1, >> > > > > @@ -115,6 +117,7 @@ typedef struct AwH3State { >> > > > > AwH3ClockCtlState ccu; >> > > > > AwCpuCfgState cpucfg; >> > > > > AwH3SysCtrlState sysctrl; >> > > > > + AwSidState sid; >> > > > > GICState gic; >> > > > > MemoryRegion sram_a1; >> > > > > MemoryRegion sram_a2; >> > > > > diff --git a/include/hw/misc/allwinner-sid.h >> > > b/include/hw/misc/allwinner-sid.h >> > > > > new file mode 100644 >> > > > > index 0000000000..41189967e2 >> > > > > --- /dev/null >> > > > > +++ b/include/hw/misc/allwinner-sid.h >> > > > > @@ -0,0 +1,61 @@ >> > > > > +/* >> > > > > + * Allwinner 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_SID_H >> > > > > +#define HW_MISC_ALLWINNER_SID_H >> > > > > + >> > > > > +#include "qemu/osdep.h" >> > > > > +#include "qom/object.h" >> > > > > +#include "hw/sysbus.h" >> > > > > +#include "qemu/uuid.h" >> > > > > + >> > > > > +/** >> > > > > + * Object model >> > > > > + * @{ >> > > > > + */ >> > > > > + >> > > > > +#define TYPE_AW_SID "allwinner-sid" >> > > > > +#define AW_SID(obj) \ >> > > > > + OBJECT_CHECK(AwSidState, (obj), TYPE_AW_SID) >> > > > > + >> > > > > +/** @} */ >> > > > > + >> > > > > +/** >> > > > > + * Allwinner Security ID object instance state >> > > > > + */ >> > > > > +typedef struct AwSidState { >> > > > > + /*< private >*/ >> > > > > + SysBusDevice parent_obj; >> > > > > + /*< public >*/ >> > > > > + >> > > > > + /** Maps I/O registers in physical memory */ >> > > > > + MemoryRegion iomem; >> > > > > + >> > > > > + /** Control register defines how and what to read */ >> > > > > + uint32_t control; >> > > > > + >> > > > > + /** RdKey register contains the data retrieved by the device >> */ >> > > > > + uint32_t rdkey; >> > > > > + >> > > > > + /** Stores the emulated device identifier */ >> > > > > + QemuUUID identifier; >> > > > > + >> > > > > +} AwSidState; >> > > > > + >> > > > > +#endif /* HW_MISC_ALLWINNER_SID_H */ >> > > > > diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c >> > > > > index e9ad6d23df..af7317e86a 100644 >> > > > > --- a/hw/arm/allwinner-h3.c >> > > > > +++ b/hw/arm/allwinner-h3.c >> > > > > @@ -36,6 +36,7 @@ const hwaddr allwinner_h3_memmap[] = { >> > > > > [AW_H3_SRAM_A2] = 0x00044000, >> > > > > [AW_H3_SRAM_C] = 0x00010000, >> > > > > [AW_H3_SYSCTRL] = 0x01c00000, >> > > > > + [AW_H3_SID] = 0x01c14000, >> > > > > [AW_H3_EHCI0] = 0x01c1a000, >> > > > > [AW_H3_OHCI0] = 0x01c1a400, >> > > > > [AW_H3_EHCI1] = 0x01c1b000, >> > > > > @@ -76,7 +77,6 @@ struct AwH3Unimplemented { >> > > > > { "mmc0", 0x01c0f000, 4 * KiB }, >> > > > > { "mmc1", 0x01c10000, 4 * KiB }, >> > > > > { "mmc2", 0x01c11000, 4 * KiB }, >> > > > > - { "sid", 0x01c14000, 1 * KiB }, >> > > > > { "crypto", 0x01c15000, 4 * KiB }, >> > > > > { "msgbox", 0x01c17000, 4 * KiB }, >> > > > > { "spinlock", 0x01c18000, 4 * KiB }, >> > > > > @@ -196,6 +196,11 @@ static void allwinner_h3_init(Object *obj) >> > > > > sysbus_init_child_obj(obj, "cpucfg", &s->cpucfg, >> > > sizeof(s->cpucfg), >> > > > > TYPE_AW_CPUCFG); >> > > > > + >> > > > > + sysbus_init_child_obj(obj, "sid", &s->sid, sizeof(s->sid), >> > > > > + TYPE_AW_SID); >> > > > > + object_property_add_alias(obj, "identifier", OBJECT(&s->sid), >> > > > > + "identifier", &error_abort); >> > > > > } >> > > > > static void allwinner_h3_realize(DeviceState *dev, Error **errp) >> > > > > @@ -316,6 +321,10 @@ static void allwinner_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 051184f14f..a7f870c88b 100644 >> > > > > --- a/hw/arm/orangepi.c >> > > > > +++ b/hw/arm/orangepi.c >> > > > > @@ -54,6 +54,10 @@ static void orangepi_init(MachineState >> *machine) >> > > > > object_property_set_int(OBJECT(s->h3), 24000000, >> "clk1-freq", >> > > > > &error_abort); >> > > > > + /* Setup SID properties */ >> > > > > + qdev_prop_set_string(DEVICE(s->h3), "identifier", >> > > > > + "8100c002-0001-0002-0003-000044556677"); >> > > > > + >> > > > > /* Mark H3 object realized */ >> > > > > object_property_set_bool(OBJECT(s->h3), true, "realized", >> > > &error_abort); >> > > > > diff --git a/hw/misc/allwinner-sid.c b/hw/misc/allwinner-sid.c >> > > > > new file mode 100644 >> > > > > index 0000000000..954de935bc >> > > > > --- /dev/null >> > > > > +++ b/hw/misc/allwinner-sid.c >> > > > > @@ -0,0 +1,170 @@ >> > > > > +/* >> > > > > + * Allwinner 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-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_sid_read(void *opaque, hwaddr offset, >> > > > > + unsigned size) >> > > > > +{ >> > > > > + const AwSidState *s = AW_SID(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: out-of-bounds offset >> > > 0x%04x\n", >> > > > > + __func__, (uint32_t)offset); >> > > > > + return 0; >> > > > > + } >> > > > > + >> > > > > + trace_allwinner_sid_read(offset, val, size); >> > > > > + >> > > > > + return val; >> > > > > +} >> > > > > + >> > > > > +static void allwinner_sid_write(void *opaque, hwaddr offset, >> > > > > + uint64_t val, unsigned size) >> > > > > +{ >> > > > > + AwSidState *s = AW_SID(opaque); >> > > > > + >> > > > > + trace_allwinner_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: out-of-bounds offset >> > > 0x%04x\n", >> > > > > + __func__, (uint32_t)offset); >> > > > > + break; >> > > > > + } >> > > > > +} >> > > > > + >> > > > > +static const MemoryRegionOps allwinner_sid_ops = { >> > > > > + .read = allwinner_sid_read, >> > > > > + .write = allwinner_sid_write, >> > > > > + .endianness = DEVICE_NATIVE_ENDIAN, >> > > > > + .valid = { >> > > > > + .min_access_size = 4, >> > > > > + .max_access_size = 4, >> > > > > + }, >> > > > > + .impl.min_access_size = 4, >> > > > > +}; >> > > > > + >> > > > > +static void allwinner_sid_reset(DeviceState *dev) >> > > > > +{ >> > > > > + AwSidState *s = AW_SID(dev); >> > > > > + >> > > > > + /* Set default values for registers */ >> > > > > + s->control = 0; >> > > > > + s->rdkey = 0; >> > > > > +} >> > > > > + >> > > > > +static void allwinner_sid_init(Object *obj) >> > > > > +{ >> > > > > + SysBusDevice *sbd = SYS_BUS_DEVICE(obj); >> > > > > + AwSidState *s = AW_SID(obj); >> > > > > + >> > > > > + /* Memory mapping */ >> > > > > + memory_region_init_io(&s->iomem, OBJECT(s), >> &allwinner_sid_ops, s, >> > > > > + TYPE_AW_SID, 1 * KiB); >> > > > > + sysbus_init_mmio(sbd, &s->iomem); >> > > > > +} >> > > > > + >> > > > > +static Property allwinner_sid_properties[] = { >> > > > > + DEFINE_PROP_UUID_NODEFAULT("identifier", AwSidState, >> identifier), >> > > > > + DEFINE_PROP_END_OF_LIST() >> > > > > +}; >> > > > > + >> > > > > +static const VMStateDescription allwinner_sid_vmstate = { >> > > > > + .name = "allwinner-sid", >> > > > > + .version_id = 1, >> > > > > + .minimum_version_id = 1, >> > > > > + .fields = (VMStateField[]) { >> > > > > + VMSTATE_UINT32(control, AwSidState), >> > > > > + VMSTATE_UINT32(rdkey, AwSidState), >> > > > > + VMSTATE_END_OF_LIST() >> > > > > + } >> > > > > +}; >> > > > > + >> > > > > +static void allwinner_sid_class_init(ObjectClass *klass, void >> *data) >> > > > > +{ >> > > > > + DeviceClass *dc = DEVICE_CLASS(klass); >> > > > > + >> > > > > + dc->reset = allwinner_sid_reset; >> > > > > + dc->vmsd = &allwinner_sid_vmstate; >> > > > > + dc->props = allwinner_sid_properties; >> > > > > +} >> > > > > + >> > > > > +static const TypeInfo allwinner_sid_info = { >> > > > > + .name = TYPE_AW_SID, >> > > > > + .parent = TYPE_SYS_BUS_DEVICE, >> > > > > + .instance_init = allwinner_sid_init, >> > > > > + .instance_size = sizeof(AwSidState), >> > > > > + .class_init = allwinner_sid_class_init, >> > > > > +}; >> > > > > + >> > > > > +static void allwinner_sid_register(void) >> > > > > +{ >> > > > > + type_register_static(&allwinner_sid_info); >> > > > > +} >> > > > > + >> > > > > +type_init(allwinner_sid_register) >> > > > > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs >> > > > > index 12c2c306b5..59500d5681 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-ccu.o >> > > > > obj-$(CONFIG_ALLWINNER_H3) += allwinner-cpucfg.o >> > > > > common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3-sysctrl.o >> > > > > +common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-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 d3e0952429..67d8bf493c 100644 >> > > > > --- a/hw/misc/trace-events >> > > > > +++ b/hw/misc/trace-events >> > > > > @@ -5,6 +5,10 @@ allwinner_cpucfg_cpu_reset(uint8_t cpu_id, >> uint32_t >> > > reset_addr) "id %u, reset_ad >> > > > > allwinner_cpucfg_read(uint64_t offset, uint64_t data, unsigned >> size) >> > > "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32 >> > > > > allwinner_cpucfg_write(uint64_t offset, uint64_t data, unsigned >> > > size) "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32 >> > > > > +# allwinner-sid.c >> > > > > +allwinner_sid_read(uint64_t offset, uint64_t data, unsigned size) >> > > "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32 >> > > > > +allwinner_sid_write(uint64_t offset, uint64_t data, unsigned >> size) >> > > "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 >> > > > -- > Niek Linnenbank > >
)-.On Wed, Feb 12, 2020 at 10:31 PM Niek Linnenbank <nieklinnenbank@gmail.com> wrote: > > Hi Corey, > > On Thu, Feb 6, 2020 at 10:09 PM Niek Linnenbank <nieklinnenbank@gmail.com> wrote: >> >> Hi Corey, >> >> On Mon, Feb 3, 2020 at 2:10 PM Corey Minyard <cminyard@mvista.com> wrote: >>> >>> On Sun, Feb 02, 2020 at 10:27:49PM +0100, Niek Linnenbank wrote: >>> > Hi Corey, >>> > >>> > Thanks for reviewing! >>> > >>> > On Mon, Jan 20, 2020 at 6:59 PM Corey Minyard <cminyard@mvista.com> wrote: >>> > >>> > > On Sat, Jan 18, 2020 at 04:25:08PM +0100, Philippe Mathieu-Daudé wrote: >>> > > > Cc'ing Corey/David for good advices about using UUID. >>> > > >>> > > Is there any reason you didn't use the built-in qemu UUID for this? It >>> > > would simplify things in general. >>> > > >>> > >>> > Currently the Allwinner SID device is using the QemuUUID type from >>> > include/qemu/uuid.h. >>> > Is that the build-in UUID you are referring to or should I use something >>> > else? >>> >>> You are using the QemuUUID type, which is of course what you should do, >>> but you aren't using the UUID generated by qemu (at least that I can find). >>> That in include/sysemu/sysemu.h and is named qemu_uuid. Whether you >>> should use that or not depends on your needs. If you just need some >>> uuid, then that's what you should probably use. If you need something >>> the user can individually control for this device, for instance, then >>> that probably won't do. >> >> >> Ah I did not know about the qemu_uuid variable, thanks for pointing that out. >> Basically the SID identifier is a number that is unique for each board that comes >> out of the factory. It is currently used by U-Boot to as input to generate a MAC address. >> >> If I understand correctly, qemu_uuid is optional and by default zero. >> However the SID value should not be zero, as otherwise U-Boot can't pick a MAC address >> resulting in a non-working ethernet device. >> >> Currently the hw/arm/orangepi.c machine specifies a fixed SID to be used for the emulated board, >> also containing a prefix (8100c002) that indicates the H3 chipset. One thing that I am strugling with is that Suggestion while reading this, you might display a warning if the user provided UUID doesn't start with 8100c002. >> I'm not able to override the property using '-global', if hw/arm/orangepi.c initializes the property with qdev_prop_set_string: >> >> $ qemu-system-arm -M orangepi-pc -kernel u-boot -nographic -nic user \ >> -global allwinner-sid.identifier=8100c002-0001-0002-0003-000044556688 >> >> If I don't set the property in hw/arm/orangepi.c, I can set it with '-global'. Do you perhaps have a >> recommendation how to improve that? Basically what is needed is that the machine sets the default >> property including the chip prefix, and that the user can override it. Although it is not required for a >> working emulated board, it would be a nice-to-have that the user can set it. > > > FYI and possibly others who have a similar usecase, I figured out how to do this. In the machine init function, > after creating the new SoC object, simply check if the identifier has a value: > > + if (qemu_uuid_is_null(&s->h3->sid.identifier)) { > + qdev_prop_set_string(DEVICE(s->h3), "identifier", > + "8100c002-0001-0002-0003-000044556677"); Similarly, display a warning "No UUID provided, using default one..." (or generate one 8100c002-XXX)? > + } > > That way, if the user passed -global to override it, the machine will not overrule the user's value > and by default the machine sets an identifier containing the H3 specific chip prefix. > [...] >>> > > > > --- /dev/null >>> > > > > +++ b/hw/misc/allwinner-sid.c >>> > > > > @@ -0,0 +1,170 @@ >>> > > > > +/* >>> > > > > + * Allwinner 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-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_sid_read(void *opaque, hwaddr offset, >>> > > > > + unsigned size) >>> > > > > +{ >>> > > > > + const AwSidState *s = AW_SID(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: out-of-bounds offset >>> > > 0x%04x\n", >>> > > > > + __func__, (uint32_t)offset); >>> > > > > + return 0; >>> > > > > + } >>> > > > > + >>> > > > > + trace_allwinner_sid_read(offset, val, size); >>> > > > > + >>> > > > > + return val; >>> > > > > +} >>> > > > > + >>> > > > > +static void allwinner_sid_write(void *opaque, hwaddr offset, >>> > > > > + uint64_t val, unsigned size) >>> > > > > +{ >>> > > > > + AwSidState *s = AW_SID(opaque); >>> > > > > + >>> > > > > + trace_allwinner_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); Maybe you want: s->rdkey = ldl_le_p(s->identifier.data); >>> > > > > + } >>> > > > > + } >>> > > > > + s->control &= ~REG_PRCTL_WRITE; >>> > > > > + break; >>> > > > > + case REG_RDKEY: /* Read Key */ >>> > > > > + break; >>> > > > > + default: >>> > > > > + qemu_log_mask(LOG_GUEST_ERROR, "%s: out-of-bounds offset >>> > > 0x%04x\n", >>> > > > > + __func__, (uint32_t)offset); >>> > > > > + break; >>> > > > > + } >>> > > > > +} > > > > > +} [...]
On Wed, Feb 12, 2020 at 11:48 PM Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > )-.On Wed, Feb 12, 2020 at 10:31 PM Niek Linnenbank > <nieklinnenbank@gmail.com> wrote: > > > > Hi Corey, > > > > On Thu, Feb 6, 2020 at 10:09 PM Niek Linnenbank < > nieklinnenbank@gmail.com> wrote: > >> > >> Hi Corey, > >> > >> On Mon, Feb 3, 2020 at 2:10 PM Corey Minyard <cminyard@mvista.com> > wrote: > >>> > >>> On Sun, Feb 02, 2020 at 10:27:49PM +0100, Niek Linnenbank wrote: > >>> > Hi Corey, > >>> > > >>> > Thanks for reviewing! > >>> > > >>> > On Mon, Jan 20, 2020 at 6:59 PM Corey Minyard <cminyard@mvista.com> > wrote: > >>> > > >>> > > On Sat, Jan 18, 2020 at 04:25:08PM +0100, Philippe Mathieu-Daudé > wrote: > >>> > > > Cc'ing Corey/David for good advices about using UUID. > >>> > > > >>> > > Is there any reason you didn't use the built-in qemu UUID for > this? It > >>> > > would simplify things in general. > >>> > > > >>> > > >>> > Currently the Allwinner SID device is using the QemuUUID type from > >>> > include/qemu/uuid.h. > >>> > Is that the build-in UUID you are referring to or should I use > something > >>> > else? > >>> > >>> You are using the QemuUUID type, which is of course what you should do, > >>> but you aren't using the UUID generated by qemu (at least that I can > find). > >>> That in include/sysemu/sysemu.h and is named qemu_uuid. Whether you > >>> should use that or not depends on your needs. If you just need some > >>> uuid, then that's what you should probably use. If you need something > >>> the user can individually control for this device, for instance, then > >>> that probably won't do. > >> > >> > >> Ah I did not know about the qemu_uuid variable, thanks for pointing > that out. > >> Basically the SID identifier is a number that is unique for each board > that comes > >> out of the factory. It is currently used by U-Boot to as input to > generate a MAC address. > >> > >> If I understand correctly, qemu_uuid is optional and by default zero. > >> However the SID value should not be zero, as otherwise U-Boot can't > pick a MAC address > >> resulting in a non-working ethernet device. > >> > >> Currently the hw/arm/orangepi.c machine specifies a fixed SID to be > used for the emulated board, > >> also containing a prefix (8100c002) that indicates the H3 chipset. One > thing that I am strugling with is that > > Suggestion while reading this, you might display a warning if the user > provided UUID doesn't start with 8100c002. > Yeah sure, I can add a warning for it. > > >> I'm not able to override the property using '-global', if > hw/arm/orangepi.c initializes the property with qdev_prop_set_string: > >> > >> $ qemu-system-arm -M orangepi-pc -kernel u-boot -nographic -nic user \ > >> -global allwinner-sid.identifier=8100c002-0001-0002-0003-000044556688 > >> > >> If I don't set the property in hw/arm/orangepi.c, I can set it with > '-global'. Do you perhaps have a > >> recommendation how to improve that? Basically what is needed is that > the machine sets the default > >> property including the chip prefix, and that the user can override it. > Although it is not required for a > >> working emulated board, it would be a nice-to-have that the user can > set it. > > > > > > FYI and possibly others who have a similar usecase, I figured out how to > do this. In the machine init function, > > after creating the new SoC object, simply check if the identifier has a > value: > > > > + if (qemu_uuid_is_null(&s->h3->sid.identifier)) { > > + qdev_prop_set_string(DEVICE(s->h3), "identifier", > > + "8100c002-0001-0002-0003-000044556677"); > > Similarly, display a warning "No UUID provided, using default one..." > (or generate one 8100c002-XXX)? > If its allright with you, I would prefer not to display a warning for the default behavior. The other boards are also not displaying such warnings and it keeps the qemu output clean. > > > + } > > > > That way, if the user passed -global to override it, the machine will > not overrule the user's value > > and by default the machine sets an identifier containing the H3 specific > chip prefix. > > > [...] > >>> > > > > --- /dev/null > >>> > > > > +++ b/hw/misc/allwinner-sid.c > >>> > > > > @@ -0,0 +1,170 @@ > >>> > > > > +/* > >>> > > > > + * Allwinner 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-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_sid_read(void *opaque, hwaddr > offset, > >>> > > > > + unsigned size) > >>> > > > > +{ > >>> > > > > + const AwSidState *s = AW_SID(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: out-of-bounds > offset > >>> > > 0x%04x\n", > >>> > > > > + __func__, (uint32_t)offset); > >>> > > > > + return 0; > >>> > > > > + } > >>> > > > > + > >>> > > > > + trace_allwinner_sid_read(offset, val, size); > >>> > > > > + > >>> > > > > + return val; > >>> > > > > +} > >>> > > > > + > >>> > > > > +static void allwinner_sid_write(void *opaque, hwaddr offset, > >>> > > > > + uint64_t val, unsigned size) > >>> > > > > +{ > >>> > > > > + AwSidState *s = AW_SID(opaque); > >>> > > > > + > >>> > > > > + trace_allwinner_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); > > Maybe you want: > > s->rdkey = ldl_le_p(s->identifier.data); > Thanks Philippe, I'll use this function to set s->rdkey. Regards, Niek > > >>> > > > > + } > >>> > > > > + } > >>> > > > > + s->control &= ~REG_PRCTL_WRITE; > >>> > > > > + break; > >>> > > > > + case REG_RDKEY: /* Read Key */ > >>> > > > > + break; > >>> > > > > + default: > >>> > > > > + qemu_log_mask(LOG_GUEST_ERROR, "%s: out-of-bounds > offset > >>> > > 0x%04x\n", > >>> > > > > + __func__, (uint32_t)offset); > >>> > > > > + break; > >>> > > > > + } > >>> > > > > +} > > > > > > +} > [...] > >
diff --git a/include/hw/arm/allwinner-h3.h b/include/hw/arm/allwinner-h3.h index 5a25a92eae..9ed365507c 100644 --- a/include/hw/arm/allwinner-h3.h +++ b/include/hw/arm/allwinner-h3.h @@ -46,6 +46,7 @@ #include "hw/misc/allwinner-h3-ccu.h" #include "hw/misc/allwinner-cpucfg.h" #include "hw/misc/allwinner-h3-sysctrl.h" +#include "hw/misc/allwinner-sid.h" #include "target/arm/cpu.h" /** @@ -63,6 +64,7 @@ enum { AW_H3_SRAM_A2, AW_H3_SRAM_C, AW_H3_SYSCTRL, + AW_H3_SID, AW_H3_EHCI0, AW_H3_OHCI0, AW_H3_EHCI1, @@ -115,6 +117,7 @@ typedef struct AwH3State { AwH3ClockCtlState ccu; AwCpuCfgState cpucfg; AwH3SysCtrlState sysctrl; + AwSidState sid; GICState gic; MemoryRegion sram_a1; MemoryRegion sram_a2; diff --git a/include/hw/misc/allwinner-sid.h b/include/hw/misc/allwinner-sid.h new file mode 100644 index 0000000000..41189967e2 --- /dev/null +++ b/include/hw/misc/allwinner-sid.h @@ -0,0 +1,61 @@ +/* + * Allwinner 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_SID_H +#define HW_MISC_ALLWINNER_SID_H + +#include "qemu/osdep.h" +#include "qom/object.h" +#include "hw/sysbus.h" +#include "qemu/uuid.h" + +/** + * Object model + * @{ + */ + +#define TYPE_AW_SID "allwinner-sid" +#define AW_SID(obj) \ + OBJECT_CHECK(AwSidState, (obj), TYPE_AW_SID) + +/** @} */ + +/** + * Allwinner Security ID object instance state + */ +typedef struct AwSidState { + /*< private >*/ + SysBusDevice parent_obj; + /*< public >*/ + + /** Maps I/O registers in physical memory */ + MemoryRegion iomem; + + /** Control register defines how and what to read */ + uint32_t control; + + /** RdKey register contains the data retrieved by the device */ + uint32_t rdkey; + + /** Stores the emulated device identifier */ + QemuUUID identifier; + +} AwSidState; + +#endif /* HW_MISC_ALLWINNER_SID_H */ diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c index e9ad6d23df..af7317e86a 100644 --- a/hw/arm/allwinner-h3.c +++ b/hw/arm/allwinner-h3.c @@ -36,6 +36,7 @@ const hwaddr allwinner_h3_memmap[] = { [AW_H3_SRAM_A2] = 0x00044000, [AW_H3_SRAM_C] = 0x00010000, [AW_H3_SYSCTRL] = 0x01c00000, + [AW_H3_SID] = 0x01c14000, [AW_H3_EHCI0] = 0x01c1a000, [AW_H3_OHCI0] = 0x01c1a400, [AW_H3_EHCI1] = 0x01c1b000, @@ -76,7 +77,6 @@ struct AwH3Unimplemented { { "mmc0", 0x01c0f000, 4 * KiB }, { "mmc1", 0x01c10000, 4 * KiB }, { "mmc2", 0x01c11000, 4 * KiB }, - { "sid", 0x01c14000, 1 * KiB }, { "crypto", 0x01c15000, 4 * KiB }, { "msgbox", 0x01c17000, 4 * KiB }, { "spinlock", 0x01c18000, 4 * KiB }, @@ -196,6 +196,11 @@ static void allwinner_h3_init(Object *obj) sysbus_init_child_obj(obj, "cpucfg", &s->cpucfg, sizeof(s->cpucfg), TYPE_AW_CPUCFG); + + sysbus_init_child_obj(obj, "sid", &s->sid, sizeof(s->sid), + TYPE_AW_SID); + object_property_add_alias(obj, "identifier", OBJECT(&s->sid), + "identifier", &error_abort); } static void allwinner_h3_realize(DeviceState *dev, Error **errp) @@ -316,6 +321,10 @@ static void allwinner_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 051184f14f..a7f870c88b 100644 --- a/hw/arm/orangepi.c +++ b/hw/arm/orangepi.c @@ -54,6 +54,10 @@ static void orangepi_init(MachineState *machine) object_property_set_int(OBJECT(s->h3), 24000000, "clk1-freq", &error_abort); + /* Setup SID properties */ + qdev_prop_set_string(DEVICE(s->h3), "identifier", + "8100c002-0001-0002-0003-000044556677"); + /* Mark H3 object realized */ object_property_set_bool(OBJECT(s->h3), true, "realized", &error_abort); diff --git a/hw/misc/allwinner-sid.c b/hw/misc/allwinner-sid.c new file mode 100644 index 0000000000..954de935bc --- /dev/null +++ b/hw/misc/allwinner-sid.c @@ -0,0 +1,170 @@ +/* + * Allwinner 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-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_sid_read(void *opaque, hwaddr offset, + unsigned size) +{ + const AwSidState *s = AW_SID(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: out-of-bounds offset 0x%04x\n", + __func__, (uint32_t)offset); + return 0; + } + + trace_allwinner_sid_read(offset, val, size); + + return val; +} + +static void allwinner_sid_write(void *opaque, hwaddr offset, + uint64_t val, unsigned size) +{ + AwSidState *s = AW_SID(opaque); + + trace_allwinner_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: out-of-bounds offset 0x%04x\n", + __func__, (uint32_t)offset); + break; + } +} + +static const MemoryRegionOps allwinner_sid_ops = { + .read = allwinner_sid_read, + .write = allwinner_sid_write, + .endianness = DEVICE_NATIVE_ENDIAN, + .valid = { + .min_access_size = 4, + .max_access_size = 4, + }, + .impl.min_access_size = 4, +}; + +static void allwinner_sid_reset(DeviceState *dev) +{ + AwSidState *s = AW_SID(dev); + + /* Set default values for registers */ + s->control = 0; + s->rdkey = 0; +} + +static void allwinner_sid_init(Object *obj) +{ + SysBusDevice *sbd = SYS_BUS_DEVICE(obj); + AwSidState *s = AW_SID(obj); + + /* Memory mapping */ + memory_region_init_io(&s->iomem, OBJECT(s), &allwinner_sid_ops, s, + TYPE_AW_SID, 1 * KiB); + sysbus_init_mmio(sbd, &s->iomem); +} + +static Property allwinner_sid_properties[] = { + DEFINE_PROP_UUID_NODEFAULT("identifier", AwSidState, identifier), + DEFINE_PROP_END_OF_LIST() +}; + +static const VMStateDescription allwinner_sid_vmstate = { + .name = "allwinner-sid", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_UINT32(control, AwSidState), + VMSTATE_UINT32(rdkey, AwSidState), + VMSTATE_END_OF_LIST() + } +}; + +static void allwinner_sid_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + + dc->reset = allwinner_sid_reset; + dc->vmsd = &allwinner_sid_vmstate; + dc->props = allwinner_sid_properties; +} + +static const TypeInfo allwinner_sid_info = { + .name = TYPE_AW_SID, + .parent = TYPE_SYS_BUS_DEVICE, + .instance_init = allwinner_sid_init, + .instance_size = sizeof(AwSidState), + .class_init = allwinner_sid_class_init, +}; + +static void allwinner_sid_register(void) +{ + type_register_static(&allwinner_sid_info); +} + +type_init(allwinner_sid_register) diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs index 12c2c306b5..59500d5681 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-ccu.o obj-$(CONFIG_ALLWINNER_H3) += allwinner-cpucfg.o common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3-sysctrl.o +common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-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 d3e0952429..67d8bf493c 100644 --- a/hw/misc/trace-events +++ b/hw/misc/trace-events @@ -5,6 +5,10 @@ allwinner_cpucfg_cpu_reset(uint8_t cpu_id, uint32_t reset_addr) "id %u, reset_ad allwinner_cpucfg_read(uint64_t offset, uint64_t data, unsigned size) "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32 allwinner_cpucfg_write(uint64_t offset, uint64_t data, unsigned size) "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32 +# allwinner-sid.c +allwinner_sid_read(uint64_t offset, uint64_t data, unsigned size) "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32 +allwinner_sid_write(uint64_t offset, uint64_t data, unsigned size) "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"
The Security Identifier device found in various Allwinner System on Chip designs gives applications a per-board unique identifier. This commit adds support for the Allwinner Security Identifier using a 128-bit UUID value as input. Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com> --- include/hw/arm/allwinner-h3.h | 3 + include/hw/misc/allwinner-sid.h | 61 ++++++++++++ hw/arm/allwinner-h3.c | 11 ++- hw/arm/orangepi.c | 4 + hw/misc/allwinner-sid.c | 170 ++++++++++++++++++++++++++++++++ hw/misc/Makefile.objs | 1 + hw/misc/trace-events | 4 + 7 files changed, 253 insertions(+), 1 deletion(-) create mode 100644 include/hw/misc/allwinner-sid.h create mode 100644 hw/misc/allwinner-sid.c