From a120157b24c78c2d890cd9793eb5a1cbbf42c9a9 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 7 Jan 2022 17:07:57 +0000 Subject: hw/intc/arm_gicv3_its: Correct off-by-one bounds check on rdbase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The checks in the ITS on the rdbase values in guest commands are off-by-one: they permit the guest to pass us a value equal to s->gicv3->num_cpu, but the valid values are 0...num_cpu-1. This meant the guest could cause us to index off the end of the s->gicv3->cpu[] array when calling gicv3_redist_process_lpi(), and we would probably crash. (This is not a security bug, because this code is only usable with emulation, not with KVM.) Cc: qemu-stable@nongnu.org Fixes: 17fb5e36aabd4b ("hw/intc: GICv3 redistributor ITS processing") Signed-off-by: Peter Maydell Reviewed-by: Alex Bennée Reviewed-by: Richard Henderson --- hw/intc/arm_gicv3_its.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index b99e63d..677b96d 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -311,7 +311,7 @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset, */ rdbase = (cte & GITS_CTE_RDBASE_PROCNUM_MASK) >> 1U; - if (rdbase > s->gicv3->num_cpu) { + if (rdbase >= s->gicv3->num_cpu) { return result; } @@ -505,7 +505,7 @@ static bool process_mapc(GICv3ITSState *s, uint32_t offset) valid = (value & CMD_FIELD_VALID_MASK); - if ((icid > s->ct.maxids.max_collids) || (rdbase > s->gicv3->num_cpu)) { + if ((icid > s->ct.maxids.max_collids) || (rdbase >= s->gicv3->num_cpu)) { qemu_log_mask(LOG_GUEST_ERROR, "ITS MAPC: invalid collection table attributes " "icid %d rdbase %" PRIu64 "\n", icid, rdbase); -- cgit v1.1 From 8d2d6dd9bb6f4a275e9acc5d97b020cd91483285 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 7 Jan 2022 17:07:58 +0000 Subject: hw/intc/arm_gicv3_its: Remove redundant ITS_CTLR_ENABLED define MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We currently define a bitmask for the GITS_CTLR ENABLED bit in two ways: as ITS_CTLR_ENABLED, and via the FIELD() macro as R_GITS_CTLR_ENABLED_MASK. Consistently use the FIELD macro version everywhere and remove the redundant ITS_CTLR_ENABLED define. Signed-off-by: Peter Maydell Reviewed-by: Alex Bennée Reviewed-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé --- hw/intc/arm_gicv3_its.c | 20 ++++++++++---------- hw/intc/gicv3_internal.h | 2 -- 2 files changed, 10 insertions(+), 12 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index 677b96d..985ae03 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -651,7 +651,7 @@ static void process_cmdq(GICv3ITSState *s) uint8_t cmd; int i; - if (!(s->ctlr & ITS_CTLR_ENABLED)) { + if (!(s->ctlr & R_GITS_CTLR_ENABLED_MASK)) { return; } @@ -887,7 +887,7 @@ static MemTxResult gicv3_its_translation_write(void *opaque, hwaddr offset, switch (offset) { case GITS_TRANSLATER: - if (s->ctlr & ITS_CTLR_ENABLED) { + if (s->ctlr & R_GITS_CTLR_ENABLED_MASK) { devid = attrs.requester_id; result = process_its_cmd(s, data, devid, NONE); } @@ -912,13 +912,13 @@ static bool its_writel(GICv3ITSState *s, hwaddr offset, switch (offset) { case GITS_CTLR: if (value & R_GITS_CTLR_ENABLED_MASK) { - s->ctlr |= ITS_CTLR_ENABLED; + s->ctlr |= R_GITS_CTLR_ENABLED_MASK; extract_table_params(s); extract_cmdq_params(s); s->creadr = 0; process_cmdq(s); } else { - s->ctlr &= ~ITS_CTLR_ENABLED; + s->ctlr &= ~R_GITS_CTLR_ENABLED_MASK; } break; case GITS_CBASER: @@ -926,7 +926,7 @@ static bool its_writel(GICv3ITSState *s, hwaddr offset, * IMPDEF choice:- GITS_CBASER register becomes RO if ITS is * already enabled */ - if (!(s->ctlr & ITS_CTLR_ENABLED)) { + if (!(s->ctlr & R_GITS_CTLR_ENABLED_MASK)) { s->cbaser = deposit64(s->cbaser, 0, 32, value); s->creadr = 0; s->cwriter = s->creadr; @@ -937,7 +937,7 @@ static bool its_writel(GICv3ITSState *s, hwaddr offset, * IMPDEF choice:- GITS_CBASER register becomes RO if ITS is * already enabled */ - if (!(s->ctlr & ITS_CTLR_ENABLED)) { + if (!(s->ctlr & R_GITS_CTLR_ENABLED_MASK)) { s->cbaser = deposit64(s->cbaser, 32, 32, value); s->creadr = 0; s->cwriter = s->creadr; @@ -979,7 +979,7 @@ static bool its_writel(GICv3ITSState *s, hwaddr offset, * IMPDEF choice:- GITS_BASERn register becomes RO if ITS is * already enabled */ - if (!(s->ctlr & ITS_CTLR_ENABLED)) { + if (!(s->ctlr & R_GITS_CTLR_ENABLED_MASK)) { index = (offset - GITS_BASER) / 8; if (offset & 7) { @@ -1076,7 +1076,7 @@ static bool its_writell(GICv3ITSState *s, hwaddr offset, * IMPDEF choice:- GITS_BASERn register becomes RO if ITS is * already enabled */ - if (!(s->ctlr & ITS_CTLR_ENABLED)) { + if (!(s->ctlr & R_GITS_CTLR_ENABLED_MASK)) { index = (offset - GITS_BASER) / 8; s->baser[index] &= GITS_BASER_RO_MASK; s->baser[index] |= (value & ~GITS_BASER_RO_MASK); @@ -1087,7 +1087,7 @@ static bool its_writell(GICv3ITSState *s, hwaddr offset, * IMPDEF choice:- GITS_CBASER register becomes RO if ITS is * already enabled */ - if (!(s->ctlr & ITS_CTLR_ENABLED)) { + if (!(s->ctlr & R_GITS_CTLR_ENABLED_MASK)) { s->cbaser = value; s->creadr = 0; s->cwriter = s->creadr; @@ -1298,7 +1298,7 @@ static void gicv3_its_reset(DeviceState *dev) static void gicv3_its_post_load(GICv3ITSState *s) { - if (s->ctlr & ITS_CTLR_ENABLED) { + if (s->ctlr & R_GITS_CTLR_ENABLED_MASK) { extract_table_params(s); extract_cmdq_params(s); } diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h index b9c3745..63de866 100644 --- a/hw/intc/gicv3_internal.h +++ b/hw/intc/gicv3_internal.h @@ -289,8 +289,6 @@ FIELD(GITS_TYPER, CIL, 36, 1) #define GITS_IDREGS 0xFFD0 -#define ITS_CTLR_ENABLED (1U) /* ITS Enabled */ - #define GITS_BASER_RO_MASK (R_GITS_BASER_ENTRYSIZE_MASK | \ R_GITS_BASER_TYPE_MASK) -- cgit v1.1 From 6c1db43de4965b5274830bbd36298638a6dbb468 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 7 Jan 2022 17:07:58 +0000 Subject: hw/intc/arm_gicv3_its: Remove maxids union from TableDesc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The TableDesc struct defines properties of the in-guest-memory tables which the guest tells us about by writing to the GITS_BASER registers. This struct currently has a union 'maxids', but all the fields of the union have the same type (uint32_t) and do the same thing (record one-greater-than the maximum ID value that can be used as an index into the table). We're about to add another table type (the GICv4 vPE table); rather than adding another specifically-named union field for that table type with the same type as the other union fields, remove the union entirely and just have a 'uint32_t max_ids' struct field. Signed-off-by: Peter Maydell Reviewed-by: Alex Bennée Reviewed-by: Richard Henderson --- hw/intc/arm_gicv3_its.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index 985ae03..f321f10 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -287,10 +287,10 @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset, * In this implementation, in case of guest errors we ignore the * command and move onto the next command in the queue. */ - if (devid > s->dt.maxids.max_devids) { + if (devid > s->dt.max_ids) { qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid command attributes: devid %d>%d", - __func__, devid, s->dt.maxids.max_devids); + __func__, devid, s->dt.max_ids); } else if (!dte_valid || !ite_valid || !cte_valid) { qemu_log_mask(LOG_GUEST_ERROR, @@ -384,7 +384,7 @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset, max_Intid = (1ULL << (GICD_TYPER_IDBITS + 1)) - 1; } - if ((devid > s->dt.maxids.max_devids) || (icid > s->ct.maxids.max_collids) + if ((devid > s->dt.max_ids) || (icid > s->ct.max_ids) || !dte_valid || (eventid > max_eventid) || (!ignore_pInt && (((pIntid < GICV3_LPI_INTID_START) || (pIntid > max_Intid)) && (pIntid != INTID_SPURIOUS)))) { @@ -505,7 +505,7 @@ static bool process_mapc(GICv3ITSState *s, uint32_t offset) valid = (value & CMD_FIELD_VALID_MASK); - if ((icid > s->ct.maxids.max_collids) || (rdbase >= s->gicv3->num_cpu)) { + if ((icid > s->ct.max_ids) || (rdbase >= s->gicv3->num_cpu)) { qemu_log_mask(LOG_GUEST_ERROR, "ITS MAPC: invalid collection table attributes " "icid %d rdbase %" PRIu64 "\n", icid, rdbase); @@ -618,7 +618,7 @@ static bool process_mapd(GICv3ITSState *s, uint64_t value, uint32_t offset) valid = (value & CMD_FIELD_VALID_MASK); - if ((devid > s->dt.maxids.max_devids) || + if ((devid > s->dt.max_ids) || (size > FIELD_EX64(s->typer, GITS_TYPER, IDBITS))) { qemu_log_mask(LOG_GUEST_ERROR, "ITS MAPD: invalid device table attributes " @@ -810,8 +810,8 @@ static void extract_table_params(GICv3ITSState *s) (page_sz / s->dt.entry_sz)); } - s->dt.maxids.max_devids = (1UL << (FIELD_EX64(s->typer, GITS_TYPER, - DEVBITS) + 1)); + s->dt.max_ids = (1UL << (FIELD_EX64(s->typer, GITS_TYPER, + DEVBITS) + 1)); s->dt.base_addr = baser_base_addr(value, page_sz); @@ -842,11 +842,11 @@ static void extract_table_params(GICv3ITSState *s) } if (FIELD_EX64(s->typer, GITS_TYPER, CIL)) { - s->ct.maxids.max_collids = (1UL << (FIELD_EX64(s->typer, - GITS_TYPER, CIDBITS) + 1)); + s->ct.max_ids = (1UL << (FIELD_EX64(s->typer, + GITS_TYPER, CIDBITS) + 1)); } else { /* 16-bit CollectionId supported when CIL == 0 */ - s->ct.maxids.max_collids = (1UL << 16); + s->ct.max_ids = (1UL << 16); } s->ct.base_addr = baser_base_addr(value, page_sz); -- cgit v1.1 From 62df780e3d4e918d984797f2d75b0cced157b757 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 7 Jan 2022 17:07:58 +0000 Subject: hw/intc/arm_gicv3_its: Don't return early in extract_table_params() loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In extract_table_params() we process each GITS_BASER register. If the register's Valid bit is not set, this means there is no in-guest-memory table and so we should not try to interpret the other fields in the register. This was incorrectly coded as a 'return' rather than a 'break', so instead of looping round to process the next GITS_BASER we would stop entirely, treating any later tables as being not valid also. This has no real guest-visible effects because (since we don't have GITS_TYPER.HCC != 0) the guest must in any case set up all the GITS_BASER to point to valid tables, so this only happens in an odd misbehaving-guest corner case. Fix the check to 'break', so that we leave the case statement and loop back around to the next GITS_BASER. Signed-off-by: Peter Maydell Reviewed-by: Alex Bennée Reviewed-by: Richard Henderson --- hw/intc/arm_gicv3_its.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index f321f10..c97b998 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -795,7 +795,7 @@ static void extract_table_params(GICv3ITSState *s) s->dt.valid = FIELD_EX64(value, GITS_BASER, VALID); if (!s->dt.valid) { - return; + break; } s->dt.page_sz = page_sz; @@ -826,7 +826,7 @@ static void extract_table_params(GICv3ITSState *s) * hence writes are discarded if ct.valid is 0 */ if (!s->ct.valid) { - return; + break; } s->ct.page_sz = page_sz; -- cgit v1.1 From e5487a413904973ca77999c904be8949da2e8f31 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 7 Jan 2022 17:07:58 +0000 Subject: hw/intc/arm_gicv3_its: Reduce code duplication in extract_table_params() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The extract_table_params() decodes the fields in the GITS_BASER registers into TableDesc structs. Since the fields are the same for all the GITS_BASER registers, there is currently a lot of code duplication within the switch (type) statement. Refactor so that the cases include only what is genuinely different for each type: the calculation of the number of bits in the ID value that indexes into the table. Signed-off-by: Peter Maydell Reviewed-by: Alex Bennée Reviewed-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé --- hw/intc/arm_gicv3_its.c | 97 ++++++++++++++++++++----------------------------- 1 file changed, 40 insertions(+), 57 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index c97b998..84808b1 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -758,6 +758,9 @@ static void extract_table_params(GICv3ITSState *s) uint64_t value; for (int i = 0; i < 8; i++) { + TableDesc *td; + int idbits; + value = s->baser[i]; if (!value) { @@ -789,73 +792,53 @@ static void extract_table_params(GICv3ITSState *s) type = FIELD_EX64(value, GITS_BASER, TYPE); switch (type) { - case GITS_BASER_TYPE_DEVICE: - memset(&s->dt, 0 , sizeof(s->dt)); - s->dt.valid = FIELD_EX64(value, GITS_BASER, VALID); - - if (!s->dt.valid) { - break; - } - - s->dt.page_sz = page_sz; - s->dt.indirect = FIELD_EX64(value, GITS_BASER, INDIRECT); - s->dt.entry_sz = FIELD_EX64(value, GITS_BASER, ENTRYSIZE); - - if (!s->dt.indirect) { - s->dt.max_entries = (num_pages * page_sz) / s->dt.entry_sz; - } else { - s->dt.max_entries = (((num_pages * page_sz) / - L1TABLE_ENTRY_SIZE) * - (page_sz / s->dt.entry_sz)); - } - - s->dt.max_ids = (1UL << (FIELD_EX64(s->typer, GITS_TYPER, - DEVBITS) + 1)); - - s->dt.base_addr = baser_base_addr(value, page_sz); - + td = &s->dt; + idbits = FIELD_EX64(s->typer, GITS_TYPER, DEVBITS) + 1; break; - case GITS_BASER_TYPE_COLLECTION: - memset(&s->ct, 0 , sizeof(s->ct)); - s->ct.valid = FIELD_EX64(value, GITS_BASER, VALID); - - /* - * GITS_TYPER.HCC is 0 for this implementation - * hence writes are discarded if ct.valid is 0 - */ - if (!s->ct.valid) { - break; - } - - s->ct.page_sz = page_sz; - s->ct.indirect = FIELD_EX64(value, GITS_BASER, INDIRECT); - s->ct.entry_sz = FIELD_EX64(value, GITS_BASER, ENTRYSIZE); - - if (!s->ct.indirect) { - s->ct.max_entries = (num_pages * page_sz) / s->ct.entry_sz; - } else { - s->ct.max_entries = (((num_pages * page_sz) / - L1TABLE_ENTRY_SIZE) * - (page_sz / s->ct.entry_sz)); - } - + td = &s->ct; if (FIELD_EX64(s->typer, GITS_TYPER, CIL)) { - s->ct.max_ids = (1UL << (FIELD_EX64(s->typer, - GITS_TYPER, CIDBITS) + 1)); + idbits = FIELD_EX64(s->typer, GITS_TYPER, CIDBITS) + 1; } else { /* 16-bit CollectionId supported when CIL == 0 */ - s->ct.max_ids = (1UL << 16); + idbits = 16; } - - s->ct.base_addr = baser_base_addr(value, page_sz); - break; - default: - break; + /* + * GITS_BASER.TYPE is read-only, so GITS_BASER_RO_MASK + * ensures we will only see type values corresponding to + * the values set up in gicv3_its_reset(). + */ + g_assert_not_reached(); + } + + memset(td, 0, sizeof(*td)); + td->valid = FIELD_EX64(value, GITS_BASER, VALID); + /* + * If GITS_BASER.Valid is 0 for any then we will not process + * interrupts. (GITS_TYPER.HCC is 0 for this implementation, so we + * do not have a special case where the GITS_BASER.Valid bit is 0 + * for the register corresponding to the Collection table but we + * still have to process interrupts using non-memory-backed + * Collection table entries.) + */ + if (!td->valid) { + continue; + } + td->page_sz = page_sz; + td->indirect = FIELD_EX64(value, GITS_BASER, INDIRECT); + td->entry_sz = FIELD_EX64(value, GITS_BASER, ENTRYSIZE); + td->base_addr = baser_base_addr(value, page_sz); + if (!td->indirect) { + td->max_entries = (num_pages * page_sz) / td->entry_sz; + } else { + td->max_entries = (((num_pages * page_sz) / + L1TABLE_ENTRY_SIZE) * + (page_sz / td->entry_sz)); } + td->max_ids = 1ULL << idbits; } } -- cgit v1.1 From 9ae85431902dfa6dba594d639d1a37d709c56a73 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 7 Jan 2022 17:07:58 +0000 Subject: hw/intc/arm_gicv3_its: Correct setting of TableDesc entry_sz MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We set the TableDesc entry_sz field from the appropriate GITS_BASER.ENTRYSIZE field. That ID register field specifies the number of bytes per table entry minus one. However when we use td->entry_sz we assume it to be the number of bytes per table entry (for instance we calculate the number of entries in a page by dividing the page size by the entry size). The effects of this bug are: * we miscalculate the maximum number of entries in the table, so our checks on guest index values are wrong (too lax) * when looking up an entry in the second level of an indirect table, we calculate an incorrect index into the L2 table. Because we make the same incorrect calculation on both reads and writes of the L2 table, the guest won't notice unless it's unlucky enough to use an index value that causes us to index off the end of the L2 table page and cause guest memory corruption in whatever follows Signed-off-by: Peter Maydell Reviewed-by: Alex Bennée Reviewed-by: Richard Henderson --- hw/intc/arm_gicv3_its.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'hw/intc') diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index 84808b1..88f4d73 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -829,7 +829,7 @@ static void extract_table_params(GICv3ITSState *s) } td->page_sz = page_sz; td->indirect = FIELD_EX64(value, GITS_BASER, INDIRECT); - td->entry_sz = FIELD_EX64(value, GITS_BASER, ENTRYSIZE); + td->entry_sz = FIELD_EX64(value, GITS_BASER, ENTRYSIZE) + 1; td->base_addr = baser_base_addr(value, page_sz); if (!td->indirect) { td->max_entries = (num_pages * page_sz) / td->entry_sz; -- cgit v1.1 From 764d6ba10cce25d20ef9f3e11a83a9783dadf65f Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 7 Jan 2022 17:07:59 +0000 Subject: hw/intc/arm_gicv3_its: Don't misuse GITS_TYPE_PHYSICAL define MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The GITS_TYPE_PHYSICAL define is the value we set the GITS_TYPER.Physical field to -- this is 1 to indicate that we support physical LPIs. (Support for virtual LPIs is the GITS_TYPER.Virtual field.) We also use this define as the *value* that we write into an interrupt translation table entry's INTTYPE field, which should be 1 for a physical interrupt and 0 for a virtual interrupt. Finally, we use it as a *mask* when we read the interrupt translation table entry INTTYPE field. Untangle this confusion: define an ITE_INTTYPE_VIRTUAL and ITE_INTTYPE_PHYSICAL to be the valid values of the ITE INTTYPE field, and replace the ad-hoc collection of ITE_ENTRY_* defines with use of the FIELD() macro to define the fields of an ITE and the FIELD_EX64() and FIELD_DP64() macros to read and write them. We use ITE in the new setup, rather than ITE_ENTRY, because ITE stands for "Interrupt translation entry" and so the extra "entry" would be redundant. We take the opportunity to correct the name of the field that holds the GICv4 'doorbell' interrupt ID (this is always the value 1023 in a GICv3, which is why we were calling it the 'spurious' field). The GITS_TYPE_PHYSICAL define is then used in only one place, where we set the initial GITS_TYPER value. Since GITS_TYPER.Physical is essentially a boolean, hiding the '1' value behind a macro is more confusing than helpful, so expand out the macro there and remove the define entirely. Signed-off-by: Peter Maydell Reviewed-by: Alex Bennée Reviewed-by: Richard Henderson --- hw/intc/arm_gicv3_its.c | 30 +++++++++++++----------------- hw/intc/gicv3_internal.h | 26 ++++++++++++++------------ 2 files changed, 27 insertions(+), 29 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index 88f4d73..15eb72a 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -156,12 +156,11 @@ static bool get_ite(GICv3ITSState *s, uint32_t eventid, uint64_t dte, 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; + if (FIELD_EX64(ite.itel, ITE_L, VALID)) { + int inttype = FIELD_EX64(ite.itel, ITE_L, INTTYPE); + if (inttype == ITE_INTTYPE_PHYSICAL) { + *pIntid = FIELD_EX64(ite.itel, ITE_L, INTID); + *icid = FIELD_EX32(ite.iteh, ITE_H, ICID); status = true; } } @@ -342,8 +341,6 @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset, MemTxResult res = MEMTX_OK; uint16_t icid = 0; uint64_t dte = 0; - IteEntry ite; - uint32_t int_spurious = INTID_SPURIOUS; bool result = false; devid = ((value & DEVID_MASK) >> DEVID_SHIFT); @@ -400,16 +397,16 @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset, */ } else { /* add ite entry to interrupt translation table */ - ite.itel = (dte_valid & TABLE_ENTRY_VALID_MASK) | - (GITS_TYPE_PHYSICAL << ITE_ENTRY_INTTYPE_SHIFT); - + IteEntry ite = {}; + ite.itel = FIELD_DP64(ite.itel, ITE_L, VALID, dte_valid); + ite.itel = FIELD_DP64(ite.itel, ITE_L, INTTYPE, ITE_INTTYPE_PHYSICAL); if (ignore_pInt) { - ite.itel |= (eventid << ITE_ENTRY_INTID_SHIFT); + ite.itel = FIELD_DP64(ite.itel, ITE_L, INTID, eventid); } else { - ite.itel |= (pIntid << ITE_ENTRY_INTID_SHIFT); + ite.itel = FIELD_DP64(ite.itel, ITE_L, INTID, pIntid); } - ite.itel |= (int_spurious << ITE_ENTRY_INTSP_SHIFT); - ite.iteh = icid; + ite.itel = FIELD_DP64(ite.itel, ITE_L, DOORBELL, INTID_SPURIOUS); + ite.iteh = FIELD_DP32(ite.iteh, ITE_H, ICID, icid); result = update_ite(s, eventid, dte, ite); } @@ -1237,8 +1234,7 @@ static void gicv3_arm_its_realize(DeviceState *dev, Error **errp) "gicv3-its-sysmem"); /* set the ITS default features supported */ - s->typer = FIELD_DP64(s->typer, GITS_TYPER, PHYSICAL, - GITS_TYPE_PHYSICAL); + s->typer = FIELD_DP64(s->typer, GITS_TYPER, PHYSICAL, 1); s->typer = FIELD_DP64(s->typer, GITS_TYPER, ITT_ENTRY_SIZE, ITS_ITT_ENTRY_SIZE - 1); s->typer = FIELD_DP64(s->typer, GITS_TYPER, IDBITS, ITS_IDBITS); diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h index 63de866..5a63e9e 100644 --- a/hw/intc/gicv3_internal.h +++ b/hw/intc/gicv3_internal.h @@ -354,28 +354,30 @@ FIELD(MAPC, RDBASE, 16, 32) #define L2_TABLE_VALID_MASK CMD_FIELD_VALID_MASK #define TABLE_ENTRY_VALID_MASK (1ULL << 0) -/** - * Default features advertised by this version of ITS - */ -/* Physical LPIs supported */ -#define GITS_TYPE_PHYSICAL (1U << 0) - /* * 12 bytes Interrupt translation Table Entry size * as per Table 5.3 in GICv3 spec * ITE Lower 8 Bytes * Bits: | 49 ... 26 | 25 ... 2 | 1 | 0 | - * Values: | 1023 | IntNum | IntType | Valid | + * Values: | Doorbell | IntNum | IntType | Valid | * ITE Higher 4 Bytes * Bits: | 31 ... 16 | 15 ...0 | * Values: | vPEID | ICID | + * (When Doorbell is unused, as it always is in GICv3, it is 1023) */ #define ITS_ITT_ENTRY_SIZE 0xC -#define ITE_ENTRY_INTTYPE_SHIFT 1 -#define ITE_ENTRY_INTID_SHIFT 2 -#define ITE_ENTRY_INTID_MASK MAKE_64BIT_MASK(2, 24) -#define ITE_ENTRY_INTSP_SHIFT 26 -#define ITE_ENTRY_ICID_MASK MAKE_64BIT_MASK(0, 16) + +FIELD(ITE_L, VALID, 0, 1) +FIELD(ITE_L, INTTYPE, 1, 1) +FIELD(ITE_L, INTID, 2, 24) +FIELD(ITE_L, DOORBELL, 26, 24) + +FIELD(ITE_H, ICID, 0, 16) +FIELD(ITE_H, VPEID, 16, 16) + +/* Possible values for ITE_L INTTYPE */ +#define ITE_INTTYPE_VIRTUAL 0 +#define ITE_INTTYPE_PHYSICAL 1 /* 16 bits EventId */ #define ITS_IDBITS GICD_TYPER_IDBITS -- cgit v1.1 From b87fab1c8e8977e8ea1233bafdbfa37090eefabf Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 7 Jan 2022 17:07:59 +0000 Subject: hw/intc/arm_gicv3_its: Correct handling of MAPI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The MAPI command takes arguments DeviceID, EventID, ICID, and is defined to be equivalent to MAPTI DeviceID, EventID, EventID, ICID. (That is, where MAPTI takes an explicit pINTID, MAPI uses the EventID as the pINTID.) We didn't quite get this right. In particular the error checks for MAPI include "EventID does not specify a valid LPI identifier", which is the same as MAPTI's error check for the pINTID field. QEMU's code skips the pINTID error check entirely in the MAPI case. We can fix this bug and in the process simplify the code by switching to the obvious implementation of setting pIntid = eventid early if ignore_pInt is true. Signed-off-by: Peter Maydell Reviewed-by: Alex Bennée Reviewed-by: Richard Henderson --- hw/intc/arm_gicv3_its.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index 15eb72a..6f21c56 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -354,7 +354,9 @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset, eventid = (value & EVENTID_MASK); - if (!ignore_pInt) { + if (ignore_pInt) { + pIntid = eventid; + } else { pIntid = ((value & pINTID_MASK) >> pINTID_SHIFT); } @@ -377,14 +379,12 @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset, max_eventid = (1UL << (((dte >> 1U) & SIZE_MASK) + 1)); - if (!ignore_pInt) { - max_Intid = (1ULL << (GICD_TYPER_IDBITS + 1)) - 1; - } + max_Intid = (1ULL << (GICD_TYPER_IDBITS + 1)) - 1; if ((devid > s->dt.max_ids) || (icid > s->ct.max_ids) || !dte_valid || (eventid > max_eventid) || - (!ignore_pInt && (((pIntid < GICV3_LPI_INTID_START) || - (pIntid > max_Intid)) && (pIntid != INTID_SPURIOUS)))) { + (((pIntid < GICV3_LPI_INTID_START) || (pIntid > max_Intid)) && + (pIntid != INTID_SPURIOUS))) { qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid command attributes " "devid %d or icid %d or eventid %d or pIntid %d or" @@ -400,11 +400,7 @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset, IteEntry ite = {}; ite.itel = FIELD_DP64(ite.itel, ITE_L, VALID, dte_valid); ite.itel = FIELD_DP64(ite.itel, ITE_L, INTTYPE, ITE_INTTYPE_PHYSICAL); - if (ignore_pInt) { - ite.itel = FIELD_DP64(ite.itel, ITE_L, INTID, eventid); - } else { - ite.itel = FIELD_DP64(ite.itel, ITE_L, INTID, pIntid); - } + ite.itel = FIELD_DP64(ite.itel, ITE_L, INTID, pIntid); ite.itel = FIELD_DP64(ite.itel, ITE_L, DOORBELL, INTID_SPURIOUS); ite.iteh = FIELD_DP32(ite.iteh, ITE_H, ICID, icid); -- cgit v1.1 From e07f844599525685db44ce74bf4ab12025d1d96a Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 7 Jan 2022 17:07:59 +0000 Subject: hw/intc/arm_gicv3_its: Use FIELD macros for DTEs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently the ITS code that reads and writes DTEs uses open-coded shift-and-mask to assemble the various fields into the 64-bit DTE word. The names of the macros used for mask and shift values are also somewhat inconsistent, and don't follow our usual convention that a MASK macro should specify the bits in their place in the word. Replace all these with use of the FIELD macro. Signed-off-by: Peter Maydell Reviewed-by: Alex Bennée Reviewed-by: Richard Henderson --- hw/intc/arm_gicv3_its.c | 20 +++++++++----------- hw/intc/gicv3_internal.h | 7 ++++--- 2 files changed, 13 insertions(+), 14 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index 6f21c56..7a217b0 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -114,7 +114,7 @@ static bool update_ite(GICv3ITSState *s, uint32_t eventid, uint64_t dte, uint64_t itt_addr; MemTxResult res = MEMTX_OK; - itt_addr = (dte & GITS_DTE_ITTADDR_MASK) >> GITS_DTE_ITTADDR_SHIFT; + itt_addr = FIELD_EX64(dte, DTE, ITTADDR); itt_addr <<= ITTADDR_SHIFT; /* 256 byte aligned */ address_space_stq_le(as, itt_addr + (eventid * (sizeof(uint64_t) + @@ -141,7 +141,7 @@ static bool get_ite(GICv3ITSState *s, uint32_t eventid, uint64_t dte, bool status = false; IteEntry ite = {}; - itt_addr = (dte & GITS_DTE_ITTADDR_MASK) >> GITS_DTE_ITTADDR_SHIFT; + itt_addr = FIELD_EX64(dte, DTE, ITTADDR); itt_addr <<= ITTADDR_SHIFT; /* 256 byte aligned */ ite.itel = address_space_ldq_le(as, itt_addr + @@ -255,10 +255,10 @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset, if (res != MEMTX_OK) { return result; } - dte_valid = dte & TABLE_ENTRY_VALID_MASK; + dte_valid = FIELD_EX64(dte, DTE, VALID); if (dte_valid) { - max_eventid = (1UL << (((dte >> 1U) & SIZE_MASK) + 1)); + max_eventid = 1UL << (FIELD_EX64(dte, DTE, SIZE) + 1); ite_valid = get_ite(s, eventid, dte, &icid, &pIntid, &res); @@ -375,10 +375,8 @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset, if (res != MEMTX_OK) { return result; } - dte_valid = dte & TABLE_ENTRY_VALID_MASK; - - max_eventid = (1UL << (((dte >> 1U) & SIZE_MASK) + 1)); - + dte_valid = FIELD_EX64(dte, DTE, VALID); + max_eventid = 1UL << (FIELD_EX64(dte, DTE, SIZE) + 1); max_Intid = (1ULL << (GICD_TYPER_IDBITS + 1)) - 1; if ((devid > s->dt.max_ids) || (icid > s->ct.max_ids) @@ -529,9 +527,9 @@ static bool update_dte(GICv3ITSState *s, uint32_t devid, bool valid, if (s->dt.valid) { if (valid) { /* add mapping entry to device table */ - dte = (valid & TABLE_ENTRY_VALID_MASK) | - ((size & SIZE_MASK) << 1U) | - (itt_addr << GITS_DTE_ITTADDR_SHIFT); + dte = FIELD_DP64(dte, DTE, VALID, 1); + dte = FIELD_DP64(dte, DTE, SIZE, size); + dte = FIELD_DP64(dte, DTE, ITTADDR, itt_addr); } } else { return true; diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h index 5a63e9e..6a3b145 100644 --- a/hw/intc/gicv3_internal.h +++ b/hw/intc/gicv3_internal.h @@ -393,9 +393,10 @@ FIELD(ITE_H, VPEID, 16, 16) * Valid = 1 bit,ITTAddr = 44 bits,Size = 5 bits */ #define GITS_DTE_SIZE (0x8ULL) -#define GITS_DTE_ITTADDR_SHIFT 6 -#define GITS_DTE_ITTADDR_MASK MAKE_64BIT_MASK(GITS_DTE_ITTADDR_SHIFT, \ - ITTADDR_LENGTH) + +FIELD(DTE, VALID, 0, 1) +FIELD(DTE, SIZE, 1, 5) +FIELD(DTE, ITTADDR, 6, 44) /* * 8 bytes Collection Table Entry size -- cgit v1.1 From 257bb6501cda75e9ba0804cd5b45e17275928252 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 7 Jan 2022 17:07:59 +0000 Subject: hw/intc/arm_gicv3_its: Correct comment about CTE RDBase field size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Alex Bennée Reviewed-by: Richard Henderson --- hw/intc/gicv3_internal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'hw/intc') diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h index 6a3b145..14e8ef6 100644 --- a/hw/intc/gicv3_internal.h +++ b/hw/intc/gicv3_internal.h @@ -400,7 +400,7 @@ FIELD(DTE, ITTADDR, 6, 44) /* * 8 bytes Collection Table Entry size - * Valid = 1 bit,RDBase = 36 bits(considering max RDBASE) + * Valid = 1 bit, RDBase = 16 bits */ #define GITS_CTE_SIZE (0x8ULL) #define GITS_CTE_RDBASE_PROCNUM_MASK MAKE_64BIT_MASK(1, RDBASE_PROCNUM_LENGTH) -- cgit v1.1 From 437dc0ea982beb11cb9b4df82baf8aefe6af661c Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 7 Jan 2022 17:07:59 +0000 Subject: hw/intc/arm_gicv3_its: Use FIELD macros for CTEs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use FIELD macros to handle CTEs, rather than ad-hoc mask-and-shift. Signed-off-by: Peter Maydell Reviewed-by: Alex Bennée Reviewed-by: Richard Henderson --- hw/intc/arm_gicv3_its.c | 7 ++++--- hw/intc/gicv3_internal.h | 3 ++- 2 files changed, 6 insertions(+), 4 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index 7a217b0..2949157 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -104,7 +104,7 @@ static bool get_cte(GICv3ITSState *s, uint16_t icid, uint64_t *cte, MEMTXATTRS_UNSPECIFIED, res); } - return (*cte & TABLE_ENTRY_VALID_MASK) != 0; + return FIELD_EX64(*cte, CTE, VALID); } static bool update_ite(GICv3ITSState *s, uint32_t eventid, uint64_t dte, @@ -308,7 +308,7 @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset, * Current implementation only supports rdbase == procnum * Hence rdbase physical address is ignored */ - rdbase = (cte & GITS_CTE_RDBASE_PROCNUM_MASK) >> 1U; + rdbase = FIELD_EX64(cte, CTE, RDBASE); if (rdbase >= s->gicv3->num_cpu) { return result; @@ -426,7 +426,8 @@ static bool update_cte(GICv3ITSState *s, uint16_t icid, bool valid, if (valid) { /* add mapping entry to collection table */ - cte = (valid & TABLE_ENTRY_VALID_MASK) | (rdbase << 1ULL); + cte = FIELD_DP64(cte, CTE, VALID, 1); + cte = FIELD_DP64(cte, CTE, RDBASE, rdbase); } /* diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h index 14e8ef6..1eeb990 100644 --- a/hw/intc/gicv3_internal.h +++ b/hw/intc/gicv3_internal.h @@ -403,7 +403,8 @@ FIELD(DTE, ITTADDR, 6, 44) * Valid = 1 bit, RDBase = 16 bits */ #define GITS_CTE_SIZE (0x8ULL) -#define GITS_CTE_RDBASE_PROCNUM_MASK MAKE_64BIT_MASK(1, RDBASE_PROCNUM_LENGTH) +FIELD(CTE, VALID, 0, 1) +FIELD(CTE, RDBASE, 1, RDBASE_PROCNUM_LENGTH) /* Special interrupt IDs */ #define INTID_SECURE 1020 -- cgit v1.1 From 80dcd37feb3a249cdd6a96826836e267df5c7077 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 7 Jan 2022 17:08:00 +0000 Subject: hw/intc/arm_gicv3_its: Fix various off-by-one errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Alex Bennée --- hw/intc/arm_gicv3_its.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index 2949157..95c1914 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -286,10 +286,10 @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset, * In this implementation, in case of guest errors we ignore the * command and move onto the next command in the queue. */ - if (devid > s->dt.max_ids) { + if (devid >= s->dt.num_ids) { qemu_log_mask(LOG_GUEST_ERROR, - "%s: invalid command attributes: devid %d>%d", - __func__, devid, s->dt.max_ids); + "%s: invalid command attributes: devid %d>=%d", + __func__, devid, s->dt.num_ids); } else if (!dte_valid || !ite_valid || !cte_valid) { qemu_log_mask(LOG_GUEST_ERROR, @@ -379,7 +379,7 @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset, max_eventid = 1UL << (FIELD_EX64(dte, DTE, SIZE) + 1); max_Intid = (1ULL << (GICD_TYPER_IDBITS + 1)) - 1; - if ((devid > s->dt.max_ids) || (icid > s->ct.max_ids) + if ((devid >= s->dt.num_ids) || (icid >= s->ct.num_ids) || !dte_valid || (eventid > max_eventid) || (((pIntid < GICV3_LPI_INTID_START) || (pIntid > max_Intid)) && (pIntid != INTID_SPURIOUS))) { @@ -497,7 +497,7 @@ static bool process_mapc(GICv3ITSState *s, uint32_t offset) valid = (value & CMD_FIELD_VALID_MASK); - if ((icid > s->ct.max_ids) || (rdbase >= s->gicv3->num_cpu)) { + if ((icid >= s->ct.num_ids) || (rdbase >= s->gicv3->num_cpu)) { qemu_log_mask(LOG_GUEST_ERROR, "ITS MAPC: invalid collection table attributes " "icid %d rdbase %" PRIu64 "\n", icid, rdbase); @@ -610,7 +610,7 @@ static bool process_mapd(GICv3ITSState *s, uint64_t value, uint32_t offset) valid = (value & CMD_FIELD_VALID_MASK); - if ((devid > s->dt.max_ids) || + if ((devid >= s->dt.num_ids) || (size > FIELD_EX64(s->typer, GITS_TYPER, IDBITS))) { qemu_log_mask(LOG_GUEST_ERROR, "ITS MAPD: invalid device table attributes " @@ -649,7 +649,7 @@ static void process_cmdq(GICv3ITSState *s) wr_offset = FIELD_EX64(s->cwriter, GITS_CWRITER, OFFSET); - if (wr_offset > s->cq.max_entries) { + if (wr_offset >= s->cq.num_entries) { qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid write offset " "%d\n", __func__, wr_offset); @@ -658,7 +658,7 @@ static void process_cmdq(GICv3ITSState *s) rd_offset = FIELD_EX64(s->creadr, GITS_CREADR, OFFSET); - if (rd_offset > s->cq.max_entries) { + if (rd_offset >= s->cq.num_entries) { qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid read offset " "%d\n", __func__, rd_offset); @@ -721,7 +721,7 @@ static void process_cmdq(GICv3ITSState *s) } if (result) { rd_offset++; - rd_offset %= s->cq.max_entries; + rd_offset %= s->cq.num_entries; s->creadr = FIELD_DP64(s->creadr, GITS_CREADR, OFFSET, rd_offset); } else { /* @@ -824,13 +824,13 @@ static void extract_table_params(GICv3ITSState *s) td->entry_sz = FIELD_EX64(value, GITS_BASER, ENTRYSIZE) + 1; td->base_addr = baser_base_addr(value, page_sz); if (!td->indirect) { - td->max_entries = (num_pages * page_sz) / td->entry_sz; + td->num_entries = (num_pages * page_sz) / td->entry_sz; } else { - td->max_entries = (((num_pages * page_sz) / + td->num_entries = (((num_pages * page_sz) / L1TABLE_ENTRY_SIZE) * (page_sz / td->entry_sz)); } - td->max_ids = 1ULL << idbits; + td->num_ids = 1ULL << idbits; } } @@ -845,7 +845,7 @@ static void extract_cmdq_params(GICv3ITSState *s) s->cq.valid = FIELD_EX64(value, GITS_CBASER, VALID); if (s->cq.valid) { - s->cq.max_entries = (num_pages * GITS_PAGE_SIZE_4K) / + s->cq.num_entries = (num_pages * GITS_PAGE_SIZE_4K) / GITS_CMDQ_ENTRY_SIZE; s->cq.base_addr = FIELD_EX64(value, GITS_CBASER, PHYADDR); s->cq.base_addr <<= R_GITS_CBASER_PHYADDR_SHIFT; -- cgit v1.1 From 7f18ac3ab3337f3c83b2ab34001ef7c1bb4a43a7 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 7 Jan 2022 17:08:00 +0000 Subject: hw/intc/arm_gicv3_its: Rename max_l2_entries to num_l2_entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Alex Bennée Reviewed-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé --- hw/intc/arm_gicv3_its.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index 95c1914..fa3cdb5 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -74,7 +74,7 @@ static bool get_cte(GICv3ITSState *s, uint16_t icid, uint64_t *cte, uint64_t value; bool valid_l2t; uint32_t l2t_id; - uint32_t max_l2_entries; + uint32_t num_l2_entries; if (s->ct.indirect) { l2t_id = icid / (s->ct.page_sz / L1TABLE_ENTRY_SIZE); @@ -88,12 +88,12 @@ static bool get_cte(GICv3ITSState *s, uint16_t icid, uint64_t *cte, valid_l2t = (value & L2_TABLE_VALID_MASK) != 0; if (valid_l2t) { - max_l2_entries = s->ct.page_sz / s->ct.entry_sz; + num_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), + ((icid % num_l2_entries) * GITS_CTE_SIZE), MEMTXATTRS_UNSPECIFIED, res); } } @@ -176,7 +176,7 @@ static uint64_t get_dte(GICv3ITSState *s, uint32_t devid, MemTxResult *res) uint64_t value; bool valid_l2t; uint32_t l2t_id; - uint32_t max_l2_entries; + uint32_t num_l2_entries; if (s->dt.indirect) { l2t_id = devid / (s->dt.page_sz / L1TABLE_ENTRY_SIZE); @@ -190,12 +190,12 @@ static uint64_t get_dte(GICv3ITSState *s, uint32_t devid, MemTxResult *res) valid_l2t = (value & L2_TABLE_VALID_MASK) != 0; if (valid_l2t) { - max_l2_entries = s->dt.page_sz / s->dt.entry_sz; + num_l2_entries = s->dt.page_sz / s->dt.entry_sz; l2t_addr = value & ((1ULL << 51) - 1); value = address_space_ldq_le(as, l2t_addr + - ((devid % max_l2_entries) * GITS_DTE_SIZE), + ((devid % num_l2_entries) * GITS_DTE_SIZE), MEMTXATTRS_UNSPECIFIED, res); } } @@ -416,7 +416,7 @@ static bool update_cte(GICv3ITSState *s, uint16_t icid, bool valid, uint64_t l2t_addr; bool valid_l2t; uint32_t l2t_id; - uint32_t max_l2_entries; + uint32_t num_l2_entries; uint64_t cte = 0; MemTxResult res = MEMTX_OK; @@ -450,12 +450,12 @@ static bool update_cte(GICv3ITSState *s, uint16_t icid, bool valid, valid_l2t = (value & L2_TABLE_VALID_MASK) != 0; if (valid_l2t) { - max_l2_entries = s->ct.page_sz / s->ct.entry_sz; + num_l2_entries = s->ct.page_sz / s->ct.entry_sz; l2t_addr = value & ((1ULL << 51) - 1); address_space_stq_le(as, l2t_addr + - ((icid % max_l2_entries) * GITS_CTE_SIZE), + ((icid % num_l2_entries) * GITS_CTE_SIZE), cte, MEMTXATTRS_UNSPECIFIED, &res); } } else { @@ -521,7 +521,7 @@ static bool update_dte(GICv3ITSState *s, uint32_t devid, bool valid, uint64_t l2t_addr; bool valid_l2t; uint32_t l2t_id; - uint32_t max_l2_entries; + uint32_t num_l2_entries; uint64_t dte = 0; MemTxResult res = MEMTX_OK; @@ -556,12 +556,12 @@ static bool update_dte(GICv3ITSState *s, uint32_t devid, bool valid, valid_l2t = (value & L2_TABLE_VALID_MASK) != 0; if (valid_l2t) { - max_l2_entries = s->dt.page_sz / s->dt.entry_sz; + num_l2_entries = s->dt.page_sz / s->dt.entry_sz; l2t_addr = value & ((1ULL << 51) - 1); address_space_stq_le(as, l2t_addr + - ((devid % max_l2_entries) * GITS_DTE_SIZE), + ((devid % num_l2_entries) * GITS_DTE_SIZE), dte, MEMTXATTRS_UNSPECIFIED, &res); } } else { -- cgit v1.1