Skip to content
This repository has been archived by the owner on Apr 13, 2019. It is now read-only.

Gdb support v2 #182

Closed
wants to merge 92 commits into from
Closed

Conversation

jim-wilson
Copy link
Contributor

The FSF GDB port now has proper target description support, aka XML register set files. This patch copies the FSF GDB xml files into qemu to replace my temporary files. It also adds the 32-bit support and the CSR support that was missing in my first part patch.

There is one odd thing here which is that the CSR xml file lists them in documentation order not numerical order, and has both v1.9.1 and v1.10 priv spec numbers. I suspect it was generated from a binutils header file. I had to clean up the existing list of CSRs to include all of the ones that GDB expects, and I had to add a table to map the CSR xml register numbers to the hardware register numbers. This table is a little awkward. If this file changes in a future gdb version, then we will need corresponding qemu changes to match.

stsquad and others added 30 commits September 17, 2018 12:37
The .gitignore was being a little over enthusiastic hiding files.

Signed-off-by: Alex Bennée <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
We need both git and a working compiler to build the tools. Although
the qemu:debian9 image also has a bunch of extra dependencies it would
be fairly unusual for a user not to already have this layer available
for one of our many other docker images so lets not complicate things.

Signed-off-by: Alex Bennée <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
This image isn't going to build anything significant as it is just
intended for building test cases. In case it does end up getting
inadvertently included in a build lets aim for the minimal possible
product.

Signed-off-by: Alex Bennée <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
As this is called directly from the Makefile while determining
dependencies and it is possible the user was configured in one window
but not have credentials in the other. Let's catch the Exceptions and
deal with it quietly.

Signed-off-by: Alex Bennée <[email protected]>
Reported-by: Peter Maydell <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
This allows some tests that just want to configure QEMU's source tree
to do so.

Signed-off-by: Alex Bennée <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Tested-by: Philippe Mathieu-Daudé <[email protected]>
Not all docker images can run the check step. Let's move everything
into a common helper so we don't need to replicate checks in the
future.

Signed-off-by: Alex Bennée <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Tested-by: Philippe Mathieu-Daudé <[email protected]>
Not all our images are able to run the tests. Rather than use features
we can just check for the existence and run-ability of gtester. If the
image has been setup for binfmt_misc it will be able to run anyway.

Signed-off-by: Alex Bennée <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Tested-by: Philippe Mathieu-Daudé <[email protected]>
Rename DOCKER_INTERMEDIATE_IMAGES to DOCKER_PARTIAL_IMAGES and add the
incomplete cross compiler images that can build tests but can't build
QEMU itself. We also add debian, debian-bootstrap and the tricode
images to the list.

Signed-off-by: Alex Bennée <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Tested-by: Philippe Mathieu-Daudé <[email protected]>
This test doesn't even build QEMU, it just builds and runs all the
unit tests. Intended to make checking unit tests on all docker images
easier.

Signed-off-by: Alex Bennée <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Tested-by: Philippe Mathieu-Daudé <[email protected]>
This allows us to run a particular test on all docker images. For
example:

  make docker-test-unit

Will run the unit tests on every supported image. At the same time
rename docker-test to docker-all-tests to be clearer.

Signed-off-by: Alex Bennée <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Tested-by: Philippe Mathieu-Daudé <[email protected]>
The addition of QEMU_TARGET was intended to ensure we fall back to
checking for the existence of an image if the build system was not
currently configured to build it. However this breaks the direct use
of the rule for building custom binfmt_misc images. We already check
for EXECUTABLE so let us just use that as a proxy for deciding if we
are just going to check the image exits.

Signed-off-by: Alex Bennée <[email protected]>
Tested-by: Philippe Mathieu-Daudé <[email protected]>
When a check fails we currently just report why we failed. This is not
totally helpful to people who want to boot-strap a new image. Report a
hint as to why it failed.

Signed-off-by: Alex Bennée <[email protected]>
Suggested-by: Fam Zheng <[email protected]>
…to Salsa

This silents the following warning:

  Cloning into './debootstrap.git'...
  warning: redirecting to https://salsa.debian.org/installer-team/debootstrap.git/

See https://lists.debian.org/debian-devel-announce/2018/01/msg00004.html

Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
Signed-off-by: Alex Bennée <[email protected]>
This is just a note that later versions of debootstrap don't
technically need this hack.

Signed-off-by: Alex Bennée <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
We do a minimum version check for the debootstrap but if the distro
has added their own minor version tick it would fail and fall-back to
the SCM version. This is sub-optimal as the latest/greatest version
may be broken at any one particular time. We fix that with a little
sed magic on the version string before passing to our ugly shell
versioning check.

Signed-off-by: Alex Bennée <[email protected]>
Tested-by: Philippe Mathieu-Daudé <[email protected]>
Setting up binfmt_misc is outside of the scope of the docker.py script
but we can at least validate it with any given executable so we have a
more useful error message than the sed line of deboostrap failing
cryptically.

Signed-off-by: Alex Bennée <[email protected]>
Reported-by: Richard Henderson <[email protected]>
The combination of being rather esoteric and needing to support mmap @
0 means this only ever worked under translation. It has now regressed
even further and is no longer useful. Kill it.

Signed-off-by: Alex Bennée <[email protected]>
Reviewed-by: Peter Maydell <[email protected]>
…nd cdrom

In ed6e216 ("linux-aio: properly bubble up errors from initialzation"),
I only added a bdrv_attach_aio_context callback for the bdrv_file
driver. There are several other drivers that use the shared
aio_plug callback, though, and they will trip the assertion added to
aio_get_linux_aio because they did not call aio_setup_linux_aio first.
Add the appropriate callback definition to the affected driver
definitions.

Fixes: ed6e216 ("linux-aio: properly bubble up errors from initialization")
Reported-by: Farhan Ali <[email protected]>
Signed-off-by: Nishanth Aravamudan <[email protected]>
Reviewed-by: John Snow <[email protected]>
Message-id: [email protected]
Cc: Eric Blake <[email protected]>
Cc: Kevin Wolf <[email protected]>
Cc: John Snow <[email protected]>
Cc: Max Reitz <[email protected]>
Cc: Stefan Hajnoczi <[email protected]>
Cc: Fam Zheng <[email protected]>
Cc: Paolo Bonzini <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Stefan Hajnoczi <[email protected]>
Calling qcrypto_init ensures that all relevant initialization is
done. In particular this honours the debugging settings and thread
settings.

Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Tested-by: Philippe Mathieu-Daudé <[email protected]>
Signed-off-by: Daniel P. Berrangé <[email protected]>
The test-vmstate test is a bit chatty because it triggers various
expected failure scenarios and the code in question uses error_report
instead of accepting 'Error **errp' parameters. To silence this test the
stubs for error_vprintf() were changed to send errors via
g_test_message() instead of stderr:

  commit 28017e0
  Author: Paolo Bonzini <[email protected]>
  Date:   Mon Oct 24 18:31:03 2016 +0200

    tests: send error_report to test log

    Implement error_vprintf to send the output of error_report to
    the test log.  This silences test-vmstate.

    Signed-off-by: Paolo Bonzini <[email protected]>
    Message-Id: <[email protected]>

Unfortunately this change has global impact across the entire test suite
and means that when tests fail for unexpected reasons, the message is
not displayed on stderr. eg when using &error_abort in a call the test
merely prints

  Unexpected error in qcrypto_tls_session_check_certificate() at crypto/tlssession.c:280:

and the actual error message is hidden, making it impossible to diagnose
the failure. This is especially problematic in CI or build systems where
it isn't possible to easily pass the --debug-log flag to tests and
re-run with the test log visible.

This change makes the previous big hammer much more nuanced, providing a
flag in the stub error_vprintf() that can used on a per-test basis to
silence the errors. Only the test-vmstate silences errors initially.

Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Tested-by: Philippe Mathieu-Daudé <[email protected]>
Signed-off-by: Daniel P. Berrangé <[email protected]>
Most of the TLS related tests are passing an in a "Error" object to
methods that are expected to fail, but then ignoring any error that is
set and instead asserting on a return value. This means that when an
error is unexpectedly raised, no information about it is printed out,
making failures hard to diagnose. Changing these tests to pass in
&error_abort will make unexpected failures print messages to stderr.

Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Tested-by: Philippe Mathieu-Daudé <[email protected]>
Signed-off-by: Daniel P. Berrangé <[email protected]>
When gnutls negotiates TLS 1.3 instead of 1.2, the order of messages
sent by the handshake changes. This exposed a logic bug in the test
suite which caused us to wait for the server to see handshake
completion, but not wait for the client to see completion. The result
was the client didn't receive the certificate for verification and the
test failed.

This is exposed in Fedora 29 rawhide which has just enabled TLS 1.3 in
its GNUTLS builds.

Reviewed-by: Eric Blake <[email protected]>
Signed-off-by: Daniel P. Berrangé <[email protected]>
I would guess it won't happen normally, but this should ease Coverity.

>>>     CID 1394385:  Integer handling issues  (OVERFLOW_BEFORE_WIDEN)
>>>     Potentially overflowing expression "pages->used * 8192U" with type "unsigned int" (32 bits, unsigned) is evaluated using 32-bit arithmetic, and then used in a context that expects an expression of type "uint64_t" (64 bits, unsigned).
854         transferred = pages->used * TARGET_PAGE_SIZE + p->packet_len;

Fixes: CID 1394385
CC: Juan Quintela <[email protected]>
Signed-off-by: Peter Xu <[email protected]>
Message-Id: <[email protected]>
Reviewed-by: Juan Quintela <[email protected]>
Signed-off-by: Dr. David Alan Gilbert <[email protected]>
We've been getting the warning:

migration_iteration_finish: Unknown ending state 2

on a cancel.

I think that's originally due to 39b9e17;  although
I've only seen the warning, I think that in some cases
that we could find the VM stays paused after a cancel where
it should restart.

Signed-off-by: Dr. David Alan Gilbert <[email protected]>
Message-Id: <[email protected]>
Reviewed-by: Peter Xu <[email protected]>
Reviewed-by: Juan Quintela <[email protected]>
Signed-off-by: Dr. David Alan Gilbert <[email protected]>
Fix outgoing migration which was crashing in
vmstate_hda_audio_stream_buf_needed, I think the problem
is that we have room for upto 4 streams in the array but only
use 2, when we come to try and save the state of the unused
streams we hit st->state == NULL.

Fixes: 280c1e1
Signed-off-by: Dr. David Alan Gilbert <[email protected]>
Message-Id: <[email protected]>
Reviewed-by: Daniel P. Berrangé <[email protected]>
Reviewed-by: Juan Quintela <[email protected]>
Signed-off-by: Dr. David Alan Gilbert <[email protected]>
We shouldn't update the received bitmap if we're the source VM.  This
fixes a breakage when release-ram is enabled on postcopy.

Signed-off-by: Peter Xu <[email protected]>
Message-Id: <[email protected]>
Reviewed-by: Juan Quintela <[email protected]>
Signed-off-by: Dr. David Alan Gilbert <[email protected]>
Postcopy recovery won't work well with release-ram capability since
release-ram will drop the page buffer as long as the page is put into
the send buffer.  So if there is a network failure happened, any page
buffers that have not yet reached the destination VM but have already
been sent from the source VM will be lost forever.  Let's refuse the
client from resuming such a postcopy migration.  Luckily release-ram was
designed to only be used when src and destination VMs are on the same
host, so it should be fine.

Signed-off-by: Peter Xu <[email protected]>
Message-Id: <[email protected]>
Reviewed-by: Juan Quintela <[email protected]>
Signed-off-by: Dr. David Alan Gilbert <[email protected]>
The only possible change of last_byte is when it reaches the edge.
Setting it every time might let last_byte contain an invalid data when
memory corruption is detected, then the check of the next byte will be
incorrect.  For example, a single page corruption at address 0x14ad000
will also lead to a "fake" corruption at 0x14ae000:

  Memory content inconsistency at 14ad000 first_byte = 44 last_byte = 44 current = ef hit_edge = 0
  Memory content inconsistency at 14ae000 first_byte = 44 last_byte = ef current = 44 hit_edge = 0

After the patch, it'll only report the corrputed page:

  Memory content inconsistency at 14ad000 first_byte = 44 last_byte = 44 current = ef hit_edge = 0

Signed-off-by: Peter Xu <[email protected]>
Message-Id: <[email protected]>
Reviewed-by: Juan Quintela <[email protected]>
Signed-off-by: Dr. David Alan Gilbert <[email protected]>
…anup_bh

migrate_fd_connect duplicate initialize expected_downtime and cleanup_bh.

Signed-off-by: Lidong Chen <[email protected]>
Message-Id: <[email protected]>
Reviewed-by: Juan Quintela <[email protected]>
Signed-off-by: Dr. David Alan Gilbert <[email protected]>
Michael Clark and others added 20 commits September 17, 2018 12:45
The constraint for `rdinstreth` was comparing the csr number to 0xc80,
which is `cycleh` instead. Fix this.

Author: Wladimir J. van der Laan <[email protected]>
Signed-off-by: Michael Clark <[email protected]>
During submission of the RISC-V port, cpu_unassigned_access
was removed because the port was not allowed to contain
printf and exit and should not abort during normal operation.

Interestingly the semantics of an unimplemented
cpu_unassigned_access were unknown, and suprisingly when the
method is not implemented then behaviour is undefined and
memory accesses succeed. This is potentially a core QEMU bug,
or something that should be documented for port maintainers.

This patch adds back a cpu_abort, as a short term workaround,
similar to the pre-submission behaviour except using the
preferred cpu_abort API vs printf and exit.

Signed-off-by: Michael Clark <[email protected]>
The previous patch in this series adds riscv_cpu_unassigned_access
and restores the previous behavior of cpu exit only using the
correct cpu_abort API. This patch removes the unimplemented code
and replaces it with code to raise a load or store access fault.

Signed-off-by: Michael Clark <[email protected]>
Because PMP is optional, but mandatorily enabled by default if present, this means all “portable” system software that switches mode from M to S or U mode, must have code to configure PMP. Note also the code linked above sets and restores the trap vector in case the PMP CSRs are not implemented and cause an illegal instruction exception. Technically the PMP CSRs should be hard-wired to zero if not implemented and they are WARL (Write Any Read Legal) so one should read back the value to check if PMP is actually present. 

For a discussion of this, see: https://groups.google.com/a/groups.riscv.org/forum/#!topic/isa-dev/OFa8TSc_PNI
Configuration register under RV32: pmpcfg0/pmpcfg1/pmpcfg2/pmpcfg3.

Configuration register under RV64: pmpcfg0/pmpcfg2.

pmpcfg_csr_write/pmpcfg_csr_read calculates the index by the following expression:`reg_index * sizeof(target_ulong)) + i`

RV32-> [0,3] [4,7] [8,11] [12,15], this is ok

RV64-> [0,7] [16,23] ,this is error

So we need add this code.
Should skip the empty PMP entry.
This allows gdb to read misa and set breakpoints.  This is a work in progress,
and has a number of obvious problems, incomplete csr support, wrong int reg
size for rv32, etc.
- Implements draft clic-spec (20180728)
  - Implements non-vectored mode and vectored mode
  - Implements mode+level+priority configuration
  - Implements mode+level+priority preemption model
  - Seperated M-mode (mtvec) and S-mode (stvec) delivery
  - CLIC supports backwards compatible CLINT mode for
    legacy interrupts using MIE/MIP,SIE/SIP (irq < 16)
    depending on mtvec (MTI,MSI) and stvec (STI,SSI)
  - CLINT mode supports S-mode stimecmp{h} and ssip{h}

- QEMU CLINT/CLIC Test Cases
  - https://github.com/michaeljclark/qemu-riscv-tests

- Adds two experimental machines
  - SiFive Freedom E-Series with CLIC
    - Implements M-mode CLINT/CLIC config memory map
    - Parameters
      - CLICINTBITS=4
      - CLICCFGMBITS=0
      - CLICCFGLBITS=4
    - Invocation
      - qemu-system-riscv{32,64} -machine sifive_ex
  - SiFive Freedom U-Series with CLIC
    - Implements M-mode and S-mode CLINT/CLIC memory map
    - Parameters
      - CLICINTBITS=8
      - CLICCFGMBITS=2
      - CLICCFGLBITS=4
    - Invocation
      - qemu-system-riscv{32,64} -machine sifive_ux

- CLIC combined CLINT/CLIC memory map
  - M-Mode CLINT = 0x02000000
    - msip       = 0x02000000 + hartid * 4
    - mtimecmp   = 0x02004000 + hartid * 4
    - mtime      = 0x0200bff8
  - S-Mode CLINT = 0x02020000
  - M-mode CLIC  = 0x02080000
    - clicintip  = 0x02080000 + hartid * 0x1000 + 0x000
    - clicintie  = 0x02080000 + hartid * 0x1000 + 0x400
    - clicintcfg = 0x02080000 + hartid * 0x1000 + 0x800
    - cliccfg    = 0x02080000 + hartid * 0x1000 + 0xc00
  - S-Mode CLIC  = 0x020c0000
    - clicintip  = 0x020c0000 + hartid * 0x1000 + 0x000
    - clicintie  = 0x020c0000 + hartid * 0x1000 + 0x400
    - clicintcfg = 0x020c0000 + hartid * 0x1000 + 0x800

- Adds CLIC interrupt tracing (`-d trace:riscv_trap,...`)
  - riscv_trap         # existing core interrupt tracing
  - sifive_clic_cfg    # CLIC global configuration
  - sifive_clic_intcfg # CLIC interrupt configuration
  - sifive_clic_intie  # CliC interrupt enable
  - sifive_clic_intip  # CLIC interrupt pending
  - sifive_clic_irq    # CLIC irq entry

- Notes / Limitations
  - Enforces clicintcfg writes based on cliccfg and mode
  - Reads/writes to intcfg/intie/intip in lower mode MMIO
    apetures are currently allowed. Access checks need to
    be added to suppress writes and hardwire read reults to
    zero for any entries that where mode < clicintcfg.mode
  - Interrupts pending bits are writable by software.
    Edge/Level configuration needs to be added to control
    software access to interrupt pending bits
  - Selective vectoring in non-vectored mode is unimplemented
  - PLIC is currently not routed via the CLIC however pending
    bits can be written by software to test pre-emption.
  - mnxti/snxti sets mstatus flags but returns 0 (slow path).
    The CLIC state is currenetly not accessible from target/riscv
    as cpu implementations can't include anything from include/hw
    so the CLIC state needs to be in a CPU accessible structure.
  - Potential race condition if an interrupt is posted
    before the CPU has received and processed an outstanding
    interrupt due to env->exccode being overwritten.
    Needs changes to the interface from the CLIC so that the
    CPU interrupt handler pulls the highest priority interrupt
    from the CLIC at the time it is woken up. This requires the
    CLIC state to be accessible from the CPU similarly to mnxti
  - CPU core changes are relatively intrusive. The CPU interrupt
    handling requires some abstraction/hooks for a more modular
    CLIC implementation. CLIC state needs to be attached to
    the CPU, and accessible to the MMIO device with hooks in
    riscv_cpu_exec_interrupt and riscv_cpu_do_interrupt

Changes since v0

- Fix array index calculation in sifive_clic_realize
- Raise CLIC_LEVEL_BITS to 8 and fix assertion
- Move CLIC parameterization constants to its header
riscv32 linux ABI is unique in that it has a 64-bit stat
structure on 32-bit vs seperate stat and stat64 syscalls.

Test program:

  #include <stdlib.h>

  int
  main (void)
  {
    int fd = open ("tmp.file", O_CREAT|O_RDWR, S_IRWXU);
    if (fd == -1) {
      perror ("open failed");
      exit (1);
    }
    struct stat buf;
    int result = fstat (fd, &buf);
    if (result == -1) {
      perror ("fstat failed");
      exit (1);
    }
    printf ("S_ISREG (buf.st_mode) = %d\n", S_ISREG (buf.st_mode));
    return 0;
  }

Expected results:

  $ riscv32-unknown-linux-gnu-gcc -O2 stat.c -o stat
  $ qemu-riscv32 stat
  S_ISREG (buf.st_mode) = 1

Signed-off-by: Michael Clark <[email protected]>
- Rename pmp_hart_has_priv to pmp_has_access as this is a more
  appropriate name for tracing
- Add tracing for CSR reads and writes to pmpcfg and pmpaddr
  using -d trace:pmpcfg_csr_read,trace:pmpcfg_csr_write,
  trace:pmpaddr_csr_read,trace:pmpaddr_csr_write
- Add tracing for PMP access and rule matching using
  -d trace:pmp_has_access,trace:pmp_rule_match
- Add early out if not all rules are present; short-circuit
  optimization bug for discontiguous rules fixed (reported by
  wxjstz <[email protected]>)
- Fix bug where TLB entries were created for rules smaller
  than the page size (4096), which caused results of rules
  with small spans to be erroneously used in subsequent accesses
- Fix integer promotion bug in pmpcfg_csr_read (also reported
  by wxjstz <[email protected]>)
- Fix bug where PMP allowed non M-mode accesses when no rules
  have been configured (default behaviour is to deny access
  to other modes until PMP has been configured. (also reported
  by wxjstz <[email protected]>)
- Fix illegal offsets for pmpcfg CSR accesses on rv64 (reported
  by wxjstz <[email protected]>)
- Use size_t for PMP CSR address offsets (unsigned int can
  result in sign extension on some 64-bit architectures)
- Add granularity parameter and mask addresss writes so that
  granularity can be detected.
- Use NA4 bit to represent terminal granule size where G > 0
  (this is implied by the specification)
- Remove redundant debugging statements (unnecessar with the
  new tracing support).
- Simplify rule matching loop and use a ternary expression
  that contains the entire rule match result in a similar
  condensed style to spike (riscv-isa-sim).

Co-authored-by: wxjstz <[email protected]>
Signed-off-by: Michael Clark <[email protected]>
- Implements draft clic-spec (20180728)
  - Implements non-vectored mode and vectored mode
  - Implements mode+level+priority configuration
  - Implements mode+level+priority preemption model
  - Seperated M-mode (mtvec) and S-mode (stvec) delivery
  - CLIC supports backwards compatible CLINT mode for
    legacy interrupts using MIE/MIP,SIE/SIP (irq < 16)
    depending on mtvec (MTI,MSI) and stvec (STI,SSI)
  - CLINT mode supports S-mode stimecmp{h} and ssip{h}

- QEMU CLINT/CLIC Test Cases
  - https://github.com/michaeljclark/qemu-riscv-tests

- Adds two experimental machines
  - SiFive Freedom E-Series with CLIC
    - Implements M-mode CLINT/CLIC config memory map
    - Parameters
      - CLICINTBITS=4
      - CLICCFGMBITS=0
      - CLICCFGLBITS=4
    - Invocation
      - qemu-system-riscv{32,64} -machine sifive_ex
  - SiFive Freedom U-Series with CLIC
    - Implements M-mode and S-mode CLINT/CLIC memory map
    - Parameters
      - CLICINTBITS=8
      - CLICCFGMBITS=2
      - CLICCFGLBITS=4
    - Invocation
      - qemu-system-riscv{32,64} -machine sifive_ux

- CLIC combined CLINT/CLIC memory map
  - M-Mode CLINT = 0x02000000
    - msip       = 0x02000000 + hartid * 4
    - mtimecmp   = 0x02004000 + hartid * 4
    - mtime      = 0x0200bff8
  - S-Mode CLINT = 0x02020000
  - M-mode CLIC  = 0x02080000
    - clicintip  = 0x02080000 + hartid * 0x1000 + 0x000
    - clicintie  = 0x02080000 + hartid * 0x1000 + 0x400
    - clicintcfg = 0x02080000 + hartid * 0x1000 + 0x800
    - cliccfg    = 0x02080000 + hartid * 0x1000 + 0xc00
  - S-Mode CLIC  = 0x020c0000
    - clicintip  = 0x020c0000 + hartid * 0x1000 + 0x000
    - clicintie  = 0x020c0000 + hartid * 0x1000 + 0x400
    - clicintcfg = 0x020c0000 + hartid * 0x1000 + 0x800

- Adds CLIC interrupt tracing (`-d trace:riscv_trap,...`)
  - riscv_trap         # existing core interrupt tracing
  - sifive_clic_cfg    # CLIC global configuration
  - sifive_clic_intcfg # CLIC interrupt configuration
  - sifive_clic_intie  # CliC interrupt enable
  - sifive_clic_intip  # CLIC interrupt pending
  - sifive_clic_irq    # CLIC irq entry

- Notes / Limitations
  - Enforces clicintcfg writes based on cliccfg and mode
  - Reads/writes to intcfg/intie/intip in lower mode MMIO
    apetures are currently allowed. Access checks need to
    be added to suppress writes and hardwire read reults to
    zero for any entries that where mode < clicintcfg.mode
  - Interrupts pending bits are writable by software.
    Edge/Level configuration needs to be added to control
    software access to interrupt pending bits
  - Selective vectoring in non-vectored mode is unimplemented
  - PLIC is currently not routed via the CLIC however pending
    bits can be written by software to test pre-emption.
  - mnxti/snxti sets mstatus flags but returns 0 (slow path).
    The CLIC state is currenetly not accessible from target/riscv
    as cpu implementations can't include anything from include/hw
    so the CLIC state needs to be in a CPU accessible structure.
  - Potential race condition if an interrupt is posted
    before the CPU has received and processed an outstanding
    interrupt due to env->exccode being overwritten.
    Needs changes to the interface from the CLIC so that the
    CPU interrupt handler pulls the highest priority interrupt
    from the CLIC at the time it is woken up. This requires the
    CLIC state to be accessible from the CPU similarly to mnxti
  - CPU core changes are relatively intrusive. The CPU interrupt
    handling requires some abstraction/hooks for a more modular
    CLIC implementation. CLIC state needs to be attached to
    the CPU, and accessible to the MMIO device with hooks in
    riscv_cpu_exec_interrupt and riscv_cpu_do_interrupt

Changes since v0

- Fix array index calculation in sifive_clic_realize
- Raise CLIC_LEVEL_BITS to 8 and fix assertion
- Move CLIC parameterization constants to its header
This allows gdb to read misa and set breakpoints.  This is a work in progress,
and has a number of obvious problems, incomplete csr support, wrong int reg
size for rv32, etc.
@jim-wilson
Copy link
Contributor Author

The first part patch is pull request #160.

@jim-wilson
Copy link
Contributor Author

Submitted upstream.

@jim-wilson
Copy link
Contributor Author

The gdb stub support is in upstream qemu now.

@jim-wilson jim-wilson closed this Mar 19, 2019
@jim-wilson jim-wilson deleted the gdb-support-v2 branch March 19, 2019 18:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.