[PATCH 3/7] target/arm: Enable FEAT_CSV2_2 for -cpu max
There is no branch prediction in TCG, therefore there is no
need to actually include the context number into the predictor.
Therefore all we need to do is add the state for SCXTNUM_ELx.
Signed-off-by: Richard Henderson
---
target/arm/cpu.h| 16 +++
target/arm/cpu64.c | 2 +-
target/arm/helper.c | 70 -
3 files changed, 86 insertions(+), 2 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index c6c6d89a69..0b89620662 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -688,6 +688,8 @@ typedef struct CPUArchState {
ARMPACKey apdb;
ARMPACKey apga;
} keys;
+
+uint64_t scxtnum_el[4];
#endif
#if defined(CONFIG_USER_ONLY)
@@ -1211,6 +1213,7 @@ void pmu_init(ARMCPU *cpu);
#define SCTLR_WXN (1U << 19)
#define SCTLR_ST (1U << 20) /* up to ??, RAZ in v6 */
#define SCTLR_UWXN(1U << 20) /* v7 onward, AArch32 only */
+#define SCTLR_TSCXT (1U << 20) /* FEAT_CSV2_1p2, AArch64 only */
#define SCTLR_FI (1U << 21) /* up to v7, v8 RES0 */
#define SCTLR_IESB(1U << 21) /* v8.2-IESB, AArch64 only */
#define SCTLR_U (1U << 22) /* up to v6, RAO in v7 */
@@ -4368,6 +4371,19 @@ static inline bool isar_feature_aa64_dit(const
ARMISARegisters *id)
return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, DIT) != 0;
}
+static inline bool isar_feature_aa64_scxtnum(const ARMISARegisters *id)
+{
+int key = FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, CSV2);
+if (key >= 2) {
+return true; /* FEAT_CSV2_2 */
+}
+if (key == 1) {
+key = FIELD_EX64(id->id_aa64pfr1, ID_AA64PFR1, CSV2_FRAC);
+return key >= 2; /* FEAT_CSV2_1p2 */
+}
+return false;
+}
+
static inline bool isar_feature_aa64_ssbs(const ARMISARegisters *id)
{
return FIELD_EX64(id->id_aa64pfr1, ID_AA64PFR1, SSBS) != 0;
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index c1006a067c..9ff08bd995 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -805,7 +805,7 @@ static void aarch64_max_initfn(Object *obj)
t = FIELD_DP64(t, ID_AA64PFR0, SVE, 1);
t = FIELD_DP64(t, ID_AA64PFR0, SEL2, 1); /* FEAT_SEL2 */
t = FIELD_DP64(t, ID_AA64PFR0, DIT, 1); /* FEAT_DIT */
-t = FIELD_DP64(t, ID_AA64PFR0, CSV2, 1); /* FEAT_CSV2 */
+t = FIELD_DP64(t, ID_AA64PFR0, CSV2, 2); /* FEAT_CSV2_2 */
cpu->isar.id_aa64pfr0 = t;
t = cpu->isar.id_aa64pfr1;
diff --git a/target/arm/helper.c b/target/arm/helper.c
index bd1c8e01cb..66af3397ee 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -1780,6 +1780,9 @@ static void scr_write(CPUARMState *env, const
ARMCPRegInfo *ri, uint64_t value)
if (cpu_isar_feature(aa64_mte, cpu)) {
valid_mask |= SCR_ATA;
}
+if (cpu_isar_feature(aa64_scxtnum, cpu)) {
+valid_mask |= SCR_ENSCXT;
+}
} else {
valid_mask &= ~(SCR_RW | SCR_ST);
if (cpu_isar_feature(aa32_ras, cpu)) {
@@ -5312,6 +5315,9 @@ static void do_hcr_write(CPUARMState *env, uint64_t
value, uint64_t valid_mask)
if (cpu_isar_feature(aa64_mte, cpu)) {
valid_mask |= HCR_ATA | HCR_DCT | HCR_TID5;
}
+if (cpu_isar_feature(aa64_scxtnum, cpu)) {
+valid_mask |= HCR_ENSCXT;
+}
}
/* Clear RES0 bits. */
@@ -5965,6 +5971,10 @@ static void define_arm_vh_e2h_redirects_aliases(ARMCPU
*cpu)
{ K(3, 0, 5, 6, 0), K(3, 4, 5, 6, 0), K(3, 5, 5, 6, 0),
"TFSR_EL1", "TFSR_EL2", "TFSR_EL12", isar_feature_aa64_mte },
+{ K(3, 0, 0xd, 0, 7), K(3, 4, 0xd, 0, 7), K(3, 5, 0xd, 0, 7),
+ "SCXTNUM_EL1", "SCXTNUM_EL2", "SCXTNUM_EL12",
+ isar_feature_aa64_scxtnum },
+
/* TODO: ARMv8.2-SPE -- PMSCR_EL2 */
/* TODO: ARMv8.4-Trace -- TRFCR_EL2 */
};
@@ -7434,7 +7444,61 @@ static const ARMCPRegInfo mte_el0_cacheop_reginfo[] = {
REGINFO_SENTINEL
};
-#endif
+static CPAccessResult access_scxtnum(CPUARMState *env, const ARMCPRegInfo *ri,
+ bool isread)
+{
+uint64_t hcr;
+
+switch (arm_current_el(env)) {
+case 0:
+hcr = arm_hcr_el2_eff(env);
+if ((hcr & (HCR_TGE | HCR_E2H)) != (HCR_TGE | HCR_E2H)) {
+if (env->cp15.sctlr_el[1] & SCTLR_TSCXT) {
+if (hcr & HCR_TGE) {
+return CP_ACCESS_TRAP_EL2;
+}
+return CP_ACCESS_TRAP;
+}
+if (arm_is_el2_enabled(env) && !(hcr & HCR_ENSCXT)) {
+return CP_ACCESS_TRAP_EL2;
+}
+} else {
+QEMU_FALLTHROUGH;
+case 1:
+if (env->cp15.sctlr_el[2] & SCTLR_TSCXT) {
+return CP_ACCESS_TRAP_EL2;
+}
+}
+QEMU_FALLTHROUGH;
+case 2:
+if (arm_feature(env, ARM_FEATURE_EL3)
+&& !(env->cp15.scr_el3 & SCR_ENSCXT)) {
+return CP_ACCESS_TRAP_EL3;
+
[PATCH 1/7] target/arm: Enable FEAT_CSV2 for -cpu max
This extension concerns branch speculation, which TCG does not implement. Thus we can trivially enable this feature. Signed-off-by: Richard Henderson --- target/arm/cpu64.c | 1 + target/arm/cpu_tcg.c | 1 + 2 files changed, 2 insertions(+) diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c index def0f1fdcb..c1006a067c 100644 --- a/target/arm/cpu64.c +++ b/target/arm/cpu64.c @@ -805,6 +805,7 @@ static void aarch64_max_initfn(Object *obj) t = FIELD_DP64(t, ID_AA64PFR0, SVE, 1); t = FIELD_DP64(t, ID_AA64PFR0, SEL2, 1); /* FEAT_SEL2 */ t = FIELD_DP64(t, ID_AA64PFR0, DIT, 1); /* FEAT_DIT */ +t = FIELD_DP64(t, ID_AA64PFR0, CSV2, 1); /* FEAT_CSV2 */ cpu->isar.id_aa64pfr0 = t; t = cpu->isar.id_aa64pfr1; diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c index 5cce9116d0..2750cbebec 100644 --- a/target/arm/cpu_tcg.c +++ b/target/arm/cpu_tcg.c @@ -71,6 +71,7 @@ void arm32_max_features(ARMCPU *cpu) cpu->isar.id_mmfr4 = t; t = cpu->isar.id_pfr0; +t = FIELD_DP32(t, ID_PFR0, CSV2, 2); /* FEAT_CVS2 */ t = FIELD_DP32(t, ID_PFR0, DIT, 1); /* FEAT_DIT */ t = FIELD_DP32(t, ID_PFR0, RAS, 1); /* FEAT_RAS */ cpu->isar.id_pfr0 = t; -- 2.25.1
[PATCH 0/7] target/arm: More trivial features, A76, N1
Based-on: 20220409000742.293691-1-richard.hender...@linaro.org
("target/arm: Implement features Debugv8p4, RAS, IESB")
3 more completely trivial features, and a 4th that merely
needs to add some state words.
Add definitions for cortex-a76 (also requires gicv4) and
neoverse-n1 (also requires gicv4.1), but we now have all
of the in-cpu features implemented.
r~
Richard Henderson (7):
target/arm: Enable FEAT_CSV2 for -cpu max
target/arm: Update ISAR fields for ARMv8.8
target/arm: Enable FEAT_CSV2_2 for -cpu max
target/arm: Enable FEAT_CSV3 for -cpu max
target/arm: Enable FEAT_DGH for -cpu max
target/arm: Define cortex-a76
target/arm: Define neoverse-n1
target/arm/cpu.h | 39 +++
hw/arm/sbsa-ref.c | 2 +
hw/arm/virt.c | 2 +
target/arm/cpu64.c | 131 +
target/arm/cpu_tcg.c | 2 +
target/arm/helper.c| 70 +++-
target/arm/translate-a64.c | 1 +
7 files changed, 246 insertions(+), 1 deletion(-)
--
2.25.1
Re: [RFC PATCH for-7.1] Remove the slirp submodule (and only compile with an external libslirp)
On 4/8/2022 12:47 PM, Thomas Huth wrote: QEMU 7.1 won't support Ubuntu 18.04 anymore, so the last big important distro that did not have a pre-packaged libslirp has been dismissed. All other major distros seem to have a libslirp package in their distribution already - according to repology.org: Fedora 34: 4.4.0 CentOS 8 (RHEL-8): 4.4.0 Debian Buster: 4.3.1 (in buster-backports) OpenSUSE Leap 15.3: 4.3.1 Ubuntu LTS 20.04: 4.1.0 FreeBSD Ports: 4.6.1 NetBSD pkgsrc: 4.3.1 Homebrew: 4.6.1 MSYS2 mingw: 4.6.1 The only one that still seems to be missing a libslirp package is OpenBSD - but I assume that they can add it to their ports system quickly if required. So there is no real urgent need for keeping the slirp submodule in the QEMU tree anymore. Thus let's drop the slirp submodule now and rely on the libslirp packages from the distributions instead. Signed-off-by: Thomas Huth I wish I had seen this earlier as our 7.1 release was just tagged. I have whipped up a port of 4.6.1 for OpenBSD as it was pretty simple. I will see about submitting it in a number of days when the tree opens.
[RFC] migration/dirtyrate: check malloc() return
Handling potential memory allocation failures in dirtyrate.
Signed-off-by: jianchunfu
---
migration/dirtyrate.c | 8
1 file changed, 8 insertions(+)
diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index aace12a787..5dd40f32c8 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -523,9 +523,17 @@ static void calculate_dirtyrate_dirty_ring(struct
DirtyRateConfig config)
}
dirty_pages = malloc(sizeof(*dirty_pages) * nvcpu);
+if (!dirty_pages) {
+error_report("malloc dirty pages for vcpus failed.");
+exit(1);
+}
DirtyStat.dirty_ring.nvcpu = nvcpu;
DirtyStat.dirty_ring.rates = malloc(sizeof(DirtyRateVcpu) * nvcpu);
+if (!DirtyStat.dirty_ring.rates) {
+error_report("malloc dirty rates for vcpu ring failed.");
+exit(1);
+}
dirtyrate_global_dirty_log_start();
--
2.18.4
Re: [PATCH 41/41] hw/arm/virt: Support TCG GICv4
On 4/8/22 07:15, Peter Maydell wrote: Add support for the TCG GICv4 to the virt board. For the board, the GICv4 is very similar to the GICv3, with the only difference being the size of the redistributor frame. The changes here are thus: * calculating virt_redist_capacity correctly for GICv4 * changing various places which were "if GICv3" to be "if not GICv2" * the commandline option handling Note that using GICv4 reduces the maximum possible number of CPUs on the virt board from 512 to 317, because we can now only fit half as many redistributors into the redistributor regions we have defined. Signed-off-by: Peter Maydell --- docs/system/arm/virt.rst | 5 ++- include/hw/arm/virt.h| 12 +-- hw/arm/virt.c| 70 ++-- 3 files changed, 67 insertions(+), 20 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 40/41] hw/arm/virt: Abstract out calculation of redistributor region capacity
On 4/8/22 07:15, Peter Maydell wrote: In several places in virt.c we calculate the number of redistributors that fit in a region of our memory map, which is the size of the region divided by the size of a single redistributor frame. For GICv4, the redistributor frame is a different size from that for GICv3. Abstract out the calculation of redistributor region capacity so that we have one place we need to change to handle GICv4 rather than several. Signed-off-by: Peter Maydell --- include/hw/arm/virt.h | 9 +++-- hw/arm/virt.c | 11 --- 2 files changed, 11 insertions(+), 9 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 39/41] hw/arm/virt: Use VIRT_GIC_VERSION_* enum values in create_gic()
On 4/8/22 07:15, Peter Maydell wrote: Everywhere we need to check which GIC version we're using, we look at vms->gic_version and use the VIRT_GIC_VERSION_* enum values, except in create_gic(), which copies vms->gic_version into a local 'int' variable and makes direct comparisons against values 2 and 3. For consistency, change this function to check the GIC version the same way we do elsewhere. This includes not implicitly relying on the enumeration type values happening to match the integer 'revision' values the GIC device object wants. Signed-off-by: Peter Maydell --- hw/arm/virt.c | 31 +++ 1 file changed, 23 insertions(+), 8 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 38/41] hw/intc/arm_gicv3: Allow 'revision' property to be set to 4
On 4/8/22 07:15, Peter Maydell wrote: Now that we have implemented all the GICv4 requirements, relax the error-checking on the GIC object's 'revision' property to allow a TCG GIC to be a GICv4, whilst still constraining the KVM GIC to GICv3. Our 'revision' property doesn't consider the possibility of wanting to specify the minor version of the GIC -- for instance there is a GICv3.1 which adds support for extended SPI and PPI ranges, among other things, and also GICv4.1. But since the QOM property is internal to QEMU, not user-facing, we can cross that bridge when we come to it. Within the GIC implementation itself code generally checks against the appropriate ID register feature bits, and the only use of s->revision is for setting those ID register bits. Signed-off-by: Peter Maydell --- hw/intc/arm_gicv3_common.c | 12 +++- hw/intc/arm_gicv3_kvm.c| 5 + 2 files changed, 12 insertions(+), 5 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 37/41] hw/intc/arm_gicv3: Update ID and feature registers for GICv4
On 4/8/22 07:15, Peter Maydell wrote: Update the various GIC ID and feature registers for GICv4: * PIDR2 [7:4] is the GIC architecture revision * GICD_TYPER.DVIS is 1 to indicate direct vLPI injection support * GICR_TYPER.VLPIS is 1 to indicate redistributor support for vLPIs * GITS_TYPER.VIRTUAL is 1 to indicate vLPI support * GITS_TYPER.VMOVP is 1 to indicate that our VMOVP implementation handles cross-ITS synchronization for the guest * ICH_VTR_EL2.nV4 is 0 to indicate direct vLPI injection support Signed-off-by: Peter Maydell --- hw/intc/gicv3_internal.h | 15 +++ hw/intc/arm_gicv3_common.c | 7 +-- hw/intc/arm_gicv3_cpuif.c | 6 +- hw/intc/arm_gicv3_dist.c | 7 --- hw/intc/arm_gicv3_its.c| 7 ++- hw/intc/arm_gicv3_redist.c | 2 +- 6 files changed, 32 insertions(+), 12 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 36/41] hw/intc/arm_gicv3_redist: Implement gicv3_redist_inv_vlpi()
On 4/8/22 07:15, Peter Maydell wrote: Implement the function gicv3_redist_inv_vlpi(), which was previously left as a stub. This is the function that does the work of the INV command for a virtual interrupt. Signed-off-by: Peter Maydell --- hw/intc/arm_gicv3_redist.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 35/41] hw/intc/arm_gicv3_redist: Implement gicv3_redist_vinvall()
On 4/8/22 07:15, Peter Maydell wrote: Implement the gicv3_redist_vinvall() function (previously left as a stub). This function handles the work of a VINVALL command: it must invalidate any cached information associated with a specific vCPU. Signed-off-by: Peter Maydell --- hw/intc/arm_gicv3_redist.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 34/41] hw/intc/arm_gicv3_redist: Implement gicv3_redist_mov_vlpi()
On 4/8/22 07:15, Peter Maydell wrote: Implement the gicv3_redist_mov_vlpi() function (previously left as a stub). This function handles the work of a VMOVI command: it marks the vLPI not-pending on the source and pending on the destination. Signed-off-by: Peter Maydell --- hw/intc/arm_gicv3_redist.c | 20 ++-- 1 file changed, 18 insertions(+), 2 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 33/41] hw/intc/arm_gicv3_redist: Use set_pending_table_bit() in mov handling
On 4/8/22 07:15, Peter Maydell wrote: We can use our new set_pending_table_bit() utility function in gicv3_redist_mov_lpi() to clear the bit in the source pending table, rather than doing the "load, clear bit, store" ourselves. Signed-off-by: Peter Maydell --- hw/intc/arm_gicv3_redist.c | 9 + 1 file changed, 1 insertion(+), 8 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 32/41] hw/intc/arm_gicv3_redist: Implement gicv3_redist_vlpi_pending()
On 4/8/22 07:15, Peter Maydell wrote:
Implement the function gicv3_redist_vlpi_pending(), which was
previously left as a stub. This is the function that is called by
the CPU interface when it changes the state of a vLPI. It's similar
to gicv3_redist_process_vlpi(), but we know that the vCPU is
definitely resident on the redistributor and the irq is in range, so
it is a bit simpler.
Signed-off-by: Peter Maydell
---
hw/intc/arm_gicv3_redist.c | 23 +--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/hw/intc/arm_gicv3_redist.c b/hw/intc/arm_gicv3_redist.c
index be36978b45c..eadf5e8265e 100644
--- a/hw/intc/arm_gicv3_redist.c
+++ b/hw/intc/arm_gicv3_redist.c
@@ -1009,9 +1009,28 @@ void gicv3_redist_movall_lpis(GICv3CPUState *src,
GICv3CPUState *dest)
void gicv3_redist_vlpi_pending(GICv3CPUState *cs, int irq, int level)
{
/*
- * The redistributor handling for changing the pending state
- * of a vLPI will be added in a subsequent commit.
+ * Change the pending state of the specified vLPI.
+ * Unlike gicv3_redist_process_vlpi(), we know here that the
+ * vCPU is definitely resident on this redistributor, and that
+ * the irq is in range.
*/
+uint64_t vptbase, ctbase;
+
+vptbase = FIELD_EX64(cs->gicr_vpendbaser, GICR_VPENDBASER, PHYADDR) << 16;
+
+if (set_pending_table_bit(cs, vptbase, irq, level)) {
+if (level) {
+/* Check whether this vLPI is now the best */
+ctbase = cs->gicr_vpropbaser & R_GICR_VPROPBASER_PHYADDR_MASK;
+update_for_one_lpi(cs, irq, ctbase, true, >hppvlpi);
+gicv3_cpuif_virt_irq_fiq_update(cs);
+} else {
+/* Only need to recalculate if this was previously the best vLPI */
+if (irq == cs->hppvlpi.irq) {
+gicv3_redist_update_vlpi(cs);
+}
+}
+}
I'll note that looks a lot like the previous patch, modulo "resident".
Perhaps this could be better factored?
Anyway,
Reviewed-by: Richard Henderson
r~
Re: [PATCH 31/41] hw/intc/arm_gicv3_redist: Implement gicv3_redist_process_vlpi()
On 4/8/22 07:15, Peter Maydell wrote: Implement the function gicv3_redist_process_vlpi(), which was left as just a stub earlier. This function deals with being handed a VLPI by the ITS. It must set the bit in the pending table. If the vCPU is currently resident we must recalculate the highest priority pending vLPI; otherwise we may need to ring a "doorbell" interrupt to let the hypervisor know it might want to reschedule the vCPU. Signed-off-by: Peter Maydell --- hw/intc/arm_gicv3_redist.c | 48 ++ 1 file changed, 44 insertions(+), 4 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 30/41] hw/intc/arm_gicv3_redist: Factor out "update bit in pending table" code
On 4/8/22 07:15, Peter Maydell wrote:
+if (extract32(pend, irq % 8, 1) == level) {
Here you assume level in {0,1}...
+pend = deposit32(pend, irq % 8, 1, level ? 1 : 0);
... and here you force it into {0,1}.
Better to have the compiler do that with bool level.
You might consider
uint8_t bit = 1 << (irq % 8);
read();
if (!(pend & bit) ^ level) {
no change
}
pend ^= bit;
write();
as a follow up; extract + deposit seems unnecessary.
Anyway, with the bool thing fixed,
Reviewed-by: Richard Henderson
r~
Re: [PATCH 29/41] hw/intc/arm_gicv3_redist: Recalculate hppvlpi on VPENDBASER writes
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 v2 1/1] hw/ppc: check if spapr_drc_index() returns NULL in spapr_nvdimm.c
spapr_nvdimm_flush_completion_cb() and flush_worker_cb() are using the
DRC object returned by spapr_drc_index() without checking it for NULL.
In this case we would be dereferencing a NULL pointer when doing
SPAPR_NVDIMM(drc->dev) and PC_DIMM(drc->dev).
This can happen if, during a scm_flush(), the DRC object is wrongly
freed/released (e.g. a bug in another part of the code).
spapr_drc_index() would then return NULL in the callbacks.
Fixes: Coverity CID 1487108, 1487178
Signed-off-by: Daniel Henrique Barboza
---
hw/ppc/spapr_nvdimm.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/hw/ppc/spapr_nvdimm.c b/hw/ppc/spapr_nvdimm.c
index c4c97da5de..04a64cada3 100644
--- a/hw/ppc/spapr_nvdimm.c
+++ b/hw/ppc/spapr_nvdimm.c
@@ -447,9 +447,15 @@ static int flush_worker_cb(void *opaque)
{
SpaprNVDIMMDeviceFlushState *state = opaque;
SpaprDrc *drc = spapr_drc_by_index(state->drcidx);
-PCDIMMDevice *dimm = PC_DIMM(drc->dev);
-HostMemoryBackend *backend = MEMORY_BACKEND(dimm->hostmem);
-int backend_fd = memory_region_get_fd(>mr);
+PCDIMMDevice *dimm;
+HostMemoryBackend *backend;
+int backend_fd;
+
+g_assert(drc != NULL);
+
+dimm = PC_DIMM(drc->dev);
+backend = MEMORY_BACKEND(dimm->hostmem);
+backend_fd = memory_region_get_fd(>mr);
if (object_property_get_bool(OBJECT(backend), "pmem", NULL)) {
MemoryRegion *mr = host_memory_backend_get_memory(dimm->hostmem);
@@ -475,7 +481,11 @@ static void spapr_nvdimm_flush_completion_cb(void *opaque,
int hcall_ret)
{
SpaprNVDIMMDeviceFlushState *state = opaque;
SpaprDrc *drc = spapr_drc_by_index(state->drcidx);
-SpaprNVDIMMDevice *s_nvdimm = SPAPR_NVDIMM(drc->dev);
+SpaprNVDIMMDevice *s_nvdimm;
+
+g_assert(drc != NULL);
+
+s_nvdimm = SPAPR_NVDIMM(drc->dev);
state->hcall_ret = hcall_ret;
QLIST_REMOVE(state, node);
--
2.35.1
[PATCH v2 0/1] Coverity fixes in hw/ppc/spapr_nvdimm.c
Changes from v1: - clarified in the commit message which kind of errors we aim to prevent - changed the H_HARDWARE return to g_assert() exit - v1 link: https://lists.gnu.org/archive/html/qemu-devel/2022-04/msg00569.html Daniel Henrique Barboza (1): hw/ppc: check if spapr_drc_index() returns NULL in spapr_nvdimm.c hw/ppc/spapr_nvdimm.c | 18 ++ 1 file changed, 14 insertions(+), 4 deletions(-) -- 2.35.1
Re: [PATCH 28/41] hw/intc/arm_gicv3_redist: Factor out "update hpplpi for all LPIs" logic
On 4/8/22 07:15, Peter Maydell wrote:
Factor out the common part of gicv3_redist_update_lpi_only() into
a new function update_for_all_lpis(), which does a full rescan
of an LPI Pending table and sets the specified PendingIrq struct
with the highest priority pending enabled LPI it finds.
Signed-off-by: Peter Maydell
---
hw/intc/arm_gicv3_redist.c | 66 ++
1 file changed, 46 insertions(+), 20 deletions(-)
diff --git a/hw/intc/arm_gicv3_redist.c b/hw/intc/arm_gicv3_redist.c
index 571e0fa8309..2379389d14e 100644
--- a/hw/intc/arm_gicv3_redist.c
+++ b/hw/intc/arm_gicv3_redist.c
@@ -103,6 +103,48 @@ static void update_for_one_lpi(GICv3CPUState *cs, int irq,
}
}
+/**
+ * update_for_all_lpis: Fully scan LPI tables and find best pending LPI
+ *
+ * @cs: GICv3CPUState
+ * @ptbase: physical address of LPI Pending table
+ * @ctbase: physical address of LPI Configuration table
+ * @ptsizebits: size of tables, specified as number of interrupt ID bits minus
1
+ * @ds: true if priority value should not be shifted
+ * @hpp: points to pending information to set
+ *
+ * Recalculate the highest priority pending enabled LPI from scratch,
+ * and set @hpp accordingly.
+ *
+ * We scan the LPI pending table @ptbase; for each pending LPI, we read the
+ * corresponding entry in the LPI configuration table @ctbase to extract
+ * the priority and enabled information.
+ *
+ * We take @ptsizebits in the form idbits-1 because this is the way that
+ * LPI table sizes are architecturally specified in GICR_PROPBASER.IDBits
+ * and in the VMAPP command's VPT_size field.
+ */
+static void update_for_all_lpis(GICv3CPUState *cs, uint64_t ptbase,
+uint64_t ctbase, unsigned ptsizebits,
+bool ds, PendingIrq *hpp)
+{
+AddressSpace *as = >gic->dma_as;
+uint8_t pend;
+uint32_t pendt_size = (1ULL << (ptsizebits + 1));
Code movement, so ok, but no need for ull.
Anyway,
Reviewed-by: Richard Henderson
r~
Re: [PATCH 27/41] hw/intc/arm_gicv3_redist: Factor out "update hpplpi for one LPI" logic
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~
Re: [PATCH 26/41] hw/intc/arm_gicv3_cpuif: Don't recalculate maintenance irq unnecessarily
On 4/8/22 07:15, Peter Maydell wrote: The maintenance interrupt state depends only on: * ICH_HCR_EL2 * ICH_LR_EL2 * ICH_VMCR_EL2 fields VENG0 and VENG1 Now we have a separate function that updates only the vIRQ and vFIQ lines, use that in places that only change state that affects vIRQ and vFIQ but not the maintenance interrupt. Signed-off-by: Peter Maydell --- hw/intc/arm_gicv3_cpuif.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 25/41] hw/intc/arm_gicv3_cpuif: Support vLPIs
On 4/8/22 07:15, Peter Maydell wrote: The CPU interface changes to support vLPIs are fairly minor: in the parts of the code that currently look at the list registers to determine the highest priority pending virtual interrupt, we must also look at the highest priority pending vLPI. To do this we change hppvi_index() to check the vLPI and return a special-case value if that is the right virtual interrupt to take. The callsites (which handle HPPIR and IAR registers and the "raise vIRQ and vFIQ lines" code) then have to handle this special-case value. This commit includes two interfaces with the as-yet-unwritten redistributor code: * the new GICv3CPUState::hppvlpi will be set by the redistributor (in the same way as the existing hpplpi does for physical LPIs) * when the CPU interface acknowledges a vLPI it needs to set it to non-pending; the new gicv3_redist_vlpi_pending() function (which matches the existing gicv3_redist_lpi_pending() used for physical LPIs) is a stub that will be filled in later Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson @@ -2632,6 +2735,12 @@ static void gicv3_cpuif_el_change_hook(ARMCPU *cpu, void *opaque) GICv3CPUState *cs = opaque; gicv3_cpuif_update(cs); +/* + * Because vLPIs are only pending in NonSecure state, + * an EL change can change the VIRQ/VFIQ status (but + * cannot affect the maintenance interrupt state) + */ +gicv3_cpuif_virt_irq_fiq_update(cs); I'm a little bit surprised this is here, and not in arm_cpu_exec_interrupt (or a subroutine). Is this because if a virq has highest priority, we have to find the highest prio physical interrupt? r~
Re: [PATCH 24/41] hw/intc/arm_gicv3_cpuif: Split "update vIRQ/vFIQ" from gicv3_cpuif_virt_update()
On 4/8/22 07:15, Peter Maydell wrote: The function gicv3_cpuif_virt_update() currently sets all of vIRQ, vFIQ and the maintenance interrupt. This implies that it has to be used quite carefully -- as the comment notes, setting the maintenance interrupt will typically cause the GIC code to be re-entered recursively. For handling vLPIs, we need the redistributor to be able to tell the cpuif to update the vIRQ and vFIQ lines when the highest priority pending vLPI changes. Since that change can't cause the maintenance interrupt state to change, we can pull the "update vIRQ/vFIQ" parts of gicv3_cpuif_virt_update() out into a separate function, which the redistributor can then call without having to worry about the reentrancy issue. Signed-off-by: Peter Maydell --- hw/intc/gicv3_internal.h | 11 +++ hw/intc/arm_gicv3_cpuif.c | 64 --- hw/intc/trace-events | 3 +- 3 files changed, 53 insertions(+), 25 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 23/41] hw/intc/arm_gicv3: Implement new GICv4 redistributor registers
On 4/8/22 07:15, Peter Maydell wrote: Implement the new GICv4 redistributor registers: GICR_VPROPBASER and GICR_VPENDBASER; for the moment we implement these as simple reads-as-written stubs, together with the necessary migration and reset handling. We don't put ID-register checks on the handling of these registers, because they are all in the only-in-v4 extra register frames, so they're not accessible in a GICv3. Signed-off-by: Peter Maydell --- GICv4.1 adds two further registers in the new VLPI frame, as well as changing the layout of VPROPBASER and VPENDBASER, but we aren't implementing v4.1 yet, just v4. --- hw/intc/gicv3_internal.h | 21 +++ include/hw/intc/arm_gicv3_common.h | 3 ++ hw/intc/arm_gicv3_common.c | 22 hw/intc/arm_gicv3_redist.c | 56 ++ 4 files changed, 102 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH 22/41] hw/intc/arm_gicv3: Implement GICv4's new redistributor frame
On 4/8/22 07:15, Peter Maydell wrote: diff --git a/hw/intc/arm_gicv3_redist.c b/hw/intc/arm_gicv3_redist.c index 7c75dd6f072..9f1fe09a78e 100644 --- a/hw/intc/arm_gicv3_redist.c +++ b/hw/intc/arm_gicv3_redist.c @@ -442,8 +442,8 @@ MemTxResult gicv3_redist_read(void *opaque, hwaddr offset, uint64_t *data, * in the memory map); if so then the GIC has multiple MemoryRegions * for the redistributors. */ -cpuidx = region->cpuidx + offset / GICV3_REDIST_SIZE; -offset %= GICV3_REDIST_SIZE; +cpuidx = region->cpuidx + offset / gicv3_redist_size(s); +offset %= gicv3_redist_size(s); cs = >cpu[cpuidx]; @@ -501,8 +501,8 @@ MemTxResult gicv3_redist_write(void *opaque, hwaddr offset, uint64_t data, * in the memory map); if so then the GIC has multiple MemoryRegions * for the redistributors. */ -cpuidx = region->cpuidx + offset / GICV3_REDIST_SIZE; -offset %= GICV3_REDIST_SIZE; +cpuidx = region->cpuidx + offset / gicv3_redist_size(s); +offset %= gicv3_redist_size(s); In these two cases, it would be better to call the function only once. Otherwise, Reviewed-by: Richard Henderson r~
Re: [PATCH 21/41] hw/intc/arm_gicv3_its: Implement VINVALL
On 4/8/22 07:15, Peter Maydell wrote: The VINVALL command should cause any cached information in the ITS or redistributor for the specified vCPU to be dropped or otherwise made consistent with the in-memory LPI configuration tables. Here we implement the command and table parsing, leaving the redistributor part as a stub for the moment, as usual. Signed-off-by: Peter Maydell --- hw/intc/gicv3_internal.h | 13 + hw/intc/arm_gicv3_its.c| 26 ++ hw/intc/arm_gicv3_redist.c | 5 + hw/intc/trace-events | 1 + 4 files changed, 45 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH 20/41] hw/intc/arm_gicv3_its: Implement VMOVI
On 4/8/22 07:15, Peter Maydell wrote: Implement the GICv4 VMOVI command, which moves the pending state of a virtual interrupt from one redistributor to another. As with MOVI, we handle the "parse and validate command arguments and table lookups" part in the ITS source file, and pass the final results to a function in the redistributor which will do the actual operation. As with the "make a VLPI pending" change, for the moment we leave that redistributor function as a stub, to be implemented in a later commit. Signed-off-by: Peter Maydell --- hw/intc/gicv3_internal.h | 23 +++ hw/intc/arm_gicv3_its.c| 82 ++ hw/intc/arm_gicv3_redist.c | 10 + hw/intc/trace-events | 1 + 4 files changed, 116 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH 17/41] hw/intc/arm_gicv3_its: Implement VSYNC
On 4/8/22 07:15, Peter Maydell wrote: The VSYNC command forces the ITS to synchronize all outstanding ITS operations for the specified vPEID, so that subsequent writse to writes. r~
Re: [PATCH 19/41] hw/intc/arm_gicv3_its: Implement INV for virtual interrupts
On 4/8/22 07:15, Peter Maydell wrote: Implement the ITS side of the handling of the INV command for virtual interrupts; as usual this calls into a redistributor function which we leave as a stub to fill in later. Signed-off-by: Peter Maydell --- hw/intc/gicv3_internal.h | 9 + hw/intc/arm_gicv3_its.c| 16 ++-- hw/intc/arm_gicv3_redist.c | 8 3 files changed, 31 insertions(+), 2 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 18/41] hw/intc/arm_gicv3_its: Implement INV command properly
On 4/8/22 07:15, Peter Maydell wrote: We were previously implementing INV (like INVALL) to just blow away cached highest-priority-pending-LPI information on all connected redistributors. For GICv4.0, this isn't going to be sufficient, because the LPI we are invalidating cached information for might be either physical or virtual, and the required action is different for those two cases. So we need to do the full process of looking up the ITE from the devid and eventid. This also means we can do the error checks that the spec lists for this command. Split out INV handling into a process_inv() function like our other command-processing functions. For the moment, stick to handling only physical LPIs; we will add the vLPI parts later. Signed-off-by: Peter Maydell --- We could also improve INVALL to only prod the one redistributor specified by the ICID in the command packet, but since INVALL is only for physical LPIs I am leaving it as it is. --- hw/intc/gicv3_internal.h | 12 + hw/intc/arm_gicv3_its.c| 50 +- hw/intc/arm_gicv3_redist.c | 11 + hw/intc/trace-events | 3 ++- 4 files changed, 74 insertions(+), 2 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 17/41] hw/intc/arm_gicv3_its: Implement VSYNC
On 4/8/22 07:15, Peter Maydell wrote: The VSYNC command forces the ITS to synchronize all outstanding ITS operations for the specified vPEID, so that subsequent writse to GITS_TRANSLATER honour them. The QEMU implementation is always in sync, so for us this is a nop, like the existing SYNC command. Signed-off-by: Peter Maydell --- hw/intc/gicv3_internal.h | 1 + hw/intc/arm_gicv3_its.c | 11 +++ hw/intc/trace-events | 1 + 3 files changed, 13 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH 16/41] hw/intc/arm_gicv3_its: Implement VMOVP
On 4/8/22 07:15, Peter Maydell wrote: Implement the GICv4 VMOVP command, which updates an entry in the vPE table to change its rdbase field. This command is unique in the ITS command set because its effects must be propagated to all the other ITSes connected to the same GIC as the ITS which executes the VMOVP command. The GICv4 spec allows two implementation choices for handling the propagation to other ITSes: * If GITS_TYPER.VMOVP is 1, the guest only needs to issue the command on one ITS, and the implementation handles the propagation to all ITSes * If GITS_TYPER.VMOVP is 0, the guest must issue the command on every ITS, and arrange for the ITSes to synchronize the updates with each other by setting ITSList and Sequence Number fields in the command packets We choose the GITS_TYPER.VMOVP = 1 approach, and synchronously execute the update on every ITS. For GICv4.1 this command has extra fields in the command packet and additional behaviour. We define the 4.1-only fields with the FIELD macro, but only implement the GICv4.0 version of the command. Note that we don't update the reported GITS_TYPER value here; we'll do that later in a commit which updates all the reported feature bit and ID register values for GICv4. Signed-off-by: Peter Maydell --- hw/intc/gicv3_internal.h | 18 ++ hw/intc/arm_gicv3_its.c | 75 hw/intc/trace-events | 1 + 3 files changed, 94 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH 15/41] hw/intc/arm_gicv3: Keep pointers to every connected ITS
On 4/8/22 07:15, Peter Maydell wrote: The GICv4 ITS VMOVP command's semantics require it to perform the operation on every ITS connected to the same GIC that the ITS that received the command is attached to. This means that the GIC object needs to keep a pointer to every ITS that is connected to it (previously it was sufficient for the ITS to have a pointer to its GIC). Add a glib ptrarray to the GICv3 object which holds pointers to every connected ITS, and make the ITS add itself to the array for the GIC it is connected to when it is realized. Note that currently all QEMU machine types with an ITS have exactly one ITS in the system, so typically the length of this ptrarray will be 1. Multiple ITSes are typically used to improve performance on real hardware, so we wouldn't need to have more than one unless we were modelling a real machine type that had multile ITSes. Signed-off-by: Peter Maydell --- hw/intc/gicv3_internal.h | 9 + include/hw/intc/arm_gicv3_common.h | 2 ++ hw/intc/arm_gicv3_common.c | 2 ++ hw/intc/arm_gicv3_its.c| 2 ++ hw/intc/arm_gicv3_its_kvm.c| 2 ++ 5 files changed, 17 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH 14/41] hw/intc/arm_gicv3_its: Handle virtual interrupts in process_its_cmd()
On 4/8/22 07:15, Peter Maydell wrote: For GICv4, interrupt table entries read by process_its_cmd() may indicate virtual LPIs which are to be directly injected into a VM. Implement the ITS side of the code for handling this. This is similar to the existing handling of physical LPIs, but instead of looking up a collection ID in a collection table, we look up a vPEID in a vPE table. As with the physical LPIs, we leave the rest of the work to code in the redistributor device. The redistributor half will be implemented in a later commit; for now we just provide a stub function which does nothing. Signed-off-by: Peter Maydell --- hw/intc/gicv3_internal.h | 17 +++ hw/intc/arm_gicv3_its.c| 99 +- hw/intc/arm_gicv3_redist.c | 9 hw/intc/trace-events | 2 + 4 files changed, 125 insertions(+), 2 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 13/41] hw/intc/arm_gicv3_its: Split out process_its_cmd() physical interrupt code
On 4/8/22 07:15, Peter Maydell wrote: Split the part of process_its_cmd() which is specific to physical interrupts into its own function. This is the part which starts by taking the ICID and looking it up in the collection table. The handling of virtual interrupts is significantly different (involving a lookup in the vPE table) so structuring the code with one sub-function for the physical interrupt case and one for the virtual interrupt case will be clearer than putting both cases in one large function. The code for handling the "remove mapping from ITE" for the DISCARD command remains in process_its_cmd() because it is common to both virtual and physical interrupts. Signed-off-by: Peter Maydell --- hw/intc/arm_gicv3_its.c | 51 ++--- 1 file changed, 33 insertions(+), 18 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v1 11/15] accel/tcg: add tb_invalidate_phys_page_range tracepoint
On 4/8/22 09:47, Alex Bennée wrote:
This gives a little more insight into what is going on as we
invalidate a range of TBs.
Signed-off-by: Alex Bennée
---
accel/tcg/translate-all.c | 9 +
accel/tcg/trace-events| 1 +
2 files changed, 10 insertions(+)
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index b0009177b9..625c46dd9b 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -1671,6 +1671,7 @@ tb_invalidate_phys_page_range__locked(struct
page_collection *pages,
TranslationBlock *tb;
tb_page_addr_t tb_start, tb_end;
int n;
+int checked = 0, removed = 0;
#ifdef TARGET_HAS_PRECISE_SMC
CPUState *cpu = current_cpu;
CPUArchState *env = NULL;
@@ -1695,6 +1696,7 @@ tb_invalidate_phys_page_range__locked(struct
page_collection *pages,
the code */
PAGE_FOR_EACH_TB(p, tb, n) {
assert_page_locked(p);
+checked++;
/* NOTE: this is subtle as a TB may span two physical pages */
if (n == 0) {
/* NOTE: tb_end may be after the end of the page, but
@@ -1728,13 +1730,20 @@ tb_invalidate_phys_page_range__locked(struct
page_collection *pages,
}
#endif /* TARGET_HAS_PRECISE_SMC */
tb_phys_invalidate__locked(tb);
+removed++;
}
}
+
+
#if !defined(CONFIG_USER_ONLY)
Spacing.
/* if no code remaining, no need to continue to use slow writes */
if (!p->first_tb) {
invalidate_page_bitmap(p);
tlb_unprotect_code(start);
+trace_tb_invalidate_phys_page_range(checked, removed, 0);
+} else {
+TranslationBlock *tb = (TranslationBlock *) p->first_tb;
+trace_tb_invalidate_phys_page_range(checked, removed, tb->pc);
Is this going to get us set without use warnings on CONFIG_USER_ONLY?
r~
r~
Re: [PATCH v1 10/15] cputlb: add tracepoints for TB invalidation
On 4/8/22 09:47, Alex Bennée wrote: Signed-off-by: Alex Bennée --- accel/tcg/translate-all.c | 2 ++ accel/tcg/trace-events| 1 + 2 files changed, 3 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH v1 09/15] cputlb: add tracepoints for the protect/unprotect helpers
On 4/8/22 09:47, Alex Bennée wrote: This helps track when pages are tagged for detecting code changes. Signed-off-by: Alex Bennée --- accel/tcg/cputlb.c | 14 ++ accel/tcg/trace-events | 3 +++ 2 files changed, 13 insertions(+), 4 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v1 07/15] disas: generalise plugin_printf and use for monitor_disas
On 4/8/22 09:47, Alex Bennée wrote: Rather than assembling our output piecemeal lets use the same approach as the plugin disas interface to build the disassembly string before printing it. Signed-off-by: Alex Bennée --- disas.c | 23 +++ 1 file changed, 15 insertions(+), 8 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v1 08/15] disas: use result of ->read_memory_func
On 4/8/22 09:47, Alex Bennée wrote: This gets especially confusing if you start plugging in host addresses from a trace and you wonder why the output keeps changing. Report when read_memory_func fails instead of blindly disassembling the buffer contents. Signed-off-by: Alex Bennée --- disas.c | 20 ++--- disas/capstone.c | 73 2 files changed, 53 insertions(+), 40 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v1 06/15] monitor: expose monitor_puts to rest of code
On 4/8/22 09:47, Alex Bennée wrote: This helps us construct strings elsewhere before echoing to the monitor. It avoids having to jump through hoops like: monitor_printf(mon, "%s", s->str); Signed-off-by: Alex Bennée --- include/monitor/monitor.h | 1 + monitor/monitor-internal.h | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v1 05/15] accel/tcg: add tb_invalidate_phy_pages_fast tracepoint
On 4/8/22 09:47, Alex Bennée wrote: These events can be very expensive for the translator so lets add a tracepoint to help with debugging what might be causing them. Clean up the comments while we are at it. Signed-off-by: Alex Bennée --- accel/tcg/translate-all.c | 15 +++ accel/tcg/trace-events| 1 + 2 files changed, 12 insertions(+), 4 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v1 04/15] accel/tcg: move trace events to correct location
On 4/8/22 09:47, Alex Bennée wrote: Signed-off-by: Alex Bennée --- accel/tcg/cputlb.c | 2 +- accel/tcg/trace-events | 4 trace-events | 4 3 files changed, 5 insertions(+), 5 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH] target/riscv: use xlen in forging isa string
On 4/9/22 02:46, Frédéric Pétrot wrote:
Since we now have xlen in misa, let's not use TARGET_LONG_BITS while
forging the isa string, and use instead riscv_cpu_mxl_bits.
Signed-off-by: Frédéric Pétrot
---
target/riscv/cpu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 0c774056c5..0644b3843e 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -984,7 +984,8 @@ char *riscv_isa_string(RISCVCPU *cpu)
int i;
const size_t maxlen = sizeof("rv128") + sizeof(riscv_single_letter_exts);
char *isa_str = g_new(char, maxlen);
-char *p = isa_str + snprintf(isa_str, maxlen, "rv%d", TARGET_LONG_BITS);
+char *p = isa_str + snprintf(isa_str, maxlen, "rv%lu",
+ riscv_cpu_mxl_bits(>env));
The fact that you need to use %lu here means riscv_cpu_mxl_bits needs fixing: use of
unsigned long is always a mistake in QEMU. Either int is fine (as in this case), or you
need uint64_t and ULL.
r~
Re: [PATCH] target/riscv: replace TARGET_LONG_BITS in gdbstub
On 4/9/22 02:46, Frédéric Pétrot wrote: Now that we have misa xlen, use that in riscv gdbstub.c instead of the TARGET_LONG_BITS define, and use riscv_cpu_mxl_bits to provide the number of bits in a consistent way. Signed-off-by: Frédéric Pétrot --- target/riscv/gdbstub.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c index 9ed049c29e..1602f76d2b 100644 --- a/target/riscv/gdbstub.c +++ b/target/riscv/gdbstub.c @@ -305,7 +305,7 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg) CPURISCVState *env = >env; GString *s = g_string_new(NULL); riscv_csr_predicate_fn predicate; -int bitsize = 16 << env->misa_mxl_max; +int bitsize = riscv_cpu_mxl_bits(env); This isn't correct, changing from using max to current mxl. You might think this is ok, because this will run up in startup, but it really runs whenever gdb attaches to the stub. Which could be anytime. r~
[PATCH for-7.1 v2 1/1] hw/ppc: use qdev to register spapr_nvdimm vmsd
Make the code a little more maintainable by using dc->vmsd to register
the vmstate instead of using vmstate_(un)register calls.
'instance_id' was being set to VMSTATE_INSTANCE_ID_ANY so there is no need
for qdev_set_legacy_instance_id() calls.
spapr_nvdimm_unrealize() was removed since it was only being used to
call vmstate_unregister().
Cc: Shivaprasad G Bhat
Signed-off-by: Daniel Henrique Barboza
---
hw/ppc/spapr_nvdimm.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/hw/ppc/spapr_nvdimm.c b/hw/ppc/spapr_nvdimm.c
index c4c97da5de..973e9d0fbe 100644
--- a/hw/ppc/spapr_nvdimm.c
+++ b/hw/ppc/spapr_nvdimm.c
@@ -866,14 +866,6 @@ static void spapr_nvdimm_realize(NVDIMMDevice *dimm, Error
**errp)
if (!is_pmem || pmem_override) {
s_nvdimm->hcall_flush_required = true;
}
-
-vmstate_register(NULL, VMSTATE_INSTANCE_ID_ANY,
- _spapr_nvdimm_states, dimm);
-}
-
-static void spapr_nvdimm_unrealize(NVDIMMDevice *dimm)
-{
-vmstate_unregister(NULL, _spapr_nvdimm_states, dimm);
}
static Property spapr_nvdimm_properties[] = {
@@ -888,8 +880,9 @@ static void spapr_nvdimm_class_init(ObjectClass *oc, void
*data)
DeviceClass *dc = DEVICE_CLASS(oc);
NVDIMMClass *nvc = NVDIMM_CLASS(oc);
+dc->vmsd = _spapr_nvdimm_states;
+
nvc->realize = spapr_nvdimm_realize;
-nvc->unrealize = spapr_nvdimm_unrealize;
device_class_set_props(dc, spapr_nvdimm_properties);
}
--
2.35.1
[PATCH for-7.1 v2 0/1] use dc->vmsd with spapr devices vmstate
Hi, This v2 contains only the last patch from v1, patch 4, given that all other patches are breaking backward migration due to how qdev_set_legacy_instance_id() works when vmstate_register() is passing an id to the vmsds. Changes from v1: - patches 1-3: removed - v1 link: https://lists.gnu.org/archive/html/qemu-devel/2022-03/msg05615.html Daniel Henrique Barboza (1): hw/ppc: use qdev to register spapr_nvdimm vmsd hw/ppc/spapr_nvdimm.c | 11 ++- 1 file changed, 2 insertions(+), 9 deletions(-) -- 2.35.1
[PATCH] target/riscv: use xlen in forging isa string
Since we now have xlen in misa, let's not use TARGET_LONG_BITS while
forging the isa string, and use instead riscv_cpu_mxl_bits.
Signed-off-by: Frédéric Pétrot
---
target/riscv/cpu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 0c774056c5..0644b3843e 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -984,7 +984,8 @@ char *riscv_isa_string(RISCVCPU *cpu)
int i;
const size_t maxlen = sizeof("rv128") + sizeof(riscv_single_letter_exts);
char *isa_str = g_new(char, maxlen);
-char *p = isa_str + snprintf(isa_str, maxlen, "rv%d", TARGET_LONG_BITS);
+char *p = isa_str + snprintf(isa_str, maxlen, "rv%lu",
+ riscv_cpu_mxl_bits(>env));
for (i = 0; i < sizeof(riscv_single_letter_exts) - 1; i++) {
if (cpu->env.misa_ext & RV(riscv_single_letter_exts[i])) {
*p++ = qemu_tolower(riscv_single_letter_exts[i]);
--
2.35.1
[PATCH] target/riscv: replace TARGET_LONG_BITS in gdbstub
Now that we have misa xlen, use that in riscv gdbstub.c instead of the
TARGET_LONG_BITS define, and use riscv_cpu_mxl_bits to provide the number of
bits in a consistent way.
Signed-off-by: Frédéric Pétrot
---
target/riscv/gdbstub.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index 9ed049c29e..1602f76d2b 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -305,7 +305,7 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int
base_reg)
CPURISCVState *env = >env;
GString *s = g_string_new(NULL);
riscv_csr_predicate_fn predicate;
-int bitsize = 16 << env->misa_mxl_max;
+int bitsize = riscv_cpu_mxl_bits(env);
int i;
/* Until gdb knows about 128-bit registers */
@@ -385,10 +385,11 @@ static int ricsv_gen_dynamic_vector_xml(CPUState *cs, int
base_reg)
for (i = 0; i < 7; i++) {
g_string_append_printf(s,
- "",
- vector_csrs[i], TARGET_LONG_BITS, base_reg++);
+ vector_csrs[i], riscv_cpu_mxl_bits(>env),
+ base_reg++);
num_regs++;
}
--
2.35.1
Re: [PATCH for-7.0] virtio-iommu: use-after-free fix
On Thu, 7 Apr 2022 at 15:50, Michael S. Tsirkin wrote: > > On Thu, Apr 07, 2022 at 11:03:16AM +0100, Peter Maydell wrote: > > On Thu, 7 Apr 2022 at 10:52, Michael S. Tsirkin wrote: > > > > > > From: Wentao Liang > > > > > > A potential Use-after-free was reported in virtio_iommu_handle_command > > > when using virtio-iommu: > > > > > > > I find a potential Use-after-free in QEMU 6.2.0, which is in > > > > virtio_iommu_handle_command() (./hw/virtio/virtio-iommu.c). > > > > So, this isn't a regression. Do you think it's critically necessary > > it goes in 7.0, or is it in the category "put it into 7.0 if we > > need an rc4 for some other reason anyway" ? > > > > (I have a feeling we'll need an rc4, but we'll see.) > > > > thanks > > -- PMM > > I am concerned it can be used to trigger a CVE but I could not > find a way. So I would say if there's an rc4 pls include it > but if not then we can pick it up in stable. We needed an rc4 for a couple of other security fixes, so I've applied this to master; thanks. -- PMM
