| Message ID | 20190617175317.27557-6-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Six minor M-profile bugfixes | expand |
On 6/17/19 10:53 AM, 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.maydell@linaro.org> > --- > hw/timer/armv7m_systick.c | 26 ++++++++++++++++++++------ > 1 file changed, 20 insertions(+), 6 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
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.maydell@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é <philmd@redhat.com>
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,
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.maydell@linaro.org> --- hw/timer/armv7m_systick.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) -- 2.20.1