Re: [PATCH 29/41] hw/intc/arm_gicv3_redist: Recalculate hppvlpi on VPENDBASER writes

2022-04-10 Thread Peter Maydell
On Sat, 9 Apr 2022 at 21:10, Richard Henderson
 wrote:
> It it really valid to write to vpendbaser with other than a 64-bit write?  I 
> suppose it's
> possible to order the 32-bit writes to make sure the update to valid comes 
> last...

Yes, that's valid. The GICv3 spec states specifically when registers are
64-bit only (see for instance GITS_SGIR). As you say, you can write the
bottom half first and the top half with the valid bit second.

thanks
-- PMM



Re: [PATCH 29/41] hw/intc/arm_gicv3_redist: Recalculate hppvlpi on VPENDBASER writes

2022-04-09 Thread Richard Henderson

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

+if (oldvalid && newvalid) {
+/*
+ * Changing other fields while VALID is 1 is UNPREDICTABLE;
+ * we choose to log and ignore the write.
+ */
+if (cs->gicr_vpendbaser ^ newval) {
+qemu_log_mask(LOG_GUEST_ERROR,
+  "%s: Changing GICR_VPENDBASER when VALID=1 "
+  "is UNPREDICTABLE\n", __func__);
+}
+return;
+}


...


@@ -493,10 +574,10 @@ static MemTxResult gicr_writel(GICv3CPUState *cs, hwaddr 
offset,
  cs->gicr_vpropbaser = deposit64(cs->gicr_vpropbaser, 32, 32, value);
  return MEMTX_OK;
  case GICR_VPENDBASER:
-cs->gicr_vpendbaser = deposit64(cs->gicr_vpendbaser, 0, 32, value);
+gicr_write_vpendbaser(cs, deposit64(cs->gicr_vpendbaser, 0, 32, 
value));
  return MEMTX_OK;
  case GICR_VPENDBASER + 4:
-cs->gicr_vpendbaser = deposit64(cs->gicr_vpendbaser, 32, 32, value);
+gicr_write_vpendbaser(cs, deposit64(cs->gicr_vpendbaser, 32, 32, 
value));
  return MEMTX_OK;
  default:
  return MEMTX_ERROR;
@@ -557,7 +638,7 @@ static MemTxResult gicr_writell(GICv3CPUState *cs, hwaddr 
offset,
  cs->gicr_vpropbaser = value;
  return MEMTX_OK;
  case GICR_VPENDBASER:
-cs->gicr_vpendbaser = value;
+gicr_write_vpendbaser(cs, value);
  return MEMTX_OK;
  default:
  return MEMTX_ERROR;


It it really valid to write to vpendbaser with other than a 64-bit write?  I suppose it's 
possible to order the 32-bit writes to make sure the update to valid comes last...


Anyway, not really related to the real logic here,
Reviewed-by: Richard Henderson 


r~



[PATCH 29/41] hw/intc/arm_gicv3_redist: Recalculate hppvlpi on VPENDBASER writes

2022-04-08 Thread Peter Maydell
The guest uses GICR_VPENDBASER to tell the redistributor when it is
scheduling or descheduling a vCPU.  When it writes and changes the
VALID bit from 0 to 1, it is scheduling a vCPU, and we must update
our view of the current highest priority pending vLPI from the new
Pending and Configuration tables.  When it writes and changes the
VALID bit from 1 to 0, it is descheduling, which means that there is
no longer a highest priority pending vLPI.

The specification allows the implementation to use part of the vLPI
Pending table as an IMPDEF area where it can cache information when a
vCPU is descheduled, so that it can avoid having to do a full rescan
of the tables when the vCPU is scheduled again.  For now, we don't
take advantage of this, and simply do a complete rescan.

Signed-off-by: Peter Maydell 
---
 hw/intc/arm_gicv3_redist.c | 87 --
 1 file changed, 84 insertions(+), 3 deletions(-)

diff --git a/hw/intc/arm_gicv3_redist.c b/hw/intc/arm_gicv3_redist.c
index 2379389d14e..bfdde36a206 100644
--- a/hw/intc/arm_gicv3_redist.c
+++ b/hw/intc/arm_gicv3_redist.c
@@ -185,6 +185,87 @@ static void gicr_write_ipriorityr(GICv3CPUState *cs, 
MemTxAttrs attrs, int irq,
 cs->gicr_ipriorityr[irq] = value;
 }
 
+static void gicv3_redist_update_vlpi_only(GICv3CPUState *cs)
+{
+uint64_t ptbase, ctbase, idbits;
+
+if (!FIELD_EX64(cs->gicr_vpendbaser, GICR_VPENDBASER, VALID)) {
+cs->hppvlpi.prio = 0xff;
+return;
+}
+
+ptbase = cs->gicr_vpendbaser & R_GICR_VPENDBASER_PHYADDR_MASK;
+ctbase = cs->gicr_vpropbaser & R_GICR_VPROPBASER_PHYADDR_MASK;
+idbits = FIELD_EX64(cs->gicr_vpropbaser, GICR_VPROPBASER, IDBITS);
+
+update_for_all_lpis(cs, ptbase, ctbase, idbits, true, &cs->hppvlpi);
+}
+
+static void gicv3_redist_update_vlpi(GICv3CPUState *cs)
+{
+gicv3_redist_update_vlpi_only(cs);
+gicv3_cpuif_virt_irq_fiq_update(cs);
+}
+
+static void gicr_write_vpendbaser(GICv3CPUState *cs, uint64_t newval)
+{
+/* Write @newval to GICR_VPENDBASER, handling its effects */
+bool oldvalid = FIELD_EX64(cs->gicr_vpendbaser, GICR_VPENDBASER, VALID);
+bool newvalid = FIELD_EX64(newval, GICR_VPENDBASER, VALID);
+bool pendinglast;
+
+/*
+ * The DIRTY bit is read-only and for us is always zero;
+ * other fields are writeable.
+ */
+newval &= R_GICR_VPENDBASER_INNERCACHE_MASK |
+R_GICR_VPENDBASER_SHAREABILITY_MASK |
+R_GICR_VPENDBASER_PHYADDR_MASK |
+R_GICR_VPENDBASER_OUTERCACHE_MASK |
+R_GICR_VPENDBASER_PENDINGLAST_MASK |
+R_GICR_VPENDBASER_IDAI_MASK |
+R_GICR_VPENDBASER_VALID_MASK;
+
+if (oldvalid && newvalid) {
+/*
+ * Changing other fields while VALID is 1 is UNPREDICTABLE;
+ * we choose to log and ignore the write.
+ */
+if (cs->gicr_vpendbaser ^ newval) {
+qemu_log_mask(LOG_GUEST_ERROR,
+  "%s: Changing GICR_VPENDBASER when VALID=1 "
+  "is UNPREDICTABLE\n", __func__);
+}
+return;
+}
+if (!oldvalid && !newvalid) {
+cs->gicr_vpendbaser = newval;
+return;
+}
+
+if (newvalid) {
+/*
+ * Valid going from 0 to 1: update hppvlpi from tables.
+ * If IDAI is 0 we are allowed to use the info we cached in
+ * the IMPDEF area of the table.
+ * PendingLast is RES1 when we make this transition.
+ */
+pendinglast = true;
+} else {
+/*
+ * Valid going from 1 to 0:
+ * Set PendingLast if there was a pending enabled interrupt
+ * for the vPE that was just descheduled.
+ * If we cache info in the IMPDEF area, write it out here.
+ */
+pendinglast = cs->hppvlpi.prio != 0xff;
+}
+
+newval = FIELD_DP64(newval, GICR_VPENDBASER, PENDINGLAST, pendinglast);
+cs->gicr_vpendbaser = newval;
+gicv3_redist_update_vlpi(cs);
+}
+
 static MemTxResult gicr_readb(GICv3CPUState *cs, hwaddr offset,
   uint64_t *data, MemTxAttrs attrs)
 {
@@ -493,10 +574,10 @@ static MemTxResult gicr_writel(GICv3CPUState *cs, hwaddr 
offset,
 cs->gicr_vpropbaser = deposit64(cs->gicr_vpropbaser, 32, 32, value);
 return MEMTX_OK;
 case GICR_VPENDBASER:
-cs->gicr_vpendbaser = deposit64(cs->gicr_vpendbaser, 0, 32, value);
+gicr_write_vpendbaser(cs, deposit64(cs->gicr_vpendbaser, 0, 32, 
value));
 return MEMTX_OK;
 case GICR_VPENDBASER + 4:
-cs->gicr_vpendbaser = deposit64(cs->gicr_vpendbaser, 32, 32, value);
+gicr_write_vpendbaser(cs, deposit64(cs->gicr_vpendbaser, 32, 32, 
value));
 return MEMTX_OK;
 default:
 return MEMTX_ERROR;
@@ -557,7 +638,7 @@ static MemTxResult gicr_writell(GICv3CPUState *cs, hwaddr 
offset,
 cs->gicr_vpropbaser = value;
 return MEMTX_OK;
 case GICR_VPENDBASER:
-cs->gicr_vpendbaser = value;
+