[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v7 04/10] hw/intc: GICv3 ITS Command processing
From: |
Neil Armstrong |
Subject: |
Re: [PATCH v7 04/10] hw/intc: GICv3 ITS Command processing |
Date: |
Fri, 6 Aug 2021 10:58:24 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 |
Hi,
On 06/08/2021 00:29, Shashi Mallela wrote:
> Added ITS command queue handling for MAPTI,MAPI commands,handled ITS
> translation which triggers an LPI via INT command as well as write
> to GITS_TRANSLATER register,defined enum to differentiate between ITS
> command interrupt trigger and GITS_TRANSLATER based interrupt trigger.
> Each of these commands make use of other functionalities implemented to
> get device table entry,collection table entry or interrupt translation
> table entry required for their processing.
>
> Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/intc/arm_gicv3_its.c | 348 +++++++++++++++++++++++++++++
> hw/intc/gicv3_internal.h | 12 +
> include/hw/intc/arm_gicv3_common.h | 2 +
> 3 files changed, 362 insertions(+)
>
> diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
> index 8bdbebbeca..35308f1c32 100644
> --- a/hw/intc/arm_gicv3_its.c
> +++ b/hw/intc/arm_gicv3_its.c
> @@ -29,6 +29,22 @@ struct GICv3ITSClass {
> void (*parent_reset)(DeviceState *dev);
> };
>
> +/*
> + * This is an internal enum used to distinguish between LPI triggered
> + * via command queue and LPI triggered via gits_translater write.
> + */
> +typedef enum ItsCmdType {
> + NONE = 0, /* internal indication for GITS_TRANSLATER write */
> + CLEAR = 1,
> + DISCARD = 2,
> + INT = 3,
> +} ItsCmdType;
> +
> +typedef struct {
> + uint32_t iteh;
> + uint64_t itel;
> +} IteEntry;
> +
> static uint64_t baser_base_addr(uint64_t value, uint32_t page_sz)
> {
> uint64_t result = 0;
> @@ -50,6 +66,320 @@ static uint64_t baser_base_addr(uint64_t value, uint32_t
> page_sz)
> return result;
> }
>
> +static bool get_cte(GICv3ITSState *s, uint16_t icid, uint64_t *cte,
> + MemTxResult *res)
> +{
> + AddressSpace *as = &s->gicv3->dma_as;
> + uint64_t l2t_addr;
> + uint64_t value;
> + bool valid_l2t;
> + uint32_t l2t_id;
> + uint32_t max_l2_entries;
> +
> + if (s->ct.indirect) {
> + l2t_id = icid / (s->ct.page_sz / L1TABLE_ENTRY_SIZE);
> +
> + value = address_space_ldq_le(as,
> + s->ct.base_addr +
> + (l2t_id * L1TABLE_ENTRY_SIZE),
> + MEMTXATTRS_UNSPECIFIED, res);
> +
> + if (*res == MEMTX_OK) {
> + valid_l2t = (value & L2_TABLE_VALID_MASK) != 0;
> +
> + if (valid_l2t) {
> + max_l2_entries = s->ct.page_sz / s->ct.entry_sz;
> +
> + l2t_addr = value & ((1ULL << 51) - 1);
> +
> + *cte = address_space_ldq_le(as, l2t_addr +
> + ((icid % max_l2_entries) *
> GITS_CTE_SIZE),
> + MEMTXATTRS_UNSPECIFIED, res);
> + }
> + }
> + } else {
> + /* Flat level table */
> + *cte = address_space_ldq_le(as, s->ct.base_addr +
> + (icid * GITS_CTE_SIZE),
> + MEMTXATTRS_UNSPECIFIED, res);
> + }
> +
> + return (*cte & TABLE_ENTRY_VALID_MASK) != 0;
> +}
> +
> +static MemTxResult update_ite(GICv3ITSState *s, uint32_t eventid, uint64_t
> dte,
> + IteEntry ite)
> +{
> + AddressSpace *as = &s->gicv3->dma_as;
> + uint64_t itt_addr;
> + MemTxResult res = MEMTX_OK;
> +
> + itt_addr = (dte & GITS_DTE_ITTADDR_MASK) >> GITS_DTE_ITTADDR_SHIFT;
> + itt_addr <<= ITTADDR_SHIFT; /* 256 byte aligned */
> +
> + address_space_stq_le(as, itt_addr + (eventid * sizeof(uint64_t)),
> + ite.itel, MEMTXATTRS_UNSPECIFIED, &res);
> +
> + if (res == MEMTX_OK) {
> + address_space_stl_le(as, itt_addr + ((eventid + sizeof(uint64_t)) *
> + sizeof(uint32_t)), ite.iteh,
> + MEMTXATTRS_UNSPECIFIED, &res);
While adding support for ITS on Zephyr, I've spotted an issue with the ITE
entry storage here.
>From eventid 0 to 7, it goes well, but from 8 and all even eventids, the table
>gets trashed.
The actual storage is:
itel: itt_addr + (eventid * 8)
iteh: itt_addr + ((eventid + 8) * 4)
For the first eventIDs, the offsets are:
EvenID itel iteh
0 0 32
1 8 36
2 16 40
3 24 44
4 32 48
5 40 52
6 48 56
7 56 60
8 64 64 <= the entry 8 simply disappears
9 72 68
10 80 72 <= trashed 9
11 88 76
12 96 80 <= trashes 10
and so on.
The correct storage would be:
address_space_stq_le(as, itt_addr + (eventid * (sizeof(uint64_t) +
sizeof(uint32_t))),
itel, MEMTXATTRS_UNSPECIFIED, &res);
if (res == MEMTX_OK) {
address_space_stl_le(as, itt_addr + (eventid * (sizeof(uint64_t) +
sizeof(uint32_t))) +
sizeof(uint32_t), iteh, MEMTXATTRS_UNSPECIFIED,
&res);
}
To store iteh right after itel.
> + }
> + return res;
> +}
> +
> +static bool get_ite(GICv3ITSState *s, uint32_t eventid, uint64_t dte,
> + uint16_t *icid, uint32_t *pIntid, MemTxResult *res)
> +{
> + AddressSpace *as = &s->gicv3->dma_as;
> + uint64_t itt_addr;
> + bool status = false;
> + IteEntry ite = {};
> +
> + itt_addr = (dte & GITS_DTE_ITTADDR_MASK) >> GITS_DTE_ITTADDR_SHIFT;
> + itt_addr <<= ITTADDR_SHIFT; /* 256 byte aligned */
> +
> + ite.itel = address_space_ldq_le(as, itt_addr +
> + (eventid * sizeof(uint64_t)),
> + MEMTXATTRS_UNSPECIFIED, res);
> +
> + if (*res == MEMTX_OK) {
> + ite.iteh = address_space_ldl_le(as, itt_addr + ((eventid +
> + sizeof(uint64_t)) * sizeof(uint32_t)),
> + MEMTXATTRS_UNSPECIFIED, res);
Same here:
itel = address_space_ldq_le(as, itt_addr + (eventid * (sizeof(uint64_t) +
sizeof(uint32_t))),
MEMTXATTRS_UNSPECIFIED, res);
if (*res == MEMTX_OK) {
iteh = address_space_ldl_le(as, itt_addr + (eventid * (sizeof(uint64_t)
+ sizeof(uint32_t))) + sizeof(uint32_t),
MEMTXATTRS_UNSPECIFIED, res);
> +
> + if (*res == MEMTX_OK) {
> + if (ite.itel & TABLE_ENTRY_VALID_MASK) {
> + if ((ite.itel >> ITE_ENTRY_INTTYPE_SHIFT) &
> + GITS_TYPE_PHYSICAL) {
> + *pIntid = (ite.itel & ITE_ENTRY_INTID_MASK) >>
> + ITE_ENTRY_INTID_SHIFT;
> + *icid = ite.iteh & ITE_ENTRY_ICID_MASK;
> + status = true;
> + }
> + }
> + }
> + }
> + return status;
> +}
> +
This fixes the issue I see when mapping multiple eventIDs, then trying to
exercise them with the INT command.
Thanks,
Neil
- Re: [PATCH v7 01/10] hw/intc: GICv3 ITS initial framework, (continued)
- [PATCH v7 03/10] hw/intc: GICv3 ITS command queue framework, Shashi Mallela, 2021/08/05
- [PATCH v7 02/10] hw/intc: GICv3 ITS register definitions added, Shashi Mallela, 2021/08/05
- [PATCH v7 06/10] hw/intc: GICv3 redistributor ITS processing, Shashi Mallela, 2021/08/05
- [PATCH v7 05/10] hw/intc: GICv3 ITS Feature enablement, Shashi Mallela, 2021/08/05
- [PATCH v7 04/10] hw/intc: GICv3 ITS Command processing, Shashi Mallela, 2021/08/05
- Re: [PATCH v7 04/10] hw/intc: GICv3 ITS Command processing,
Neil Armstrong <=
- [PATCH v7 07/10] hw/arm/sbsa-ref: add ITS support in SBSA GIC, Shashi Mallela, 2021/08/05
- [PATCH v7 08/10] tests/data/acpi/virt: Add IORT files for ITS, Shashi Mallela, 2021/08/05
- [PATCH v7 09/10] hw/arm/virt: add ITS support in virt GIC, Shashi Mallela, 2021/08/05
- [PATCH v7 10/10] tests/data/acpi/virt: Update IORT files for ITS, Shashi Mallela, 2021/08/05