The 'sfence.vma' instruction is privileged, and should only ever be allowed
when executing in supervisor mode or higher.
Jonathan
Signed-off-by: Jonathan Behrens
---
target/riscv/op_helper.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/target/riscv/op_helper.c b
Public bug reported:
QEMU 3.1 incorrectly decodes the "c.unimp" instruction (opcode 0x)
as an "addi4spn fp, 0" when either of the two following bytes are non-
zero. This is because the ctx->opcode value used when decoding the
instruction is actually filled with a 32-bit load (to handle normal
Public bug reported:
I attempted to run qemu with a ram disk. However, when reading the
contents of the disk from within the VM I only get back zeros.
I was able to trace the issue to a mismatch of expectations on line 93
of hw/riscv/virt.c. Specifically, when running in 32-bit mode the value
of
Signed-off-by: Jonathan Behrens
---
hw/riscv/virt.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index fc4c6b306e..3526463034 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -29,7 +29,6 @@
#include "hw/sysbus.h"
#include "h
be open to other suggestions.
Jonathan
On Mon, Apr 15, 2019 at 8:23 PM Alistair Francis
wrote:
> On Fri, Apr 12, 2019 at 12:04 PM Jonathan Behrens
> wrote:
> >
> > Currently mcounteren.TM acts as though it is hardwired to zero, even
> though
> > QEMU
> >
Currently mcounteren.TM acts as though it is hardwired to zero, even though
QEMU
allows it to be set. This change resolves the issue by allowing reads to the
time and timeh control registers when running in a privileged mode where
such
accesses are allowed.
Signed-off-by: Jonathan Behrens
Just to double check, nothing further on this is need from me, right? It is
set to be merged onto the master branch once the 4.0 release is out?
Jonathan
On Wed, Apr 3, 2019 at 7:11 PM Alistair Francis
wrote:
> On Mon, Apr 1, 2019 at 1:39 PM Jonathan Behrens
> wrote:
> >
> &g
Sure, I've just sent a corrected version.
Thanks,
Jonathan
On Tue, May 28, 2019 at 1:09 PM Palmer Dabbelt wrote:
> On Tue, 30 Apr 2019 10:36:01 PDT (-0700), finte...@gmail.com wrote:
> > Currently mcounteren.TM acts as though it is hardwired to zero, even
> though
> > QEMU allows it to be set.
is stored in the time_freq field of each
hart so that it is accessible during CSR reads, but must be the same across all
harts. Each board can initialize it to a custom value, although all currently
use a 10 MHz frequency.
Signed-off-by: Jonathan Behrens
---
hw/riscv/riscv_hart.c | 4
not?) which would leave only the firmware only and firmware + kernel
variants. What do other people think about this?
Jonathan
On Mon, May 20, 2019 at 12:56 PM Alistair Francis
wrote:
> On Sat, May 18, 2019 at 2:57 PM Jonathan Behrens
> wrote:
> >
> > > I've never
in time for 4.1.
On Fri, Jun 14, 2019 at 8:55 AM Jonathan Behrens
wrote:
> I'm not sure that is accurate. Based on the discussion here
> <https://forums.sifive.com/t/possible-bug-in-counteren-csrs/2123> the
> HiFive Unleashed actually does support reading the timer CSR from
> unpriv
or something simpler that just
> hard
> > codes mcounteren.TM to zero (so QEMU is at least conformant) can be
> merged
> > in time for 4.1.
> >
> > On Fri, Jun 14, 2019 at 8:55 AM Jonathan Behrens
> > wrote:
> >
> >> I'm not sure that is accurate. Bas
I was actually just writing up the same thing. Shifting all the addresses
in the ELF file by 2 or 4MB is somewhat surprising behavior, especially
because this will apply to segments that are mapped even at much higher
addresses. If you want a segment aligned to a 1GB superpage boundary you
now
Signed-off-by: Jonathan Behrens
---
hw/net/cadence_gem.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 7f63411430..37cb8a4e5c 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -146,7 +146,6 @@
#define GEM_DESCONF7
> I've never been fully convinced of this, why not just use the generic
loader?
If I understand you are proposing passing bbl (or other firmware) with the
-kernel flag, and then vmlinux (or another kernel) with the -initrd flag?
Wouldn't this result in losing the ability to pass a real init
This patch assumes that translation failure should always raise a paging
fault, but it should be possible for it to raise an access fault as well
(since according to the spec "PMP checks are also applied to
page-table accesses for virtual-address translation, for which the
effective
Hesham,
I don't think this is quite right. If I understand correctly, PMP
permissions are only validated on TLB fills, not on all accesses. (Is
anyone able to confirm this?) If so, this function can't just validate the
range of a single access and then place the entire page into the TLB.
However,
Jonathan Behrens (2):
target/riscv: virt machine shouldn't assume ELF entry is DRAM base
target/riscv: Add support for -bios "firmware_filename" flag
hw/riscv/virt.c | 93 -
1 file changed, 69 insertions(+), 24 deletions(-)
--
2.20.1
and kernel binaries by
passing both the -bios and -kernel flags.
This is based on a previously proposed patch by Michael Clark:
https://patchwork.kernel.org/patch/10419975/
Signed-off-by: Jonathan Behrens
---
hw/riscv/virt.c | 66 -
1 file changed, 55
be to somehow use the kernel_high address in that computation
instead.
Signed-off-by: Jonathan Behrens
---
hw/riscv/virt.c | 33 +
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index fc4c6b306e..87cc08016b 100644
Currently mcounteren.TM acts as though it is hardwired to zero, even though
QEMU allows it to be set. This change resolves the issue by allowing reads
to the time and timeh control registers when running in a privileged mode
where such accesses are allowed.
The frequency of the time register is
p
> > register are read-only." Further, if an interrupt is not delegated to
> mode
> > x,
> > then "the corresponding bits in xip [...] should appear to be hardwired
> to
> > zero. This patch implements both of those requirements.
> >
> > Signed-off-by: Jo
ts.
Signed-off-by: Jonathan Behrens
---
target/riscv/csr.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 1ec1222da1..fff7d834e8 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -246,6 +246,7 @@ static con
There is an analogous change for ARM here:
https://patchwork.kernel.org/patch/10649857
---
target/riscv/csr.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 6083c782a1..1ec1222da1 100644
--- a/target/riscv/csr.c
+++
Argh, meant to include a signed off by line:
Signed-off-by: Jonathan Behrens
On Mon, May 6, 2019 at 11:31 AM Jonathan Behrens wrote:
> There is an analogous change for ARM here:
> https://patchwork.kernel.org/patch/10649857
> ---
> target/riscv/csr.c | 4 +++-
> 1 file change
ts.
Signed-off-by: Jonathan Behrens
---
target/riscv/csr.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 1ec1222da1..fff7d834e8 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -246,6 +246,7 @@ static con
There is an analogous change for ARM here:
https://patchwork.kernel.org/patch/10649857
Signed-off-by: Jonathan Behrens
---
target/riscv/csr.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 6083c782a1..1ec1222da1 100644
I think this patch is slightly incorrect. If the PMP region is valid for
the size of the access, but not the rest of the page then a few lines down
in this function the entire page should not be placed into the TLB. Instead
only the portion of the page that passed the access check should be
l 2, 2019 at 8:20 AM Alistair Francis
> wrote:
> >
> > On Mon, Jul 1, 2019 at 8:56 AM wrote:
> > >
> > > From: Jonathan Behrens
> > >
> > > QEMU currently always triggers an illegal instruction exception when
> > > code attempts to read th
Signed-off-by: Jonathan Behrens
---
target/riscv/op_helper.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 331cc36232..2e5a980192 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
Unless I'm missing something, the virt board doesn't support HTIF and
should not be including this header.
Jonathan
Signed-off-by: Jonathan Behrens
---
hw/riscv/virt.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index fc4c6b306e..3526463034 100644
should have a new version out in the next few days.
On Thu, Apr 25, 2019 at 5:44 PM Palmer Dabbelt wrote:
> On Fri, 19 Apr 2019 16:05:35 PDT (-0700), alistai...@gmail.com wrote:
> > On Mon, Apr 15, 2019 at 5:46 PM Jonathan Behrens
> wrote:
> >>
> >> For any
e named counters but not the
remaining 29 unnamed "high performance counters".
On Fri, Jun 28, 2019 at 5:03 PM Alistair Francis
wrote:
> On Fri, Jun 28, 2019 at 1:12 PM wrote:
> >
> > From: Jonathan Behrens
> >
> > QEMU currently always triggers an illegal instruction
5 Jun 2019 23:58:34 PDT (-0700), bmeng...@gmail.com wrote:
> > >> > On Wed, Jun 26, 2019 at 4:23 AM Jonathan Behrens <
> finte...@gmail.com> wrote:
> > >> >>
> > >> >> I just did some testing on a HiFive Unleashed board and can
> confirm what
Ping! What is the status of this patch?
On Wed, Jul 3, 2019 at 2:02 PM Jonathan Behrens
wrote:
> Bin, that proposal proved to be somewhat more controversial than I was
> expecting, since it was different than how currently available hardware
> worked. This option seemed much mo
Is there a reason to guarantee support of a particular draft extension
version once it has been superseded by a subsequent version? I understand
why it was done for priv-1.9.1, but going forward I'm skeptical there will
be much/any code out in the wild that depends on old draft versions of
Has there been testing with "-smp 2"? A while back I thought I read that
the included uboot firmware was using a hard-coded device tree that
indicated 4+1 CPUs, which I would have expected to cause Linux boot issues?
Jonathan
On Sun, Sep 15, 2019 at 1:31 PM Palmer Dabbelt wrote:
> On Sun, 15
=1, I think the result would actually be more correct this
way: the bits of mie are supposed to always reflect sie rather than vsie).
Jonathan
On Tue, Sep 17, 2019 at 7:37 PM Alistair Francis
wrote:
> On Wed, Sep 11, 2019 at 7:55 AM Jonathan Behrens
> wrote:
> >
>
Version 0.4 of the hypervisor spec no longer talks about swapping
registers. Instead when running in VS-mode some of the supervisor registers
are "aliased" and actually refer to alternate versions. Implementations are
of course still allowed to do swapping internally if desired, but it adds
I'm not familiar with QEMU conventions on this, but would it make sense to
require having exactly 5 CPUs to match the real board?
Jonathan
On Mon, Aug 5, 2019 at 12:05 PM Bin Meng wrote:
> It is not useful if we only have one management CPU.
>
> Signed-off-by: Bin Meng
> ---
>
>
riority=7 is now assumed.
Reviewed-by: Jonathan Behrens
On Mon, Aug 5, 2019 at 12:10 PM Bin Meng wrote:
> This removes "reg-names" and "riscv,max-priority" properties of the
> PLIC node from device tree, and updates its compatible string, to
> keep in sync with
Reviewed-by: Jonathan Behrens
On Mon, Aug 5, 2019 at 12:07 PM Bin Meng wrote:
> This updates the UART base address to match the hardware.
>
> Signed-off-by: Bin Meng
> ---
>
> hw/riscv/sifive_u.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
Unless I'm missing something, this is the only place that QEMU checks the
privilege level for read and writes to CSRs. The exact computation used
here won't work with the hypervisor extension, but we also can't just get
rid of privilege checking entirely...
Jonathan
On Thu, Jul 25, 2019 at 2:56
Reviewed-by: Jonathan Behrens
On Thu, Jul 25, 2019 at 2:56 PM Alistair Francis
wrote:
> Signed-off-by: Alistair Francis
> ---
> hw/riscv/sifive_plic.c | 12
> include/hw/riscv/sifive_plic.h | 3 ---
> 2 files changed, 15 deletions(-)
>
>
M-mode.
I also tested by booting RVirt <https://github.com/mit-pdos/RVirt> with a
Linux guest and found that this patch caused it to instantly crash because
guest CSR accesses weren't intercepted.
Jonathan
On Fri, Jul 26, 2019 at 4:28 PM Alistair Francis
wrote:
> On Thu, Jul 25, 2019
On Thu, Sep 19, 2019 at 10:50 AM Richard Henderson
wrote:
>
> On 9/18/19 4:47 PM, Alistair Francis wrote:
> > I'm not a fan of the pointer method that I'm using, but to me it seems
> > the least worst in terms of handling future code, keeping everythign
> > consistnent and avoiding complex access
This patch enables a debugger to read and write the current privilege level via
a special "priv" register. When compiled with CONFIG_USER_ONLY the register is
still visible but is hardwired to zero.
Signed-off-by: Jonathan Behrens
---
gdb-xml/riscv-32bit-cpu.xml | 1 +
gdb-xml/r
Any code whose behavior is changed by this patch is wrong, so it doesn't
seem like it matters much whether this is merged or not. Personally I'd
lean towards making sure that attempts to use PTEs with the reserved bit
set would always fault, rather than wrapping around to low memory and
perhaps
> The specification is very clear: these bits are not part of ppn, not
> part of the translation target address. The current code is against
> the riscv-privilege specification.
If all of the reserved bits are zero then the patch changes nothing.
Further the only normative mention of the reserved
3 [Machine]
Changlog V3:
- Break patch into series
- Make priv a virtual register
Changelog V2:
- Use PRV_H and PRV_S instead of integer literals
Jonathan Behrens (3)
target/riscv: Tell gdbstub the correct number of CSRs
target/riscv: Expose priv register for GDB for reads
target/riscv: Mak
On Mon, Oct 7, 2019 at 2:36 PM Alistair Francis wrote:
> On Fri, Oct 4, 2019 at 8:18 AM Jonathan Behrens wrote:
> > @@ -296,6 +302,14 @@ int riscv_cpu_gdb_write_register(CPUState *cs, uint8_t
> > *mem_buf, int n)
> > } else if (n == 32) {
> >
On Mon, Oct 7, 2019 at 5:17 PM Jim Wilson wrote:
> On 10/4/19 8:16 AM, Jonathan Behrens wrote:
> > diff --git a/gdb-xml/riscv-32bit-cpu.xml b/gdb-xml/riscv-32bit-cpu.xml
> > index 0d07aaec85..d6d76aafd8 100644
> > --- a/gdb-xml/riscv-32bit-cpu.xml
> > +++ b/
-by: Jonathan Behrens
---
target/riscv/gdbstub.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index 33cf7c4c7d..bc84b599c2 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -387,6 +387,15 @@ static int
This patch enables a debugger to read the current privilege level via a virtual
"priv" register. When compiled with CONFIG_USER_ONLY the register is still
visible but always reports the value zero.
Signed-off-by: Jonathan Behrens
---
configure | 4 ++--
gdb
On Mon, Oct 7, 2019 at 8:15 PM Jonathan Behrens wrote:
>
> The third patch in this series makes the priv virtual register writitable. I'm
> not entirely sure this is a good idea, so I split it out into its own patch.
> In
> particular, this change will conflict with the hype
If the number of registers reported to the gdbstub code does not match the
number in the associated XML file, then the register numbers used by the stub
may get out of sync with a remote GDB instance.
Signed-off-by: Jonathan Behrens
---
target/riscv/gdbstub.c | 4 ++--
1 file changed, 2
On Tue, Oct 8, 2019 at 5:54 AM Bin Meng wrote:
>
> On Tue, Oct 8, 2019 at 8:15 AM Jonathan Behrens wrote:
> >
> > If the number of registers reported to the gdbstub code does not match the
> > number in the associated XML file, then the register numbers used by the
On Tue, Oct 8, 2019 at 8:27 AM Bin Meng wrote:
>
> On Tue, Oct 8, 2019 at 8:18 AM Jonathan Behrens wrote:
> >
> > This patch enables a debugger to read the current privilege level via a
> > virtual
> > "priv" register. When compiled with CONFIG_USE
This patch enables a debugger to read and write the current privilege level via
a special "priv" register. When compiled with CONFIG_USER_ONLY the register is
still visible but is hardwired to zero.
Signed-off-by: Jonathan Behrens
---
gdb-xml/riscv-32bit-cpu.xml | 1 +
gdb-xml/r
How do you know that the access won't straddle a page boundary? Is there a
guarantee somewhere that size=0 means that the access is naturally aligned?
Jonathan
On Fri, Oct 11, 2019 at 7:14 PM Dayeol Lee wrote:
> riscv_cpu_tlb_fill() uses the `size` parameter to check PMP violation
> using
There is nowhere in the spec that ever says what hardware has to do if
any of those reserved bits are non-zero. Hardware is certainly not
required to ignore them and treat the PTE as being valid (which is
what this patch does). I'd argue that since only buggy code would ever
set these bits, QEMU
-by: Jonathan Behrens
Reviewed-by: Bin Meng
Tested-by: Bin Meng
Reviewed-by: Alistair Francis
---
target/riscv/gdbstub.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index 1f71604b78..1a7947e019 100644
--- a/target/riscv/gdbstub.c
Make priv a virtual register
Changelog V2:
- Use PRV_H and PRV_S instead of integer literals
Jonathan Behrens (3)
target/riscv: Tell gdbstub the correct number of CSRs
target/riscv: Expose priv register for GDB for reads
target/riscv: Make the priv register writable by GDB
configure
If the number of registers reported to the gdbstub code does not match the
number in the associated XML file, then the register numbers used by the stub
may get out of sync with a remote GDB instance.
Signed-off-by: Jonathan Behrens
Reviewed-by: Bin Meng
Reviewed-by: Alistair Francis
This patch enables a debugger to read the current privilege level via a virtual
"priv" register. When compiled with CONFIG_USER_ONLY the register is still
visible but always reports the value zero.
Signed-off-by: Jonathan Behrens
---
configure | 4 ++--
gdb
When I looked through the relevant code a few months ago, I couldn't find
anywhere that could actually be agnostic to whether the real or virtual
registers were in effect (other than emulating the actual CSR modification
instructions). For almost all state, the VS behavior is filtered by HS-mode
PM Palmer Dabbelt wrote:
> On Wed, 14 Aug 2019 20:19:39 PDT (-0700), jonat...@fintelia.io wrote:
> > Ping! What is the status of this patch?
>
> Sorry, I must have lost track of it. I've added it to my patch queue.
>
> >
> > On Wed, Jul 3, 2019 at 2:02 PM J
This series doesn't seem to touch mcounteren.TM which should be hardwired
to zero if no callback in provided, and writable otherwise. (I had a patch
to do the hardwiring unconditionally, but it seems to have been
accidentally dropped.) Other than that, I think the design is quite good.
Jonathan
Looks good to me. Though this is I think the third bug in privilege
checking in op_helper.c which is only like 150 lines long total. It would
be really good to fully double check that there aren't any more lurking
there...
Reviewed-by: Jonathan Behrens >
On Tue, Jan 21, 2020 at 12:45 AM Alist
I don't think this was ever merged?
On Wed, Jul 3, 2019 at 10:37 PM wrote:
> Patchew URL:
> https://patchew.org/QEMU/20190703190715.5328-1-jonat...@fintelia.io/
>
>
>
> Hi,
>
> This series failed the asan build test. Please find the testing commands
> and
> their output below. If you have
new version with that changed.
Jonathan
On Thu, Jan 23, 2020 at 6:35 PM Richard Henderson <
richard.hender...@linaro.org> wrote:
> On 1/23/20 9:52 AM, Jonathan Behrens wrote:
> > +if (!(env->priv >= PRV_S) ||
>
> For integers, !(x >= y) is a poor way to write x < y.
>
>
> r~
>
>
The text you are referencing (the couple italic paragraphs below section
2.8 in the unprivileged ISA) is non-normative and "can be skipped if the
reader is only interested in the specification itself". This convention of
making indented italic text non-normative is described at the bottom of
page
s S-mode to selectively permit WFI in U-mode."
Signed-off-by: Jonathan Behrens
Reviewed-by: Alistair Francis
---
target/riscv/op_helper.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 331cc36232..2e5a
tand anything. However, I'd attribute that more to
luck than elegance of a particular option.
Jonathan
On Tue, Jan 21, 2020 at 7:01 PM Alistair Francis
wrote:
> On Tue, Jan 21, 2020 at 10:56 PM Jonathan Behrens
> wrote:
> >
> > When I looked through the relevant code a few mon
Yeah, I've run into the same issue. Even if you ask Gmail to treat the
email as plain text it still takes the liberty of hard wrapping lines
without asking you. It really is a shame that there isn't a good way to
submit patches via a web UI, the art of git send-email + manual SMTP isn't
as
75 matches
Mail list logo
