All of lore.kernel.org
 help / color / mirror / Atom feed
From: Richard Henderson <richard.henderson@linaro.org>
To: Peter Maydell <peter.maydell@linaro.org>,
	qemu-arm@nongnu.org, qemu-devel@nongnu.org
Cc: "Shashi Mallela" <shashi.mallela@linaro.org>,
	"Alex Bennée" <alex.bennee@linaro.org>
Subject: Re: [PATCH 10/13] hw/intc/arm_gicv3_its: Drop TableDesc and CmdQDesc valid fields
Date: Thu, 3 Feb 2022 15:18:29 +1100	[thread overview]
Message-ID: <abc3e104-7606-5a66-754d-99c55f1e8571@linaro.org> (raw)
In-Reply-To: <20220201193207.2771604-11-peter.maydell@linaro.org>

On 2/2/22 06:32, Peter Maydell wrote:
> 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>
> ---
>   include/hw/intc/arm_gicv3_its_common.h |  2 --
>   hw/intc/arm_gicv3_its.c                | 31 ++++++++++++--------------
>   2 files changed, 14 insertions(+), 19 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~


  reply	other threads:[~2022-02-03  4:24 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-01 19:31 [PATCH 00/13] hw/intc/arm_gicv3_its: more cleanups, bugfixes Peter Maydell
2022-02-01 19:31 ` [PATCH 01/13] hw/intc/arm_gicv3_its: Use address_space_map() to access command queue packets Peter Maydell
2022-02-03  2:15   ` Richard Henderson
2022-02-01 19:31 ` [PATCH 02/13] hw/intc/arm_gicv3_its: Keep DTEs as a struct, not a raw uint64_t Peter Maydell
2022-02-03  2:23   ` Richard Henderson
2022-02-01 19:31 ` [PATCH 03/13] hw/intc/arm_gicv3_its: Pass DTEntry to update_dte() Peter Maydell
2022-02-03  2:30   ` Richard Henderson
2022-02-01 19:31 ` [PATCH 04/13] hw/intc/arm_gicv3_its: Keep CTEs as a struct, not a raw uint64_t Peter Maydell
2022-02-03  2:58   ` Richard Henderson
2022-02-01 19:31 ` [PATCH 05/13] hw/intc/arm_gicv3_its: Pass CTEntry to update_cte() Peter Maydell
2022-02-03  3:00   ` Richard Henderson
2022-02-01 19:32 ` [PATCH 06/13] hw/intc/arm_gicv3_its: Fix address calculation in get_ite() and update_ite() Peter Maydell
2022-02-03  3:59   ` Richard Henderson
2022-02-03 10:45     ` Peter Maydell
2022-02-03 22:02       ` Richard Henderson
2022-02-01 19:32 ` [PATCH 07/13] hw/intc/arm_gicv3_its: Avoid nested ifs in get_ite() Peter Maydell
2022-02-03  4:01   ` Richard Henderson
2022-02-01 19:32 ` [PATCH 08/13] hw/intc/arm_gicv3_its: Pass ITE values back from get_ite() via a struct Peter Maydell
2022-02-03  4:05   ` Richard Henderson
2022-02-01 19:32 ` [PATCH 09/13] hw/intc/arm_gicv3_its: Make update_ite() use ITEntry Peter Maydell
2022-02-03  4:09   ` Richard Henderson
2022-02-01 19:32 ` [PATCH 10/13] hw/intc/arm_gicv3_its: Drop TableDesc and CmdQDesc valid fields Peter Maydell
2022-02-03  4:18   ` Richard Henderson [this message]
2022-02-01 19:32 ` [PATCH 11/13] hw/intc/arm_gicv3_its: In MAPC with V=0, don't check rdbase field Peter Maydell
2022-02-03  4:24   ` Richard Henderson
2022-02-01 19:32 ` [PATCH 12/13] hw/intc/arm_gicv3_its: Don't allow intid 1023 in MAPI/MAPTI Peter Maydell
2022-02-03  4:25   ` Richard Henderson
2022-02-01 19:32 ` [PATCH 13/13] hw/intc/arm_gicv3_its: Split error checks Peter Maydell
2022-02-03  4:26   ` Richard Henderson
2022-02-07 17:56 ` [PATCH 00/13] hw/intc/arm_gicv3_its: more cleanups, bugfixes 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=abc3e104-7606-5a66-754d-99c55f1e8571@linaro.org \
    --to=richard.henderson@linaro.org \
    --cc=alex.bennee@linaro.org \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.