On Wed, Jan 31, 2018 at 9:10 AM, Philippe Mathieu-Daudé <f4...@amsat.org> wrote:
> On 01/15/2018 10:37 PM, Andrey Smirnov wrote:
>> Add code to emulate SNVS IP-block. Currently only the bits needed to
>> be able to emulate machine shutdown are implemented.
>>
>> Cc: Peter Maydell <peter.mayd...@linaro.org>
>> Cc: Jason Wang <jasow...@redhat.com>
>> Cc: Philippe Mathieu-Daudé <f4...@amsat.org>
>> Cc: qemu-devel@nongnu.org
>> Cc: qemu-...@nongnu.org
>> Cc: yurov...@gmail.com
>> Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
>> Signed-off-by: Andrey Smirnov <andrew.smir...@gmail.com>
>> ---
>> hw/misc/Makefile.objs | 1 +
>> hw/misc/imx7_snvs.c | 83
>> +++++++++++++++++++++++++++++++++++++++++++++
>> include/hw/misc/imx7_snvs.h | 35 +++++++++++++++++++
>> 3 files changed, 119 insertions(+)
>> create mode 100644 hw/misc/imx7_snvs.c
>> create mode 100644 include/hw/misc/imx7_snvs.h
>>
>> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
>> index 4b2b705a6c..019886912c 100644
>> --- a/hw/misc/Makefile.objs
>> +++ b/hw/misc/Makefile.objs
>> @@ -35,6 +35,7 @@ obj-$(CONFIG_IMX) += imx6_ccm.o
>> obj-$(CONFIG_IMX) += imx6_src.o
>> obj-$(CONFIG_IMX) += imx7_ccm.o
>> obj-$(CONFIG_IMX) += imx2_wdt.o
>> +obj-$(CONFIG_IMX) += imx7_snvs.o
>> obj-$(CONFIG_MILKYMIST) += milkymist-hpdmc.o
>> obj-$(CONFIG_MILKYMIST) += milkymist-pfpu.o
>> obj-$(CONFIG_MAINSTONE) += mst_fpga.o
>> diff --git a/hw/misc/imx7_snvs.c b/hw/misc/imx7_snvs.c
>> new file mode 100644
>> index 0000000000..670b9f4639
>> --- /dev/null
>> +++ b/hw/misc/imx7_snvs.c
>> @@ -0,0 +1,83 @@
>> +/*
>> + * IMX7 Secure Non-Volatile Storage
>> + *
>> + * Copyright (c) 2017, Impinj, Inc.
>> + *
>> + * Author: Andrey Smirnov <andrew.smir...@gmail.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + *
>> + * Bare minimum emulation code needed to support being able to shut
>> + * down linux guest gracefully.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "hw/misc/imx7_snvs.h"
>> +#include "qemu/log.h"
>> +#include "sysemu/sysemu.h"
>> +
>> +static uint64_t imx7_snvs_read(void *opaque, hwaddr offset, unsigned size)
>> +{
>> + return 0;
>> +}
>> +
>> +static void imx7_snvs_write(void *opaque, hwaddr offset,
>> + uint64_t v, unsigned size)
>> +{
>> + const uint32_t value = v;
>> + const uint32_t mask = SNVS_LPCR_TOP | SNVS_LPCR_DP_EN;
>> +
>> + if (offset == SNVS_LPCR && ((value & mask) == mask)) {
>> + qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
>> + }
>> +}
>> +
>> +static const struct MemoryRegionOps imx7_snvs_ops = {
>> + .read = imx7_snvs_read,
>
> Same here, you can remove the imx7_snvs_read() function since
> memory_region_dispatch_read() takes care of this and return 0.
>
Hmm, I don't think I agree both from reading code and trying this out.
Without .read callback the call chain ends up being the following
memory_region_dispatch_read() -> memory_region_dispatch_read1() ->
access_with_adjusted_size(..., memory_region_oldmmio_read_accessor,
...) -> memory_region_oldmmio_read_accessor() ->
mr->ops->old_mmio.read[ctz32(size)]() -> SEGFAULT
As much as I'd love to get rid of dummy .read callback, I don't see a
way to do that, unfortunately.
Thanks,
Andrey Smirnov
