On Mon, Dec 30, 2019 at 3:49 PM Philippe Mathieu-Daudé <phi...@redhat.com> wrote:
> On 12/18/19 9:49 PM, Niek Linnenbank wrote: > > Hi Philippe, > > > > On Tue, Dec 17, 2019 at 8:45 AM Philippe Mathieu-Daudé > > <phi...@redhat.com <mailto:phi...@redhat.com>> wrote: > > > > Hi Niek, > > > > On 12/17/19 12:35 AM, Niek Linnenbank wrote: > > > The Security Identifier device in Allwinner H3 System on Chip > > > gives applications a per-board unique identifier. This commit > > > adds support for the Allwinner H3 Security Identifier using > > > a 128-bit UUID value as input. > > > > > > Signed-off-by: Niek Linnenbank <nieklinnenb...@gmail.com > > <mailto:nieklinnenb...@gmail.com>> > > > --- > > > include/hw/arm/allwinner-h3.h | 2 + > > > include/hw/misc/allwinner-h3-sid.h | 40 +++++++ > > > hw/arm/allwinner-h3.c | 7 ++ > > > hw/arm/orangepi.c | 4 + > > > hw/misc/allwinner-h3-sid.c | 179 > > +++++++++++++++++++++++++++++ > > > hw/misc/Makefile.objs | 1 + > > > hw/misc/trace-events | 4 + > > > 7 files changed, 237 insertions(+) > > > create mode 100644 include/hw/misc/allwinner-h3-sid.h > > > create mode 100644 hw/misc/allwinner-h3-sid.c > > > > > > diff --git a/include/hw/arm/allwinner-h3.h > > b/include/hw/arm/allwinner-h3.h > > > index 8128ae6131..c98c1972a6 100644 > > > --- a/include/hw/arm/allwinner-h3.h > > > +++ b/include/hw/arm/allwinner-h3.h > > > @@ -29,6 +29,7 @@ > > > #include "hw/misc/allwinner-h3-clk.h" > > > #include "hw/misc/allwinner-h3-cpucfg.h" > > > #include "hw/misc/allwinner-h3-syscon.h" > > > +#include "hw/misc/allwinner-h3-sid.h" > > > #include "target/arm/cpu.h" > > > > > > enum { > > > @@ -77,6 +78,7 @@ typedef struct AwH3State { > > > AwH3ClockState ccu; > > > AwH3CpuCfgState cpucfg; > > > AwH3SysconState syscon; > > > + AwH3SidState sid; > > > GICState gic; > > > MemoryRegion sram_a1; > > > MemoryRegion sram_a2; > > > diff --git a/include/hw/misc/allwinner-h3-sid.h > > b/include/hw/misc/allwinner-h3-sid.h > > > new file mode 100644 > > > index 0000000000..79c9a24459 > > > --- /dev/null > > > +++ b/include/hw/misc/allwinner-h3-sid.h > > > @@ -0,0 +1,40 @@ > > > +/* > > > + * Allwinner H3 Security ID emulation > > > + * > > > + * Copyright (C) 2019 Niek Linnenbank <nieklinnenb...@gmail.com > > <mailto:nieklinnenb...@gmail.com>> > > > + * > > > + * This program is free software: you can redistribute it and/or > > modify > > > + * it under the terms of the GNU General Public License as > > published by > > > + * the Free Software Foundation, either version 2 of the > License, or > > > + * (at your option) any later version. > > > + * > > > + * This program is distributed in the hope that it will be > useful, > > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > > + * GNU General Public License for more details. > > > + * > > > + * You should have received a copy of the GNU General Public > License > > > + * along with this program. If not, see > > <http://www.gnu.org/licenses/>. > > > + */ > > > + > > > +#ifndef HW_MISC_ALLWINNER_H3_SID_H > > > +#define HW_MISC_ALLWINNER_H3_SID_H > > > + > > > +#include "hw/sysbus.h" > > > +#include "qemu/uuid.h" > > > + > > > +#define TYPE_AW_H3_SID "allwinner-h3-sid" > > > +#define AW_H3_SID(obj) OBJECT_CHECK(AwH3SidState, (obj), > > TYPE_AW_H3_SID) > > > + > > > +typedef struct AwH3SidState { > > > + /*< private >*/ > > > + SysBusDevice parent_obj; > > > + /*< public >*/ > > > + > > > + MemoryRegion iomem; > > > + uint32_t control; > > > + uint32_t rdkey; > > > + QemuUUID identifier; > > > +} AwH3SidState; > > > + > > > +#endif > > > diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c > > > index 1a9748ab2e..ba34f905cd 100644 > > > --- a/hw/arm/allwinner-h3.c > > > +++ b/hw/arm/allwinner-h3.c > > > @@ -196,6 +196,9 @@ static void aw_h3_init(Object *obj) > > > > > > sysbus_init_child_obj(obj, "cpucfg", &s->cpucfg, > > sizeof(s->cpucfg), > > > TYPE_AW_H3_CPUCFG); > > > + > > > + sysbus_init_child_obj(obj, "sid", &s->sid, sizeof(s->sid), > > > + TYPE_AW_H3_SID); > > > > Here add a property alias: > > > > object_property_add_alias(obj, "identifier", > OBJECT(&s->sid), > > "identifier", &error_abort); > > > > > } > > > > > > static void aw_h3_realize(DeviceState *dev, Error **errp) > > > @@ -332,6 +335,10 @@ static void aw_h3_realize(DeviceState *dev, > > Error **errp) > > > qdev_init_nofail(DEVICE(&s->cpucfg)); > > > sysbus_mmio_map(SYS_BUS_DEVICE(&s->cpucfg), 0, > > s->memmap[AW_H3_CPUCFG]); > > > > > > + /* Security Identifier */ > > > + qdev_init_nofail(DEVICE(&s->sid)); > > > + sysbus_mmio_map(SYS_BUS_DEVICE(&s->sid), 0, > > s->memmap[AW_H3_SID]); > > > + > > > /* Universal Serial Bus */ > > > sysbus_create_simple(TYPE_AW_H3_EHCI, > s->memmap[AW_H3_EHCI0], > > > qdev_get_gpio_in(DEVICE(&s->gic), > > > diff --git a/hw/arm/orangepi.c b/hw/arm/orangepi.c > > > index 62cefc8c06..b01c4b4f01 100644 > > > --- a/hw/arm/orangepi.c > > > +++ b/hw/arm/orangepi.c > > > @@ -62,6 +62,10 @@ static void orangepi_init(MachineState > *machine) > > > exit(1); > > > } > > > > > > + /* Setup SID properties */ > > > + qdev_prop_set_string(DEVICE(&s->h3->sid), "identifier", > > > + "8100c002-0001-0002-0003-000044556677"); > > > > And here use the alias: > > > > qdev_prop_set_string(DEVICE(&s->h3), "identifier", > > > "8100c002-0001-0002-0003-000044556677"); > > > > > > Ah OK, I see what you mean. The boards should be using the SoC object > > only and > > not directly any of its sub devices, correct? > > > > > > What means this value? Don't you want to be able to set it from > command > > line? > > > > The first word 0x02c00081 is the identifying word for the H3 SoC in the > > SID data. > > After that come the per-device unique specific bytes. This is documented > > at the end of this page in 'Currently known SID's' on the > > linux-sunxi.org <http://linux-sunxi.org> Wiki: > > https://linux-sunxi.org/SID_Register_Guide > > > > The remaining parts of this value I simply made up without any real > meaning. > > And yes, it would in fact make sense to have the user be able to > > override it from the command line. > > It is used by U-boot as an input for generating the MAC address. Linux > > also reads it, but I did not investigate > > how it us used there. I think I did make a TODO of using a cmdline > > argument, but later forgot to actually implement it. > > > > Do you have a suggestion how to best provide the command line argument? > > I do see '-device driver[,prop=value]' > > is there in the --help for qemu-system-arm, but it looks like that > > should be used by the user for adding PCI / USB devices? > > Look for '-global' in the manpage: > > > -global driver.prop=value > -global driver=driver,property=property,value=value > > Set default value of driver's property prop to value. > > In particular, you can use this to set driver properties > for devices which are created automatically by the machine > model. To create a device which is not created automatically > and set properties on it, use -device. > > -global driver.prop=value is shorthand for > -global driver=driver,property=prop,value=value. > > So this should work for your device: > > -global > allwinner-h3-sid.identifier=8100c002-0001-0002-0003-000044556677 > Thanks, I tried this, however currently its not applied when used. I think its because the orangepi.c file sets the property as well. Its not a blocking problem if the user can't override the SID value, but it would be a nice-to-have. U-Boot uses its value for MAC generation and that can be overridden as well by U-Boot itself. > > > > > > > /* Mark H3 object realized */ > > > object_property_set_bool(OBJECT(s->h3), true, "realized", > > &error_abort); > > > if (error_abort != NULL) { > > > diff --git a/hw/misc/allwinner-h3-sid.c > b/hw/misc/allwinner-h3-sid.c > > > new file mode 100644 > > > index 0000000000..c472f2bcc6 > > > --- /dev/null > > > +++ b/hw/misc/allwinner-h3-sid.c > > > @@ -0,0 +1,179 @@ > > > +/* > > > + * Allwinner H3 Security ID emulation > > > + * > > > + * Copyright (C) 2019 Niek Linnenbank <nieklinnenb...@gmail.com > > <mailto:nieklinnenb...@gmail.com>> > > > + * > > > + * This program is free software: you can redistribute it and/or > > modify > > > + * it under the terms of the GNU General Public License as > > published by > > > + * the Free Software Foundation, either version 2 of the > License, or > > > + * (at your option) any later version. > > > + * > > > + * This program is distributed in the hope that it will be > useful, > > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > > + * GNU General Public License for more details. > > > + * > > > + * You should have received a copy of the GNU General Public > License > > > + * along with this program. If not, see > > <http://www.gnu.org/licenses/>. > > > + */ > > > + > > > +#include "qemu/osdep.h" > > > +#include "qemu/units.h" > > > +#include "hw/sysbus.h" > > > +#include "migration/vmstate.h" > > > +#include "qemu/log.h" > > > +#include "qemu/module.h" > > > +#include "qemu/guest-random.h" > > > +#include "qapi/error.h" > > > +#include "hw/qdev-properties.h" > > > +#include "hw/misc/allwinner-h3-sid.h" > > > +#include "trace.h" > > > + > > > +/* SID register offsets */ > > > +enum { > > > + REG_PRCTL = 0x40, /* Control */ > > > + REG_RDKEY = 0x60, /* Read Key */ > > > +}; > > > + > > > +/* SID register flags */ > > > +enum { > > > + REG_PRCTL_WRITE = 0x0002, /* Unknown write flag */ > > > + REG_PRCTL_OP_LOCK = 0xAC00, /* Lock operation */ > > > +}; > > > + > > > +static uint64_t allwinner_h3_sid_read(void *opaque, hwaddr > offset, > > > + unsigned size) > > > +{ > > > + const AwH3SidState *s = (AwH3SidState *)opaque; > > > + uint64_t val = 0; > > > + > > > + switch (offset) { > > > + case REG_PRCTL: /* Control */ > > > + val = s->control; > > > + break; > > > + case REG_RDKEY: /* Read Key */ > > > + val = s->rdkey; > > > + break; > > > + default: > > > + qemu_log_mask(LOG_GUEST_ERROR, "%s: bad read offset > > 0x%04x\n", > > > + __func__, (uint32_t)offset); > > > + return 0; > > > + } > > > + > > > + trace_allwinner_h3_sid_read(offset, val, size); > > > + > > > + return val; > > > +} > > > + > > > +static void allwinner_h3_sid_write(void *opaque, hwaddr offset, > > > + uint64_t val, unsigned size) > > > +{ > > > + AwH3SidState *s = (AwH3SidState *)opaque; > > > + > > > + trace_allwinner_h3_sid_write(offset, val, size); > > > + > > > + switch (offset) { > > > + case REG_PRCTL: /* Control */ > > > + s->control = val; > > > + > > > + if ((s->control & REG_PRCTL_OP_LOCK) && > > > + (s->control & REG_PRCTL_WRITE)) { > > > + uint32_t id = s->control >> 16; > > > + > > > + if (id < sizeof(QemuUUID)) { > > > + s->rdkey = (s->identifier.data[id]) | > > > + (s->identifier.data[id + 1] << 8) | > > > + (s->identifier.data[id + 2] << 16) | > > > + (s->identifier.data[id + 3] << 24); > > > > This is: > > > > s->rdkey = ldl_le_p(&s->identifier.data[id]); > > > > > + } > > > + } > > > + s->control &= ~REG_PRCTL_WRITE; > > > + break; > > > + case REG_RDKEY: /* Read Key */ > > > > Read in a write()? > > > > Maybe we can simply /* fall through */ LOG_GUEST_ERROR? > > > > > > When writing this module, I looked at how U-Boot is using the SID > > registers and simply > > named the registers after the names used by U-Boot. You can find this > > part in arch/arm/mach-sunxi/cpu_info.c:111, > > functions sun8i_efuse_read() and sunxi_get_sid(). U-Boot defines > > SIDC_RDKEY, so I named the register also rdkey. > > I used the U-Boot source because the Allwinner H3 datasheet does not > > document the registers. Later I > > found the SID page on the linux-sunxi wiki that I mentioned earlier, and > > they also describe the same register names: > > > > https://linux-sunxi.org/SID_Register_Guide > > Hmm this page describe this register as RW, odd. I think this is wrong > because we deal with a fuse, and we program it via the REG_PRKEY > register. Does Linux/U-Boot do write access to this register? > We can start logging LOG_GUEST_ERROR, and correct it if we notice this > register is really writable (which I doubt). > > Yes it seems so. The PRCTL is the "Control" register (R/W) and the RDKEY the "Data" register (RO). You can see the Linux implementation in drivers/nvmem/sunxi_sid.c For U-Boot the file is in arch/arm/mach-sunxi/cpu_info.c, functions sunxi_get_sid and sun8i_efuse_read. Regards, Niek > > > > I suspect the information on this page is written based on the source > > code from the original SDK (which I did not study btw) > [...] > > -- Niek Linnenbank
