On 6/17/19 7:53 PM, Peter Maydell wrote: > Like most of the v7M memory mapped system registers, the systick > registers are accessible to privileged code only and user accesses > must generate a BusFault. We implement that for registers in > the NVIC proper already, but missed it for systick since we > implement it as a separate device. Correct the omission. > > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> > --- > hw/timer/armv7m_systick.c | 26 ++++++++++++++++++++------ > 1 file changed, 20 insertions(+), 6 deletions(-) > > diff --git a/hw/timer/armv7m_systick.c b/hw/timer/armv7m_systick.c > index a17317ce2fe..94640743b5d 100644 > --- a/hw/timer/armv7m_systick.c > +++ b/hw/timer/armv7m_systick.c > @@ -75,11 +75,17 @@ static void systick_timer_tick(void *opaque) > } > } > > -static uint64_t systick_read(void *opaque, hwaddr addr, unsigned size) > +static MemTxResult systick_read(void *opaque, hwaddr addr, uint64_t *data, > + unsigned size, MemTxAttrs attrs) > { > SysTickState *s = opaque; > uint32_t val; > > + if (attrs.user) { > + /* Generate BusFault for unprivileged accesses */ > + return MEMTX_ERROR; > + } > + > switch (addr) { > case 0x0: /* SysTick Control and Status. */ > val = s->control; > @@ -121,14 +127,21 @@ static uint64_t systick_read(void *opaque, hwaddr addr, > unsigned size) > } > > trace_systick_read(addr, val, size); > - return val; > + *data = val; > + return MEMTX_OK; > } > > -static void systick_write(void *opaque, hwaddr addr, > - uint64_t value, unsigned size) > +static MemTxResult systick_write(void *opaque, hwaddr addr, > + uint64_t value, unsigned size, > + MemTxAttrs attrs) > { > SysTickState *s = opaque; > > + if (attrs.user) { > + /* Generate BusFault for unprivileged accesses */ > + return MEMTX_ERROR; > + } > + > trace_systick_write(addr, value, size); > > switch (addr) { > @@ -172,11 +185,12 @@ static void systick_write(void *opaque, hwaddr addr, > qemu_log_mask(LOG_GUEST_ERROR, > "SysTick: Bad write offset 0x%" HWADDR_PRIx "\n", > addr); > } > + return MEMTX_OK; > } > > static const MemoryRegionOps systick_ops = { > - .read = systick_read, > - .write = systick_write, > + .read_with_attrs = systick_read, > + .write_with_attrs = systick_write, > .endianness = DEVICE_NATIVE_ENDIAN, > .valid.min_access_size = 4, > .valid.max_access_size = 4, >
Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com>