Re: [PATCH 27/41] hw/intc/arm_gicv3_redist: Factor out "update hpplpi for one LPI" logic

2022-04-09 Thread Richard Henderson

On 4/8/22 07:15, Peter Maydell wrote:

Currently the functions which update the highest priority pending LPI
information by looking at the LPI Pending and Configuration tables
are hard-coded to use the physical LPI tables addressed by
GICR_PENDBASER and GICR_PROPBASER.  To support virtual LPIs we will
need to do essentially the same job, but looking at the current
virtual LPI Pending and Configuration tables and updating cs->hppvlpi
instead of cs->hpplpi.

Factor out the common part of the gicv3_redist_check_lpi_priority()
function into a new update_for_one_lpi() function, which updates
a PendingIrq struct if the specified LPI is higher priority than
what is currently recorded there.

Signed-off-by: Peter Maydell
---
  hw/intc/arm_gicv3_redist.c | 74 --
  1 file changed, 47 insertions(+), 27 deletions(-)


Reviewed-by: Richard Henderson 

r~



[PATCH 27/41] hw/intc/arm_gicv3_redist: Factor out "update hpplpi for one LPI" logic

2022-04-08 Thread Peter Maydell
Currently the functions which update the highest priority pending LPI
information by looking at the LPI Pending and Configuration tables
are hard-coded to use the physical LPI tables addressed by
GICR_PENDBASER and GICR_PROPBASER.  To support virtual LPIs we will
need to do essentially the same job, but looking at the current
virtual LPI Pending and Configuration tables and updating cs->hppvlpi
instead of cs->hpplpi.

Factor out the common part of the gicv3_redist_check_lpi_priority()
function into a new update_for_one_lpi() function, which updates
a PendingIrq struct if the specified LPI is higher priority than
what is currently recorded there.

Signed-off-by: Peter Maydell 
---
 hw/intc/arm_gicv3_redist.c | 74 --
 1 file changed, 47 insertions(+), 27 deletions(-)

diff --git a/hw/intc/arm_gicv3_redist.c b/hw/intc/arm_gicv3_redist.c
index 3464972c139..571e0fa8309 100644
--- a/hw/intc/arm_gicv3_redist.c
+++ b/hw/intc/arm_gicv3_redist.c
@@ -60,6 +60,49 @@ static uint32_t gicr_read_bitmap_reg(GICv3CPUState *cs, 
MemTxAttrs attrs,
 return reg;
 }
 
+/**
+ * update_for_one_lpi: Update pending information if this LPI is better
+ *
+ * @cs: GICv3CPUState
+ * @irq: interrupt to look up in the LPI Configuration table
+ * @ctbase: physical address of the LPI Configuration table to use
+ * @ds: true if priority value should not be shifted
+ * @hpp: points to pending information to update
+ *
+ * Look up @irq in the Configuration table specified by @ctbase
+ * to see if it is enabled and what its priority is. If it is an
+ * enabled interrupt with a higher priority than that currently
+ * recorded in @hpp, update @hpp.
+ */
+static void update_for_one_lpi(GICv3CPUState *cs, int irq,
+   uint64_t ctbase, bool ds, PendingIrq *hpp)
+{
+uint8_t lpite;
+uint8_t prio;
+
+address_space_read(&cs->gic->dma_as,
+   ctbase + ((irq - GICV3_LPI_INTID_START) * 
sizeof(lpite)),
+   MEMTXATTRS_UNSPECIFIED, &lpite, sizeof(lpite));
+
+if (!(lpite & LPI_CTE_ENABLED)) {
+return;
+}
+
+if (ds) {
+prio = lpite & LPI_PRIORITY_MASK;
+} else {
+prio = ((lpite & LPI_PRIORITY_MASK) >> 1) | 0x80;
+}
+
+if ((prio < hpp->prio) ||
+((prio == hpp->prio) && (irq <= hpp->irq))) {
+hpp->irq = irq;
+hpp->prio = prio;
+/* LPIs and vLPIs are always non-secure Grp1 interrupts */
+hpp->grp = GICV3_G1NS;
+}
+}
+
 static uint8_t gicr_read_ipriorityr(GICv3CPUState *cs, MemTxAttrs attrs,
 int irq)
 {
@@ -598,34 +641,11 @@ MemTxResult gicv3_redist_write(void *opaque, hwaddr 
offset, uint64_t data,
 
 static void gicv3_redist_check_lpi_priority(GICv3CPUState *cs, int irq)
 {
-AddressSpace *as = &cs->gic->dma_as;
-uint64_t lpict_baddr;
-uint8_t lpite;
-uint8_t prio;
+uint64_t lpict_baddr = cs->gicr_propbaser & R_GICR_PROPBASER_PHYADDR_MASK;
 
-lpict_baddr = cs->gicr_propbaser & R_GICR_PROPBASER_PHYADDR_MASK;
-
-address_space_read(as, lpict_baddr + ((irq - GICV3_LPI_INTID_START) *
-   sizeof(lpite)), MEMTXATTRS_UNSPECIFIED, &lpite,
-   sizeof(lpite));
-
-if (!(lpite & LPI_CTE_ENABLED)) {
-return;
-}
-
-if (cs->gic->gicd_ctlr & GICD_CTLR_DS) {
-prio = lpite & LPI_PRIORITY_MASK;
-} else {
-prio = ((lpite & LPI_PRIORITY_MASK) >> 1) | 0x80;
-}
-
-if ((prio < cs->hpplpi.prio) ||
-((prio == cs->hpplpi.prio) && (irq <= cs->hpplpi.irq))) {
-cs->hpplpi.irq = irq;
-cs->hpplpi.prio = prio;
-/* LPIs are always non-secure Grp1 interrupts */
-cs->hpplpi.grp = GICV3_G1NS;
-}
+update_for_one_lpi(cs, irq, lpict_baddr,
+   cs->gic->gicd_ctlr & GICD_CTLR_DS,
+   &cs->hpplpi);
 }
 
 void gicv3_redist_update_lpi_only(GICv3CPUState *cs)
-- 
2.25.1