The GICv4 defines a new in-guest-memory table for the ITS: this is
the vPE table. Implement the new GITS_BASER2 register which the
guest uses to tell the ITS where the vPE table is located, including
the decode of the register fields into the TableDesc structure which
we do for the GITS_BASER<n> when the guest enables the ITS.
We guard provision of the new register with the its_feature_virtual()
function, which does a check of the GITS_TYPER.Virtual bit which
indicates presence of ITS support for virtual LPIs. Since this bit
is currently always zero, GICv4-specific features will not be
accessible to the guest yet.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220408141550.1271295-8-peter.maydell@linaro.org
In process_mapti() we check interrupt IDs to see whether they are
in the valid LPI range. Factor this out into its own utility
function, as we're going to want it elsewhere too for GICv4.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220408141550.1271295-7-peter.maydell@linaro.org
We use the common function gicv3_idreg() to supply the CoreSight ID
register values for the GICv3 for the copies of these ID registers in
the distributor, redistributor and ITS register frames. This isn't
quite correct, because while most of the register values are the
same, the PIDR0 value should vary to indicate which of these three
frames it is. (You can see this and also the correct values of these
PIDR0 registers by looking at the GIC-600 or GIC-700 TRMs, for
example.)
Make gicv3_idreg() take an extra argument for the PIDR0 value.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220408141550.1271295-5-peter.maydell@linaro.org
Boards using the GICv3 need to configure it with both the total
number of CPUs and also the sizes of all the memory regions which
contain redistributors (one redistributor per CPU). At the moment
the GICv3 checks that the number of CPUs specified is not too many to
fit in the defined redistributor regions, but in fact the code
assumes that the two match exactly. For instance when we set the
GICR_TYPER.Last bit on the final redistributor in each region, we
assume that we don't need to consider the possibility of a region
being only half full of redistributors or even completely empty. We
also assume in gicv3_redist_read() and gicv3_redist_write() that we
can calculate the CPU index from the offset within the MemoryRegion
and that this will always be in range.
Fortunately all the board code sets the redistributor region sizes to
exactly match the CPU count, so this isn't a visible bug. We could
in theory make the GIC code handle non-full redistributor regions, or
have it automatically reduce the provided region sizes to match the
CPU count, but the simplest thing is just to strengthen the error
check and insist that the CPU count and redistributor region size
settings match exactly, since all the board code does that anyway.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220408141550.1271295-4-peter.maydell@linaro.org
In the GICv3 code we implicitly rely on there being at least one CPU
and thus at least one redistributor and CPU interface. Sanity-check
that the property the board code sets is not zero.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220408141550.1271295-3-peter.maydell@linaro.org
In commit b6f96009ac we split do_process_its_cmd() from
process_its_cmd(), but forgot the usual blank line between function
definitions. Add it.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220408141550.1271295-2-peter.maydell@linaro.org
This commit implements reset function of all ACLINT devices.
ACLINT device reset will clear MTIME and MSIP register to 0.
Depend on RISC-V ACLINT spec v1.0-rc4:
https://github.com/riscv/riscv-aclint/blob/v1.0-rc4/riscv-aclint.adoc
Signed-off-by: Jim Shu <jim.shu@sifive.com>
Reviewed-by: Frank Chang <frank.chang@sifive.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-Id: <20220420080901.14655-5-frank.chang@sifive.com>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
RISC-V privilege spec defines that mtime is exposed as a memory-mapped
machine-mode read-write register. However, as QEMU uses host monotonic
timer as timer source, this makes mtime to be read-only in RISC-V
ACLINT.
This patch makes mtime to be writable by recording the time delta value
between the mtime value to be written and the timer value at the time
mtime is written. Time delta value is then added back whenever the timer
value is retrieved.
Signed-off-by: Frank Chang <frank.chang@sifive.com>
Reviewed-by: Jim Shu <jim.shu@sifive.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-Id: <20220420080901.14655-4-frank.chang@sifive.com>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
RISC-V privilege spec defines that:
* In RV32, memory-mapped writes to mtimecmp modify only one 32-bit part
of the register.
* For RV64, naturally aligned 64-bit memory accesses to the mtime and
mtimecmp registers are additionally supported and are atomic.
It's possible to perform both 32/64-bit read/write accesses to both
mtimecmp and mtime registers.
Signed-off-by: Frank Chang <frank.chang@sifive.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Jim Shu <jim.shu@sifive.com>
Message-Id: <20220420080901.14655-3-frank.chang@sifive.com>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
If device's MemoryRegion doesn't have .impl.[min|max]_access_size
declaration, the default access_size_min would be 1 byte and
access_size_max would be 4 bytes (see: softmmu/memory.c).
This will cause a 64-bit memory access to ACLINT to be splitted into
two 32-bit memory accesses.
Signed-off-by: Frank Chang <frank.chang@sifive.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Jim Shu <jim.shu@sifive.com>
Message-Id: <20220420080901.14655-2-frank.chang@sifive.com>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
While at it, replace '%x' with '%u' as suggested by Philippe Mathieu-Daudé.
Also fixes a GCC 12.0.1 -Wformat-overflow false-positive.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20220420132624.2439741-16-marcandre.lureau@redhat.com>
Switch the creation of the combiner devices to the new-style
"embedded in state struct" approach, so we can easily refer
to the object elsewhere during realize.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220404154658.565020-18-peter.maydell@linaro.org
The function exynos4210_combiner_get_gpioin() currently lives in
exynos4210_combiner.c, but it isn't really part of the combiner
device itself -- it is a function that implements the wiring up of
some interrupt sources to multiple combiner inputs. Move it to live
with the other SoC-level code in exynos4210.c, along with a few
macros previously defined in exynos4210.h which are now used only
in exynos4210.c.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220404154658.565020-11-peter.maydell@linaro.org
Switch the creation of the external GIC to the new-style "embedded in
state struct" approach, so we can easily refer to the object
elsewhere during realize.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220404154658.565020-9-peter.maydell@linaro.org
The function exynos4210_init_board_irqs() currently lives in
exynos4210_gic.c, but it isn't really part of the exynos4210.gic
device -- it is a function that implements (some of) the wiring up of
interrupts between the SoC's GIC and combiner components. This means
it fits better in exynos4210.c, which is the SoC-level code. Move it
there. Similarly, exynos4210_git_irq() is used almost only in the
SoC-level code, so move it too.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220404154658.565020-8-peter.maydell@linaro.org
Fix a missing set of spaces around '-' in the definition of
combiner_grp_to_gic_id[]. We're about to move this code, so
fix the style issue first to keep checkpatch happy with the
code-motion patch.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220404154658.565020-7-peter.maydell@linaro.org
The exynos4210 code currently has two very similar arrays of IRQs:
* board_irqs is a field of the Exynos4210Irq struct which is filled
in by exynos4210_init_board_irqs() with the appropriate qemu_irqs
for each IRQ the board/SoC can assert
* irq_table is a set of qemu_irqs pointed to from the
Exynos4210State struct. It's allocated in exynos4210_init_irq,
and the only behaviour these irqs have is that they pass on the
level to the equivalent board_irqs[] irq
The extra indirection through irq_table is unnecessary, so coalesce
these into a single irq_table[] array as a direct field in
Exynos4210State which exynos4210_init_board_irqs() fills in.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220404154658.565020-6-peter.maydell@linaro.org
Now we have removed the only use of TYPE_EXYNOS4210_IRQ_GATE we can
delete the device entirely.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com>
Message-id: 20220404154658.565020-3-peter.maydell@linaro.org
Replace the global variables with inlined helper functions. getpagesize() is very
likely annotated with a "const" function attribute (at least with glibc), and thus
optimization should apply even better.
This avoids the need for a constructor initialization too.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20220323155743.1585078-12-marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
In commit 84d43d2e82 we rearranged the logging of errors in
process_mapc(), and inadvertently dropped the trailing newlines
from the log messages. Restore them. The same commit also
attempted to switch the ICID printing to hex (which is how we
print ICIDs elsewhere) but only did half the job, adding the
0x prefix but leaving the format string at %d; correct to %x.
Fixes: 84d43d2e82 ("hw/intc/arm_gicv3_its: In MAPC with V=0, don't check rdbase field")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
g_new(T, n) is neater than g_malloc(sizeof(T) * n). It's also safer,
for two reasons. One, it catches multiplication overflowing size_t.
Two, it returns T * rather than void *, which lets the compiler catch
more type errors.
This commit only touches allocations with size arguments of the form
sizeof(T).
Patch created mechanically with:
$ spatch --in-place --sp-file scripts/coccinelle/use-g_new-etc.cocci \
--macro-file scripts/cocci-macro-file.h FILES...
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20220315144156.1595462-4-armbru@redhat.com>
Reviewed-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
CONFIG_ARM_GIC_TCG actually guards the compilation of TCG GICv3
specific files. So let's rename it into CONFIG_ARM_GICV3_TCG
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Message-id: 20220308182452.223473-2-eric.auger@redhat.com
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Xive2EndSource objects can only be instantiated through a Xive2Router
(PnvXive2).
Reported-by: Thomas Huth <thuth@redhat.com>
Fixes: f8a233dedf ("ppc/xive2: Introduce a XIVE2 core framework")
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
The trace_gicv3_icv_hppir_read trace event takes an integer value
which it uses to form the register name, which should be either
ICV_HPPIR0 or ICV_HPPIR1. We were passing in the 'grp' variable for
this, but that is either GICV3_G0 or GICV3_G1NS, which happen to be 0
and 2, which meant that tracing for the ICV_HPPIR1 register was
incorrectly printed as ICV_HPPIR2.
Use the same approach we do for all the other similar trace events,
and pass in 'ri->crm == 8 ? 0 : 1', deriving the index value
directly from the ARMCPRegInfo struct.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220303202341.2232284-6-peter.maydell@linaro.org
We forgot a space in some log messages, so the output ended
up looking like
gicv3_dist_write: invalid guest write at offset 0000000000008000size 8
with a missing space before "size". Add the missing spaces.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220303202341.2232284-5-peter.maydell@linaro.org
The GICv3 has some registers that support byte accesses, and some
that support 8-byte accesses. Our TCG implementation implements all
of this, switching on the 'size' argument and handling the registers
that must support reads of that size while logging an error for
attempted accesses to registers that do not support that size access.
However we forgot to tell the core memory subsystem about this by
specifying the .impl and .valid fields in the MemoryRegionOps struct,
so the core was happily simulating 8 byte accesses by combining two 4
byte accesses. This doesn't have much guest-visible effect, since
there aren't many 8 byte registers and they all support being written
in two 4 byte parts.
Set the .impl and .valid fields to say that all sizes from 1 to 8
bytes are both valid and implemented by the device.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220303202341.2232284-4-peter.maydell@linaro.org
For debugging guest use of the ITS, it can be helpful to trace
when the ITS reads and writes the in-memory tables.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220303202341.2232284-3-peter.maydell@linaro.org
When debugging code that's using the ITS, it's helpful to
see tracing of the ITS commands that the guest executes. Add
suitable trace events.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220303202341.2232284-2-peter.maydell@linaro.org
The RISC-V AIA (Advanced Interrupt Architecture) defines a new
interrupt controller for MSIs (message signal interrupts) called
IMSIC (Incoming Message Signal Interrupt Controller). The IMSIC
is per-HART device and also suppport virtualizaiton of MSIs using
dedicated VS-level guest interrupt files.
This patch adds device emulation for RISC-V AIA IMSIC which
supports M-level, S-level, and VS-level MSIs.
Signed-off-by: Anup Patel <anup.patel@wdc.com>
Signed-off-by: Anup Patel <anup@brainfault.org>
Reviewed-by: Frank Chang <frank.chang@sifive.com>
Message-Id: <20220220085526.808674-3-anup@brainfault.org>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
The XIVE interrupt controller on P10 can automatically save and
restore the state of the interrupt registers under the internal NVP
structure representing the VCPU. This saves a costly store/load in
guest entries and exits.
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Add GEN1 config even if we don't use it yet in the core framework.
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
The thread interrupt management area (TIMA) is a set of pages mapped
in the Hypervisor and in the guest OS address space giving access to
the interrupt thread context registers for interrupt management, ACK,
EOI, CPPR, etc.
XIVE2 changes slightly the TIMA layout with extra bits for the new
features, larger CAM lines and the controller provides configuration
switches for backward compatibility. This is called the XIVE2
P9-compat mode, of Gen1 TIMA. It impacts the layout of the TIMA and
the availability of the internal features associated with it,
Automatic Save & Restore for instance. Using a P9 layout also means
setting the controller in such a mode at init time.
As the OPAL driver initializes the XIVE2 controller with a XIVE2/P10
TIMA directly, the XIVE2 model only has a simple support for the
compat mode in the OS TIMA.
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Only the CAM line updates done by the hypervisor are specific to
POWER10. Instead of duplicating the TM ops table, we handle these
commands locally under the PowerNV XIVE2 model.
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
These bits control the availability of interrupt features : StoreEOI,
PHB PQ_disable, PHB Address-Based Trigger and the overall XIVE
exploitation mode. These bits can be set at early boot time of the
system to activate/deactivate a feature for testing purposes. The
default value should be '1'.
The 'XIVE exploitation mode' bit is a software bit that skiboot could
use to disable the XIVE OS interface and propose a P8 style XICS
interface instead. There are no plans for that for the moment.
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
The PQ_disable configuration bit disables the check done on the PQ
state bits when processing new MSI interrupts. When bit 9 is enabled,
the PHB forwards any MSI trigger to the XIVE interrupt controller
without checking the PQ state bits. The XIVE IC knows from the trigger
message that the PQ bits have not been checked and performs the check
locally.
This configuration bit only applies to MSIs and LSIs are still checked
on the PHB to handle the assertion level.
PQ_disable enablement is a requirement for StoreEOI.
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
The trigger message coming from a HW source contains a special bit
informing the XIVE interrupt controller that the PQ bits have been
checked at the source or not. Depending on the value, the IC can
perform the check and the state transition locally using its own PQ
state bits.
The following changes add new accessors to the XiveRouter required to
query and update the PQ state bits. This only applies to the PowerNV
machine. sPAPR accessors are provided but the pSeries machine should
not be concerned by such complex configuration for the moment.
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
This is an internal offset used to inject triggers when the PQ state
bits are not controlled locally. Such as for LSIs when the PHB5 are
using the Address-Based Interrupt Trigger mode and on the END.
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
The XIVE2 interrupt controller of the POWER10 processor follows the
same logic than on POWER9 but the HW interface has been largely
reviewed. It has a new register interface, different BARs, extra
VSDs, new layout for the XIVE2 structures, and a set of new features
which are described below.
This is a model of the POWER10 XIVE2 interrupt controller for the
PowerNV machine. It focuses primarily on the needs of the skiboot
firmware but some initial hypervisor support is implemented for KVM
use (escalation).
Support for new features will be implemented in time and will require
new support from the OS.
* XIVE2 BARS
The interrupt controller BARs have a different layout outlined below.
Each sub-engine has now own its range and the indirect TIMA access was
replaced with a set of pages, one per CPU, under the IC BAR:
- IC BAR (Interrupt Controller)
. 4 pages, one per sub-engine
. 128 indirect TIMA pages
- TM BAR (Thread Interrupt Management Area)
. 4 pages
- ESB BAR (ESB pages for IPIs)
. up to 1TB
- END BAR (ESB pages for ENDs)
. up to 2TB
- NVC BAR (Notification Virtual Crowd)
. up to 128
- NVPG BAR (Notification Virtual Process and Group)
. up to 1TB
- Direct mapped Thread Context Area (reads & writes)
OPAL does not use the grouping and crowd capability.
* Virtual Structure Tables
XIVE2 adds new tables types and also changes the field layout of the END
and NVP Virtualization Structure Descriptors.
- EAS
- END new layout
- NVT was splitted in :
. NVP (Processor), 32B
. NVG (Group), 32B
. NVC (Crowd == P9 block group) 32B
- IC for remote configuration
- SYNC for cache injection
- ERQ for event input queue
The setup is slighly different on XIVE2 because the indexing has changed
for some of the tables, block ID or the chip topology ID can be used.
* XIVE2 features
SCOM and MMIO registers have a new layout and XIVE2 adds a new global
capability and configuration registers.
The lowlevel hardware offers a set of new features among which :
- a configurable number of priorities : 1 - 8
- StoreEOI with load-after-store ordering is activated by default
- Gen2 TIMA layout
- A P9-compat mode, or Gen1, TIMA toggle bit for SW compatibility
- increase to 24bit for VP number
Other features will have some impact on the Hypervisor and guest OS
when activated, but this is not required for initial support of the
controller.
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
The VP space is larger in XIVE2 (P10), 24 bits instead of 19bits on
XIVE (P9), and the CAM line can use a 7bits or 8bits thread id.
For now, we only use 7bits thread ids, same as P9, but because of the
change of the size of the VP space, the CAM matching routine is
different between P9 and P10. It is easier to duplicate the whole
routine than to add extra handlers in xive_presenter_tctx_match() used
for P9.
We might come with a better solution later on, after we have added
some more support for the XIVE2 controller.
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
The XIVE2 interrupt controller of the POWER10 processor as the same
logic as on POWER9 but its SW interface has been largely reworked. The
interrupt controller has a new register interface, different BARs,
extra VSDs. These will be described when we add the device model for
the baremetal machine.
The XIVE internal structures for the EAS, END, NVT have different
layouts which is a problem for the current core XIVE framework. To
avoid adding too much complexity in the XIVE models, a new XIVE2 core
framework is introduced. It duplicates the models which are closely
linked to the XIVE internal structures : Xive2Router and
Xive2ENDSource and reuses the XiveSource, XivePresenter, XiveTCTX
models, as they are more generic.
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
More than 1k of TypeInfo instances are already marked as const. Mark the
remaining ones, too.
This commit was created with:
git grep -z -l 'static TypeInfo' -- '*.c' | \
xargs -0 sed -i 's/static TypeInfo/static const TypeInfo/'
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Acked-by: Corey Minyard <cminyard@mvista.com>
Message-id: 20220117145805.173070-2-shentey@gmail.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
The RISC-V AIA (Advanced Interrupt Architecture) defines a new
interrupt controller for wired interrupts called APLIC (Advanced
Platform Level Interrupt Controller). The APLIC is capabable of
forwarding wired interupts to RISC-V HARTs directly or as MSIs
(Message Signaled Interupts).
This patch adds device emulation for RISC-V AIA APLIC.
Signed-off-by: Anup Patel <anup.patel@wdc.com>
Signed-off-by: Anup Patel <anup@brainfault.org>
Reviewed-by: Frank Chang <frank.chang@sifive.com>
Message-id: 20220204174700.534953-19-anup@brainfault.org
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
In most of the ITS command processing, we check different error
possibilities one at a time and log them appropriately. In
process_mapti() and process_mapd() we have code which checks
multiple error cases at once, which means the logging is less
specific than it could be. Split those cases up.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220201193207.2771604-14-peter.maydell@linaro.org
When handling MAPI/MAPTI, we allow the supplied interrupt ID to be
either 1023 or something in the valid LPI range. This is a mistake:
only a real valid LPI is allowed. (The general behaviour of the ITS
is that most interrupt ID fields require a value in the LPI range;
the exception is that fields specifying a doorbell value, which are
all in GICv4 commands, allow also 1023 to mean "no doorbell".)
Remove the condition that incorrectly allows 1023 here.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220201193207.2771604-13-peter.maydell@linaro.org
In the MAPC command, if V=0 this is a request to delete a collection
table entry and the rdbase field of the command packet will not be
used. In particular, the specification says that the "UNPREDICTABLE
if rdbase is not valid" only applies for V=1.
We were doing a check-and-log-guest-error on rdbase regardless of
whether the V bit was set, and also (harmlessly but confusingly)
storing the contents of the rdbase field into the updated collection
table entry. Update the code so that if V=0 we don't check or use
the rdbase field value.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220201193207.2771604-12-peter.maydell@linaro.org
Currently we track in the TableDesc and CmdQDesc structs the state of
the GITS_BASER<n> and GITS_CBASER Valid bits. However we aren't very
consistent abut checking the valid field: we test it in update_cte()
and update_dte(), but not anywhere else we look things up in tables.
The GIC specification says that it is UNPREDICTABLE if a guest fails
to set any of these Valid bits before enabling the ITS via
GITS_CTLR.Enabled. So we can choose to handle Valid == 0 as
equivalent to a zero-length table. This is in fact how we're already
catching this case in most of the table-access paths: when Valid is 0
we leave the num_entries fields in TableDesc or CmdQDesc set to zero,
and then the out-of-bounds check "index >= num_entries" that we have
to do anyway before doing any of these table lookups will always be
true, catching the no-valid-table case without any extra code.
So we can remove the checks on the valid field from update_cte()
and update_dte(): since these happen after the bounds check there
was never any case when the test could fail. That means the valid
fields would be entirely unused, so just remove them.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220201193207.2771604-11-peter.maydell@linaro.org
Make the update_ite() struct use the new ITEntry struct, so that
callers don't need to assemble the in-memory ITE data themselves, and
only get_ite() and update_ite() need to care about that in-memory
layout. We can then drop the no-longer-used IteEntry struct
definition.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220201193207.2771604-10-peter.maydell@linaro.org
In get_ite() we currently return the caller some of the fields of an
Interrupt Table Entry via a set of pointer arguments, and validate
some of them internally (interrupt type and valid bit) to return a
simple true/false 'valid' indication. Define a new ITEntry struct
which has all the fields that the in-memory ITE has, and bring the
get_ite() function in to line with get_dte() and get_cte().
This paves the way for handling virtual interrupts, which will want
a different subset of the fields in the ITE. Handling them under
the old "lots of pointer arguments" scheme would have meant a
confusingly large set of arguments for this function.
The new struct ITEntry is obviously confusably similar to the
existing IteEntry struct, whose fields are the raw 12 bytes
of the in-memory ITE. In the next commit we will make update_ite()
use ITEntry instead of IteEntry, which will allow us to delete
the IteEntry struct and remove the confusion.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220201193207.2771604-9-peter.maydell@linaro.org
The get_ite() code has some awkward nested if statements; clean
them up by returning early if the memory accesses fail.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220201193207.2771604-8-peter.maydell@linaro.org
In get_ite() and update_ite() we work with a 12-byte in-guest-memory
table entry, which we intend to handle as an 8-byte value followed by
a 4-byte value. Unfortunately the calculation of the address of the
4-byte value is wrong, because we write it as:
table_base_address + (index * entrysize) + 4
(obfuscated by the way the expression has been written)
when it should be + 8. This bug meant that we overwrote the top
bytes of the 8-byte value with the 4-byte value. There are no
guest-visible effects because the top half of the 8-byte value
contains only the doorbell interrupt field, which is used only in
GICv4, and the two bugs in the "write ITE" and "read ITE" codepaths
cancel each other out.
We can't simply change the calculation, because this would break
migration of a (TCG) guest from the old version of QEMU which had
in-guest-memory interrupt tables written using the buggy version of
update_ite(). We must also at the same time change the layout of the
fields within the ITE_L and ITE_H values so that the in-memory
locations of the fields we care about (VALID, INTTYPE, INTID and
ICID) stay the same.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220201193207.2771604-7-peter.maydell@linaro.org
Make update_cte() take a CTEntry struct rather than all the fields
of the new CTE as separate arguments.
This brings it into line with the update_dte() API.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220201193207.2771604-6-peter.maydell@linaro.org
In the ITS, a CTE is an entry in the collection table, which contains
multiple fields. Currently the function get_cte() which reads one
entry from the device table returns a success/failure boolean and
passes back the raw 64-bit integer CTE value via a pointer argument.
We then extract fields from the CTE as we need them.
Create a real C struct with the same fields as the CTE, and
populate it in get_cte(), so that that function and update_cte()
are the only ones which need to care about the in-guest-memory
format of the CTE.
This brings get_cte()'s API into line with get_dte().
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220201193207.2771604-5-peter.maydell@linaro.org
Make update_dte() take a DTEntry struct rather than all the fields of
the new DTE as separate arguments.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220201193207.2771604-4-peter.maydell@linaro.org
In the ITS, a DTE is an entry in the device table, which contains
multiple fields. Currently the function get_dte() which reads one
entry from the device table returns it as a raw 64-bit integer,
which we then pass around in that form, only extracting fields
from it as we need them.
Create a real C struct with the same fields as the DTE, and
populate it in get_dte(), so that that function and update_dte()
are the only ones that need to care about the in-guest-memory
format of the DTE.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220201193207.2771604-3-peter.maydell@linaro.org
Currently the ITS accesses each 8-byte doubleword in a 4-doubleword
command packet with a separate address_space_ldq_le() call. This is
awkward because the individual command processing functions have
ended up with code to handle "load more doublewords out of the
packet", which is both unwieldy and also a potential source of bugs
because it's not obvious when looking at a line that pulls a field
out of the 'value' variable which of the 4 doublewords that variable
currently holds.
Switch to using address_space_map() to map the whole command packet
at once and fish the four doublewords out of it. Then each process_*
function can start with a few lines of code that extract the fields
it cares about.
This requires us to split out the guts of process_its_cmd() into a
new do_process_its_cmd(), because we were previously overloading the
value and offset arguments as a backdoor way to directly pass the
devid and eventid from a write to GITS_TRANSLATER. The new
do_process_its_cmd() takes those arguments directly, and
process_its_cmd() is just a wrapper that does the "read fields from
command packet" part.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220201193207.2771604-2-peter.maydell@linaro.org
* Update copyright dates to 2022
* hw/armv7m: Fix broken VMStateDescription
* hw/char/exynos4210_uart: Fix crash on trying to load VM state
* rtc: Move RTC function prototypes to their own header
* xlnx-versal-virt: Support PMC SLCR
* xlnx-versal-virt: Support OSPI flash memory controller
* scripts: Explain the difference between linux-headers and standard-headers
* target/arm: Log CPU index in 'Taking exception' log
* arm_gicv3_its: Various bugfixes and cleanups
* arm_gicv3_its: Implement the missing MOVI and MOVALL commands
* ast2600: Fix address mapping of second SPI controller
* target/arm: Use correct entrypoint for SVC taken from Hyp to Hyp
-----BEGIN PGP SIGNATURE-----
iQJNBAABCAA3FiEE4aXFk81BneKOgxXPPCUl7RQ2DN4FAmH0C+AZHHBldGVyLm1h
eWRlbGxAbGluYXJvLm9yZwAKCRA8JSXtFDYM3gG4D/9biXPVdkOd7lIslRX0ihRg
AZkZrMNk6VF/MW6xJNVWWd+44cyjLopFqF5dS+Vjebt7pEtZvxY0K5mYmzClk6lg
2U89gWuLEDJDKNVfKAmsmj24Os4xRj4sJPq/Mee8lsBdOAwEQ3C36p0RnWGBcTJN
9VfzRMSGvdjQFJjGAaro078zrA1Q11msA4BbLht+YGTE1aeyryyfF/qGSRlrlTn8
+r0ZWBD4ttz8IsqSLtnpQvT6EbL79w0jBywVauVzCOGQGpti3HdHJNYR7cKgTMja
Hffx6f6iv/O4SAUUGS0WMWdfW/MEVxOFxJ7Zc2twGqDMuVWlFiLT0X1MZuHi0FpG
CjbhTsvJIrKom1Ib+LPkWscrlHHEf0cvME0WokErLOJDXvbqKj04oOkpQmqUIv0+
5j7o4mlQFuLXIyzcrBZxmwT/Ekg8KZA8aUR0ddUd0vBmGMdO2En/c4Qr/x4H2gXH
HL/18oPRaSV6mP08mxcda+hJ9m5MC+7l0+KKoDfaPM9d4hl5StI0zTlH+5ffbK+m
UWthMnrrZw2ZU8AzGPZxOAW5K5S3XOso5Z9credkRGuSDriaGuNY0s5gSvNawZGe
ioIrUl50t+5/o2+tba7FA2ePiGeC9/zS671zHG9Rdpe86JpJXCzWO7OYiVulV3Yu
dmQYrhgnUqNjh3SAiXUFVA==
=m7N5
-----END PGP SIGNATURE-----
Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20220128' into staging
target-arm queue:
* Update copyright dates to 2022
* hw/armv7m: Fix broken VMStateDescription
* hw/char/exynos4210_uart: Fix crash on trying to load VM state
* rtc: Move RTC function prototypes to their own header
* xlnx-versal-virt: Support PMC SLCR
* xlnx-versal-virt: Support OSPI flash memory controller
* scripts: Explain the difference between linux-headers and standard-headers
* target/arm: Log CPU index in 'Taking exception' log
* arm_gicv3_its: Various bugfixes and cleanups
* arm_gicv3_its: Implement the missing MOVI and MOVALL commands
* ast2600: Fix address mapping of second SPI controller
* target/arm: Use correct entrypoint for SVC taken from Hyp to Hyp
# gpg: Signature made Fri 28 Jan 2022 15:29:36 GMT
# gpg: using RSA key E1A5C593CD419DE28E8315CF3C2525ED14360CDE
# gpg: issuer "peter.maydell@linaro.org"
# gpg: Good signature from "Peter Maydell <peter.maydell@linaro.org>" [ultimate]
# gpg: aka "Peter Maydell <pmaydell@gmail.com>" [ultimate]
# gpg: aka "Peter Maydell <pmaydell@chiark.greenend.org.uk>" [ultimate]
# Primary key fingerprint: E1A5 C593 CD41 9DE2 8E83 15CF 3C25 25ED 1436 0CDE
* remotes/pmaydell/tags/pull-target-arm-20220128: (32 commits)
target/arm: Use correct entrypoint for SVC taken from Hyp to Hyp
hw/arm: ast2600: Fix address mapping of second SPI controller
hw/intc/arm_gicv3_its: Implement MOVI
hw/intc/arm_gicv3_its: Implement MOVALL
hw/intc/arm_gicv3_its: Check table bounds against correct limit
hw/intc/arm_gicv3_its: Make GITS_BASER<n> RAZ/WI for unimplemented registers
hw/intc/arm_gicv3_its: Provide read accessor for translation_ops
hw/intc/arm_gicv3: Set GICR_CTLR.CES if LPIs are supported
hw/intc/arm_gicv3_redist: Remove unnecessary zero checks
hw/intc/arm_gicv3_its: Sort ITS command list into numeric order
hw/intc/arm_gicv3: Honour GICD_CTLR.EnableGrp1NS for LPIs
hw/intc/arm_gicv3_its: Don't clear GITS_CWRITER on writes to GITS_CBASER
hw/intc/arm_gicv3_its: Don't clear GITS_CREADR when GITS_CTLR.ENABLED is set
hw/intc/arm_gicv3: Initialise dma_as in GIC, not ITS
hw/intc/arm_gicv3_its: Add tracepoints
target/arm: Log CPU index in 'Taking exception' log
scripts: Explain the difference between linux-headers and standard-headers
MAINTAINERS: Remove myself (for raspi).
MAINTAINERS: Add an entry for Xilinx Versal OSPI
hw/arm/xlnx-versal-virt: Connect mt35xu01g flashes to the OSPI
...
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Implement the ITS MOVI command. This command specifies a (physical) LPI
by DeviceID and EventID and provides a new ICID for it. The ITS must
find the interrupt translation table entry for the LPI, which will
tell it the old ICID. It then moves the pending state of the LPI from
the old redistributor to the new one and updates the ICID field in
the translation table entry.
This is another GICv3 ITS command that we forgot to implement. Linux
does use this one, but only if the guest powers off one of its CPUs.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220122182444.724087-15-peter.maydell@linaro.org
Implement the ITS MOVALL command, which takes all the pending
interrupts on a source redistributor and makes the not-pending on
that source redistributor and pending on a destination redistributor.
This is a GICv3 ITS command which we forgot to implement. (It is
not used by Linux guests.)
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220122182444.724087-14-peter.maydell@linaro.org
Currently when we fill in a TableDesc based on the value the guest
has written to the GITS_BASER<n> register, we calculate both:
* num_entries : the number of entries in the table, constrained
by the amount of memory the guest has given it
* num_ids : the number of IDs we support for this table,
constrained by the implementation choices and the architecture
(eg DeviceIDs are 16 bits, so num_ids is 1 << 16)
When validating ITS commands, however, we check only num_ids,
thus allowing a broken guest to specify table entries that
index off the end of it. This will only corrupt guest memory,
but the ITS is supposed to reject such commands as invalid.
Instead of calculating both num_entries and num_ids, set
num_entries to the minimum of the two limits, and check that.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220122182444.724087-13-peter.maydell@linaro.org
The ITS has a bank of 8 GITS_BASER<n> registers, which allow the
guest to specify the base address of various data tables. Each
register has a read-only type field indicating which table it is for
and a read-write field where the guest can write in the base address
(among other things). We currently allow the guest to write the
writeable fields for all eight registers, even if the type field is 0
indicating "Unimplemented". This means the guest can provoke QEMU
into asserting by writing an address into one of these unimplemented
base registers, which bypasses the "if (!value) continue" check in
extract_table_params() and lets us hit the assertion that the type
field is one of the permitted table types.
Prevent the assertion by not allowing the guest to write to the
unimplemented base registers. This means their value will remain 0
and extract_table_params() will ignore them.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220122182444.724087-12-peter.maydell@linaro.org
The MemoryRegionOps gicv3_its_translation_ops currently provides only
a .write_with_attrs function, because the only register in this
region is the write-only GITS_TRANSLATER. However, if you don't
provide a read function and the guest tries reading from this memory
region, QEMU will crash because
memory_region_read_with_attrs_accessor() calls a NULL pointer.
Add a read function which always returns 0, to cover both bogus
attempts to read GITS_TRANSLATER and also reads from the rest of the
region, which is documented to be reserved, RES0.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220122182444.724087-11-peter.maydell@linaro.org
The GICR_CTLR.CES bit is a read-only bit which is set to 1 to indicate
that the GICR_CTLR.EnableLPIs bit can be written to 0 to disable
LPIs (as opposed to allowing LPIs to be enabled but not subsequently
disabled). Our implementation permits this, so advertise it
by setting CES to 1.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220122182444.724087-10-peter.maydell@linaro.org
The ITS-related parts of the redistributor code make some checks for
whether registers like GICR_PROPBASER and GICR_PENDBASER are zero.
There is no requirement in the specification for treating zeroes in
these address registers specially -- they contain guest physical
addresses and it is entirely valid (if unusual) for the guest to
choose to put the tables they address at guest physical address zero.
We use these values only to calculate guest addresses, and attempts
by the guest to use a bad address will be handled by the
address_space_* functions which we use to do the loads and stores.
Remove the unnecessary checks.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220122182444.724087-9-peter.maydell@linaro.org
The list of #defines for the ITS command packet numbers is neither
in alphabetical nor numeric order. Sort it into numeric order.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220122182444.724087-8-peter.maydell@linaro.org
The GICD_CTLR distributor register has enable bits which control
whether the different interrupt groups (Group 0, Non-secure Group 1
and Secure Group 1) are forwarded to the CPU. We get this right for
traditional interrupts, but forgot to account for it when adding
LPIs. LPIs are always Group 1 NS and if the EnableGrp1NS bit is not
set we must not forward them to the CPU.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220122182444.724087-7-peter.maydell@linaro.org
The ITS specification says that when the guest writes to GITS_CBASER
this causes GITS_CREADR to be cleared. However it does not have an
equivalent clause for GITS_CWRITER. (This is because GITS_CREADR is
read-only, but GITS_CWRITER is writable and the guest can initialize
it.) Remove the code that clears GITS_CWRITER on GITS_CBASER writes.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220122182444.724087-6-peter.maydell@linaro.org
The current ITS code clears GITS_CREADR when GITS_CTLR.ENABLED is set.
This is not correct -- guest code can validly clear ENABLED and then
set it again and expect the ITS to continue processing where it left
off. Remove the erroneous assignment.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220122182444.724087-5-peter.maydell@linaro.org
In our implementation, all ITSes connected to a GIC share a single
AddressSpace, which we keep in the GICv3State::dma_as field and
initialized based on the GIC's 'sysmem' property. The right place
to set it up by calling address_space_init() is therefore in the
GIC's realize method, not the ITS's realize.
This fixes a theoretical bug where QEMU hangs on startup if the board
model creates two ITSes connected to the same GIC -- we would call
address_space_init() twice on the same AddressSpace*, which creates
an infinite loop in the QTAILQ that softmmu/memory.c uses to store
its list of AddressSpaces and causes any subsequent attempt to
iterate through that list to loop forever. There aren't any board
models like that in the tree at the moment, though.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220122182444.724087-4-peter.maydell@linaro.org
The ITS currently has no tracepoints; add a minimal set
that allows basic monitoring of guest register accesses and
reading of commands from the command queue.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220122182444.724087-3-peter.maydell@linaro.org
The ldq_be_dma() routine was recently changed to return a result of
the transaction. Use it when loading the virtual structure descriptors
in the XIVE PowerNV model.
Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Message-Id: <20220124081635.3672439-1-clg@kaod.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
The functions are only used within their respective source files, so no
need for exporting.
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Message-Id: <20220116122327.73048-1-shentey@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Get kernel and fdt start address in virt.c, and pass them to KVM
when cpu reset. Add kvm_riscv.h to place riscv specific interface.
In addition, PLIC is created without M-mode PLIC contexts when KVM
is enabled.
Signed-off-by: Yifei Jiang <jiangyifei@huawei.com>
Signed-off-by: Mingwang Li <limingwang@huawei.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
Message-id: 20220112081329.1835-7-jiangyifei@huawei.com
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Quoting Peter Maydell:
"These MEMTX_* aren't from the memory transaction
API functions; they're just being used by gicd_readl() and
friends as a way to indicate a success/failure so that the
actual MemoryRegionOps read/write fns like gicv3_dist_read()
can log a guest error."
We are going to introduce more MemTxResult bits, so it is
safer to check for !MEMTX_OK rather than MEMTX_ERROR.
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
In process_its_cmd(), we read an ICID out of the interrupt table
entry, and then use it as an index into the collection table. Add a
check that it is within range for the collection table first.
This check is not strictly necessary, because:
* we range check the ICID from the guest before writing it into
the interrupt table entry, so the the only way to get an
out of range ICID in process_its_cmd() is if a badly-behaved
guest is writing directly to the interrupt table memory
* the collection table is in guest memory, so QEMU won't fall
over if we read off the end of it
However, it seems clearer to include the check.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-id: 20220111171048.3545974-14-peter.maydell@linaro.org
In a few places in the ITS command handling functions, we were
doing the range-check of an event ID or device ID only after using
it as a table index; move the checks to before the uses.
This misordering wouldn't have very bad effects because the
tables are in guest memory anyway.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-id: 20220111171048.3545974-13-peter.maydell@linaro.org
The ITS has several tables which all share a similar format,
described by the TableDesc struct: the guest may configure them
to be a single-level table or a two-level table. Currently we
open-code the process of finding the table entry in all the
functions which read or write the device table or the collection
table. Factor out the "get the address of the table entry"
logic into a new function, so that the code which needs to
read or write a table entry only needs to call table_entry_addr()
and then perform a suitable load or store to that address.
Note that the error handling is slightly complicated because
we want to handle two cases differently:
* failure to read the L1 table entry should end up causing
a command stall, like other kinds of DMA error
* an L1 table entry that says there is no L2 table for this
index (ie whose valid bit is 0) must result in us treating
the table entry as not-valid on read, and discarding
writes (this is mandated by the spec)
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-id: 20220111171048.3545974-12-peter.maydell@linaro.org
Fix process_mapd() to consistently return CMD_STALL for memory
errors and CMD_CONTINUE for parameter errors, as we claim in the
comments that we do.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220111171048.3545974-11-peter.maydell@linaro.org
Fix process_mapc() to consistently return CMD_STALL for memory
errors and CMD_CONTINUE for parameter errors, as we claim in the
comments that we do.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220111171048.3545974-10-peter.maydell@linaro.org
Fix process_mapti() to consistently return CMD_STALL for memory
errors and CMD_CONTINUE for parameter errors, as we claim in the
comments that we do.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220111171048.3545974-9-peter.maydell@linaro.org
Refactor process_its_cmd() so that it consistently uses
the structure
do thing;
if (error condition) {
return early;
}
do next thing;
rather than doing some of the work nested inside if (not error)
code blocks.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220111171048.3545974-8-peter.maydell@linaro.org
Fix process_its_cmd() to consistently return CMD_STALL for
memory errors and CMD_CONTINUE for parameter errors, as
we claim in the comments that we do.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220111171048.3545974-7-peter.maydell@linaro.org
When an ITS detects an error in a command, it has an
implementation-defined (CONSTRAINED UNPREDICTABLE) choice of whether
to ignore the command, proceeding to the next one in the queue, or to
stall the ITS command queue, processing nothing further. The
behaviour required when the read of the command packet from memory
fails is less clearly documented, but the same set of choices as for
command errors seem reasonable.
The intention of the QEMU implementation, as documented in the
comments, is that if we encounter a memory error reading the command
packet or one of the various data tables then we should stall, but
for command parameter errors we should ignore the queue and continue.
However, we don't actually do this. To get the desired behaviour,
the various process_* functions need to return true to cause
process_cmdq() to advance to the next command and keep processing,
and false to stall command processing. What they mostly do is return
false for any kind of error.
To make the code clearer, replace the 'bool' return from the process_
functions with an enum which may be either CMD_STALL or CMD_CONTINUE.
In this commit no behaviour changes; in subsequent commits we will
adjust the error-return paths for the process_ functions one by one.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20220111171048.3545974-6-peter.maydell@linaro.org
In process_cmdq(), we read 64 bits of the command packet, which
contain the command identifier, which we then switch() on to dispatch
to an appropriate sub-function. However, if address_space_ldq_le()
reports a memory transaction failure, we still read the command
identifier out of the data and switch() on it. Restructure the code
so that we stop immediately (stalling the command queue) in this
case.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220111171048.3545974-5-peter.maydell@linaro.org
process_its_cmd() returns a bool, like all the other process_ functions.
However we were putting its return value into 'res', not 'result',
which meant we would ignore it when deciding whether to continue
or stall the command queue. Fix the typo.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20220111171048.3545974-4-peter.maydell@linaro.org
The bounds check on the number of interrupt IDs is correct, but
doesn't match our convention; change the variable name, initialize it
to the 2^n value rather than (2^n)-1, and use >= instead of > in the
comparison.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-id: 20220111171048.3545974-3-peter.maydell@linaro.org
In process_its_cmd() and process_mapti() we must check the
event ID against a limit defined by the size field in the DTE,
which specifies the number of ID bits minus one. Convert
this code to our num_foo convention:
* change the variable names
* use uint64_t and 1ULL when calculating the number
of valid event IDs, because DTE.SIZE is 5 bits and
so num_eventids may be up to 2^32
* fix the off-by-one error in the comparison
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-id: 20220111171048.3545974-2-peter.maydell@linaro.org
When running Linux on a machine with GICv2, the kernel can crash while
processing an interrupt and can subsequently start a kdump kernel from
the active interrupt handler. In such a case, the crashed kernel might
not gracefully signal the end of interrupt to the GICv2 hardware. The
kdump kernel will however try to reset the GIC state on startup to get
the controller into a sane state, in particular the kernel writes ones
to GICD_ICACTIVERn and wipes out GICC_APRn to make sure that no
interrupt is active.
The patch adds a logic to recalculate the running priority when
GICC_APRn/GICC_NSAPRn is written which makes sure that the mentioned
reset works with the GICv2 emulation in QEMU too and the kdump kernel
starts receiving interrupts.
The described scenario can be reproduced on an AArch64 QEMU virt machine
with a kdump-enabled Linux system by using the softdog module. The kdump
kernel will hang at some point because QEMU still thinks the running
priority is that of the timer interrupt and asserts no new interrupts to
the system:
$ modprobe softdog soft_margin=10 soft_panic=1
$ cat > /dev/watchdog
[Press Enter to start the watchdog, wait for its timeout and observe
that the kdump kernel hangs on startup.]
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
Message-id: 20220113151916.17978-3-ppavlu@suse.cz
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Implement support for reading GICC_IIDR. This register is used by the
Linux kernel to recognize that GICv2 with GICC_APRn is present.
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
Message-id: 20220113151916.17978-2-ppavlu@suse.cz
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
We can remove the original sifive_plic_irqs_pending() function and
instead just use the sifive_plic_claim() function (renamed to
sifive_plic_claimed()) to determine if any interrupts are pending.
This requires move the side effects outside of sifive_plic_claimed(),
but as they are only invoked once that isn't a problem.
We have also removed all of the old #ifdef debugging logs, so let's
cleanup the last remaining debug function while we are here.
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Message-Id: <20220105213937.1113508-5-alistair.francis@opensource.wdc.com>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Message-Id: <20220105213937.1113508-4-alistair.francis@opensource.wdc.com>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Message-Id: <20220105213937.1113508-3-alistair.francis@opensource.wdc.com>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Message-Id: <20220105213937.1113508-2-alistair.francis@opensource.wdc.com>
In several places we have a local variable max_l2_entries which is
the number of entries which will fit in a level 2 table. The
calculations done on this value are correct; rename it to
num_l2_entries to fit the convention we're using in this code.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
The ITS code has to check whether various parameters passed in
commands are in-bounds, where the limit is defined in terms of the
number of bits that are available for the parameter. (For example,
the GITS_TYPER.Devbits ID register field specifies the number of
DeviceID bits minus 1, and device IDs passed in the MAPTI and MAPD
command packets must fit in that many bits.)
Currently we have off-by-one bugs in many of these bounds checks.
The typical problem is that we define a max_foo as 1 << n. In
the Devbits example, we set
s->dt.max_ids = 1UL << (GITS_TYPER.Devbits + 1).
However later when we do the bounds check we write
if (devid > s->dt.max_ids) { /* command error */ }
which incorrectly permits a devid of 1 << n.
These bugs will not cause QEMU crashes because the ID values being
checked are only used for accesses into tables held in guest memory
which we access with address_space_*() functions, but they are
incorrect behaviour of our emulation.
Fix them by standardizing on this pattern:
* bounds limits are named num_foos and are the 2^n value
(equal to the number of valid foo values)
* bounds checks are either
if (fooid < num_foos) { good }
or
if (fooid >= num_foos) { bad }
In this commit we fix the handling of the number of IDs
in the device table and the collection table, and the number
of commands that will fit in the command queue.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Use FIELD macros to handle CTEs, rather than ad-hoc mask-and-shift.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
The comment says that in our CTE format the RDBase field is 36 bits;
in fact for us it is only 16 bits, because we use the RDBase format
where it specifies a 16-bit CPU number. The code already uses
RDBASE_PROCNUM_LENGTH (16) as the field width, so fix the comment
to match it.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Peter Maydell
Jim Shu
Frank Chang
Richard Henderson
Marc-André Lureau
Markus Armbruster
Eric Auger
Cédric Le Goater
Anup Patel
Bernhard Beschow
Yifei Jiang
Philippe Mathieu-Daudé
Petr Pavlu
Alistair Francis