From: Peter Maydell <peter.maydell@linaro.org>
To: Shashi Mallela <shashi.mallela@linaro.org>
Cc: Leif Lindholm <leif@nuviainc.com>,
QEMU Developers <qemu-devel@nongnu.org>,
qemu-arm <qemu-arm@nongnu.org>,
Radoslaw Biernacki <rad@semihalf.com>
Subject: Re: [PATCH v4 4/8] hw/intc: GICv3 ITS Command processing
Date: Tue, 8 Jun 2021 11:45:47 +0100 [thread overview]
Message-ID: <CAFEAcA983niE409cJtWAZ3A03uR21eZGmGsncR09aQ9QLc=syw@mail.gmail.com> (raw)
In-Reply-To: <20210602180042.111347-5-shashi.mallela@linaro.org>
On Wed, 2 Jun 2021 at 19:00, Shashi Mallela <shashi.mallela@linaro.org> 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>
> ---
> hw/intc/arm_gicv3_its.c | 334 +++++++++++++++++++++++++++++
> hw/intc/gicv3_internal.h | 12 ++
> include/hw/intc/arm_gicv3_common.h | 2 +
> 3 files changed, 348 insertions(+)
>
> diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
> index 6551c577b3..82bb5b84ef 100644
> --- a/hw/intc/arm_gicv3_its.c
> +++ b/hw/intc/arm_gicv3_its.c
> @@ -28,6 +28,13 @@ struct GICv3ITSClass {
> void (*parent_reset)(DeviceState *dev);
> };
>
> +typedef enum ItsCmdType {
> + NONE = 0, /* internal indication for GITS_TRANSLATER write */
> + CLEAR = 1,
> + DISCARD = 2,
> + INT = 3,
> +} ItsCmdType;
> +
> static uint64_t baser_base_addr(uint64_t value, uint32_t page_sz)
> {
> uint64_t result = 0;
> @@ -49,6 +56,315 @@ 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;
> + bool status = false;
> +
> + 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 >> VALID_SHIFT) & VALID_MASK;
VALID_MASK should be the mask in its shifted location
(for consistency with how the FIELD macros do it). Then
this is just
valid_l2t = (value & 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);
> + }
> +
> + if (*cte & VALID_MASK) {
> + status = true;
> + }
> +
> + return status;
You don't need the 'status' variable, you can just
return (*cte & VALID_MASK) != 0;
(Looks like this code is already assuming VALID_MASK is the mask
in its shifted location, and so inconsistent with your current definition ?)
> +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;
> + uint64_t itel = 0;
> + uint32_t iteh = 0;
> +
> + itt_addr = (dte >> 6ULL) & ITTADDR_MASK;
> + itt_addr <<= ITTADDR_SHIFT; /* 256 byte aligned */
> +
> + itel = address_space_ldq_le(as, itt_addr + (eventid * sizeof(uint64_t)),
> + MEMTXATTRS_UNSPECIFIED, res);
> +
> + if (*res == MEMTX_OK) {
> + iteh = address_space_ldl_le(as, itt_addr + ((eventid +
> + sizeof(uint64_t)) * sizeof(uint32_t)),
> + MEMTXATTRS_UNSPECIFIED, res);
> +
> + if (*res == MEMTX_OK) {
> + if (itel & VALID_MASK) {
> + if ((itel >> ITE_ENTRY_INTTYPE_SHIFT) & GITS_TYPE_PHYSICAL) {
> + *pIntid = (itel >> ITE_ENTRY_INTID_SHIFT) &
> + ITE_ENTRY_INTID_MASK;
More _MASK constants that don't have the same semantics as the
registerfields versions. Please can you change all of these ?
> + *icid = iteh & ITE_ENTRY_ICID_MASK;
> + status = true;
> + }
> + }
> + }
> + }
> + return status;
> +}
> +
> + if ((devid > s->dt.max_devids) || !dte_valid || !ite_valid ||
> + !cte_valid || (eventid > max_eventid)) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: invalid interrupt translation table attributes "
> + "devid %d or eventid %d\n",
> + __func__, devid, eventid);
> + /*
> + * in this implementation,in case of error
Another missing space after comma.
> + /*
> + * in this implementation,in case of error
And again.
> + * we ignore this command and move onto the next
> + * command in the queue
> + */
thanks
-- PMM
next prev parent reply other threads:[~2021-06-08 10:47 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-02 18:00 [PATCH v4 0/8] GICv3 LPI and ITS feature implementation Shashi Mallela
2021-06-02 18:00 ` [PATCH v4 1/8] hw/intc: GICv3 ITS initial framework Shashi Mallela
2021-06-08 10:02 ` Peter Maydell
2021-06-11 16:21 ` Eric Auger
2021-06-11 17:23 ` Shashi Mallela
2021-07-06 7:38 ` Eric Auger
2021-07-06 13:24 ` shashi.mallela
2021-07-06 14:04 ` Eric Auger
2021-07-06 14:18 ` shashi.mallela
2021-06-12 6:52 ` Eric Auger
2021-07-06 7:29 ` Eric Auger
2021-06-02 18:00 ` [PATCH v4 2/8] hw/intc: GICv3 ITS register definitions added Shashi Mallela
2021-06-08 10:31 ` Peter Maydell
2021-06-12 6:08 ` Eric Auger
2021-06-16 21:02 ` shashi.mallela
2021-06-21 9:51 ` Eric Auger
2021-06-28 21:51 ` shashi.mallela
2021-06-02 18:00 ` [PATCH v4 3/8] hw/intc: GICv3 ITS command queue framework Shashi Mallela
2021-06-08 10:38 ` Peter Maydell
2021-06-13 14:13 ` Eric Auger
2021-06-16 21:02 ` shashi.mallela
2021-06-21 10:03 ` Eric Auger
2021-06-28 21:58 ` shashi.mallela
2021-06-13 14:39 ` Eric Auger
2021-06-28 15:55 ` shashi.mallela
2021-06-02 18:00 ` [PATCH v4 4/8] hw/intc: GICv3 ITS Command processing Shashi Mallela
2021-06-08 10:45 ` Peter Maydell [this message]
2021-06-13 15:55 ` Eric Auger
2021-06-16 21:02 ` shashi.mallela
2021-06-21 10:13 ` Eric Auger
2021-06-28 22:04 ` shashi.mallela
2021-06-02 18:00 ` [PATCH v4 5/8] hw/intc: GICv3 ITS Feature enablement Shashi Mallela
2021-06-08 10:57 ` Peter Maydell
2021-06-02 18:00 ` [PATCH v4 6/8] hw/intc: GICv3 redistributor ITS processing Shashi Mallela
2021-06-08 13:57 ` Peter Maydell
2021-06-10 23:39 ` Shashi Mallela
2021-06-11 8:30 ` Peter Maydell
2021-06-15 2:23 ` Shashi Mallela
2021-06-13 16:26 ` Eric Auger
2021-06-16 21:02 ` shashi.mallela
2021-06-02 18:00 ` [PATCH v4 7/8] hw/arm/sbsa-ref: add ITS support in SBSA GIC Shashi Mallela
2021-06-03 11:42 ` Leif Lindholm
2021-06-03 15:31 ` shashi.mallela
2021-06-04 10:42 ` Leif Lindholm
2021-06-04 15:36 ` shashi.mallela
2021-07-08 19:40 ` Leif Lindholm
2021-07-08 20:05 ` Peter Maydell
2021-07-08 22:05 ` Leif Lindholm
2021-08-05 20:10 ` shashi.mallela
2021-06-02 18:00 ` [PATCH v4 8/8] hw/arm/virt: add ITS support in virt GIC Shashi Mallela
2021-06-08 11:00 ` Peter Maydell
2021-06-08 10:00 ` [PATCH v4 0/8] GICv3 LPI and ITS feature implementation Peter Maydell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAFEAcA983niE409cJtWAZ3A03uR21eZGmGsncR09aQ9QLc=syw@mail.gmail.com' \
--to=peter.maydell@linaro.org \
--cc=leif@nuviainc.com \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=rad@semihalf.com \
--cc=shashi.mallela@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).