Re: [PATCH 3/3] target/hppa: Fix unit carry conditions
On 3/25/24 04:04, Richard Henderson wrote:
Split do_unit_cond to do_unit_zero_cond to only handle
conditions versus zero. These are the only ones that
are legal for UXOR. Simplify trans_uxor accordingly.
Rename do_unit to do_unit_addsub, since xor has been split.
Properly compute carry-out bits for add and subtract,
mirroring the code in do_add and do_sub.
Signed-off-by: Richard Henderson
This patch triggers a failure in SECTION 055 (32-bit)
ERROR 0999 IN SECTION 055
UNEXPECTED TRAP# 13
IN:
0x001a2b2c: uaddcm,tc,shc r13,r14,r15
r13..r15:
---
target/hppa/translate.c | 214
1 file changed, 109 insertions(+), 105 deletions(-)
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 3fc3e7754c..2bf213c938 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -936,98 +936,44 @@ static DisasCond do_sed_cond(DisasContext *ctx, unsigned
orig, bool d,
return do_log_cond(ctx, c * 2 + f, d, res);
}
-/* Similar, but for unit conditions. */
-
-static DisasCond do_unit_cond(unsigned cf, bool d, TCGv_i64 res,
- TCGv_i64 in1, TCGv_i64 in2)
+/* Similar, but for unit zero conditions. */
+static DisasCond do_unit_zero_cond(unsigned cf, bool d, TCGv_i64 res)
{
-DisasCond cond;
-TCGv_i64 tmp, cb = NULL;
+TCGv_i64 tmp;
uint64_t d_repl = d ? 0x00010001ull : 1;
-
-if (cf & 8) {
-/* Since we want to test lots of carry-out bits all at once, do not
- * do our normal thing and compute carry-in of bit B+1 since that
- * leaves us with carry bits spread across two words.
- */
-cb = tcg_temp_new_i64();
-tmp = tcg_temp_new_i64();
-tcg_gen_or_i64(cb, in1, in2);
-tcg_gen_and_i64(tmp, in1, in2);
-tcg_gen_andc_i64(cb, cb, res);
-tcg_gen_or_i64(cb, cb, tmp);
-}
+uint64_t ones = 0, sgns = 0;
switch (cf >> 1) {
-case 0: /* never / TR */
-cond = cond_make_f();
-break;
-
case 1: /* SBW / NBW */
if (d) {
-tmp = tcg_temp_new_i64();
-tcg_gen_subi_i64(tmp, res, d_repl * 0x0001u);
-tcg_gen_andc_i64(tmp, tmp, res);
-tcg_gen_andi_i64(tmp, tmp, d_repl * 0x8000u);
-cond = cond_make_0(TCG_COND_NE, tmp);
-} else {
-/* undefined */
-cond = cond_make_f();
+ones = d_repl;
+sgns = d_repl << 31;
}
break;
-
case 2: /* SBZ / NBZ */
-/* See hasless(v,1) from
- * https://graphics.stanford.edu/~seander/bithacks.html#ZeroInWord
- */
-tmp = tcg_temp_new_i64();
-tcg_gen_subi_i64(tmp, res, d_repl * 0x01010101u);
-tcg_gen_andc_i64(tmp, tmp, res);
-tcg_gen_andi_i64(tmp, tmp, d_repl * 0x80808080u);
-cond = cond_make_0(TCG_COND_NE, tmp);
+ones = d_repl * 0x01010101u;
+sgns = ones << 7;
break;
-
case 3: /* SHZ / NHZ */
-tmp = tcg_temp_new_i64();
-tcg_gen_subi_i64(tmp, res, d_repl * 0x00010001u);
-tcg_gen_andc_i64(tmp, tmp, res);
-tcg_gen_andi_i64(tmp, tmp, d_repl * 0x80008000u);
-cond = cond_make_0(TCG_COND_NE, tmp);
+ones = d_repl * 0x00010001u;
+sgns = ones << 15;
break;
-
-case 4: /* SDC / NDC */
-tcg_gen_andi_i64(cb, cb, d_repl * 0xu);
-cond = cond_make_0(TCG_COND_NE, cb);
-break;
-
-case 5: /* SWC / NWC */
-if (d) {
-tcg_gen_andi_i64(cb, cb, d_repl * 0x8000u);
-cond = cond_make_0(TCG_COND_NE, cb);
-} else {
-/* undefined */
-cond = cond_make_f();
-}
-break;
-
-case 6: /* SBC / NBC */
-tcg_gen_andi_i64(cb, cb, d_repl * 0x80808080u);
-cond = cond_make_0(TCG_COND_NE, cb);
-break;
-
-case 7: /* SHC / NHC */
-tcg_gen_andi_i64(cb, cb, d_repl * 0x80008000u);
-cond = cond_make_0(TCG_COND_NE, cb);
-break;
-
-default:
-g_assert_not_reached();
}
-if (cf & 1) {
-cond.c = tcg_invert_cond(cond.c);
+if (ones == 0) {
+/* Undefined, or 0/1 (never/always). */
+return cf & 1 ? cond_make_t() : cond_make_f();
}
-return cond;
+/*
+ * See hasless(v,1) from
+ * https://graphics.stanford.edu/~seander/bithacks.html#ZeroInWord
+ */
+tmp = tcg_temp_new_i64();
+tcg_gen_subi_i64(tmp, res, ones);
+tcg_gen_andc_i64(tmp, tmp, res);
+
+return cond_make_tmp(cf & 1 ? TCG_COND_TSTEQ : TCG_COND_TSTNE,
+ tmp, tcg_constant_i64(sgns));
}
static TCGv_i64 get_carry(DisasContext *ctx, bool d,
@@ -1330,34 +1276,82 @@ static bool do_log_reg(DisasContext *ctx, arg_rrr_cf_d
*a,
return nullify_end(ctx);
}
-static void do_unit(DisasContext *ctx, unsigned rt, TCGv_i64 in1,
-
Re: [PATCH v3 12/17] esp.c: prevent cmdfifo overflow in esp_cdb_ready()
On 24/3/24 20:17, Mark Cave-Ayland wrote:
During normal use the cmdfifo will never wrap internally and cmdfifo_cdb_offset
will always indicate the start of the SCSI CDB. However it is possible that a
malicious guest could issue an invalid ESP command sequence such that cmdfifo
wraps internally and cmdfifo_cdb_offset could point beyond the end of the FIFO
data buffer.
Add an extra check to fifo8_peek_buf() to ensure that if the cmdfifo has wrapped
internally then esp_cdb_ready() will exit rather than allow scsi_cdb_length() to
access data outside the cmdfifo data buffer.
Reported-by: Chuhong Yuan
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index f47abc36d6..d8db33b921 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -429,13 +429,23 @@ static bool esp_cdb_ready(ESPState *s)
{
int len = fifo8_num_used(>cmdfifo) - s->cmdfifo_cdb_offset;
const uint8_t *pbuf;
+uint32_t n;
int cdblen;
if (len <= 0) {
return false;
}
-pbuf = fifo8_peek_buf(>cmdfifo, len, NULL);
+pbuf = fifo8_peek_buf(>cmdfifo, len, );
+if (n < len) {
+/*
+ * In normal use the cmdfifo should never wrap, but include this check
+ * to prevent a malicious guest from reading past the end of the
+ * cmdfifo data buffer below
+ */
Can we qemu_log_mask(LOG_GUEST_ERROR) something here?
+return false;
+}
+
cdblen = scsi_cdb_length((uint8_t *)[s->cmdfifo_cdb_offset]);
return cdblen < 0 ? false : (len >= cdblen);
Re: [PATCH v3 03/17] esp.c: replace esp_fifo_pop_buf() with esp_fifo8_pop_buf() in do_message_phase()
On 24/3/24 20:16, Mark Cave-Ayland wrote: The aim is to restrict the esp_fifo_*() functions so that they only operate on the hardware FIFO. When reading from cmdfifo in do_message_phase() use the underlying esp_fifo8_pop_buf() function directly. Signed-off-by: Mark Cave-Ayland --- hw/scsi/esp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v3 01/17] esp.c: move esp_fifo_pop_buf() internals to new esp_fifo8_pop_buf() function
On 24/3/24 20:16, Mark Cave-Ayland wrote:
Update esp_fifo_pop_buf() to be a simple wrapper onto the new
esp_fifo8_pop_buf()
function.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
+static uint32_t esp_fifo_pop_buf(Fifo8 *fifo, uint8_t *dest, int maxlen)
If future cleanups, maxlen can be unsigned (size_t), anyhow:
Reviewed-by: Philippe Mathieu-Daudé
+{
+return esp_fifo8_pop_buf(fifo, dest, maxlen);
+}
+
static uint32_t esp_get_tc(ESPState *s)
{
uint32_t dmalen;
Re: [PATCH v3 02/17] esp.c: replace esp_fifo_pop_buf() with esp_fifo8_pop_buf() in do_command_phase()
On 24/3/24 20:16, Mark Cave-Ayland wrote: The aim is to restrict the esp_fifo_*() functions so that they only operate on the hardware FIFO. When reading from cmdfifo in do_command_phase() use the underlying esp_fifo8_pop_buf() function directly. Signed-off-by: Mark Cave-Ayland --- hw/scsi/esp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé
[RFC PATCH 11/12] hw/arm/smmuv3: Add property for OAS
Add property that sets the OAS of the SMMU, this in not used in this
patch.
Signed-off-by: Mostafa Saleh
---
hw/arm/smmuv3-internal.h | 2 +-
hw/arm/smmuv3.c | 27 ++-
include/hw/arm/smmuv3.h | 1 +
3 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
index a7d53b3854..9bb4ec9ec6 100644
--- a/hw/arm/smmuv3-internal.h
+++ b/hw/arm/smmuv3-internal.h
@@ -105,7 +105,7 @@ REG32(IDR5,0x14)
FIELD(IDR5, VAX,10, 2);
FIELD(IDR5, STALL_MAX, 16, 16);
-#define SMMU_IDR5_OAS 4
+#define SMMU_IDR5_OAS_DEF 4
REG32(IIDR,0x18)
REG32(AIDR,0x1c)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 2a29e3bccb..9d0db25379 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -299,7 +299,7 @@ static void smmuv3_init_regs(SMMUv3State *s)
s->idr[3] = FIELD_DP32(s->idr[3], IDR3, RIL, 1);
s->idr[3] = FIELD_DP32(s->idr[3], IDR3, BBML, 2);
-s->idr[5] = FIELD_DP32(s->idr[5], IDR5, OAS, SMMU_IDR5_OAS); /* 44 bits */
+s->idr[5] = FIELD_DP32(s->idr[5], IDR5, OAS, s->oas);
/* 4K, 16K and 64K granule support */
s->idr[5] = FIELD_DP32(s->idr[5], IDR5, GRAN4K, 1);
s->idr[5] = FIELD_DP32(s->idr[5], IDR5, GRAN16K, 1);
@@ -1869,11 +1869,34 @@ static const VMStateDescription vmstate_gbpa = {
}
};
+static const VMStateDescription vmstate_oas = {
+.name = "smmuv3/oas",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (const VMStateField[]) {
+VMSTATE_INT32(oas, SMMUv3State),
+VMSTATE_END_OF_LIST()
+}
+};
+
+static int smmuv3_preload(void *opaque)
+{
+SMMUv3State *s = opaque;
+
+/*
+ * In case it wasn't migrated, use the value used
+ * by older QEMU.
+ */
+s->oas = SMMU_IDR5_OAS_DEF;
+return 0;
+}
+
static const VMStateDescription vmstate_smmuv3 = {
.name = "smmuv3",
.version_id = 1,
.minimum_version_id = 1,
.priority = MIG_PRI_IOMMU,
+.pre_load = smmuv3_preload,
.fields = (const VMStateField[]) {
VMSTATE_UINT32(features, SMMUv3State),
VMSTATE_UINT8(sid_size, SMMUv3State),
@@ -1901,6 +1924,7 @@ static const VMStateDescription vmstate_smmuv3 = {
},
.subsections = (const VMStateDescription * const []) {
_gbpa,
+_oas,
NULL
}
};
@@ -1913,6 +1937,7 @@ static Property smmuv3_properties[] = {
* Defaults to stage 1
*/
DEFINE_PROP_STRING("stage", SMMUv3State, stage),
+DEFINE_PROP_INT32("oas", SMMUv3State, oas, SMMU_IDR5_OAS_DEF),
DEFINE_PROP_END_OF_LIST()
};
diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
index d183a62766..00a9eb4467 100644
--- a/include/hw/arm/smmuv3.h
+++ b/include/hw/arm/smmuv3.h
@@ -63,6 +63,7 @@ struct SMMUv3State {
qemu_irq irq[4];
QemuMutex mutex;
char *stage;
+int32_t oas;
};
typedef enum {
--
2.44.0.396.g6e790dbe36-goog
[RFC PATCH 07/12] hw/arm/smmu-common: Support nested translation
When nested translation is requested, we need to do:
- Translate stage-1 IPA using stage-2 to a physical address.
- Translate stage-1 PTW walks using stage-2.
- Combine both to create a single TLB entry
Signed-off-by: Mostafa Saleh
---
hw/arm/smmu-common.c | 135 ---
hw/arm/trace-events | 2 +-
include/hw/arm/smmu-common.h | 3 +-
3 files changed, 113 insertions(+), 27 deletions(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index f0905c28cf..da8776ecec 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -119,7 +119,7 @@ void smmu_iotlb_insert(SMMUState *bs, SMMUTransCfg *cfg,
SMMUTLBEntry *new)
*key = smmu_get_iotlb_key(cfg->asid, cfg->s2cfg.vmid, new->entry.iova,
tg, new->level, stage_tag);
trace_smmu_iotlb_insert(cfg->asid, cfg->s2cfg.vmid, new->entry.iova,
-tg, new->level, stage_tag);
+tg, new->level, new->entry.addr_mask, stage_tag);
g_hash_table_insert(bs->iotlb, key, new);
}
@@ -305,6 +305,27 @@ SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg,
dma_addr_t iova)
return NULL;
}
+/* Return the correct table address based on configuration. */
+static inline int translate_table_s1(dma_addr_t *table_addr, SMMUTransCfg *cfg,
+ SMMUPTWEventInfo *info, SMMUState *bs)
+{
+dma_addr_t addr = *table_addr;
+SMMUTLBEntry *cached_entry;
+
+if (cfg->stage != SMMU_NESTED) {
+return 0;
+}
+
+CALL_FUNC_CFG_S2(cfg, cached_entry, smmu_translate,
+ bs, cfg, addr, IOMMU_RO, info);
+
+if (cached_entry) {
+*table_addr = CACHED_ENTRY_TO_ADDR(cached_entry, addr);
+return 0;
+}
+return -EINVAL;
+}
+
/**
* smmu_ptw_64_s1 - VMSAv8-64 Walk of the page tables for a given IOVA
* @cfg: translation config
@@ -320,7 +341,8 @@ SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t
iova)
*/
static int smmu_ptw_64_s1(SMMUTransCfg *cfg,
dma_addr_t iova, IOMMUAccessFlags perm,
- SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
+ SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info,
+ SMMUState *bs)
{
dma_addr_t baseaddr, indexmask;
SMMUStage stage = cfg->stage;
@@ -368,6 +390,10 @@ static int smmu_ptw_64_s1(SMMUTransCfg *cfg,
goto error;
}
baseaddr = get_table_pte_address(pte, granule_sz);
+/* In case of failure, retain stage-2 fault. */
+if (translate_table_s1(, cfg, info, bs)) {
+goto error_no_stage;
+}
level++;
continue;
} else if (is_page_pte(pte, level)) {
@@ -403,7 +429,7 @@ static int smmu_ptw_64_s1(SMMUTransCfg *cfg,
tlbe->entry.translated_addr = gpa;
tlbe->entry.iova = iova & ~mask;
tlbe->entry.addr_mask = mask;
-tlbe->entry.perm = PTE_AP_TO_PERM(ap);
+tlbe->parent_perm = tlbe->entry.perm = PTE_AP_TO_PERM(ap);
tlbe->level = level;
tlbe->granule = granule_sz;
return 0;
@@ -412,6 +438,7 @@ static int smmu_ptw_64_s1(SMMUTransCfg *cfg,
error:
info->stage = SMMU_STAGE_1;
+error_no_stage:
tlbe->entry.perm = IOMMU_NONE;
return -EINVAL;
}
@@ -524,7 +551,7 @@ static int smmu_ptw_64_s2(SMMUTransCfg *cfg,
tlbe->entry.translated_addr = gpa;
tlbe->entry.iova = ipa & ~mask;
tlbe->entry.addr_mask = mask;
-tlbe->entry.perm = s2ap;
+tlbe->parent_perm = tlbe->entry.perm = s2ap;
tlbe->level = level;
tlbe->granule = granule_sz;
return 0;
@@ -537,6 +564,35 @@ error:
return -EINVAL;
}
+/* Combine 2 TLB enteries and return in tlbe. */
+static void combine_tlb(SMMUTLBEntry *tlbe, SMMUTLBEntry *tlbe_s2,
+dma_addr_t iova, SMMUTransCfg *cfg)
+{
+if (cfg->stage == SMMU_NESTED) {
+
+/*
+ * tg and level are used from stage-1, while the addr mask can be
+ * smaller in case stage-2 size(based on granule and level) was
+ * smaller than stage-1.
+ * That should have no impact on:
+ * - lookup: as iova is properly aligned with the stage-1 level and
+ * granule.
+ * - Invalidation: as it uses the entry mask.
+ */
+tlbe->entry.addr_mask = MIN(tlbe->entry.addr_mask,
+tlbe_s2->entry.addr_mask);
+tlbe->entry.translated_addr = CACHED_ENTRY_TO_ADDR(tlbe_s2,
+ tlbe->entry.translated_addr);
+
+/* parent_perm has s2 perm while perm has s1 perm. */
+tlbe->parent_perm = tlbe_s2->entry.perm;
+return;
+}
+
+/* That was not nested, use the s2. */
+memcpy(tlbe, tlbe_s2,
[RFC PATCH 12/12] hw/arm/virt: Set SMMU OAS based on CPU PARANGE
Use the new SMMU property to make the SMMU OAS match the CPU PARANGE.
That's according to SMMU manual ARM IHI 0070F.b:
6.3.6 SMMU_IDR5, OAS must match the system physical address size.
Signed-off-by: Mostafa Saleh
---
hw/arm/virt.c | 14 --
target/arm/cpu.h | 2 ++
target/arm/cpu64.c | 5 +
3 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 0af1943697..599c0f752b 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -235,6 +235,13 @@ static bool ns_el2_virt_timer_present(void)
arm_feature(env, ARM_FEATURE_EL2) && cpu_isar_feature(aa64_vh, cpu);
}
+/* We rely on CPU to define system OAS. */
+static int32_t get_system_oas(void)
+{
+ARMCPU *cpu = ARM_CPU(qemu_get_cpu(0));
+return cpu_arm_get_oas(cpu);
+}
+
static void create_fdt(VirtMachineState *vms)
{
MachineState *ms = MACHINE(vms);
@@ -1340,7 +1347,7 @@ static void create_pcie_irq_map(const MachineState *ms,
}
static void create_smmu(const VirtMachineState *vms,
-PCIBus *bus)
+PCIBus *bus, int32_t oas)
{
char *node;
const char compat[] = "arm,smmu-v3";
@@ -1360,6 +1367,9 @@ static void create_smmu(const VirtMachineState *vms,
object_property_set_link(OBJECT(dev), "primary-bus", OBJECT(bus),
_abort);
+
+qdev_prop_set_uint64(dev, "oas", oas);
+
sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);
sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
for (i = 0; i < NUM_SMMU_IRQS; i++) {
@@ -1534,7 +1544,7 @@ static void create_pcie(VirtMachineState *vms)
switch (vms->iommu) {
case VIRT_IOMMU_SMMUV3:
-create_smmu(vms, vms->bus);
+create_smmu(vms, vms->bus, get_system_oas());
qemu_fdt_setprop_cells(ms->fdt, nodename, "iommu-map",
0x0, vms->iommu_phandle, 0x0, 0x1);
break;
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index a5b3d8f7da..14ef1a9d37 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3408,4 +3408,6 @@ static inline target_ulong cpu_untagged_addr(CPUState
*cs, target_ulong x)
}
#endif
+int32_t cpu_arm_get_oas(ARMCPU *cpu);
+
#endif
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 985b1efe16..08da83c082 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -787,6 +787,11 @@ static const gchar *aarch64_gdb_arch_name(CPUState *cs)
return "aarch64";
}
+int32_t cpu_arm_get_oas(ARMCPU *cpu)
+{
+return FIELD_EX64(cpu->isar.id_aa64mmfr0, ID_AA64MMFR0, PARANGE);
+}
+
static void aarch64_cpu_class_init(ObjectClass *oc, void *data)
{
CPUClass *cc = CPU_CLASS(oc);
--
2.44.0.396.g6e790dbe36-goog
Re: [PATCH v2] contrib/plugins/execlog: Fix compiler warning
On 3/25/24 13:58, Peter Maydell wrote:
On Mon, 25 Mar 2024 at 06:41, Pierrick Bouvier
wrote:
On 3/25/24 10:06, Yao Xingtao wrote:
diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
index a1dfd59ab7..09654910ee 100644
--- a/contrib/plugins/execlog.c
+++ b/contrib/plugins/execlog.c
@@ -327,8 +327,13 @@ static GPtrArray *registers_init(int vcpu_index)
for (int p = 0; p < rmatches->len; p++) {
g_autoptr(GPatternSpec) pat =
g_pattern_spec_new(rmatches->pdata[p]);
g_autofree gchar *rd_lower = g_utf8_strdown(rd->name, -1);
+#if GLIB_CHECK_VERSION(2, 70, 0)
+if (g_pattern_spec_match_string(pat, rd->name) ||
+g_pattern_spec_match_string(pat, rd_lower)) {
+#else
if (g_pattern_match_string(pat, rd->name) ||
g_pattern_match_string(pat, rd_lower)) {
+#endif
Register *reg = init_vcpu_register(rd);
g_ptr_array_add(registers, reg);
As suggested by Peter on previous version, you can declare a new
function `g_pattern_match_string_qemu` in include/glib-compat.h which
abstract this.
We should have an abstraction function, but it should *not*
be in glib-compat.h, but local to this plugin's .c file. This is
because the plugins are deliberately standalone binaries which do not
rely on any of QEMU's include files or build process (you'll
see they don't use osdep.h, for example).
Sorry, I misunderstood that, as it was discussed with Alex that maybe
plugins should be able to access glib-compat.h.
thanks
-- PMM
[RFC PATCH 10/12] hw/arm/smmu: Refactor SMMU OAS
SMMUv3 OAS is hardcoded to 44 bits, for nested configurations that
can be a problem as stage-2 might be shared with the CPU which might
have different PARANGE, and according to SMMU manual ARM IHI 0070F.b:
6.3.6 SMMU_IDR5, OAS must match the system physical address size.
This patch doesn't change the SMMU OAS, but refactors the code to
make it easier to do that:
- Rely everywhere on IDR5 for reading OAS instead of using the macro so
it is easier just change IDR5 and it propagages correctly.
- Make the code work if OAS is 52 bits.
- Remove unused functions/macros: pa_range/MAX_PA
Signed-off-by: Mostafa Saleh
---
hw/arm/smmu-common.c | 7 ---
hw/arm/smmuv3-internal.h | 15 ++-
hw/arm/smmuv3.c | 35 ---
3 files changed, 34 insertions(+), 23 deletions(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index da8776ecec..a4196ddd22 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -359,7 +359,8 @@ static int smmu_ptw_64_s1(SMMUTransCfg *cfg,
inputsize = 64 - tt->tsz;
level = 4 - (inputsize - 4) / stride;
indexmask = VMSA_IDXMSK(inputsize, stride, level);
-baseaddr = extract64(tt->ttb, 0, 48);
+
+baseaddr = extract64(tt->ttb, 0, cfg->oas);
baseaddr &= ~indexmask;
while (level < VMSA_LEVELS) {
@@ -472,8 +473,8 @@ static int smmu_ptw_64_s2(SMMUTransCfg *cfg,
* Get the ttb from concatenated structure.
* The offset is the idx * size of each ttb(number of ptes * (sizeof(pte))
*/
-uint64_t baseaddr = extract64(cfg->s2cfg.vttb, 0, 48) + (1 << stride) *
- idx * sizeof(uint64_t);
+uint64_t baseaddr = extract64(cfg->s2cfg.vttb, 0, cfg->s2cfg.eff_ps) +
+ (1 << stride) * idx * sizeof(uint64_t);
dma_addr_t indexmask = VMSA_IDXMSK(inputsize, stride, level);
baseaddr &= ~indexmask;
diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
index e4dd11e1e6..a7d53b3854 100644
--- a/hw/arm/smmuv3-internal.h
+++ b/hw/arm/smmuv3-internal.h
@@ -592,23 +592,12 @@ static inline int oas2bits(int oas_field)
return 44;
case 5:
return 48;
+case 6:
+return 52;
}
return -1;
}
-static inline int pa_range(STE *ste)
-{
-int oas_field = MIN(STE_S2PS(ste), SMMU_IDR5_OAS);
-
-if (!STE_S2AA64(ste)) {
-return 40;
-}
-
-return oas2bits(oas_field);
-}
-
-#define MAX_PA(ste) ((1 << pa_range(ste)) - 1)
-
/* CD fields */
#define CD_VALID(x) extract32((x)->word[0], 31, 1)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 288e7cf1ae..2a29e3bccb 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -411,10 +411,10 @@ static bool s2t0sz_valid(SMMUTransCfg *cfg)
}
if (cfg->s2cfg.granule_sz == 16) {
-return (cfg->s2cfg.tsz >= 64 - oas2bits(SMMU_IDR5_OAS));
+return (cfg->s2cfg.tsz >= 64 - cfg->s2cfg.eff_ps);
}
-return (cfg->s2cfg.tsz >= MAX(64 - oas2bits(SMMU_IDR5_OAS), 16));
+return (cfg->s2cfg.tsz >= MAX(64 - cfg->s2cfg.eff_ps, 16));
}
/*
@@ -435,8 +435,11 @@ static bool s2_pgtable_config_valid(uint8_t sl0, uint8_t
t0sz, uint8_t gran)
return nr_concat <= VMSA_MAX_S2_CONCAT;
}
-static int decode_ste_s2_cfg(SMMUTransCfg *cfg, STE *ste)
+static int decode_ste_s2_cfg(SMMUv3State *s, SMMUTransCfg *cfg,
+ STE *ste)
{
+uint8_t oas = FIELD_EX32(s->idr[5], IDR5, OAS);
+
if (STE_S2AA64(ste) == 0x0) {
qemu_log_mask(LOG_UNIMP,
"SMMUv3 AArch32 tables not supported\n");
@@ -469,7 +472,15 @@ static int decode_ste_s2_cfg(SMMUTransCfg *cfg, STE *ste)
}
/* For AA64, The effective S2PS size is capped to the OAS. */
-cfg->s2cfg.eff_ps = oas2bits(MIN(STE_S2PS(ste), SMMU_IDR5_OAS));
+cfg->s2cfg.eff_ps = oas2bits(MIN(STE_S2PS(ste), oas));
+/*
+ * For SMMUv3.1 and later, when OAS == IAS == 52, the stage 2 input
+ * range is further limited to 48 bits unless STE.S2TG indicates a
+ * 64KB granule.
+ */
+if (cfg->s2cfg.granule_sz != 16) {
+cfg->s2cfg.eff_ps = MIN(cfg->s2cfg.eff_ps, 48);
+}
/*
* It is ILLEGAL for the address in S2TTB to be outside the range
* described by the effective S2PS value.
@@ -545,6 +556,7 @@ static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg,
STE *ste, SMMUEventInfo *event)
{
uint32_t config;
+uint8_t oas = FIELD_EX32(s->idr[5], IDR5, OAS);
int ret;
if (!STE_VALID(ste)) {
@@ -591,8 +603,8 @@ static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg,
* Stage-1 OAS defaults to OAS even if not enabled as it would be used
* in input address check for stage-2.
*/
-cfg->oas = oas2bits(SMMU_IDR5_OAS);
-ret = decode_ste_s2_cfg(cfg, ste);
+cfg->oas = oas2bits(oas);
+ret = decode_ste_s2_cfg(s, cfg, ste);
if (ret) {
goto bad_ste;
[RFC PATCH 08/12] hw/arm/smmuv3: Support and advertise nesting
Everything is in place, add the last missing bits:
- Handle fault checking according to the actual PTW event and not the
the translation stage.
- Consolidate parsing of STE cfg and setting translation stage.
Advertise nesting if stage requested is "nested".
Signed-off-by: Mostafa Saleh
---
hw/arm/smmuv3.c | 51 ++---
1 file changed, 36 insertions(+), 15 deletions(-)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 32a1838576..e5373f4cfe 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -34,9 +34,10 @@
#include "smmuv3-internal.h"
#include "smmu-internal.h"
-#define PTW_RECORD_FAULT(cfg) (((cfg)->stage == SMMU_STAGE_1) ? \
- (cfg)->record_faults : \
- (cfg)->s2cfg.record_faults)
+#define PTW_RECORD_FAULT(ptw_info, cfg) (((ptw_info).stage == SMMU_STAGE_1 && \
+(cfg)->record_faults) || \
+((ptw_info).stage == SMMU_STAGE_2 && \
+(cfg)->s2cfg.record_faults))
/**
* smmuv3_trigger_irq - pulse @irq if enabled and update
@@ -260,6 +261,9 @@ static void smmuv3_init_regs(SMMUv3State *s)
/* Based on sys property, the stages supported in smmu will be
advertised.*/
if (s->stage && !strcmp("2", s->stage)) {
s->idr[0] = FIELD_DP32(s->idr[0], IDR0, S2P, 1);
+} else if (s->stage && !strcmp("nested", s->stage)) {
+s->idr[0] = FIELD_DP32(s->idr[0], IDR0, S1P, 1);
+s->idr[0] = FIELD_DP32(s->idr[0], IDR0, S2P, 1);
} else {
s->idr[0] = FIELD_DP32(s->idr[0], IDR0, S1P, 1);
}
@@ -425,8 +429,6 @@ static bool s2_pgtable_config_valid(uint8_t sl0, uint8_t
t0sz, uint8_t gran)
static int decode_ste_s2_cfg(SMMUTransCfg *cfg, STE *ste)
{
-cfg->stage = SMMU_STAGE_2;
-
if (STE_S2AA64(ste) == 0x0) {
qemu_log_mask(LOG_UNIMP,
"SMMUv3 AArch32 tables not supported\n");
@@ -509,6 +511,27 @@ bad_ste:
return -EINVAL;
}
+static void decode_ste_config(SMMUTransCfg *cfg, uint32_t config)
+{
+
+if (STE_CFG_ABORT(config)) {
+cfg->aborted = true;
+return;
+}
+if (STE_CFG_BYPASS(config)) {
+cfg->bypassed = true;
+return;
+}
+
+if (STE_CFG_S1_ENABLED(config)) {
+cfg->stage |= SMMU_STAGE_1;
+}
+
+if (STE_CFG_S2_ENABLED(config)) {
+cfg->stage |= SMMU_STAGE_2;
+}
+}
+
/* Returns < 0 in case of invalid STE, 0 otherwise */
static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg,
STE *ste, SMMUEventInfo *event)
@@ -525,16 +548,15 @@ static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg,
config = STE_CONFIG(ste);
-if (STE_CFG_ABORT(config)) {
-cfg->aborted = true;
+decode_ste_config(cfg, config);
+
+if (cfg->aborted) {
return 0;
}
-if (STE_CFG_BYPASS(config)) {
-cfg->bypassed = true;
+if (cfg->bypassed) {
return 0;
}
-
/*
* If a stage is enabled in SW while not advertised, throw bad ste
* according to user manual(IHI0070E) "5.2 Stream Table Entry".
@@ -704,7 +726,6 @@ static int decode_cd(SMMUv3State *s, SMMUTransCfg *cfg,
/* we support only those at the moment */
cfg->aa64 = true;
-cfg->stage = SMMU_STAGE_1;
cfg->oas = oas2bits(CD_IPS(cd));
cfg->oas = MIN(oas2bits(SMMU_IDR5_OAS), cfg->oas);
@@ -887,28 +908,28 @@ static SMMUTranslationStatus
smmuv3_do_translate(SMMUv3State *s, hwaddr addr,
event->u.f_walk_eabt.addr2 = ptw_info.addr;
break;
case SMMU_PTW_ERR_TRANSLATION:
-if (PTW_RECORD_FAULT(cfg)) {
+if (PTW_RECORD_FAULT(ptw_info, cfg)) {
event->type = SMMU_EVT_F_TRANSLATION;
event->u.f_translation.addr = addr;
event->u.f_translation.rnw = flag & 0x1;
}
break;
case SMMU_PTW_ERR_ADDR_SIZE:
-if (PTW_RECORD_FAULT(cfg)) {
+if (PTW_RECORD_FAULT(ptw_info, cfg)) {
event->type = SMMU_EVT_F_ADDR_SIZE;
event->u.f_addr_size.addr = addr;
event->u.f_addr_size.rnw = flag & 0x1;
}
break;
case SMMU_PTW_ERR_ACCESS:
-if (PTW_RECORD_FAULT(cfg)) {
+if (PTW_RECORD_FAULT(ptw_info, cfg)) {
event->type = SMMU_EVT_F_ACCESS;
event->u.f_access.addr = addr;
event->u.f_access.rnw = flag & 0x1;
}
break;
case SMMU_PTW_ERR_PERMISSION:
-if (PTW_RECORD_FAULT(cfg)) {
+if (PTW_RECORD_FAULT(ptw_info, cfg)) {
event->type = SMMU_EVT_F_PERMISSION;
event->u.f_permission.addr = addr;
event->u.f_permission.rnw = flag & 0x1;
--
2.44.0.396.g6e790dbe36-goog
[RFC PATCH 04/12] hw/arm/smmu: Support nesting in commands
Commands had assumptions about VMID and ASID being mutually exclusive
and the same for stage-1 and stage-2. As we are going to support
nesting, we need to implement them properly:
- CMD_TLBI_NH_ASID: Used to ignore VMID as it was not used in stage-1
instances, now we read it from the command and invalidate by
ASID + VMID if stage-2 exists.
- CMD_TLBI_NH_ALL: Use to invalidate all as VMID were not used in
stage-1 instances, now it invalidates stage-1 by vmid, and this
command is decoupled from CMD_TLBI_NSNH_ALL which invalidates all
stages.
- CMD_TLBI_NH_VAA, SMMU_CMD_TLBI_NH_VA: Used to ignore VMID also.
- CMD_TLBI_S2_IPA: Now invalidates stage-2 only.
Signed-off-by: Mostafa Saleh
---
hw/arm/smmu-common.c | 34 +-
hw/arm/smmuv3.c | 47 +++-
hw/arm/trace-events | 7 +++---
include/hw/arm/smmu-common.h | 4 +--
4 files changed, 64 insertions(+), 28 deletions(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 677dcf9a13..f0905c28cf 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -129,22 +129,24 @@ void smmu_iotlb_inv_all(SMMUState *s)
g_hash_table_remove_all(s->iotlb);
}
-static gboolean smmu_hash_remove_by_asid(gpointer key, gpointer value,
- gpointer user_data)
+static gboolean smmu_hash_remove_by_asid_vmid(gpointer key, gpointer value,
+ gpointer user_data)
{
-uint16_t asid = *(uint16_t *)user_data;
+SMMUIOTLBPageInvInfo *info = (SMMUIOTLBPageInvInfo *)user_data;
SMMUIOTLBKey *iotlb_key = (SMMUIOTLBKey *)key;
-return SMMU_IOTLB_ASID(*iotlb_key) == asid;
+return (SMMU_IOTLB_ASID(*iotlb_key) == info->asid) &&
+(SMMU_IOTLB_VMID(*iotlb_key) == info->vmid);
}
static gboolean smmu_hash_remove_by_vmid(gpointer key, gpointer value,
gpointer user_data)
{
-uint16_t vmid = *(uint16_t *)user_data;
+SMMUIOTLBPageInvInfo *info = (SMMUIOTLBPageInvInfo *)user_data;
SMMUIOTLBKey *iotlb_key = (SMMUIOTLBKey *)key;
-return SMMU_IOTLB_VMID(*iotlb_key) == vmid;
+return (SMMU_IOTLB_VMID(*iotlb_key) == info->vmid) &&
+(info->stage & SMMU_IOTLB_STAGE(*iotlb_key));
}
static gboolean smmu_hash_remove_by_asid_vmid_iova(gpointer key, gpointer
value,
@@ -198,16 +200,26 @@ void smmu_iotlb_inv_iova(SMMUState *s, int asid, int
vmid, dma_addr_t iova,
);
}
-void smmu_iotlb_inv_asid(SMMUState *s, uint16_t asid)
+void smmu_iotlb_inv_asid_vmid(SMMUState *s, uint16_t asid, uint16_t vmid)
{
+SMMUIOTLBPageInvInfo info = {
+.asid = asid,
+.vmid = vmid,
+};
+
trace_smmu_iotlb_inv_asid(asid);
-g_hash_table_foreach_remove(s->iotlb, smmu_hash_remove_by_asid, );
+g_hash_table_foreach_remove(s->iotlb, smmu_hash_remove_by_asid_vmid,
);
}
-inline void smmu_iotlb_inv_vmid(SMMUState *s, uint16_t vmid)
+inline void smmu_iotlb_inv_vmid(SMMUState *s, uint16_t vmid, SMMUStage stage)
{
-trace_smmu_iotlb_inv_vmid(vmid);
-g_hash_table_foreach_remove(s->iotlb, smmu_hash_remove_by_vmid, );
+SMMUIOTLBPageInvInfo info = {
+.vmid = vmid,
+.stage = stage,
+};
+
+trace_smmu_iotlb_inv_vmid(vmid, stage);
+g_hash_table_foreach_remove(s->iotlb, smmu_hash_remove_by_vmid, );
}
/* VMSAv8-64 Translation */
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index b27bf297e1..9460fff0ed 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -1060,7 +1060,7 @@ static void smmuv3_inv_notifiers_iova(SMMUState *s, int
asid, int vmid,
}
}
-static void smmuv3_range_inval(SMMUState *s, Cmd *cmd)
+static void smmuv3_range_inval(SMMUState *s, Cmd *cmd, SMMUStage stage)
{
dma_addr_t end, addr = CMD_ADDR(cmd);
uint8_t type = CMD_TYPE(cmd);
@@ -1085,9 +1085,9 @@ static void smmuv3_range_inval(SMMUState *s, Cmd *cmd)
}
if (!tg) {
-trace_smmuv3_range_inval(vmid, asid, addr, tg, 1, ttl, leaf);
+trace_smmuv3_range_inval(vmid, asid, addr, tg, 1, ttl, leaf, stage);
smmuv3_inv_notifiers_iova(s, asid, vmid, addr, tg, 1);
-smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, 1, ttl, SMMU_NESTED);
+smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, 1, ttl, stage);
return;
}
@@ -1103,10 +1103,10 @@ static void smmuv3_range_inval(SMMUState *s, Cmd *cmd)
uint64_t mask = dma_aligned_pow2_mask(addr, end, 64);
num_pages = (mask + 1) >> granule;
-trace_smmuv3_range_inval(vmid, asid, addr, tg, num_pages, ttl, leaf);
+trace_smmuv3_range_inval(vmid, asid, addr, tg,
+ num_pages, ttl, leaf, stage);
smmuv3_inv_notifiers_iova(s, asid, vmid, addr, tg, num_pages);
-smmu_iotlb_inv_iova(s, asid, vmid, addr, tg,
-num_pages, ttl, SMMU_NESTED);
+smmu_iotlb_inv_iova(s,
[RFC PATCH 01/12] hw/arm/smmu: Use enum for SMMU stage
Currently, translation stage is represented as an int, where 1 is stage-1 and
2 is stage-2, when nested is added, 3 would be confusing to represent nesting,
so we use an enum instead.
While keeping the same values, this is useful for:
- Doing tricks with bit masks, where BIT(0) is stage-1 and BIT(1) is
stage-2 and both is nested.
- Tracing, as stage is printed as int.
Signed-off-by: Mostafa Saleh
---
hw/arm/smmu-common.c | 14 +++---
hw/arm/smmuv3.c | 15 ---
include/hw/arm/smmu-common.h | 11 +--
3 files changed, 24 insertions(+), 16 deletions(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 4caedb4998..3a7c350aca 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -304,7 +304,7 @@ static int smmu_ptw_64_s1(SMMUTransCfg *cfg,
SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
{
dma_addr_t baseaddr, indexmask;
-int stage = cfg->stage;
+SMMUStage stage = cfg->stage;
SMMUTransTableInfo *tt = select_tt(cfg, iova);
uint8_t level, granule_sz, inputsize, stride;
@@ -392,7 +392,7 @@ static int smmu_ptw_64_s1(SMMUTransCfg *cfg,
info->type = SMMU_PTW_ERR_TRANSLATION;
error:
-info->stage = 1;
+info->stage = SMMU_STAGE_1;
tlbe->entry.perm = IOMMU_NONE;
return -EINVAL;
}
@@ -415,7 +415,7 @@ static int smmu_ptw_64_s2(SMMUTransCfg *cfg,
dma_addr_t ipa, IOMMUAccessFlags perm,
SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
{
-const int stage = 2;
+const SMMUStage stage = SMMU_STAGE_2;
int granule_sz = cfg->s2cfg.granule_sz;
/* ARM DDI0487I.a: Table D8-7. */
int inputsize = 64 - cfg->s2cfg.tsz;
@@ -513,7 +513,7 @@ static int smmu_ptw_64_s2(SMMUTransCfg *cfg,
info->type = SMMU_PTW_ERR_TRANSLATION;
error:
-info->stage = 2;
+info->stage = SMMU_STAGE_2;
tlbe->entry.perm = IOMMU_NONE;
return -EINVAL;
}
@@ -532,9 +532,9 @@ error:
int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm,
SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
{
-if (cfg->stage == 1) {
+if (cfg->stage == SMMU_STAGE_1) {
return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info);
-} else if (cfg->stage == 2) {
+} else if (cfg->stage == SMMU_STAGE_2) {
/*
* If bypassing stage 1(or unimplemented), the input address is passed
* directly to stage 2 as IPA. If the input address of a transaction
@@ -543,7 +543,7 @@ int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova,
IOMMUAccessFlags perm,
*/
if (iova >= (1ULL << cfg->oas)) {
info->type = SMMU_PTW_ERR_ADDR_SIZE;
-info->stage = 1;
+info->stage = SMMU_STAGE_1;
tlbe->entry.perm = IOMMU_NONE;
return -EINVAL;
}
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 9eb56a70f3..50e5a72d54 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -34,7 +34,8 @@
#include "smmuv3-internal.h"
#include "smmu-internal.h"
-#define PTW_RECORD_FAULT(cfg) (((cfg)->stage == 1) ? (cfg)->record_faults : \
+#define PTW_RECORD_FAULT(cfg) (((cfg)->stage == SMMU_STAGE_1) ? \
+ (cfg)->record_faults : \
(cfg)->s2cfg.record_faults)
/**
@@ -402,7 +403,7 @@ static bool s2_pgtable_config_valid(uint8_t sl0, uint8_t
t0sz, uint8_t gran)
static int decode_ste_s2_cfg(SMMUTransCfg *cfg, STE *ste)
{
-cfg->stage = 2;
+cfg->stage = SMMU_STAGE_2;
if (STE_S2AA64(ste) == 0x0) {
qemu_log_mask(LOG_UNIMP,
@@ -678,7 +679,7 @@ static int decode_cd(SMMUTransCfg *cfg, CD *cd,
SMMUEventInfo *event)
/* we support only those at the moment */
cfg->aa64 = true;
-cfg->stage = 1;
+cfg->stage = SMMU_STAGE_1;
cfg->oas = oas2bits(CD_IPS(cd));
cfg->oas = MIN(oas2bits(SMMU_IDR5_OAS), cfg->oas);
@@ -762,7 +763,7 @@ static int smmuv3_decode_config(IOMMUMemoryRegion *mr,
SMMUTransCfg *cfg,
return ret;
}
-if (cfg->aborted || cfg->bypassed || (cfg->stage == 2)) {
+if (cfg->aborted || cfg->bypassed || (cfg->stage == SMMU_STAGE_2)) {
return 0;
}
@@ -882,7 +883,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion
*mr, hwaddr addr,
goto epilogue;
}
-if (cfg->stage == 1) {
+if (cfg->stage == SMMU_STAGE_1) {
/* Select stage1 translation table. */
tt = select_tt(cfg, addr);
if (!tt) {
@@ -919,7 +920,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion
*mr, hwaddr addr,
* nesting is not supported. So it is sufficient to check the
* translation stage to know the TLB stage for now.
*/
-event.u.f_walk_eabt.s2 = (cfg->stage == 2);
+event.u.f_walk_eabt.s2 = (cfg->stage == SMMU_STAGE_2);
if (PTW_RECORD_FAULT(cfg)) {
event.type =
[RFC PATCH 09/12] hw/arm/smmuv3: Advertise S2FWB
QEMU doesn's support memory attributes, so FWB is NOP, this
might change in the future if memory attributre would be supported.
Signed-off-by: Mostafa Saleh
---
hw/arm/smmuv3.c | 8
1 file changed, 8 insertions(+)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index e5373f4cfe..288e7cf1ae 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -287,6 +287,14 @@ static void smmuv3_init_regs(SMMUv3State *s)
if (FIELD_EX32(s->idr[0], IDR0, S2P)) {
/* XNX is a stage-2-specific feature */
s->idr[3] = FIELD_DP32(s->idr[3], IDR3, XNX, 1);
+if (FIELD_EX32(s->idr[0], IDR0, S1P)) {
+/*
+ * QEMU doesn's support memory attributes, so FWB is NOP, this
+ * might change in the future if memory attributre would be
+ * supported.
+ */
+ s->idr[3] = FIELD_DP32(s->idr[3], IDR3, FWB, 1);
+}
}
s->idr[3] = FIELD_DP32(s->idr[3], IDR3, RIL, 1);
s->idr[3] = FIELD_DP32(s->idr[3], IDR3, BBML, 2);
--
2.44.0.396.g6e790dbe36-goog
[RFC PATCH 05/12] hw/arm/smmuv3: Support nested SMMUs in smmuv3_notify_iova()
IOMMUTLBEvent only understands IOVA, for stage-2 only SMMUs keep
the implementation, while only notify for stage-1 invalidation
in case of nesting.
Signed-off-by: Mostafa Saleh
---
hw/arm/smmuv3.c | 23 +++
hw/arm/trace-events | 2 +-
2 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 9460fff0ed..d9ee203d09 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -993,7 +993,7 @@ static void smmuv3_notify_iova(IOMMUMemoryRegion *mr,
IOMMUNotifier *n,
int asid, int vmid,
dma_addr_t iova, uint8_t tg,
- uint64_t num_pages)
+ uint64_t num_pages, int stage)
{
SMMUDevice *sdev = container_of(mr, SMMUDevice, iommu);
IOMMUTLBEvent event;
@@ -1017,14 +1017,21 @@ static void smmuv3_notify_iova(IOMMUMemoryRegion *mr,
return;
}
-if (STAGE1_SUPPORTED(s)) {
+/*
+ * IOMMUTLBEvent only understands IOVA, for stage-2 only SMMUs
+ * keep the implementation, while only notify for stage-1
+ * invalidation in case of nesting.
+ */
+if (stage == SMMU_STAGE_1) {
tt = select_tt(cfg, iova);
if (!tt) {
return;
}
granule = tt->granule_sz;
-} else {
+} else if (!STAGE1_SUPPORTED(s)) {
granule = cfg->s2cfg.granule_sz;
+} else {
+return;
}
} else {
@@ -1043,7 +1050,7 @@ static void smmuv3_notify_iova(IOMMUMemoryRegion *mr,
/* invalidate an asid/vmid/iova range tuple in all mr's */
static void smmuv3_inv_notifiers_iova(SMMUState *s, int asid, int vmid,
dma_addr_t iova, uint8_t tg,
- uint64_t num_pages)
+ uint64_t num_pages, int stage)
{
SMMUDevice *sdev;
@@ -1052,10 +1059,10 @@ static void smmuv3_inv_notifiers_iova(SMMUState *s, int
asid, int vmid,
IOMMUNotifier *n;
trace_smmuv3_inv_notifiers_iova(mr->parent_obj.name, asid, vmid,
-iova, tg, num_pages);
+iova, tg, num_pages, stage);
IOMMU_NOTIFIER_FOREACH(n, mr) {
-smmuv3_notify_iova(mr, n, asid, vmid, iova, tg, num_pages);
+smmuv3_notify_iova(mr, n, asid, vmid, iova, tg, num_pages, stage);
}
}
}
@@ -1086,7 +1093,7 @@ static void smmuv3_range_inval(SMMUState *s, Cmd *cmd,
SMMUStage stage)
if (!tg) {
trace_smmuv3_range_inval(vmid, asid, addr, tg, 1, ttl, leaf, stage);
-smmuv3_inv_notifiers_iova(s, asid, vmid, addr, tg, 1);
+smmuv3_inv_notifiers_iova(s, asid, vmid, addr, tg, 1, stage);
smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, 1, ttl, stage);
return;
}
@@ -1105,7 +1112,7 @@ static void smmuv3_range_inval(SMMUState *s, Cmd *cmd,
SMMUStage stage)
num_pages = (mask + 1) >> granule;
trace_smmuv3_range_inval(vmid, asid, addr, tg,
num_pages, ttl, leaf, stage);
-smmuv3_inv_notifiers_iova(s, asid, vmid, addr, tg, num_pages);
+smmuv3_inv_notifiers_iova(s, asid, vmid, addr, tg, num_pages, stage);
smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, num_pages, ttl, stage);
addr += mask + 1;
}
diff --git a/hw/arm/trace-events b/hw/arm/trace-events
index 73cec52d21..34b10af83f 100644
--- a/hw/arm/trace-events
+++ b/hw/arm/trace-events
@@ -55,7 +55,7 @@ smmuv3_cmdq_tlbi_s12_vmid(uint16_t vmid) "vmid=%d"
smmuv3_config_cache_inv(uint32_t sid) "Config cache INV for sid=0x%x"
smmuv3_notify_flag_add(const char *iommu) "ADD SMMUNotifier node for iommu
mr=%s"
smmuv3_notify_flag_del(const char *iommu) "DEL SMMUNotifier node for iommu
mr=%s"
-smmuv3_inv_notifiers_iova(const char *name, uint16_t asid, uint16_t vmid,
uint64_t iova, uint8_t tg, uint64_t num_pages) "iommu mr=%s asid=%d vmid=%d
iova=0x%"PRIx64" tg=%d num_pages=0x%"PRIx64
+smmuv3_inv_notifiers_iova(const char *name, uint16_t asid, uint16_t vmid,
uint64_t iova, uint8_t tg, uint64_t num_pages, int stage) "iommu mr=%s asid=%d
vmid=%d iova=0x%"PRIx64" tg=%d num_pages=0x%"PRIx64" stage=%d"
# strongarm.c
strongarm_uart_update_parameters(const char *label, int speed, char parity,
int data_bits, int stop_bits) "%s speed=%d parity=%c data=%d stop=%d"
--
2.44.0.396.g6e790dbe36-goog
[RFC PATCH 00/12] SMMUv3 nested translation support
Currently, QEMU supports emulating either stage-1 or stage-2 SMMUs but not nested instances. This patch series adds support for nested translation in SMMUv3, this is controlled by property “arm-smmuv3.stage=nested”, and advertised to guests as (IDR0.S1P == 1 && IDR0.S2P == 2) Main changes(architecture): 1) CDs are considered IPA and translated with stage-2. 2) TTBx and tables for stage-1 are considered IPA and translated with stage-2. 3) Translate the IPA address with stage-2. TLBs: == TLBs are the most tricky part. 1) General design Unified(Combined) design is used, where a new tag is added "stage" which has 2 valid values: - STAGE_1: Meaning this entry translates VA to PADDR, it can be cached from fully nested configuration or from stage-1 only. It doesn't support separate cached entries (VA to IPA). - STAGE_2: Meaning this translates IPA to PADDR, cached from stage-2 only configuration. TLBs are also modified to cache 2 permissions, a new permission added "parent_perm." For non-nested configuration, perm == parent_perm and nothing changes. This is used to know which stage to use in case there is a permission fault from a TLB entry. 2) Caching in TLB Stage-1 and stage-2 are inserted in the TLB as is. For nested translation, both entries are combined into one TLB entry. Everything is used from stage-1, except: - transatled_addr from stage-2. - parent_perm is from stage-2. - addr_mask: is the minimum of both. 3) TLB Lookup For stage-1 and nested translations, it look for STAGE_1 entries. For stage-2 it look for STAGE_2 TLB entries. 4) TLB invalidation - Stage-1 commands (CMD_TLBI_NH_VAA, SMMU_CMD_TLBI_NH_VA, SMMU_CMD_TLBI_NH_ALL): Invalidate TLBs tagged with SMMU_STAGE_1. - Stage-2 commands (CMD_TLBI_S2_IPA): Invalidate TLBs tagged with SMMU_STAGE_2. - All (SMMU_CMD_TLBI_S12_VMALL): Will invalidate both, this is communicated to the TLB as SMMU_NESTED which is (SMMU_STAGE_1 | SMMU_STAGE_2) which uses it as a mask. As far as I understand, this is compliant with the ARM architecture, based on: - ARM ARM DDI 0487J.a: RLGSCG, RTVTYQ, RGNJPZ - ARM IHI 0070F.b: 16.2 Caching An alternative approach would be to instantiate 2 TLBs, one per each stage. I haven’t investigated that. Others === - Advertise SMMUv3.2-S2FWB, it is NOP for QEMU as it doesn’t support attributes. - OAS: A typical setup with nesting is to share CPU stage-2 with the SMMU, and according to the user manual, SMMU OAS must match the system physical address. This was discussed before in https://lore.kernel.org/all/20230226220650.1480786-11-smost...@google.com/ The implementation here, follows the discussion, where migration is added and oas is set up from the board (virt). However, the OAS is chosen based on the CPU PARANGE as there is no fixed one. - For nested configuration, IOVA notifier only notifies for stage-1 invalidations (as far as I understand this is the intended behaviour as it notifies for IOVA) - Stop ignoring VMID for stage-1 if stage-2 is also supported. Future improvements: = 1) One small improvement, that I don’t think it’s worth the extra complexity, is in case of Stage-1 TLB miss for nested translation, we can do stage-1 walk and lookup for stage-2 TLBs, instead of doing the full walk. 2) Patch 0006 (hw/arm/smmuv3: Translate CD and TT using stage-2 table) introduces a macro to use functions that rely on cfg for stage-2, I don’t like it. However, I didn’t find a simple way around it, either we change many functions to have a separate stage argument, or add another arg in config, which is probably more code. Testing 1) IOMMUFD + VFIO Kernel: https://lore.kernel.org/all/cover.1683688960.git.nicol...@nvidia.com/ VMM: https://qemu-devel.nongnu.narkive.com/o815DqpI/rfc-v5-0-8-arm-smmuv3-emulation-support By assigning “virtio-net-pci,netdev=net0,disable-legacy=on,iommu_platform=on,ats=on”, to a guest VM (on top of QEMU guest) with VIFO and IOMMUFD. 2) Work in progress prototype I am hacking on for nesting on KVM (this is nowhere near complete, and misses many stuff but it doesn't require VMs/VFIO) also with virtio-net-pci and git cloning a bunch of stuff and also observing traces. https://android-kvm.googlesource.com/linux/+log/refs/heads/smostafa/android15-6.6-smmu-nesting-wip hw/arm/smmuv3: Split smmuv3_translate() better viewed with --color-moved Mostafa Saleh (12): hw/arm/smmu: Use enum for SMMU stage hw/arm/smmu: Split smmuv3_translate() hw/arm/smmu: Add stage to TLB hw/arm/smmu: Support nesting in commands hw/arm/smmuv3: Support nested SMMUs in smmuv3_notify_iova() hw/arm/smmuv3: Translate CD and TT using stage-2 table hw/arm/smmu-common: Support nested translation hw/arm/smmuv3: Support and advertise nesting hw/arm/smmuv3: Advertise S2FWB
[RFC PATCH 06/12] hw/arm/smmuv3: Translate CD and TT using stage-2 table
According to the user manual (ARM IHI 0070 F.b),
In "5.2 Stream Table Entry":
[51:6] S1ContextPtr
If Config[1] == 1 (stage 2 enabled), this pointer is an IPA translated by
stage 2 and the programmed value must be within the range of the IAS.
In "5.4.1 CD notes":
The translation table walks performed from TTB0 or TTB1 are always performed
in IPA space if stage 2 translations are enabled.
So translate both the CD and the TTBx in this patch if nested
translation is requested.
Signed-off-by: Mostafa Saleh
---
hw/arm/smmuv3.c | 49 ++--
include/hw/arm/smmu-common.h | 15 +++
2 files changed, 57 insertions(+), 7 deletions(-)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index d9ee203d09..32a1838576 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -337,14 +337,36 @@ static int smmu_get_ste(SMMUv3State *s, dma_addr_t addr,
STE *buf,
}
+static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr,
+ SMMUTransCfg *cfg,
+ SMMUEventInfo *event,
+ IOMMUAccessFlags flag,
+ SMMUTLBEntry **out_entry);
/* @ssid > 0 not supported yet */
-static int smmu_get_cd(SMMUv3State *s, STE *ste, uint32_t ssid,
- CD *buf, SMMUEventInfo *event)
+static int smmu_get_cd(SMMUv3State *s, STE *ste, SMMUTransCfg *cfg,
+ uint32_t ssid, CD *buf, SMMUEventInfo *event)
{
dma_addr_t addr = STE_CTXPTR(ste);
int ret, i;
+SMMUTranslationStatus status;
+SMMUTLBEntry *entry;
trace_smmuv3_get_cd(addr);
+
+if (cfg->stage == SMMU_NESTED) {
+CALL_FUNC_CFG_S2(cfg, status, smmuv3_do_translate, s, addr,
+ cfg, event, IOMMU_RO, );
+/*
+ * It is not clear what should happen if this fails, so we return here
+ * which gets propagated as a translation error.
+ */
+if (status != SMMU_TRANS_SUCCESS) {
+return -EINVAL;
+}
+
+addr = CACHED_ENTRY_TO_ADDR(entry, addr);
+}
+
/* TODO: guarantee 64-bit single-copy atomicity */
ret = dma_memory_read(_space_memory, addr, buf, sizeof(*buf),
MEMTXATTRS_UNSPECIFIED);
@@ -659,10 +681,13 @@ static int smmu_find_ste(SMMUv3State *s, uint32_t sid,
STE *ste,
return 0;
}
-static int decode_cd(SMMUTransCfg *cfg, CD *cd, SMMUEventInfo *event)
+static int decode_cd(SMMUv3State *s, SMMUTransCfg *cfg,
+ CD *cd, SMMUEventInfo *event)
{
int ret = -EINVAL;
int i;
+SMMUTranslationStatus status;
+SMMUTLBEntry *entry;
if (!CD_VALID(cd) || !CD_AARCH64(cd)) {
goto bad_cd;
@@ -713,6 +738,17 @@ static int decode_cd(SMMUTransCfg *cfg, CD *cd,
SMMUEventInfo *event)
tt->tsz = tsz;
tt->ttb = CD_TTB(cd, i);
+
+/* Translate the TTBx, from IPA to PA if nesting is enabled. */
+if (cfg->stage == SMMU_NESTED) {
+CALL_FUNC_CFG_S2(cfg, status, smmuv3_do_translate, s,
+ tt->ttb, cfg, event, IOMMU_RO, );
+/* See smmu_get_cd(). */
+if (status != SMMU_TRANS_SUCCESS) {
+return -EINVAL;
+}
+tt->ttb = CACHED_ENTRY_TO_ADDR(entry, tt->ttb);
+}
if (tt->ttb & ~(MAKE_64BIT_MASK(0, cfg->oas))) {
goto bad_cd;
}
@@ -767,12 +803,12 @@ static int smmuv3_decode_config(IOMMUMemoryRegion *mr,
SMMUTransCfg *cfg,
return 0;
}
-ret = smmu_get_cd(s, , 0 /* ssid */, , event);
+ret = smmu_get_cd(s, , cfg, 0 /* ssid */, , event);
if (ret) {
return ret;
}
-return decode_cd(cfg, , event);
+return decode_cd(s, cfg, , event);
}
/**
@@ -942,8 +978,7 @@ epilogue:
switch (status) {
case SMMU_TRANS_SUCCESS:
entry.perm = cached_entry->entry.perm;
-entry.translated_addr = cached_entry->entry.translated_addr +
-(addr & cached_entry->entry.addr_mask);
+entry.translated_addr = CACHED_ENTRY_TO_ADDR(cached_entry, addr);
entry.addr_mask = cached_entry->entry.addr_mask;
trace_smmuv3_translate_success(mr->parent_obj.name, sid, addr,
entry.translated_addr, entry.perm,
diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index 6d3bf5316b..c0969e461d 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -39,6 +39,21 @@
#define SMMU_STAGE_TO_TLB_TAG(stage)(((stage) == SMMU_NESTED) ? \
SMMU_STAGE_1 : (stage))
+
+#define CACHED_ENTRY_TO_ADDR(ent, addr) (ent)->entry.translated_addr + \
+ ((addr) & (ent)->entry.addr_mask);
+
+/*
+ * From nested
[RFC PATCH 02/12] hw/arm/smmu: Split smmuv3_translate()
smmuv3_translate() does everything from STE/CD parsing to TLB lookup
and PTW.
Soon, when nesting is supported, stage-1 data (tt, CD) needs to be
translated using stage-2.
Split smmuv3_translate() to 3 functions:
- smmu_translate(): in smmu-common.c, which does the TLB lookup, PTW,
TLB insertion, all the functions are already there, this just puts
them together.
This also simplifies the code as it consolidates event generation
in case of TLB lookup permission failure or in TT selection.
- smmuv3_do_translate(): in smmuv3.c, Calls smmu_translate() and does
the event population in case of errors.
- smmuv3_translate(), now calls smmuv3_do_translate() for
translation while the rest is the same.
Also, add stage in trace_smmuv3_translate_success()
Signed-off-by: Mostafa Saleh
---
hw/arm/smmu-common.c | 59
hw/arm/smmuv3.c | 175 +--
hw/arm/trace-events | 2 +-
include/hw/arm/smmu-common.h | 5 +
4 files changed, 130 insertions(+), 111 deletions(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 3a7c350aca..20630eb670 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -554,6 +554,65 @@ int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova,
IOMMUAccessFlags perm,
g_assert_not_reached();
}
+SMMUTLBEntry *smmu_translate(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t addr,
+ IOMMUAccessFlags flag, SMMUPTWEventInfo *info)
+{
+uint64_t page_mask, aligned_addr;
+SMMUTLBEntry *cached_entry = NULL;
+SMMUTransTableInfo *tt;
+int status;
+
+/*
+ * Combined attributes used for TLB lookup, as only one stage is supported,
+ * it will hold attributes based on the enabled stage.
+ */
+SMMUTransTableInfo tt_combined;
+
+if (cfg->stage == SMMU_STAGE_1) {
+/* Select stage1 translation table. */
+tt = select_tt(cfg, addr);
+if (!tt) {
+info->type = SMMU_PTW_ERR_TRANSLATION;
+info->stage = SMMU_STAGE_1;
+return NULL;
+}
+tt_combined.granule_sz = tt->granule_sz;
+tt_combined.tsz = tt->tsz;
+
+} else {
+/* Stage2. */
+tt_combined.granule_sz = cfg->s2cfg.granule_sz;
+tt_combined.tsz = cfg->s2cfg.tsz;
+}
+
+/*
+ * TLB lookup looks for granule and input size for a translation stage,
+ * as only one stage is supported right now, choose the right values
+ * from the configuration.
+ */
+page_mask = (1ULL << tt_combined.granule_sz) - 1;
+aligned_addr = addr & ~page_mask;
+
+cached_entry = smmu_iotlb_lookup(bs, cfg, _combined, aligned_addr);
+if (cached_entry) {
+if ((flag & IOMMU_WO) && !(cached_entry->entry.perm & IOMMU_WO)) {
+info->type = SMMU_PTW_ERR_PERMISSION;
+info->stage = cfg->stage;
+return NULL;
+}
+return cached_entry;
+}
+
+cached_entry = g_new0(SMMUTLBEntry, 1);
+status = smmu_ptw(cfg, aligned_addr, flag, cached_entry, info);
+if (status) {
+g_free(cached_entry);
+return NULL;
+}
+smmu_iotlb_insert(bs, cfg, cached_entry);
+return cached_entry;
+}
+
/**
* The bus number is used for lookup when SID based invalidation occurs.
* In that case we lazily populate the SMMUPciBus array from the bus hash
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 50e5a72d54..f081ff0cc4 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -827,6 +827,67 @@ static void smmuv3_flush_config(SMMUDevice *sdev)
g_hash_table_remove(bc->configs, sdev);
}
+/* Do translation with TLB lookup. */
+static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr,
+ SMMUTransCfg *cfg,
+ SMMUEventInfo *event,
+ IOMMUAccessFlags flag,
+ SMMUTLBEntry **out_entry)
+{
+SMMUPTWEventInfo ptw_info = {};
+SMMUState *bs = ARM_SMMU(s);
+SMMUTLBEntry *cached_entry = NULL;
+
+cached_entry = smmu_translate(bs, cfg, addr, flag, _info);
+if (!cached_entry) {
+/* All faults from PTW has S2 field. */
+event->u.f_walk_eabt.s2 = (ptw_info.stage == SMMU_STAGE_2);
+switch (ptw_info.type) {
+case SMMU_PTW_ERR_WALK_EABT:
+event->type = SMMU_EVT_F_WALK_EABT;
+event->u.f_walk_eabt.addr = addr;
+event->u.f_walk_eabt.rnw = flag & 0x1;
+event->u.f_walk_eabt.class = 0x1;
+event->u.f_walk_eabt.addr2 = ptw_info.addr;
+break;
+case SMMU_PTW_ERR_TRANSLATION:
+if (PTW_RECORD_FAULT(cfg)) {
+event->type = SMMU_EVT_F_TRANSLATION;
+event->u.f_translation.addr = addr;
+event->u.f_translation.rnw = flag & 0x1;
+}
+
[RFC PATCH 03/12] hw/arm/smmu: Add stage to TLB
TLBs for nesting will be extended to be combined, a new index is added
"stage", with 2 valid values:
- SMMU_STAGE_1: Meaning this translates VA to PADDR, this entry can
be cached from fully nested configuration or from stage-1 only.
We don't support separate cached entries (VA to IPA)
- SMMU_STAGE_2: Meaning this translates IPA to PADDR, cached from
stage-2 only configuration.
For TLB invalidation:
- by VA: Invalidate TLBs tagged with SMMU_STAGE_1
- by IPA: Invalidate TLBs tagged with SMMU_STAGE_2
- All: Will invalidate both, this is communicated to the TLB as
SMMU_NESTED which is (SMMU_STAGE_1 | SMMU_STAGE_2) which uses
it as a mask.
This briefly described in the user manual (ARM IHI 0070 F.b) in
"16.2.1 Caching combined structures".
Signed-off-by: Mostafa Saleh
---
hw/arm/smmu-common.c | 27 +--
hw/arm/smmu-internal.h | 2 ++
hw/arm/smmuv3.c | 5 +++--
hw/arm/trace-events | 3 ++-
include/hw/arm/smmu-common.h | 8 ++--
5 files changed, 30 insertions(+), 15 deletions(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 20630eb670..677dcf9a13 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -38,7 +38,7 @@ static guint smmu_iotlb_key_hash(gconstpointer v)
/* Jenkins hash */
a = b = c = JHASH_INITVAL + sizeof(*key);
-a += key->asid + key->vmid + key->level + key->tg;
+a += key->asid + key->vmid + key->level + key->tg + key->stage;
b += extract64(key->iova, 0, 32);
c += extract64(key->iova, 32, 32);
@@ -54,14 +54,14 @@ static gboolean smmu_iotlb_key_equal(gconstpointer v1,
gconstpointer v2)
return (k1->asid == k2->asid) && (k1->iova == k2->iova) &&
(k1->level == k2->level) && (k1->tg == k2->tg) &&
- (k1->vmid == k2->vmid);
+ (k1->vmid == k2->vmid) && (k1->stage == k2->stage);
}
SMMUIOTLBKey smmu_get_iotlb_key(uint16_t asid, uint16_t vmid, uint64_t iova,
-uint8_t tg, uint8_t level)
+uint8_t tg, uint8_t level, SMMUStage stage)
{
SMMUIOTLBKey key = {.asid = asid, .vmid = vmid, .iova = iova,
-.tg = tg, .level = level};
+.tg = tg, .level = level, .stage = stage};
return key;
}
@@ -81,7 +81,8 @@ SMMUTLBEntry *smmu_iotlb_lookup(SMMUState *bs, SMMUTransCfg
*cfg,
SMMUIOTLBKey key;
key = smmu_get_iotlb_key(cfg->asid, cfg->s2cfg.vmid,
- iova & ~mask, tg, level);
+ iova & ~mask, tg, level,
+ SMMU_STAGE_TO_TLB_TAG(cfg->stage));
entry = g_hash_table_lookup(bs->iotlb, );
if (entry) {
break;
@@ -109,15 +110,16 @@ void smmu_iotlb_insert(SMMUState *bs, SMMUTransCfg *cfg,
SMMUTLBEntry *new)
{
SMMUIOTLBKey *key = g_new0(SMMUIOTLBKey, 1);
uint8_t tg = (new->granule - 10) / 2;
+SMMUStage stage_tag = SMMU_STAGE_TO_TLB_TAG(cfg->stage);
if (g_hash_table_size(bs->iotlb) >= SMMU_IOTLB_MAX_SIZE) {
smmu_iotlb_inv_all(bs);
}
*key = smmu_get_iotlb_key(cfg->asid, cfg->s2cfg.vmid, new->entry.iova,
- tg, new->level);
+ tg, new->level, stage_tag);
trace_smmu_iotlb_insert(cfg->asid, cfg->s2cfg.vmid, new->entry.iova,
-tg, new->level);
+tg, new->level, stage_tag);
g_hash_table_insert(bs->iotlb, key, new);
}
@@ -159,18 +161,22 @@ static gboolean
smmu_hash_remove_by_asid_vmid_iova(gpointer key, gpointer value,
if (info->vmid >= 0 && info->vmid != SMMU_IOTLB_VMID(iotlb_key)) {
return false;
}
+if (!(info->stage & SMMU_IOTLB_STAGE(iotlb_key))) {
+return false;
+}
return ((info->iova & ~entry->addr_mask) == entry->iova) ||
((entry->iova & ~info->mask) == info->iova);
}
void smmu_iotlb_inv_iova(SMMUState *s, int asid, int vmid, dma_addr_t iova,
- uint8_t tg, uint64_t num_pages, uint8_t ttl)
+ uint8_t tg, uint64_t num_pages, uint8_t ttl,
+ SMMUStage stage)
{
/* if tg is not set we use 4KB range invalidation */
uint8_t granule = tg ? tg * 2 + 10 : 12;
if (ttl && (num_pages == 1) && (asid >= 0)) {
-SMMUIOTLBKey key = smmu_get_iotlb_key(asid, vmid, iova, tg, ttl);
+SMMUIOTLBKey key = smmu_get_iotlb_key(asid, vmid, iova, tg, ttl,
stage);
if (g_hash_table_remove(s->iotlb, )) {
return;
@@ -184,6 +190,7 @@ void smmu_iotlb_inv_iova(SMMUState *s, int asid, int vmid,
dma_addr_t iova,
SMMUIOTLBPageInvInfo info = {
.asid = asid, .iova = iova,
.vmid = vmid,
+.stage = stage,
.mask = (num_pages * 1 << granule) - 1};
g_hash_table_foreach_remove(s->iotlb,
@@ -597,7 +604,7 @@ SMMUTLBEntry
Re: [RFC v2 2/2] hw/riscv: Add server platform reference machine
On 3/23/2024 3:14 AM, Atish Kumar Patra wrote:
> On Tue, Mar 12, 2024 at 6:53 AM Fei Wu wrote:
>>
>> The RISC-V Server Platform specification[1] defines a standardized set
>> of hardware and software capabilities, that portable system software,
>> such as OS and hypervisors can rely on being present in a RISC-V server
>> platform.
>>
[...]
>> +
>> +static void finalize_fdt(RVSPMachineState *s)
>> +{
>> +uint32_t phandle = 1, irq_mmio_phandle = 1, msi_pcie_phandle = 1;
>> +uint32_t irq_pcie_phandle = 1;
>> +
>> +create_fdt_sockets(s, rvsp_ref_memmap, , _mmio_phandle,
>> + _pcie_phandle, _pcie_phandle);
>> +
>> +create_fdt_pcie(s, rvsp_ref_memmap, irq_pcie_phandle, msi_pcie_phandle);
>> +
>> +create_fdt_reset(s, rvsp_ref_memmap, );
>> +
>> +create_fdt_uart(s, rvsp_ref_memmap, irq_mmio_phandle);
>> +
>> +create_fdt_rtc(s, rvsp_ref_memmap, irq_mmio_phandle);
>
>
> We need a minimalistic DT for firmwares which probably don't use rtc, pcie
> etc.
> Does EDK2 plan to generate ACPI tables from these DT ? Otherwise, we
> can get rid of these.
>
Yes, I agree. Eventually we can remove many of these fdt when EDK2 is
able to generate the ACPI tables directly. I add it here as EDK2 has not
been adapted yet, the system won't boot up with the upstream EDK2 if
e.g. fdt of pcie is removed.
Thanks,
Fei.
> As Heinrich said, Linux kernels should boot using ACPI only.
>
Re: [PATCH] qapi: document InputMultiTouchType
marcandre.lur...@redhat.com writes: > From: Marc-André Lureau > > Signed-off-by: Marc-André Lureau Reviewed-by: Markus Armbruster Queued, thanks!
Re: [PULL 1/1] target/loongarch: Fix qemu-system-loongarch64 assert failed with the option '-d int'
Cc: qemu-sta...@nongnu.org 在 2024/3/22 下午10:58, Michael Tokarev 写道: 22.03.2024 13:03, Song Gao : qemu-system-loongarch64 assert failed with the option '-d int', the helper_idle() raise an exception EXCP_HLT, but the exception name is undefined. Signed-off-by: Song Gao Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20240321123606.1704900-1-gaos...@loongson.cn> Is this another qemu-stable material? You Cc'd it to me but I'm not sure what should I do with it. For patches suitable for -stable, please Cc: qemu-sta...@nongnu.org. Thanks, /mjt
Re: [PATCH v2] contrib/plugins/execlog: Fix compiler warning
On Mon, 25 Mar 2024 at 06:41, Pierrick Bouvier
wrote:
>
> On 3/25/24 10:06, Yao Xingtao wrote:
> > diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
> > index a1dfd59ab7..09654910ee 100644
> > --- a/contrib/plugins/execlog.c
> > +++ b/contrib/plugins/execlog.c
> > @@ -327,8 +327,13 @@ static GPtrArray *registers_init(int vcpu_index)
> > for (int p = 0; p < rmatches->len; p++) {
> > g_autoptr(GPatternSpec) pat =
> > g_pattern_spec_new(rmatches->pdata[p]);
> > g_autofree gchar *rd_lower = g_utf8_strdown(rd->name, -1);
> > +#if GLIB_CHECK_VERSION(2, 70, 0)
> > +if (g_pattern_spec_match_string(pat, rd->name) ||
> > +g_pattern_spec_match_string(pat, rd_lower)) {
> > +#else
> > if (g_pattern_match_string(pat, rd->name) ||
> > g_pattern_match_string(pat, rd_lower)) {
> > +#endif
> > Register *reg = init_vcpu_register(rd);
> > g_ptr_array_add(registers, reg);
> >
>
> As suggested by Peter on previous version, you can declare a new
> function `g_pattern_match_string_qemu` in include/glib-compat.h which
> abstract this.
We should have an abstraction function, but it should *not*
be in glib-compat.h, but local to this plugin's .c file. This is
because the plugins are deliberately standalone binaries which do not
rely on any of QEMU's include files or build process (you'll
see they don't use osdep.h, for example).
thanks
-- PMM
[PATCH] qapi: document InputMultiTouchType
From: Marc-André Lureau
Signed-off-by: Marc-André Lureau
---
qapi/ui.json | 12
1 file changed, 12 insertions(+)
diff --git a/qapi/ui.json b/qapi/ui.json
index 5744c24e3c..53d9143054 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1080,6 +1080,16 @@
#
# Type of a multi-touch event.
#
+# @begin: A new touch event sequence has just started.
+#
+# @update: A touch event sequence has been updated.
+#
+# @end: A touch event sequence has finished.
+#
+# @cancel: A touch event sequence has been canceled.
+#
+# @data: Absolute position data.
+#
# Since: 8.1
##
{ 'enum' : 'InputMultiTouchType',
@@ -1137,6 +1147,8 @@
#
# MultiTouch input event.
#
+# @type: The type of multi-touch event.
+#
# @slot: Which slot has generated the event.
#
# @tracking-id: ID to correlate this event with previously generated
--
2.44.0
Re: [PATCH 2/3] target/hppa: Optimize UADDCM with no condition
On 3/25/24 04:04, Richard Henderson wrote:
With r1 as zero is by far the only usage of UADDCM, as the easiest
way to invert a register. The compiler does occasionally use the
addition step as well, and we can simplify that to avoid a temp
and write directly into the destination.
Signed-off-by: Richard Henderson
Reviewed-by: Helge Deller
Tested-by: Helge Deller
Helge
---
target/hppa/translate.c | 24 ++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index a3f425d861..3fc3e7754c 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -2763,9 +2763,29 @@ static bool do_uaddcm(DisasContext *ctx, arg_rrr_cf_d
*a, bool is_tc)
{
TCGv_i64 tcg_r1, tcg_r2, tmp;
-if (a->cf) {
-nullify_over(ctx);
+if (a->cf == 0) {
+tcg_r2 = load_gpr(ctx, a->r2);
+tmp = dest_gpr(ctx, a->t);
+
+if (a->r1 == 0) {
+/* UADDCM r0,src,dst is the common idiom for dst = ~src. */
+tcg_gen_not_i64(tmp, tcg_r2);
+} else {
+/*
+ * Recall that r1 - r2 == r1 + ~r2 + 1.
+ * Thus r1 + ~r2 == r1 - r2 - 1,
+ * which does not require an extra temporary.
+ */
+tcg_r1 = load_gpr(ctx, a->r1);
+tcg_gen_sub_i64(tmp, tcg_r1, tcg_r2);
+tcg_gen_subi_i64(tmp, tmp, 1);
+}
+save_gpr(ctx, a->t, tmp);
+cond_free(>null_cond);
+return true;
}
+
+nullify_over(ctx);
tcg_r1 = load_gpr(ctx, a->r1);
tcg_r2 = load_gpr(ctx, a->r2);
tmp = tcg_temp_new_i64();
Re: [PATCH 1/3] targt/hppa: Fix DCOR reconstruction of carry bits
On 3/25/24 04:04, Richard Henderson wrote:
The carry bits for each nibble N are located in bit (N+1)*4,
so the shift by 3 was off by one. Furthermore, the carry bit
for the most significant carry bit is indeed located in bit 64,
which is located in a different storage word.
Use a double-word shift-right to reassemble into a single word
and place them all at bit 0 of their respective nibbles.
Signed-off-by: Richard Henderson
Reviewed-by: Helge Deller
Tested-by: Helge Deller
Helge
---
target/hppa/translate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index e041310207..a3f425d861 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -2791,7 +2791,7 @@ static bool do_dcor(DisasContext *ctx, arg_rr_cf_d *a,
bool is_i)
nullify_over(ctx);
tmp = tcg_temp_new_i64();
-tcg_gen_shri_i64(tmp, cpu_psw_cb, 3);
+tcg_gen_extract2_i64(tmp, cpu_psw_cb, cpu_psw_cb_msb, 4);
if (!is_i) {
tcg_gen_not_i64(tmp, tmp);
}
Re: [PATCH v2 0/2] ARM Sbsa-ref: Enable CPU cluster topology
> W dniu 22.03.2024 o 19:51, Peter Maydell pisze: > > On Tue, 12 Mar 2024 at 08:32, Xiong Yining > > >> xiongyining1480 (2): > >>hw/arm/sbsa-ref:Enable CPU cluster on ARM sbsa machine > >>hw/arm/sbsa-ref: Add cpu-map to device tree > > > > Thanks for these patches. I think we should squash the two > > patches together into one, because the first patch is only > > a single line, and also because we shouldn't say that the > > machine supports cluster topology until it actually does > > by putting the information into the device tree. fully agree > > There's no rush, because we're now in softfreeze for 9.0, so these > > will have to wait until 9.0 is released (in about a month's time). > > > I'm also a bit confused by the Reviewed-by: tag from Marcin on patch 2, > > because I can't see that in my mail archives of the discussion on version > > 1 of this patchset, only a Tested-by. > > Marcin, are you OK with these patches? > > I only tested them. They are fine, will check on Monday. > > > Also, is this change to the DTB something that would require an > > increase in the sbsa-ref platform version number, or not? > > TF-A will check for "/cpus/cpu-map" node and if it is missing then will > not provide it to EDK2. So far I did not saw patches for firmware side. I send a patch in TF-A to check "/cpus/cpu-map" node https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/27189/1#message-2c29be6b8b9b4fd3fef23ba7be6fe6fc3a2d0aef. It can be used with this patch in qemu. > I would add bump of platform version to 0.4 one. It is cheap operation > and so far (from firmware side) we check for >= 0.3 only. > > > Should we adjust the documentation in docs/system/arm/sbsa.rst to > > mention that the DTB might have cluster topology information? > > Yes. I will send an update to mention that NUMA configuration can be > there too (we already export it from TF-A to EDK2 via SMC calls). 信息安全声明：本邮件包含信息归发件人所在组织所有,发件人所在组织对该邮件拥有所有权利。请接收者注意保密,未经发件人书面许可,不得向任何第三方组织和个人透露本邮件所含信息。 Information Security Notice: The information contained in this mail is solely property of the sender's organization.This mail communication is confidential.Recipients named above are obligated to maintain secrecy and are not permitted to disclose the contents of this communication to others.
Re: [PATCH 01/10] qtest/phb4: Add testbench for PHB4
Hello Saif,
On 3/21/24 11:04, Saif Abrar wrote:
New qtest TB added for PHB4.
TB reads PHB Version register and asserts that
bits[24:31] have value 0xA5.
Signed-off-by: Saif Abrar
---
tests/qtest/meson.build | 1 +
tests/qtest/pnv-phb4-test.c | 74 +
2 files changed, 75 insertions(+)
create mode 100644 tests/qtest/pnv-phb4-test.c
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 36c5c13a7b..4795e51c17 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -168,6 +168,7 @@ qtests_ppc64 = \
(config_all_devices.has_key('CONFIG_PSERIES') ? ['device-plug-test'] : [])
+ \
(config_all_devices.has_key('CONFIG_POWERNV') ? ['pnv-xscom-test'] : []) +
\
(config_all_devices.has_key('CONFIG_POWERNV') ? ['pnv-host-i2c-test'] : [])
+ \
+ (config_all_devices.has_key('CONFIG_POWERNV') ? ['pnv-phb4-test'] : []) +
\
(config_all_devices.has_key('CONFIG_PSERIES') ? ['rtas-test'] : []) +
\
(slirp.found() ? ['pxe-test'] : []) + \
(config_all_devices.has_key('CONFIG_USB_UHCI') ? ['usb-hcd-uhci-test'] :
[]) + \
diff --git a/tests/qtest/pnv-phb4-test.c b/tests/qtest/pnv-phb4-test.c
new file mode 100644
index 00..e3b809e9c4
--- /dev/null
+++ b/tests/qtest/pnv-phb4-test.c
@@ -0,0 +1,74 @@
+/*
+ * QTest testcase for PowerNV PHB4
+ *
+ * Copyright (c) 2024, IBM Corporation.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "hw/pci-host/pnv_phb4_regs.h"
+
+#define P10_XSCOM_BASE 0x000603fcull
+#define PHB4_MMIO 0x000600c3c000ull
+#define PHB4_XSCOM 0x8010900ull
+
+#define PPC_BIT(bit)(0x8000ULL >> (bit))
+#define PPC_BITMASK(bs, be) ((PPC_BIT(bs) - PPC_BIT(be)) | PPC_BIT(bs))
+
+static uint64_t pnv_xscom_addr(uint32_t pcba)
+{
+return P10_XSCOM_BASE | ((uint64_t) pcba << 3);
+}
+
+static uint64_t pnv_phb4_xscom_addr(uint32_t reg)
+{
+return pnv_xscom_addr(PHB4_XSCOM + reg);
+}
Please use tests/qtest/pnv-xscom.h instead.
+/*
+ * XSCOM read/write is indirect in PHB4:
+ * Write 'SCOM - HV Indirect Address Register'
+ * with register-offset to read/write.
+ - bit[0]: Valid Bit
+ - bit[51:61]: Indirect Address(00:10)
+ * Read/write 'SCOM - HV Indirect Data Register' to get/set the value.
+ */
+
+static uint64_t pnv_phb4_xscom_read(QTestState *qts, uint32_t reg)
+{
+qtest_writeq(qts, pnv_phb4_xscom_addr(PHB_SCOM_HV_IND_ADDR),
+PPC_BIT(0) | reg);
+return qtest_readq(qts, pnv_phb4_xscom_addr(PHB_SCOM_HV_IND_DATA));
+}
+/* Assert that 'PHB - Version Register Offset 0x0800' bits-[24:31] are 0xA5 */
+static void phb4_version_test(QTestState *qts)
+{
+uint64_t ver = pnv_phb4_xscom_read(qts, PHB_VERSION);
+
+/* PHB Version register [24:31]: Major Revision ID 0xA5 */
+ver = ver >> (63 - 31);
+g_assert_cmpuint(ver, ==, 0xA5);
+}
+
+static void test_phb4(void)
+{
+QTestState *qts = NULL;
+
+qts = qtest_initf("-machine powernv10 -accel tcg -nographic -d unimp");
"-nographic -d unimp" is not needed.
+
+/* Make sure test is running on PHB */
+phb4_version_test(qts);
Please add similar tests for phb[345]. See tests/qtest/pnv-xscom-test.c.
Thanks,
C.
+
+qtest_quit(qts);
+}
+
+int main(int argc, char **argv)
+{
+g_test_init(, , NULL);
+qtest_add_func("phb4", test_phb4);
+return g_test_run();
+}
Let's close member documentation gaps
If you're cc'ed, I have a bit of doc work for you. Search for your
name to find it.
The QAPI generator forces you to document your stuff. Except for
commands, events, enum and object types listed in pragma
documentation-exceptions, the generator silently defaults missing
documentation to "Not documented". Right now, we're using this loophole
some 500 times.
Most of the offenders are enumeration values. Their meaning is perhaps
easier to guess than the meaning of command arguments, member data, and
object type members. Ignoring enumerations leaves 62 offenders. Let's
examine them.
= qapi/block-core.json
* DummyBlockCoreForceArrays
Not actually part of the external interface, ignore.
* Qcow2OverlapCheckFlags
If the user needs to know what the flags do, then the flags need to be
documented. Else, they should not be part of the stable interface.
Vladimir, if the former, please fix. If the latter, please mark them
unstable.
* ThrottleGroupProperties
The unstable properties you're not supposed to use are undocumented.
Tolerable, I guess.
* XDbgBlockGraph
Only user is x-debug-query-block-graph, which is for debugging.
Tolerable, I guess.
* blockdev-reopen
The documentation refers to the argument ("the given set of options"),
but since it lacks a formal @option: section, the generator concludes
it doesn't, and supplies its "Not documented" description.
Embarrassing. Kevin or Hanna, please fix.
= qapi/machine-target.json
* query-cpu-model-baseline
* query-cpu-model-comparison
The documentation refers to the arguments ("two CPU models"), but
since it lacks formal @modela: and @modelb: sections, the generator
concludes it doesn't, and supplies its "Not documented" description.
Embarrassing. David, please fix.
* query-cpu-model-expansion
Likewise, only the references to the arguments are even more vague.
David, please fix.
= qapi/machine.json
* DummyForceArrays
Not actually part of the external interface, ignore.
= qapi/net.json
* String
Lack of the @str: section produces an embarrassing "Not documented" in
the generated documentation. I can post a patch to make it less
embarrassing. I doubt we can make it actually good, as generic
wrapper types like this one have meaning only in the context they are
used. Therefore, their meaning can be usefully explained only at
their uses, not their definition.
= qapi/pci.json
* PciMemoryRegion
Michael or Marcel, please document @address.
= qapi/rocker.json
* query-rocker
* query-rocker-ports
Jiri, please document the argument.
= qapi/run-state.json
* GuestPanicInformationHyperV
Paolo, please document the members.
* watchdog-set-action
Paolo, please document the argument, or ask me to do it for you.
= qapi/stats.json
* StatsFilter
Paolo, please document @providers.
* StatsValue
Paolo, please document @boolean.
* query-stats-schemas
Paolo, please document the argument.
= qapi/transaction.json
* AbortWrapper
* BlockDirtyBitmapAddWrapper
* BlockDirtyBitmapMergeWrapper
* BlockDirtyBitmapWrapper
* BlockdevBackupWrapper
* BlockdevSnapshotInternalWrapper
* BlockdevSnapshotSyncWrapper
* BlockdevSnapshotWrapper
* DriveBackupWrapper
Kevin or Hana, please document the member.
Similar wrapper types elsewhere simply steal from the wrapped type's
description. Trouble is the ones wrapped here lack a description.
= qapi/ui.json
* InputMultiTouchEvent
Marc-André, please document @type.
= qapi/virtio.json
* DummyVirtioForceArrays
Not actually part of the external interface, ignore.
Re: [PATCH 11/26] runstate: skip initial CPU reset if reset is not actually possible
On 22/3/24 19:11, Paolo Bonzini wrote: Right now, the system reset is concluded by a call to cpu_synchronize_all_post_reset() in order to sync any changes that the machine reset callback applied to the CPU state. However, for VMs with encrypted state such as SEV-ES guests (currently the only case of guests with non-resettable CPUs) this cannot be done, because guest state has already been finalized by machine-init-done notifiers. cpu_synchronize_all_post_reset() does nothing on these guests, and actually we would like to make it fail if called once guest has been encrypted. So, assume that boards that support non-resettable CPUs do not touch CPU state and that all such setup is done before, at the time of cpu_synchronize_all_post_init(). Signed-off-by: Paolo Bonzini --- system/runstate.c | 15 ++- roms/edk2 | 2 +- Without submodule change: Reviewed-by: Philippe Mathieu-Daudé 2 files changed, 15 insertions(+), 2 deletions(-)
Re: [PATCH 18/26] kvm: Introduce support for memory_attributes
On 22/3/24 19:11, Paolo Bonzini wrote: From: Xiaoyao Li Introduce the helper functions to set the attributes of a range of memory to private or shared. This is necessary to notify KVM the private/shared attribute of each gpa range. KVM needs the information to decide the GPA needs to be mapped at hva-based shared memory or guest_memfd based private memory. Signed-off-by: Xiaoyao Li Message-ID: <20240320083945.991426-11-michael.r...@amd.com> Signed-off-by: Paolo Bonzini --- include/sysemu/kvm.h | 4 accel/kvm/kvm-all.c | 31 +++ 2 files changed, 35 insertions(+) diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h index 54f4d83a370..bda309d5ffa 100644 --- a/include/sysemu/kvm.h +++ b/include/sysemu/kvm.h @@ -536,4 +536,8 @@ void kvm_mark_guest_state_protected(void); * reported for the VM. */ bool kvm_hwpoisoned_mem(void); + +int kvm_set_memory_attributes_private(hwaddr start, hwaddr size); +int kvm_set_memory_attributes_shared(hwaddr start, hwaddr size); uint64_t size? (kvm_memory_attributes::size is __u64).
Re: [PATCH RESEND v3 0/3] i386: Fix Hyper-V Gen1 guests stuck on boot with 'hv-passthrough'
Vitaly Kuznetsov writes: > Changes since 'RESEND v2': > - Included 'docs/system: Add recommendations to Hyper-V enlightenments doc' > in the set as it also requires a "RESEND") Ping) > > Hyper-V Gen1 guests are getting stuck on boot when 'hv-passthrough' is > used. While 'hv-passthrough' is a debug only feature, this significantly > limit its usefullness. While debugging the problem, I found that there are > two loosely connected issues: > - 'hv-passthrough' enables 'hv-syndbg' and this is undesired. > - 'hv-syndbg's support by KVM is detected incorrectly when !CONFIG_SYNDBG. > > Fix both issues; exclude 'hv-syndbg' from 'hv-passthrough' and don't allow > to turn on 'hv-syndbg' for !CONFIG_SYNDBG builds. > > Vitaly Kuznetsov (3): > i386: Fix conditional CONFIG_SYNDBG enablement > i386: Exclude 'hv-syndbg' from 'hv-passthrough' > docs/system: Add recommendations to Hyper-V enlightenments doc > > docs/system/i386/hyperv.rst | 43 + > target/i386/cpu.c | 2 ++ > target/i386/kvm/kvm.c | 18 ++-- > 3 files changed, 53 insertions(+), 10 deletions(-) -- Vitaly
Re: [PATCH 15/26] target/i386: Implement mc->kvm_type() to get VM type
On 22/3/24 19:11, Paolo Bonzini wrote: KVM is introducing a new API to create confidential guests, which will be used by TDX and SEV-SNP but is also available for SEV and SEV-ES. The API uses the VM type argument to KVM_CREATE_VM to identify which confidential computing technology to use. Since there are no other expected uses of VM types, delegate mc->kvm_type() for x86 boards to the confidential-guest-support s/mc/cgs/ here and in subject? object pointed to by ms->cgs. For example, if a sev-guest object is specified to confidential-guest-support, like, qemu -machine ...,confidential-guest-support=sev0 \ -object sev-guest,id=sev0,... it will check if a VM type KVM_X86_SEV_VM or KVM_X86_SEV_ES_VM is supported, and if so use them together with the KVM_SEV_INIT2 function of the KVM_MEMORY_ENCRYPT_OP ioctl. If not, it will fall back to KVM_SEV_INIT and KVM_SEV_ES_INIT. This is a preparatory work towards TDX and SEV-SNP support, but it will also enable support for VMSA features such as DebugSwap, which are only available via KVM_SEV_INIT2. Co-developed-by: Xiaoyao Li Signed-off-by: Xiaoyao Li Signed-off-by: Paolo Bonzini --- target/i386/confidential-guest.h | 19 ++ target/i386/kvm/kvm_i386.h | 2 ++ hw/i386/x86.c| 11 target/i386/kvm/kvm.c| 44 4 files changed, 76 insertions(+)
Re: [PATCH 13/26] KVM: remove kvm_arch_cpu_check_are_resettable
On 22/3/24 19:11, Paolo Bonzini wrote: Board reset requires writing a fresh CPU state. As far as KVM is concerned, the only thing that blocks reset is that CPU state is encrypted; therefore, kvm_cpus_are_resettable() can simply check if that is the case. Signed-off-by: Paolo Bonzini --- include/sysemu/kvm.h | 10 -- accel/kvm/kvm-accel-ops.c | 2 +- accel/kvm/kvm-all.c| 5 - target/arm/kvm.c | 5 - target/i386/kvm/kvm.c | 5 - target/loongarch/kvm/kvm.c | 5 - target/mips/kvm.c | 5 - target/ppc/kvm.c | 5 - target/riscv/kvm/kvm-cpu.c | 5 - target/s390x/kvm/kvm.c | 5 - 10 files changed, 1 insertion(+), 51 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 12/26] KVM: track whether guest state is encrypted
On 22/3/24 19:11, Paolo Bonzini wrote: So far, KVM has allowed KVM_GET/SET_* ioctls to execute even if the guest state is encrypted, in which case they do nothing. For the new API using VM types, instead, the ioctls will fail which is a safer and more robust approach. The new API will be the only one available for SEV-SNP and TDX, but it is also usable for SEV and SEV-ES. In preparation for that, require architecture-specific KVM code to communicate the point at which guest state is protected (which must be after kvm_cpu_synchronize_post_init(), though that might change in the future in order to suppor migration). From that point, skip reading registers so that cpu->vcpu_dirty is never true: if it ever becomes true, kvm_arch_put_registers() will fail miserably. Signed-off-by: Paolo Bonzini --- include/sysemu/kvm.h | 2 ++ include/sysemu/kvm_int.h | 1 + accel/kvm/kvm-all.c | 14 -- target/i386/sev.c| 1 + 4 files changed, 16 insertions(+), 2 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PULL 20/34] tests/libqos: add riscv/virt machine nodes
On 08/03/2024 12.11, Alistair Francis wrote: From: Daniel Henrique Barboza Add a RISC-V 'virt' machine to the graph. This implementation is a modified copy of the existing arm machine in arm-virt-machine.c It contains a virtio-mmio and a generic-pcihost controller. The generic-pcihost controller hardcodes assumptions from the ARM 'virt' machine, like ecam and pio_base addresses, so we'll add an extra step to set its parameters after creating it. Our command line is incremented with 'aclint' parameters to allow the machine to run MSI tests. Signed-off-by: Daniel Henrique Barboza Acked-by: Alistair Francis Acked-by: Thomas Huth Message-ID: <20240217192607.32565-7-dbarb...@ventanamicro.com> Signed-off-by: Alistair Francis --- Hi! I noticed that "make check SPEED=slow" is now failing on the qos-test with both, qemu-system-riscv32 and qemu-system-riscv64. Seems like it fails with the virtio-9p test, when I run the qos-test manually, I get: $ MALLOC_PERTURB_=21 V=2 QTEST_QEMU_BINARY=./qemu-system-riscv64 \ tests/qtest/qos-test -m slow ... # Start of local tests # starting QEMU: exec ./qemu-system-riscv64 -qtest unix:/tmp/qtest-211303.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-211303.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -M virt,aclint=on,aia=aplic-imsic -fsdev local,id=fsdev0,path='/home/thuth/tmp/qemu-build/qtest-9p-local-MBCML2',security_model=mapped-xattr -device virtio-9p-pci,fsdev=fsdev0,addr=04.0,mount_tag=qtest -accel qtest ok 168 /riscv64/virt/generic-pcihost/pci-bus-generic/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/local/config Received response 7 (RLERROR) instead of 73 (RMKDIR) Rlerror has errno 17 (File exists) ** ERROR:../../devel/qemu/tests/qtest/libqos/virtio-9p-client.c:275:v9fs_req_recv: assertion failed (hdr.id == id): (7 == 73) not ok /riscv64/virt/generic-pcihost/pci-bus-generic/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/local/create_dir - ERROR:../../devel/qemu/tests/qtest/libqos/virtio-9p-client.c:275:v9fs_req_recv: assertion failed (hdr.id == id): (7 == 73) Bail out! Aborted (core dumped) Could you please have a look? ... or if it is too cumbersome to fix, could we please always skip the virtio-9p local tests on riscv ? Thomas
Re: [PATCH-for-9.0 1/2] hw/clock: Let clock_set_mul_div() return boolean value
On 09:40 Mon 25 Mar , Luc Michel wrote:
> On 16:58 Fri 22 Mar , Philippe Mathieu-Daudé wrote:
> > Let clock_set_mul_div() return a boolean value whether the
> > clock has been updated or not, similarly to clock_set().
> >
> > Signed-off-by: Philippe Mathieu-Daudé
>
> Acked-by: Luc Michel
Sorry, I forgot, as Peter suggested, can you add a word in the doc
about this?
Something in the vein of:
+ Similary to ``clock_set()``, ``clock_set_mul_div()`` returns ``true`` if
+ the clock state was modified, that it, if the multiplier or the diviser
+ or both were changed by the call.
+
Note that ``clock_set_mul_div()`` does not automatically
call ``clock_propagate()``. If you make a runtime change to the
multiplier or divider you must call clock_propagate() yourself.
Thanks!
--
Luc
>
> > ---
> > include/hw/clock.h | 4 +++-
> > hw/core/clock.c| 8 +++-
> > 2 files changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/hw/clock.h b/include/hw/clock.h
> > index bb12117f67..eb58599131 100644
> > --- a/include/hw/clock.h
> > +++ b/include/hw/clock.h
> > @@ -357,6 +357,8 @@ char *clock_display_freq(Clock *clk);
> > * @multiplier: multiplier value
> > * @divider: divider value
> > *
> > + * @return: true if the clock is changed.
> > + *
> > * By default, a Clock's children will all run with the same period
> > * as their parent. This function allows you to adjust the multiplier
> > * and divider used to derive the child clock frequency.
> > @@ -374,6 +376,6 @@ char *clock_display_freq(Clock *clk);
> > * Note that this function does not call clock_propagate(); the
> > * caller should do that if necessary.
> > */
> > -void clock_set_mul_div(Clock *clk, uint32_t multiplier, uint32_t divider);
> > +bool clock_set_mul_div(Clock *clk, uint32_t multiplier, uint32_t divider);
> >
> > #endif /* QEMU_HW_CLOCK_H */
> > diff --git a/hw/core/clock.c b/hw/core/clock.c
> > index d82e44cd1a..a19c7db7df 100644
> > --- a/hw/core/clock.c
> > +++ b/hw/core/clock.c
> > @@ -143,14 +143,20 @@ char *clock_display_freq(Clock *clk)
> > return freq_to_str(clock_get_hz(clk));
> > }
> >
> > -void clock_set_mul_div(Clock *clk, uint32_t multiplier, uint32_t divider)
> > +bool clock_set_mul_div(Clock *clk, uint32_t multiplier, uint32_t divider)
> > {
> > assert(divider != 0);
> >
> > +if (clk->multiplier == multiplier && clk->divider == divider) {
> > +return false;
> > +}
> > +
> > trace_clock_set_mul_div(CLOCK_PATH(clk), clk->multiplier, multiplier,
> > clk->divider, divider);
> > clk->multiplier = multiplier;
> > clk->divider = divider;
> > +
> > +return true;
> > }
> >
> > static void clock_initfn(Object *obj)
> > --
> > 2.41.0
> >
>
--
Re: [PATCH 11/26] runstate: skip initial CPU reset if reset is not actually possible
On Fri, Mar 22, 2024 at 07:11:01PM +0100, Paolo Bonzini wrote: > Right now, the system reset is concluded by a call to > cpu_synchronize_all_post_reset() in order to sync any changes > that the machine reset callback applied to the CPU state. > > However, for VMs with encrypted state such as SEV-ES guests (currently > the only case of guests with non-resettable CPUs) this cannot be done, > because guest state has already been finalized by machine-init-done notifiers. > cpu_synchronize_all_post_reset() does nothing on these guests, and actually > we would like to make it fail if called once guest has been encrypted. > So, assume that boards that support non-resettable CPUs do not touch > CPU state and that all such setup is done before, at the time of > cpu_synchronize_all_post_init(). > > Signed-off-by: Paolo Bonzini > --- > system/runstate.c | 15 ++- > roms/edk2 | 2 +- > 2 files changed, 15 insertions(+), 2 deletions(-) Accidental submodule change here : > diff --git a/roms/edk2 b/roms/edk2 > index edc6681206c..819cfc6b42a 16 > --- a/roms/edk2 > +++ b/roms/edk2 > @@ -1 +1 @@ > -Subproject commit edc6681206c1a8791981a2f911d2fb8b3d2f5768 > +Subproject commit 819cfc6b42a68790a23509e4fcc58ceb70e1965e > -- > 2.44.0 > > With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH-for-9.0 2/2] hw/misc/stm32l4x5_rcc: Propagate period when enabling a clock
On 16:39 Fri 22 Mar , Peter Maydell wrote:
> On Fri, 22 Mar 2024 at 15:59, Philippe Mathieu-Daudé
> wrote:
> >
> > From: Arnaud Minier
> >
> > The "clock_set_mul_div" function doesn't propagate the clock period
> > to the children if it is changed (e.g. by enabling/disabling a clock
> > multiplexer).
> > This was overlooked during the implementation due to late changes.
> >
> > This commit propagates the change if the multiplier or divider changes.
> >
> > Fixes: ec7d83acbd ("hw/misc/stm32l4x5_rcc: Add an internal clock
> > multiplexer object")
> > Signed-off-by: Arnaud Minier
> > Signed-off-by: Inès Varhol
> > Message-ID: <20240317103918.44375-2-arnaud.min...@telecom-paris.fr>
> > [PMD: Check clock_set_mul_div() return value]
> > Signed-off-by: Philippe Mathieu-Daudé
> > ---
> > hw/misc/stm32l4x5_rcc.c | 5 -
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/misc/stm32l4x5_rcc.c b/hw/misc/stm32l4x5_rcc.c
> > index bc2d63528b..7ad628b296 100644
> > --- a/hw/misc/stm32l4x5_rcc.c
> > +++ b/hw/misc/stm32l4x5_rcc.c
> > @@ -59,7 +59,10 @@ static void clock_mux_update(RccClockMuxState *mux, bool
> > bypass_source)
> > freq_multiplier = mux->divider;
> > }
> >
> > -clock_set_mul_div(mux->out, freq_multiplier, mux->multiplier);
> > +if (clock_set_mul_div(mux->out, freq_multiplier, mux->multiplier)) {
> > +clock_propagate(mux->out);
> > +}
> > +
> > clock_update(mux->out, clock_get(current_source));
>
> clock_update() also calls clock_propagate(), so this doesn't
> seem entirely right: shouldn't we figure out whether we need to
> do a clock_propagate() and do it once? (Maybe what seems odd to me
> is that clock_set() does clock_propagate() for you but
> clock_set_mul_div() does not...)
clock_set() does not call clock_propagate(). clock_update() is a
clock_set() followed by a clock_propagate() if the period changed.
I think this is where the problem comes from here. clock_update() call
won't call clock_propagate() if the clock period does not change.
I think you'll want something like:
bool changed;
changed = clock_set_mul_div(mux->out, freq_multiplier, mux->multiplexer);
changed ||= clock_set(clock_get(current_source));
if (changed) {
clock_propagate(mux->out);
}
Thanks,
--
Luc
>
> (Also I think we should have the information we need now to be able
> to do the "reduce log spam" in the comment -- if neither
> clock_set_mul_div() nor clock_update() needed to do anything
> then we didn't actually change the config.)
>
> -- PMM
--
Re: [PATCH v8] arm/kvm: Enable support for KVM_ARM_VCPU_PMU_V3_FILTER
On Mon, Mar 25, 2024 at 01:35:58PM +0800, Shaoqin Huang wrote: > Hi Daniel, > > Thanks for your reviewing. I see your comments in the v7. > > I have some doubts about what you said about the QAPI. Do you want me to > convert the current design into the QAPI parsing like the > IOThreadVirtQueueMapping? And we need to add new json definition in the > qapi/ directory? Yes, you would define a type in the qapi dir similar to how is done for IOThreadVirtQueueMapping, and then you can use that in the property setter method. With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PATCH v10 08/23] target/arm: Handle IS/FS in ISR_EL1 for NMI, VINMI and VFNMI
Add IS and FS bit in ISR_EL1 and handle the read. With CPU_INTERRUPT_NMI or
CPU_INTERRUPT_VINMI, both CPSR_I and ISR_IS must be set. With
CPU_INTERRUPT_VFNMI, both CPSR_F and ISR_FS must be set.
Signed-off-by: Jinjie Ruan
Reviewed-by: Richard Henderson
---
v9:
- CPU_INTERRUPT_VNMI -> CPU_INTERRUPT_VINMI.
- Handle CPSR_F and ISR_FS according to CPU_INTERRUPT_VFNMI instead of
CPU_INTERRUPT_VFIQ and HCRX_EL2.VFNMI.
- Update the commit message.
v7:
- env->cp15.hcrx_el2 -> arm_hcrx_el2_eff().
- Add Reviewed-by.
v6:
- Verify that HCR_EL2.VF is set before checking VFNMI.
v4；
- Also handle VNMI.
v3:
- CPU_INTERRUPT_NMI do not set FIQ, so remove it.
- With CPU_INTERRUPT_NMI, both CPSR_I and ISR_IS must be set.
---
target/arm/cpu.h| 2 ++
target/arm/helper.c | 13 +
2 files changed, 15 insertions(+)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 08a6bc50de..97997dbd08 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1398,6 +1398,8 @@ void pmu_init(ARMCPU *cpu);
#define CPSR_N (1U << 31)
#define CPSR_NZCV (CPSR_N | CPSR_Z | CPSR_C | CPSR_V)
#define CPSR_AIF (CPSR_A | CPSR_I | CPSR_F)
+#define ISR_FS (1U << 9)
+#define ISR_IS (1U << 10)
#define CPSR_IT (CPSR_IT_0_1 | CPSR_IT_2_7)
#define CACHED_CPSR_BITS (CPSR_T | CPSR_AIF | CPSR_GE | CPSR_IT | CPSR_Q \
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 077c9a6923..b57114d35d 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2021,16 +2021,29 @@ static uint64_t isr_read(CPUARMState *env, const
ARMCPRegInfo *ri)
if (cs->interrupt_request & CPU_INTERRUPT_VIRQ) {
ret |= CPSR_I;
}
+if (cs->interrupt_request & CPU_INTERRUPT_VINMI) {
+ret |= ISR_IS;
+ret |= CPSR_I;
+}
} else {
if (cs->interrupt_request & CPU_INTERRUPT_HARD) {
ret |= CPSR_I;
}
+
+if (cs->interrupt_request & CPU_INTERRUPT_NMI) {
+ret |= ISR_IS;
+ret |= CPSR_I;
+}
}
if (hcr_el2 & HCR_FMO) {
if (cs->interrupt_request & CPU_INTERRUPT_VFIQ) {
ret |= CPSR_F;
}
+if (cs->interrupt_request & CPU_INTERRUPT_VFNMI) {
+ret |= ISR_FS;
+ret |= CPSR_F;
+}
} else {
if (cs->interrupt_request & CPU_INTERRUPT_FIQ) {
ret |= CPSR_F;
--
2.34.1
[PATCH v10 14/23] hw/intc/arm_gicv3: Add irq non-maskable property
A SPI, PPI or SGI interrupt can have non-maskable property. So maintain
non-maskable property in PendingIrq and GICR/GICD. Since add new device
state, it also needs to be migrated, so also save NMI info in
vmstate_gicv3_cpu and vmstate_gicv3.
Signed-off-by: Jinjie Ruan
Acked-by: Richard Henderson
---
v10:
- superprio -> nmi, gicr_isuperprio -> gicr_inmir0.
- Save NMI state in vmstate_gicv3_cpu and vmstate_gicv3.
- Update the commit message.
v3:
- Place this ahead of implement GICR_INMIR.
- Add Acked-by.
---
hw/intc/arm_gicv3_common.c | 44 ++
include/hw/intc/arm_gicv3_common.h | 4 +++
2 files changed, 48 insertions(+)
diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index 2d2cea6858..be76ae0be6 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -164,6 +164,24 @@ const VMStateDescription vmstate_gicv3_gicv4 = {
}
};
+static bool nmi_needed(void *opaque)
+{
+GICv3CPUState *cs = opaque;
+
+return cs->gic->nmi_support != 0;
+}
+
+static const VMStateDescription vmstate_gicv3_cpu_nmi = {
+.name = "arm_gicv3_cpu/nmi",
+.version_id = 1,
+.minimum_version_id = 1,
+.needed = nmi_needed,
+.fields = (const VMStateField[]) {
+VMSTATE_UINT32(gicr_inmir0, GICv3CPUState),
+VMSTATE_END_OF_LIST()
+}
+};
+
static const VMStateDescription vmstate_gicv3_cpu = {
.name = "arm_gicv3_cpu",
.version_id = 1,
@@ -197,6 +215,10 @@ static const VMStateDescription vmstate_gicv3_cpu = {
_gicv3_cpu_sre_el1,
_gicv3_gicv4,
NULL
+},
+.subsections = (const VMStateDescription * const []) {
+_gicv3_cpu_nmi,
+NULL
}
};
@@ -238,6 +260,24 @@ const VMStateDescription
vmstate_gicv3_gicd_no_migration_shift_bug = {
}
};
+static bool needed_nmi(void *opaque)
+{
+GICv3State *cs = opaque;
+
+return cs->nmi_support != 0;
+}
+
+const VMStateDescription vmstate_gicv3_gicd_nmi = {
+.name = "arm_gicv3/gicd_nmi",
+.version_id = 1,
+.minimum_version_id = 1,
+.needed = needed_nmi,
+.fields = (const VMStateField[]) {
+VMSTATE_UINT32_ARRAY(nmi, GICv3State, GICV3_BMP_SIZE),
+VMSTATE_END_OF_LIST()
+}
+};
+
static const VMStateDescription vmstate_gicv3 = {
.name = "arm_gicv3",
.version_id = 1,
@@ -267,6 +307,10 @@ static const VMStateDescription vmstate_gicv3 = {
.subsections = (const VMStateDescription * const []) {
_gicv3_gicd_no_migration_shift_bug,
NULL
+},
+.subsections = (const VMStateDescription * const []) {
+_gicv3_gicd_nmi,
+NULL
}
};
diff --git a/include/hw/intc/arm_gicv3_common.h
b/include/hw/intc/arm_gicv3_common.h
index 4358c5319c..88533749eb 100644
--- a/include/hw/intc/arm_gicv3_common.h
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -146,6 +146,7 @@ typedef struct {
int irq;
uint8_t prio;
int grp;
+bool nmi;
} PendingIrq;
struct GICv3CPUState {
@@ -172,6 +173,7 @@ struct GICv3CPUState {
uint32_t gicr_ienabler0;
uint32_t gicr_ipendr0;
uint32_t gicr_iactiver0;
+uint32_t gicr_inmir0;
uint32_t edge_trigger; /* ICFGR0 and ICFGR1 even bits */
uint32_t gicr_igrpmodr0;
uint32_t gicr_nsacr;
@@ -275,6 +277,7 @@ struct GICv3State {
GIC_DECLARE_BITMAP(active); /* GICD_ISACTIVER */
GIC_DECLARE_BITMAP(level);/* Current level */
GIC_DECLARE_BITMAP(edge_trigger); /* GICD_ICFGR even bits */
+GIC_DECLARE_BITMAP(nmi); /* GICD_INMIR */
uint8_t gicd_ipriority[GICV3_MAXIRQ];
uint64_t gicd_irouter[GICV3_MAXIRQ];
/* Cached information: pointer to the cpu i/f for the CPUs specified
@@ -314,6 +317,7 @@ GICV3_BITMAP_ACCESSORS(pending)
GICV3_BITMAP_ACCESSORS(active)
GICV3_BITMAP_ACCESSORS(level)
GICV3_BITMAP_ACCESSORS(edge_trigger)
+GICV3_BITMAP_ACCESSORS(nmi)
#define TYPE_ARM_GICV3_COMMON "arm-gicv3-common"
typedef struct ARMGICv3CommonClass ARMGICv3CommonClass;
--
2.34.1
[PATCH v10 01/23] target/arm: Handle HCR_EL2 accesses for bits introduced with FEAT_NMI
FEAT_NMI defines another three new bits in HCRX_EL2: TALLINT, HCRX_VINMI and
HCRX_VFNMI. When the feature is enabled, allow these bits to be written in
HCRX_EL2.
Signed-off-by: Jinjie Ruan
Reviewed-by: Richard Henderson
---
v9:
- Declare cpu variable to reuse latter.
v4:
- Update the comment for FEAT_NMI in hcrx_write().
- Update the commit message, s/thress/three/g.
v3:
- Add Reviewed-by.
- Add HCRX_VINMI and HCRX_VFNMI support in HCRX_EL2.
- Upate the commit messsage.
---
target/arm/cpu-features.h | 5 +
target/arm/helper.c | 9 -
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/target/arm/cpu-features.h b/target/arm/cpu-features.h
index e5758d9fbc..b300d0446d 100644
--- a/target/arm/cpu-features.h
+++ b/target/arm/cpu-features.h
@@ -681,6 +681,11 @@ static inline bool isar_feature_aa64_sme(const
ARMISARegisters *id)
return FIELD_EX64(id->id_aa64pfr1, ID_AA64PFR1, SME) != 0;
}
+static inline bool isar_feature_aa64_nmi(const ARMISARegisters *id)
+{
+return FIELD_EX64(id->id_aa64pfr1, ID_AA64PFR1, NMI) != 0;
+}
+
static inline bool isar_feature_aa64_tgran4_lpa2(const ARMISARegisters *id)
{
return FIELD_SEX64(id->id_aa64mmfr0, ID_AA64MMFR0, TGRAN4) >= 1;
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 3f3a5b55d4..7d6c6e9878 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6183,13 +6183,20 @@ bool el_is_in_host(CPUARMState *env, int el)
static void hcrx_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
{
+ARMCPU *cpu = env_archcpu(env);
+
uint64_t valid_mask = 0;
/* FEAT_MOPS adds MSCEn and MCE2 */
-if (cpu_isar_feature(aa64_mops, env_archcpu(env))) {
+if (cpu_isar_feature(aa64_mops, cpu)) {
valid_mask |= HCRX_MSCEN | HCRX_MCE2;
}
+/* FEAT_NMI adds TALLINT, VINMI and VFNMI */
+if (cpu_isar_feature(aa64_nmi, cpu)) {
+valid_mask |= HCRX_TALLINT | HCRX_VINMI | HCRX_VFNMI;
+}
+
/* Clear RES0 bits. */
env->cp15.hcrx_el2 = value & valid_mask;
}
--
2.34.1
[PATCH v10 17/23] hw/intc/arm_gicv3: Add NMI handling CPU interface registers
Add the NMIAR CPU interface registers which deal with acknowledging NMI.
When introduce NMI interrupt, there are some updates to the semantics for the
register ICC_IAR1_EL1 and ICC_HPPIR1_EL1. For ICC_IAR1_EL1 register, it
should return 1022 if the intid has non-maskable property. And for
ICC_NMIAR1_EL1 register, it should return 1023 if the intid do not have
non-maskable property. Howerever, these are not necessary for ICC_HPPIR1_EL1
register.
And the APR and RPR has NMI bits which should be handled correctly.
Signed-off-by: Jinjie Ruan
Reviewed-by: Richard Henderson
---
v10:
- is_nmi -> nmi.
- is_hppi -> hppi.
- Exchange the order of nmi and hppi parameters.
- superprio -> nmi.
- Handle APR and RPR NMI bits.
- Update the commit message, super priority -> non-maskable property.
v7:
- Add Reviewed-by.
v4:
- Define ICC_NMIAR1_EL1 only if FEAT_GICv3_NMI is implemented.
- Check sctrl_elx.SCTLR_NMI to return 1022 for icc_iar1_read().
- Add gicv3_icc_nmiar1_read() trace event.
- Do not check icc_hppi_can_preempt() for icc_nmiar1_read().
- Add icv_nmiar1_read() and call it when EL2Enabled() and HCR_EL2.IMO == '1'
---
hw/intc/arm_gicv3_cpuif.c | 115 ++
hw/intc/gicv3_internal.h | 5 ++
hw/intc/trace-events | 1 +
3 files changed, 110 insertions(+), 11 deletions(-)
diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index e1a60d8c15..76e2286e70 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -795,6 +795,13 @@ static uint64_t icv_iar_read(CPUARMState *env, const
ARMCPRegInfo *ri)
return intid;
}
+static uint64_t icv_nmiar1_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+/* todo */
+uint64_t intid = INTID_SPURIOUS;
+return intid;
+}
+
static uint32_t icc_fullprio_mask(GICv3CPUState *cs)
{
/*
@@ -825,11 +832,15 @@ static inline int icc_num_aprs(GICv3CPUState *cs)
return aprmax;
}
-static int icc_highest_active_prio(GICv3CPUState *cs)
+static uint64_t icc_highest_active_prio(GICv3CPUState *cs)
{
/* Calculate the current running priority based on the set bits
* in the Active Priority Registers.
*/
+ARMCPU *cpu = ARM_CPU(cs->cpu);
+CPUARMState *env = >env;
+
+uint64_t prio;
int i;
for (i = 0; i < icc_num_aprs(cs); i++) {
@@ -839,7 +850,32 @@ static int icc_highest_active_prio(GICv3CPUState *cs)
if (!apr) {
continue;
}
-return (i * 32 + ctz32(apr)) << (icc_min_bpr(cs) + 1);
+prio = (i * 32 + ctz32(apr)) << (icc_min_bpr(cs) + 1);
+
+if (cs->gic->nmi_support) {
+if (cs->gic->gicd_ctlr & GICD_CTLR_DS) {
+if ((cs->icc_apr[GICV3_G0][i] & ICC_AP1R_EL1_NMI) ||
+(cs->icc_apr[GICV3_G1][i] & ICC_AP1R_EL1_NMI) ||
+(cs->icc_apr[GICV3_G1NS][i] & ICC_AP1R_EL1_NMI)) {
+prio |= ICC_RPR_EL1_NMI;
+}
+} else if (!arm_is_secure(env)) {
+if (cs->icc_apr[GICV3_G1NS][i] & ICC_AP1R_EL1_NMI) {
+prio |= ICC_RPR_EL1_NMI;
+}
+} else {
+if (cs->icc_apr[GICV3_G1][i] & ICC_AP1R_EL1_NMI) {
+prio |= ICC_RPR_EL1_NMI;
+}
+}
+
+if (arm_feature(env, ARM_FEATURE_EL3) &&
+cs->icc_apr[GICV3_G1NS][i] & ICC_AP1R_EL1_NMI) {
+prio |= ICC_RPR_EL1_NSNMI;
+}
+}
+
+return prio;
}
/* No current active interrupts: return idle priority */
return 0xff;
@@ -896,7 +932,7 @@ static bool icc_hppi_can_preempt(GICv3CPUState *cs)
/* Return true if we have a pending interrupt of sufficient
* priority to preempt.
*/
-int rprio;
+uint64_t rprio;
uint32_t mask;
if (icc_no_enabled_hppi(cs)) {
@@ -1034,7 +1070,7 @@ static void icc_pmr_write(CPUARMState *env, const
ARMCPRegInfo *ri,
gicv3_cpuif_update(cs);
}
-static void icc_activate_irq(GICv3CPUState *cs, int irq)
+static void icc_activate_irq(GICv3CPUState *cs, int irq, bool nmi)
{
/* Move the interrupt from the Pending state to Active, and update
* the Active Priority Registers
@@ -1047,6 +1083,10 @@ static void icc_activate_irq(GICv3CPUState *cs, int irq)
cs->icc_apr[cs->hppi.grp][regno] |= (1 << regbit);
+if (cs->gic->nmi_support) {
+cs->icc_apr[cs->hppi.grp][regno] |= (nmi ? ICC_AP1R_EL1_NMI : 0);
+}
+
if (irq < GIC_INTERNAL) {
cs->gicr_iactiver0 = deposit32(cs->gicr_iactiver0, irq, 1, 1);
cs->gicr_ipendr0 = deposit32(cs->gicr_ipendr0, irq, 1, 0);
@@ -1097,7 +1137,8 @@ static uint64_t icc_hppir0_value(GICv3CPUState *cs,
CPUARMState *env)
return cs->hppi.irq;
}
-static uint64_t icc_hppir1_value(GICv3CPUState *cs, CPUARMState *env)
+static uint64_t icc_hppir1_value(GICv3CPUState *cs, CPUARMState *env, bool
hppi,
+ bool nmi)
{
/*
[PATCH v10 09/23] target/arm: Handle PSTATE.ALLINT on taking an exception
Set or clear PSTATE.ALLINT on taking an exception to ELx according to the
SCTLR_ELx.SPINTMASK bit.
Signed-off-by: Jinjie Ruan
Reviewed-by: Richard Henderson
---
v9:
- Not check SCTLR_NMI in arm_cpu_do_interrupt_aarch64().
v3:
- Add Reviewed-by.
---
target/arm/helper.c | 8
1 file changed, 8 insertions(+)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index b57114d35d..967e833ee8 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -11730,6 +11730,14 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
}
}
+if (cpu_isar_feature(aa64_nmi, cpu)) {
+if (!(env->cp15.sctlr_el[new_el] & SCTLR_SPINTMASK)) {
+new_mode |= PSTATE_ALLINT;
+} else {
+new_mode &= ~PSTATE_ALLINT;
+}
+}
+
pstate_write(env, PSTATE_DAIF | new_mode);
env->aarch64 = true;
aarch64_restore_sp(env, new_el);
--
2.34.1
[PATCH v10 16/23] hw/intc/arm_gicv3: Implement GICD_INMIR
Add GICD_INMIR, GICD_INMIRnE register and support access GICD_INMIR0.
Signed-off-by: Jinjie Ruan
Reviewed-by: Richard Henderson
---
v10:
- superprio -> nmi.
v4:
- Make the GICD_INMIR implementation more clearer.
- Udpate the commit message.
v3:
- Add Reviewed-by.
---
hw/intc/arm_gicv3_dist.c | 34 ++
hw/intc/gicv3_internal.h | 2 ++
2 files changed, 36 insertions(+)
diff --git a/hw/intc/arm_gicv3_dist.c b/hw/intc/arm_gicv3_dist.c
index 22ddc0d666..d8207acb22 100644
--- a/hw/intc/arm_gicv3_dist.c
+++ b/hw/intc/arm_gicv3_dist.c
@@ -89,6 +89,29 @@ static int gicd_ns_access(GICv3State *s, int irq)
return extract32(s->gicd_nsacr[irq / 16], (irq % 16) * 2, 2);
}
+static void gicd_write_bitmap_reg(GICv3State *s, MemTxAttrs attrs,
+ uint32_t *bmp, maskfn *maskfn,
+ int offset, uint32_t val)
+{
+/*
+ * Helper routine to implement writing to a "set" register
+ * (GICD_INMIR, etc).
+ * Semantics implemented here:
+ * RAZ/WI for SGIs, PPIs, unimplemented IRQs
+ * Bits corresponding to Group 0 or Secure Group 1 interrupts RAZ/WI.
+ * offset should be the offset in bytes of the register from the start
+ * of its group.
+ */
+int irq = offset * 8;
+
+if (irq < GIC_INTERNAL || irq >= s->num_irq) {
+return;
+}
+val &= mask_group_and_nsacr(s, attrs, maskfn, irq);
+*gic_bmp_ptr32(bmp, irq) = val;
+gicv3_update(s, irq, 32);
+}
+
static void gicd_write_set_bitmap_reg(GICv3State *s, MemTxAttrs attrs,
uint32_t *bmp,
maskfn *maskfn,
@@ -545,6 +568,11 @@ static bool gicd_readl(GICv3State *s, hwaddr offset,
/* RAZ/WI since affinity routing is always enabled */
*data = 0;
return true;
+case GICD_INMIR ... GICD_INMIR + 0x7f:
+*data = (!s->nmi_support) ? 0 :
+gicd_read_bitmap_reg(s, attrs, s->nmi, NULL,
+ offset - GICD_INMIR);
+return true;
case GICD_IROUTER ... GICD_IROUTER + 0x1fdf:
{
uint64_t r;
@@ -754,6 +782,12 @@ static bool gicd_writel(GICv3State *s, hwaddr offset,
case GICD_SPENDSGIR ... GICD_SPENDSGIR + 0xf:
/* RAZ/WI since affinity routing is always enabled */
return true;
+case GICD_INMIR ... GICD_INMIR + 0x7f:
+if (s->nmi_support) {
+gicd_write_bitmap_reg(s, attrs, s->nmi, NULL,
+ offset - GICD_INMIR, value);
+}
+return true;
case GICD_IROUTER ... GICD_IROUTER + 0x1fdf:
{
uint64_t r;
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index 21697ecf39..8d793243f4 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -52,6 +52,8 @@
#define GICD_SGIR0x0F00
#define GICD_CPENDSGIR 0x0F10
#define GICD_SPENDSGIR 0x0F20
+#define GICD_INMIR 0x0F80
+#define GICD_INMIRnE 0x3B00
#define GICD_IROUTER 0x6000
#define GICD_IDREGS 0xFFD0
--
2.34.1
[PATCH v10 19/23] hw/intc/arm_gicv3: Implement NMI interrupt prioirty
If GICD_CTLR_DS bit is zero and the NMI is non-secure, the NMI prioirty
is higher than 0x80, otherwise it is higher than 0x0. And save NMI
super prioirty information in hppi.superprio to deliver NMI exception.
Since both GICR and GICD can deliver NMI, it is both necessary to check
whether the pending irq is NMI in gicv3_redist_update_noirqset and
gicv3_update_noirqset. And In irqbetter(), only a non-NMI with the same
priority and a smaller interrupt number can be preempted but not NMI.
Signed-off-by: Jinjie Ruan
Reviewed-by: Richard Henderson
---
v10:
- has_superprio -> nmi.
- superpriority -> non-maskable property.
- gicr_isuperprio -> gicr_inmir0.
- superprio -> nmi.
v8:
- Add Reviewed-by.
v7:
- Reorder the irqbetter() code for clarity.
- Eliminate the has_superprio local variable for gicv3_get_priority().
- false -> cs->hpplpi.superprio in gicv3_redist_update_noirqset().
- 0x0 -> false in arm_gicv3_common_reset_hold().
- Clear superprio in several places for hppi, hpplpi and hppvlpi.
v6:
- Put the "extract superprio info" logic into gicv3_get_priority().
- Update the comment in irqbetter().
- Reset the cs->hppi.superprio to 0x0.
- Set hppi.superprio to false for LPI.
v4:
- Replace is_nmi with has_superprio to not a mix NMI and superpriority.
- Update the comment in irqbetter().
- Extract gicv3_get_priority() to avoid code repeat.
---
v3:
- Add missing brace
---
hw/intc/arm_gicv3.c| 67 +-
hw/intc/arm_gicv3_common.c | 3 ++
hw/intc/arm_gicv3_redist.c | 3 ++
3 files changed, 64 insertions(+), 9 deletions(-)
diff --git a/hw/intc/arm_gicv3.c b/hw/intc/arm_gicv3.c
index 0b8f79a122..6704190d9d 100644
--- a/hw/intc/arm_gicv3.c
+++ b/hw/intc/arm_gicv3.c
@@ -21,7 +21,7 @@
#include "hw/intc/arm_gicv3.h"
#include "gicv3_internal.h"
-static bool irqbetter(GICv3CPUState *cs, int irq, uint8_t prio)
+static bool irqbetter(GICv3CPUState *cs, int irq, uint8_t prio, bool nmi)
{
/* Return true if this IRQ at this priority should take
* precedence over the current recorded highest priority
@@ -30,14 +30,23 @@ static bool irqbetter(GICv3CPUState *cs, int irq, uint8_t
prio)
* is the same as this one (a property which the calling code
* relies on).
*/
-if (prio < cs->hppi.prio) {
-return true;
+if (prio != cs->hppi.prio) {
+return prio < cs->hppi.prio;
+}
+
+/*
+ * The same priority IRQ with non-maskable property should signal to
+ * the CPU as it have the priority higher than the labelled 0x80 or 0x00.
+ */
+if (nmi != cs->hppi.nmi) {
+return nmi;
}
+
/* If multiple pending interrupts have the same priority then it is an
* IMPDEF choice which of them to signal to the CPU. We choose to
* signal the one with the lowest interrupt number.
*/
-if (prio == cs->hppi.prio && irq <= cs->hppi.irq) {
+if (irq <= cs->hppi.irq) {
return true;
}
return false;
@@ -129,6 +138,40 @@ static uint32_t gicr_int_pending(GICv3CPUState *cs)
return pend;
}
+static bool gicv3_get_priority(GICv3CPUState *cs, bool is_redist,
+ uint8_t *prio, int irq)
+{
+uint32_t nmi = 0x0;
+
+if (is_redist) {
+nmi = extract32(cs->gicr_inmir0, irq, 1);
+} else {
+nmi = *gic_bmp_ptr32(cs->gic->nmi, irq);
+nmi = nmi & (1 << (irq & 0x1f));
+}
+
+if (nmi) {
+/* DS = 0 & Non-secure NMI */
+if (!(cs->gic->gicd_ctlr & GICD_CTLR_DS) &&
+((is_redist && extract32(cs->gicr_igroupr0, irq, 1)) ||
+ (!is_redist && gicv3_gicd_group_test(cs->gic, irq {
+*prio = 0x80;
+} else {
+*prio = 0x0;
+}
+
+return true;
+}
+
+if (is_redist) {
+*prio = cs->gicr_ipriorityr[irq];
+} else {
+*prio = cs->gic->gicd_ipriority[irq];
+}
+
+return false;
+}
+
/* Update the interrupt status after state in a redistributor
* or CPU interface has changed, but don't tell the CPU i/f.
*/
@@ -141,6 +184,7 @@ static void gicv3_redist_update_noirqset(GICv3CPUState *cs)
uint8_t prio;
int i;
uint32_t pend;
+bool nmi = false;
/* Find out which redistributor interrupts are eligible to be
* signaled to the CPU interface.
@@ -152,10 +196,11 @@ static void gicv3_redist_update_noirqset(GICv3CPUState
*cs)
if (!(pend & (1 << i))) {
continue;
}
-prio = cs->gicr_ipriorityr[i];
-if (irqbetter(cs, i, prio)) {
+nmi = gicv3_get_priority(cs, true, , i);
+if (irqbetter(cs, i, prio, nmi)) {
cs->hppi.irq = i;
cs->hppi.prio = prio;
+cs->hppi.nmi = nmi;
seenbetter = true;
}
}
@@ -168,9 +213,10 @@ static void gicv3_redist_update_noirqset(GICv3CPUState *cs)
if ((cs->gicr_ctlr & GICR_CTLR_ENABLE_LPIS) &&
[PATCH v10 21/23] hw/intc/arm_gicv3: Report the VINMI interrupt
In vCPU Interface, if the vIRQ has the non-maskable property, report
vINMI to the corresponding vPE.
Signed-off-by: Jinjie Ruan
Reviewed-by: Richard Henderson
---
v10:
- Update the commit message, superpriority -> non-maskable.
v9:
- Update the commit subject and message, vNMI -> vINMI.
v6:
- Add Reviewed-by.
---
hw/intc/arm_gicv3_cpuif.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index 4cd84b142e..cc5ff124d6 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -476,6 +476,7 @@ void gicv3_cpuif_virt_irq_fiq_update(GICv3CPUState *cs)
int idx;
int irqlevel = 0;
int fiqlevel = 0;
+int nmilevel = 0;
idx = hppvi_index(cs);
trace_gicv3_cpuif_virt_update(gicv3_redist_affid(cs), idx,
@@ -493,9 +494,17 @@ void gicv3_cpuif_virt_irq_fiq_update(GICv3CPUState *cs)
uint64_t lr = cs->ich_lr_el2[idx];
if (icv_hppi_can_preempt(cs, lr)) {
-/* Virtual interrupts are simple: G0 are always FIQ, and G1 IRQ */
+/*
+ * Virtual interrupts are simple: G0 are always FIQ, and G1 are
+ * IRQ or NMI which depends on the ICH_LR_EL2.NMI to have
+ * non-maskable property.
+ */
if (lr & ICH_LR_EL2_GROUP) {
-irqlevel = 1;
+if (cs->gic->nmi_support && (lr & ICH_LR_EL2_NMI)) {
+nmilevel = 1;
+} else {
+irqlevel = 1;
+}
} else {
fiqlevel = 1;
}
@@ -505,6 +514,7 @@ void gicv3_cpuif_virt_irq_fiq_update(GICv3CPUState *cs)
trace_gicv3_cpuif_virt_set_irqs(gicv3_redist_affid(cs), fiqlevel,
irqlevel);
qemu_set_irq(cs->parent_vfiq, fiqlevel);
qemu_set_irq(cs->parent_virq, irqlevel);
+qemu_set_irq(cs->parent_vnmi, nmilevel);
}
static void gicv3_cpuif_virt_update(GICv3CPUState *cs)
--
2.34.1
[PATCH v10 04/23] target/arm: Implement ALLINT MSR (immediate)
Add ALLINT MSR (immediate) to decodetree, in which the CRm is 0b000x. The
EL0 check is necessary to ALLINT, and the EL1 check is necessary when
imm == 1. So implement it inline for EL2/3, or EL1 with imm==0. Avoid the
unconditional write to pc and use raise_exception_ra to unwind.
Signed-off-by: Jinjie Ruan
Reviewed-by: Richard Henderson
---
v10:
- Correct the exception_target_el(env) to 2, since it is a hypervisor trap
from EL1 to EL2.
v7:
- Add Reviewed-by.
v6:
- Fix DISAS_TOO_MANY to DISAS_UPDATE_EXIT and add the comment.
v5:
- Drop the & 1 in trans_MSR_i_ALLINT().
- Simplify and merge msr_i_allint() and allint_check().
- Rename msr_i_allint() to msr_set_allint_el1().
v4:
- Fix the ALLINT MSR (immediate) decodetree implementation.
- Remove arm_is_el2_enabled() check in allint_check().
- Update env->allint to env->pstate.
- Only call allint_check() when imm == 1.
- Simplify the allint_check() to not pass "op" and extract.
- Implement it inline for EL2/3, or EL1 with imm==0.
- Pass (a->imm & 1) * PSTATE_ALLINT (i64) to simplfy the ALLINT set/clear.
v3:
- Remove EL0 check in allint_check().
- Add TALLINT check for EL1 in allint_check().
- Remove unnecessarily arm_rebuild_hflags() in msr_i_allint helper.
---
target/arm/tcg/a64.decode | 1 +
target/arm/tcg/helper-a64.c| 12
target/arm/tcg/helper-a64.h| 1 +
target/arm/tcg/translate-a64.c | 19 +++
4 files changed, 33 insertions(+)
diff --git a/target/arm/tcg/a64.decode b/target/arm/tcg/a64.decode
index 8a20dce3c8..0e7656fd15 100644
--- a/target/arm/tcg/a64.decode
+++ b/target/arm/tcg/a64.decode
@@ -207,6 +207,7 @@ MSR_i_DIT 1101 0101 0 011 0100 010 1
@msr_i
MSR_i_TCO 1101 0101 0 011 0100 100 1 @msr_i
MSR_i_DAIFSET 1101 0101 0 011 0100 110 1 @msr_i
MSR_i_DAIFCLEAR 1101 0101 0 011 0100 111 1 @msr_i
+MSR_i_ALLINT1101 0101 0 001 0100 000 imm:1 000 1
MSR_i_SVCR 1101 0101 0 011 0100 0 mask:2 imm:1 011 1
# MRS, MSR (register), SYS, SYSL. These are all essentially the
diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c
index ebaa7f00df..673e949422 100644
--- a/target/arm/tcg/helper-a64.c
+++ b/target/arm/tcg/helper-a64.c
@@ -66,6 +66,18 @@ void HELPER(msr_i_spsel)(CPUARMState *env, uint32_t imm)
update_spsel(env, imm);
}
+void HELPER(msr_set_allint_el1)(CPUARMState *env)
+{
+/* ALLINT update to PSTATE. */
+if (arm_hcrx_el2_eff(env) & HCRX_TALLINT) {
+raise_exception_ra(env, EXCP_UDEF,
+ syn_aa64_sysregtrap(0, 1, 0, 4, 1, 0x1f, 0), 2,
+ GETPC());
+}
+
+env->pstate |= PSTATE_ALLINT;
+}
+
static void daif_check(CPUARMState *env, uint32_t op,
uint32_t imm, uintptr_t ra)
{
diff --git a/target/arm/tcg/helper-a64.h b/target/arm/tcg/helper-a64.h
index 575a5dab7d..0518165399 100644
--- a/target/arm/tcg/helper-a64.h
+++ b/target/arm/tcg/helper-a64.h
@@ -22,6 +22,7 @@ DEF_HELPER_FLAGS_1(rbit64, TCG_CALL_NO_RWG_SE, i64, i64)
DEF_HELPER_2(msr_i_spsel, void, env, i32)
DEF_HELPER_2(msr_i_daifset, void, env, i32)
DEF_HELPER_2(msr_i_daifclear, void, env, i32)
+DEF_HELPER_1(msr_set_allint_el1, void, env)
DEF_HELPER_3(vfp_cmph_a64, i64, f16, f16, ptr)
DEF_HELPER_3(vfp_cmpeh_a64, i64, f16, f16, ptr)
DEF_HELPER_3(vfp_cmps_a64, i64, f32, f32, ptr)
diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
index 340265beb0..21758b290d 100644
--- a/target/arm/tcg/translate-a64.c
+++ b/target/arm/tcg/translate-a64.c
@@ -2036,6 +2036,25 @@ static bool trans_MSR_i_DAIFCLEAR(DisasContext *s, arg_i
*a)
return true;
}
+static bool trans_MSR_i_ALLINT(DisasContext *s, arg_i *a)
+{
+if (!dc_isar_feature(aa64_nmi, s) || s->current_el == 0) {
+return false;
+}
+
+if (a->imm == 0) {
+clear_pstate_bits(PSTATE_ALLINT);
+} else if (s->current_el > 1) {
+set_pstate_bits(PSTATE_ALLINT);
+} else {
+gen_helper_msr_set_allint_el1(tcg_env);
+}
+
+/* Exit the cpu loop to re-evaluate pending IRQs. */
+s->base.is_jmp = DISAS_UPDATE_EXIT;
+return true;
+}
+
static bool trans_MSR_i_SVCR(DisasContext *s, arg_MSR_i_SVCR *a)
{
if (!dc_isar_feature(aa64_sme, s) || a->mask == 0) {
--
2.34.1
[PATCH v10 23/23] hw/arm/virt: Add FEAT_GICv3_NMI feature support in virt GIC
A PE that implements FEAT_NMI and FEAT_GICv3 also implements
FEAT_GICv3_NMI. A PE that does not implement FEAT_NMI, does not implement
FEAT_GICv3_NMI
So included support FEAT_GICv3_NMI feature as part of virt platform
GIC initialization if FEAT_NMI and FEAT_GICv3 supported.
Signed-off-by: Jinjie Ruan
Reviewed-by: Richard Henderson
---
v4:
- Add Reviewed-by.
v3:
- Adjust to be the last after add FEAT_NMI to max.
- Check whether support FEAT_NMI and FEAT_GICv3 for FEAT_GICv3_NMI.
---
hw/arm/virt.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index ef2e6c2c4d..63d9f5b553 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -729,6 +729,19 @@ static void create_v2m(VirtMachineState *vms)
vms->msi_controller = VIRT_MSI_CTRL_GICV2M;
}
+/*
+ * A PE that implements FEAT_NMI and FEAT_GICv3 also implements
+ * FEAT_GICv3_NMI. A PE that does not implement FEAT_NMI, does not implement
+ * FEAT_GICv3_NMI.
+ */
+static bool gicv3_nmi_present(VirtMachineState *vms)
+{
+ARMCPU *cpu = ARM_CPU(qemu_get_cpu(0));
+
+return cpu_isar_feature(aa64_nmi, cpu) &&
+ (vms->gic_version != VIRT_GIC_VERSION_2);
+}
+
static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
{
MachineState *ms = MACHINE(vms);
@@ -802,6 +815,11 @@ static void create_gic(VirtMachineState *vms, MemoryRegion
*mem)
vms->virt);
}
}
+
+if (gicv3_nmi_present(vms)) {
+qdev_prop_set_bit(vms->gic, "has-nmi", true);
+}
+
gicbusdev = SYS_BUS_DEVICE(vms->gic);
sysbus_realize_and_unref(gicbusdev, _fatal);
sysbus_mmio_map(gicbusdev, 0, vms->memmap[VIRT_GIC_DIST].base);
--
2.34.1
[PATCH v10 11/23] hw/intc/arm_gicv3: Add external IRQ lines for NMI
Augment the GICv3's QOM device interface by adding one
new set of sysbus IRQ line, to signal NMI to each CPU.
Signed-off-by: Jinjie Ruan
Reviewed-by: Richard Henderson
---
v4:
- Add Reviewed-by.
v3:
- Add support for VNMI.
---
hw/intc/arm_gicv3_common.c | 6 ++
include/hw/intc/arm_gic_common.h | 2 ++
include/hw/intc/arm_gicv3_common.h | 2 ++
3 files changed, 10 insertions(+)
diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index cb55c72681..c52f060026 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -299,6 +299,12 @@ void gicv3_init_irqs_and_mmio(GICv3State *s,
qemu_irq_handler handler,
for (i = 0; i < s->num_cpu; i++) {
sysbus_init_irq(sbd, >cpu[i].parent_vfiq);
}
+for (i = 0; i < s->num_cpu; i++) {
+sysbus_init_irq(sbd, >cpu[i].parent_nmi);
+}
+for (i = 0; i < s->num_cpu; i++) {
+sysbus_init_irq(sbd, >cpu[i].parent_vnmi);
+}
memory_region_init_io(>iomem_dist, OBJECT(s), ops, s,
"gicv3_dist", 0x1);
diff --git a/include/hw/intc/arm_gic_common.h b/include/hw/intc/arm_gic_common.h
index 7080375008..97fea4102d 100644
--- a/include/hw/intc/arm_gic_common.h
+++ b/include/hw/intc/arm_gic_common.h
@@ -71,6 +71,8 @@ struct GICState {
qemu_irq parent_fiq[GIC_NCPU];
qemu_irq parent_virq[GIC_NCPU];
qemu_irq parent_vfiq[GIC_NCPU];
+qemu_irq parent_nmi[GIC_NCPU];
+qemu_irq parent_vnmi[GIC_NCPU];
qemu_irq maintenance_irq[GIC_NCPU];
/* GICD_CTLR; for a GIC with the security extensions the NS banked version
diff --git a/include/hw/intc/arm_gicv3_common.h
b/include/hw/intc/arm_gicv3_common.h
index 4e2fb518e7..7324c7d983 100644
--- a/include/hw/intc/arm_gicv3_common.h
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -155,6 +155,8 @@ struct GICv3CPUState {
qemu_irq parent_fiq;
qemu_irq parent_virq;
qemu_irq parent_vfiq;
+qemu_irq parent_nmi;
+qemu_irq parent_vnmi;
/* Redistributor */
uint32_t level; /* Current IRQ level */
--
2.34.1
[PATCH v10 10/23] hw/arm/virt: Wire NMI and VINMI irq lines from GIC to CPU
Wire the new NMI and VINMI interrupt line from the GIC to each CPU.
Signed-off-by: Jinjie Ruan
Reviewed-by: Richard Henderson
---
v9:
- Rename ARM_CPU_VNMI to ARM_CPU_VINMI.
- Update the commit message.
v4:
- Add Reviewed-by.
v3:
- Also add VNMI wire.
---
hw/arm/virt.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index a9a913aead..ef2e6c2c4d 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -821,7 +821,8 @@ static void create_gic(VirtMachineState *vms, MemoryRegion
*mem)
/* Wire the outputs from each CPU's generic timer and the GICv3
* maintenance interrupt signal to the appropriate GIC PPI inputs,
- * and the GIC's IRQ/FIQ/VIRQ/VFIQ interrupt outputs to the CPU's inputs.
+ * and the GIC's IRQ/FIQ/VIRQ/VFIQ/NMI/VINMI interrupt outputs to the
+ * CPU's inputs.
*/
for (i = 0; i < smp_cpus; i++) {
DeviceState *cpudev = DEVICE(qemu_get_cpu(i));
@@ -865,6 +866,10 @@ static void create_gic(VirtMachineState *vms, MemoryRegion
*mem)
qdev_get_gpio_in(cpudev, ARM_CPU_VIRQ));
sysbus_connect_irq(gicbusdev, i + 3 * smp_cpus,
qdev_get_gpio_in(cpudev, ARM_CPU_VFIQ));
+sysbus_connect_irq(gicbusdev, i + 4 * smp_cpus,
+ qdev_get_gpio_in(cpudev, ARM_CPU_NMI));
+sysbus_connect_irq(gicbusdev, i + 5 * smp_cpus,
+ qdev_get_gpio_in(cpudev, ARM_CPU_VINMI));
}
fdt_add_gic_node(vms);
--
2.34.1
[PATCH v10 15/23] hw/intc/arm_gicv3_redist: Implement GICR_INMIR0
Add GICR_INMIR0 register and support access GICR_INMIR0.
Signed-off-by: Jinjie Ruan
Reviewed-by: Richard Henderson
---
v10:
- gicr_isuperprio -> gicr_inmir0.
v6:
- Add Reviewed-by.
v4:
- Make the GICR_INMIR0 implementation more clearer.
---
hw/intc/arm_gicv3_redist.c | 19 +++
hw/intc/gicv3_internal.h | 1 +
2 files changed, 20 insertions(+)
diff --git a/hw/intc/arm_gicv3_redist.c b/hw/intc/arm_gicv3_redist.c
index 8153525849..ed1f9d1e44 100644
--- a/hw/intc/arm_gicv3_redist.c
+++ b/hw/intc/arm_gicv3_redist.c
@@ -35,6 +35,15 @@ static int gicr_ns_access(GICv3CPUState *cs, int irq)
return extract32(cs->gicr_nsacr, irq * 2, 2);
}
+static void gicr_write_bitmap_reg(GICv3CPUState *cs, MemTxAttrs attrs,
+ uint32_t *reg, uint32_t val)
+{
+/* Helper routine to implement writing to a "set" register */
+val &= mask_group(cs, attrs);
+*reg = val;
+gicv3_redist_update(cs);
+}
+
static void gicr_write_set_bitmap_reg(GICv3CPUState *cs, MemTxAttrs attrs,
uint32_t *reg, uint32_t val)
{
@@ -406,6 +415,10 @@ static MemTxResult gicr_readl(GICv3CPUState *cs, hwaddr
offset,
*data = value;
return MEMTX_OK;
}
+case GICR_INMIR0:
+*data = cs->gic->nmi_support ?
+gicr_read_bitmap_reg(cs, attrs, cs->gicr_inmir0) : 0;
+return MEMTX_OK;
case GICR_ICFGR0:
case GICR_ICFGR1:
{
@@ -555,6 +568,12 @@ static MemTxResult gicr_writel(GICv3CPUState *cs, hwaddr
offset,
gicv3_redist_update(cs);
return MEMTX_OK;
}
+case GICR_INMIR0:
+if (cs->gic->nmi_support) {
+gicr_write_bitmap_reg(cs, attrs, >gicr_inmir0, value);
+}
+return MEMTX_OK;
+
case GICR_ICFGR0:
/* Register is all RAZ/WI or RAO/WI bits */
return MEMTX_OK;
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index 8f4ebed2f4..21697ecf39 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -110,6 +110,7 @@
#define GICR_ICFGR1 (GICR_SGI_OFFSET + 0x0C04)
#define GICR_IGRPMODR0(GICR_SGI_OFFSET + 0x0D00)
#define GICR_NSACR(GICR_SGI_OFFSET + 0x0E00)
+#define GICR_INMIR0 (GICR_SGI_OFFSET + 0x0F80)
/* VLPI redistributor registers, offsets from VLPI_base */
#define GICR_VPROPBASER (GICR_VLPI_OFFSET + 0x70)
--
2.34.1
[PATCH v10 18/23] hw/intc/arm_gicv3: Handle icv_nmiar1_read() for icc_nmiar1_read()
Implement icv_nmiar1_read() for icc_nmiar1_read(), so add definition for
ICH_LR_EL2.NMI and ICH_AP1R_EL2.NMI bit.
If FEAT_GICv3_NMI is supported, ich_ap_write() should consider ICV_AP1R_EL1.NMI
bit. In icv_activate_irq() and icv_eoir_write(), the ICV_AP1R_EL1.NMI bit
should be set or clear according to the Non-maskable property. And the RPR
priority should also update the NMI bit according to the APR priority NMI bit.
By the way, add gicv3_icv_nmiar1_read trace event.
If the hpp irq is a NMI, the icv iar read should return 1022 and trap for
NMI again
Signed-off-by: Jinjie Ruan
Reviewed-by: Richard Henderson
---
v10:
- Rename ICH_AP1R_EL2_NMI to ICV_AP1R_EL1_NMI.
- Add ICV_RPR_EL1_NMI definition.
- Set ICV_RPR_EL1.NMI according to the ICV_AP1R_EL1.NMI in
ich_highest_active_virt_prio().
v9:
- Correct the INTID_NMI logic.
v8:
- Fix an unexpected interrupt bug when sending VNMI by running qemu VM.
v7:
- Add Reviewed-by.
v6:
- Implement icv_nmiar1_read().
---
hw/intc/arm_gicv3_cpuif.c | 79 +--
hw/intc/gicv3_internal.h | 4 ++
hw/intc/trace-events | 1 +
3 files changed, 73 insertions(+), 11 deletions(-)
diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index 76e2286e70..e0dc76df2f 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -149,12 +149,13 @@ static uint32_t icv_fullprio_mask(GICv3CPUState *cs)
return (~0U << (8 - cs->vpribits)) & 0xff;
}
-static int ich_highest_active_virt_prio(GICv3CPUState *cs)
+static uint64_t ich_highest_active_virt_prio(GICv3CPUState *cs)
{
/* Calculate the current running priority based on the set bits
* in the ICH Active Priority Registers.
*/
int i;
+uint64_t prio;
int aprmax = ich_num_aprs(cs);
for (i = 0; i < aprmax; i++) {
@@ -164,7 +165,15 @@ static int ich_highest_active_virt_prio(GICv3CPUState *cs)
if (!apr) {
continue;
}
-return (i * 32 + ctz32(apr)) << (icv_min_vbpr(cs) + 1);
+prio = (i * 32 + ctz32(apr)) << (icv_min_vbpr(cs) + 1);
+
+if (cs->gic->nmi_support &&
+cs->ich_apr[GICV3_G1NS][i] & ICV_AP1R_EL1_NMI) {
+prio |= ICV_RPR_EL1_NMI;
+}
+
+return prio;
+
}
/* No current active interrupts: return idle priority */
return 0xff;
@@ -289,7 +298,8 @@ static bool icv_hppi_can_preempt(GICv3CPUState *cs,
uint64_t lr)
* equivalent of these checks.
*/
int grp;
-uint32_t mask, prio, rprio, vpmr;
+uint32_t mask, prio, vpmr;
+uint64_t rprio;
if (!(cs->ich_hcr_el2 & ICH_HCR_EL2_EN)) {
/* Virtual interface disabled */
@@ -336,7 +346,8 @@ static bool icv_hppvlpi_can_preempt(GICv3CPUState *cs)
* We can assume we're Non-secure because hppvi_index() already
* tested for that.
*/
-uint32_t mask, rprio, vpmr;
+uint32_t mask, vpmr;
+uint64_t rprio;
if (!(cs->ich_hcr_el2 & ICH_HCR_EL2_EN)) {
/* Virtual interface disabled */
@@ -550,7 +561,11 @@ static void icv_ap_write(CPUARMState *env, const
ARMCPRegInfo *ri,
trace_gicv3_icv_ap_write(ri->crm & 1, regno, gicv3_redist_affid(cs),
value);
-cs->ich_apr[grp][regno] = value & 0xU;
+if (cs->gic->nmi_support) {
+cs->ich_apr[grp][regno] = value & (0xU | ICV_AP1R_EL1_NMI);
+} else {
+cs->ich_apr[grp][regno] = value & 0xU;
+}
gicv3_cpuif_virt_irq_fiq_update(cs);
return;
@@ -697,7 +712,7 @@ static void icv_ctlr_write(CPUARMState *env, const
ARMCPRegInfo *ri,
static uint64_t icv_rpr_read(CPUARMState *env, const ARMCPRegInfo *ri)
{
GICv3CPUState *cs = icc_cs_from_env(env);
-int prio = ich_highest_active_virt_prio(cs);
+uint64_t prio = ich_highest_active_virt_prio(cs);
trace_gicv3_icv_rpr_read(gicv3_redist_affid(cs), prio);
return prio;
@@ -728,7 +743,7 @@ static uint64_t icv_hppir_read(CPUARMState *env, const
ARMCPRegInfo *ri)
return value;
}
-static void icv_activate_irq(GICv3CPUState *cs, int idx, int grp)
+static void icv_activate_irq(GICv3CPUState *cs, int idx, int grp, bool nmi)
{
/* Activate the interrupt in the specified list register
* by moving it from Pending to Active state, and update the
@@ -742,7 +757,12 @@ static void icv_activate_irq(GICv3CPUState *cs, int idx,
int grp)
cs->ich_lr_el2[idx] &= ~ICH_LR_EL2_STATE_PENDING_BIT;
cs->ich_lr_el2[idx] |= ICH_LR_EL2_STATE_ACTIVE_BIT;
-cs->ich_apr[grp][regno] |= (1 << regbit);
+
+if (cs->gic->nmi_support) {
+cs->ich_apr[grp][regno] |= (1 << regbit) | (nmi ? ICV_AP1R_EL1_NMI :
0);
+} else {
+cs->ich_apr[grp][regno] |= (1 << regbit);
+}
}
static void icv_activate_vlpi(GICv3CPUState *cs)
@@ -776,7 +796,11 @@ static uint64_t icv_iar_read(CPUARMState *env, const
ARMCPRegInfo *ri)
if (thisgrp == grp && icv_hppi_can_preempt(cs, lr)) {
intid = ich_lr_vintid(lr);
[PATCH v10 13/23] hw/intc: Enable FEAT_GICv3_NMI Feature
Added properties to enable FEAT_GICv3_NMI feature, setup distributor
and redistributor registers to indicate NMI support.
Signed-off-by: Jinjie Ruan
Reviewed-by: Richard Henderson
---
v10:
- Adjust to before add irq non-maskable property.
v4:
- Add Reviewed-by.
---
hw/intc/arm_gicv3_common.c | 1 +
hw/intc/arm_gicv3_dist.c | 2 ++
hw/intc/gicv3_internal.h | 1 +
include/hw/intc/arm_gicv3_common.h | 1 +
4 files changed, 5 insertions(+)
diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index c52f060026..2d2cea6858 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -569,6 +569,7 @@ static Property arm_gicv3_common_properties[] = {
DEFINE_PROP_UINT32("num-irq", GICv3State, num_irq, 32),
DEFINE_PROP_UINT32("revision", GICv3State, revision, 3),
DEFINE_PROP_BOOL("has-lpi", GICv3State, lpi_enable, 0),
+DEFINE_PROP_BOOL("has-nmi", GICv3State, nmi_support, 0),
DEFINE_PROP_BOOL("has-security-extensions", GICv3State, security_extn, 0),
/*
* Compatibility property: force 8 bits of physical priority, even
diff --git a/hw/intc/arm_gicv3_dist.c b/hw/intc/arm_gicv3_dist.c
index 35e850685c..22ddc0d666 100644
--- a/hw/intc/arm_gicv3_dist.c
+++ b/hw/intc/arm_gicv3_dist.c
@@ -389,6 +389,7 @@ static bool gicd_readl(GICv3State *s, hwaddr offset,
* by GICD_TYPER.IDbits)
* MBIS == 0 (message-based SPIs not supported)
* SecurityExtn == 1 if security extns supported
+ * NMI = 1 if Non-maskable interrupt property is supported
* CPUNumber == 0 since for us ARE is always 1
* ITLinesNumber == (((max SPI IntID + 1) / 32) - 1)
*/
@@ -402,6 +403,7 @@ static bool gicd_readl(GICv3State *s, hwaddr offset,
bool dvis = s->revision >= 4;
*data = (1 << 25) | (1 << 24) | (dvis << 18) | (sec_extn << 10) |
+(s->nmi_support << GICD_TYPER_NMI_SHIFT) |
(s->lpi_enable << GICD_TYPER_LPIS_SHIFT) |
(0xf << 19) | itlinesnumber;
return true;
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index 29d5cdc1b6..8f4ebed2f4 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -68,6 +68,7 @@
#define GICD_CTLR_E1NWF (1U << 7)
#define GICD_CTLR_RWP (1U << 31)
+#define GICD_TYPER_NMI_SHIFT 9
#define GICD_TYPER_LPIS_SHIFT 17
/* 16 bits EventId */
diff --git a/include/hw/intc/arm_gicv3_common.h
b/include/hw/intc/arm_gicv3_common.h
index 7324c7d983..4358c5319c 100644
--- a/include/hw/intc/arm_gicv3_common.h
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -249,6 +249,7 @@ struct GICv3State {
uint32_t num_irq;
uint32_t revision;
bool lpi_enable;
+bool nmi_support;
bool security_extn;
bool force_8bit_prio;
bool irq_reset_nonsecure;
--
2.34.1
[PATCH v10 12/23] target/arm: Handle NMI in arm_cpu_do_interrupt_aarch64()
According to Arm GIC section 4.6.3 Interrupt superpriority, the interrupt with superpriority is always IRQ, never FIQ, so the NMI exception trap entry behave like IRQ. And VINMI(vIRQ with Superpriority) can be raised from the GIC or come from the hcrx_el2.HCRX_VINMI bit, VFNMI(vFIQ with Superpriority) come from the hcrx_el2.HCRX_VFNMI bit. Signed-off-by: Jinjie Ruan Reviewed-by: Richard Henderson --- v9: - Update the commit message. - Handle VINMI and VFNMI. v7: - Add Reviewed-by. v6: - Not combine VFNMI with CPU_INTERRUPT_VNMI. v4: - Also handle VNMI in arm_cpu_do_interrupt_aarch64(). v3: - Remove the FIQ NMI handle. --- target/arm/helper.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/target/arm/helper.c b/target/arm/helper.c index 967e833ee8..eef37b801d 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -11650,10 +11650,13 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs) break; case EXCP_IRQ: case EXCP_VIRQ: +case EXCP_NMI: +case EXCP_VINMI: addr += 0x80; break; case EXCP_FIQ: case EXCP_VFIQ: +case EXCP_VFNMI: addr += 0x100; break; case EXCP_VSERR: -- 2.34.1
[PATCH v10 22/23] target/arm: Add FEAT_NMI to max
Enable FEAT_NMI on the 'max' CPU. Signed-off-by: Jinjie Ruan Reviewed-by: Richard Henderson --- v3: - Add Reviewed-by. - Sorted to last. --- docs/system/arm/emulation.rst | 1 + target/arm/tcg/cpu64.c| 1 + 2 files changed, 2 insertions(+) diff --git a/docs/system/arm/emulation.rst b/docs/system/arm/emulation.rst index 2a7bbb82dc..a9ae7ede9f 100644 --- a/docs/system/arm/emulation.rst +++ b/docs/system/arm/emulation.rst @@ -64,6 +64,7 @@ the following architecture extensions: - FEAT_MTE (Memory Tagging Extension) - FEAT_MTE2 (Memory Tagging Extension) - FEAT_MTE3 (MTE Asymmetric Fault Handling) +- FEAT_NMI (Non-maskable Interrupt) - FEAT_NV (Nested Virtualization) - FEAT_NV2 (Enhanced nested virtualization support) - FEAT_PACIMP (Pointer authentication - IMPLEMENTATION DEFINED algorithm) diff --git a/target/arm/tcg/cpu64.c b/target/arm/tcg/cpu64.c index 9f7a9f3d2c..62c4663512 100644 --- a/target/arm/tcg/cpu64.c +++ b/target/arm/tcg/cpu64.c @@ -1175,6 +1175,7 @@ void aarch64_max_tcg_initfn(Object *obj) t = FIELD_DP64(t, ID_AA64PFR1, RAS_FRAC, 0); /* FEAT_RASv1p1 + FEAT_DoubleFault */ t = FIELD_DP64(t, ID_AA64PFR1, SME, 1); /* FEAT_SME */ t = FIELD_DP64(t, ID_AA64PFR1, CSV2_FRAC, 0); /* FEAT_CSV2_2 */ +t = FIELD_DP64(t, ID_AA64PFR1, NMI, 1); /* FEAT_NMI */ cpu->isar.id_aa64pfr1 = t; t = cpu->isar.id_aa64mmfr0; -- 2.34.1
[PATCH v10 03/23] target/arm: Add support for FEAT_NMI, Non-maskable Interrupt
Add support for FEAT_NMI. NMI (FEAT_NMI) is an mandatory feature in
ARMv8.8-A and ARM v9.3-A.
Signed-off-by: Jinjie Ruan
Reviewed-by: Richard Henderson
---
v3:
- Add Reviewed-by.
- Adjust to before the MSR patches.
---
target/arm/internals.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index dd3da211a3..516e0584bf 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1229,6 +1229,9 @@ static inline uint32_t aarch64_pstate_valid_mask(const
ARMISARegisters *id)
if (isar_feature_aa64_mte(id)) {
valid |= PSTATE_TCO;
}
+if (isar_feature_aa64_nmi(id)) {
+valid |= PSTATE_ALLINT;
+}
return valid;
}
--
2.34.1
[PATCH v10 00/23] target/arm: Implement FEAT_NMI and FEAT_GICv3_NMI
This patch set implements FEAT_NMI and FEAT_GICv3_NMI for armv8. These introduce support for a new category of interrupts in the architecture which we can use to provide NMI like functionality. There are two modes for using this FEAT_NMI. When PSTATE.ALLINT or PSTATE.SP & SCTLR_ELx.SCTLR_SPINTMASK is set, any entry to ELx causes all interrupts including those with superpriority to be masked on entry to ELn until the mask is explicitly removed by software or hardware. PSTATE.ALLINT can be managed by software using the new register control ALLINT.ALLINT. Independent controls are provided for this feature at each EL, usage at EL1 should not disrupt EL2 or EL3. I have tested it with the following linux patches which try to support FEAT_NMI in linux kernel: https://lore.kernel.org/linux-arm-kernel/Y4sH5qX5bK9xfEBp@lpieralisi/T/#mb4ba4a2c045bf72c10c2202c1dd1b82d3240dc88 In the test, SGI, PPI and SPI interrupts can all be set to have super priority to be converted to a hardware NMI interrupt. The SGI is tested with kernel IPI as NMI framework, softlockup, hardlockup and kgdb test cases, and the PPI interrupt is tested with "perf top" command with hardware NMI enabled, and the SPI interrupt is tested with a custom test module, in which NMI interrupts can be received and sent normally. And the Virtual NMI(VNMI) SGI, PPI and SPI interrupts has also been tested in nested QEMU Virtual Machine with host "virtualization=true". The SGI VNMI is tested by accessing GICR_INMIR0 and GICR_ISPENDR0 with devmem command, as well as hardlockup and kgdb testcases. The PPI VNMI is tested by accessing GICR_INMIR0 and GICR_ISPENDR0 with devmem command, as well as "perf top" command with hardware NMI enabled, which works well. The SPI VNMI is tested with a custom test module, in which SPI VNMI can be sent from the GIC and received normally. +-+ | Distributor | +-+ SPI | NMI | NMI \/\/ ++ +---+ | Redist | | Redist| ++ +---+ SGI | NMI PPI | NMI \/\/ +-+ +---+ |CPU Interface| ... | CPU Interface | +-+ +---+ | NMI | NMI \/\/ +-+ +-+ | PE | | PE | +-+ +-+ Changes in v10: - Correct the exception_target_el(env) to 2 in msr_set_allint_el1 helper, since it is a hypervisor trap from EL1 to EL2. - In arm_cpu_exec_interrupt(), if SCTLR_ELx.NMI is 0, NMI -> IRQ, VINMI -> VIRQ, VFNMI -> VFIQ. - Make arm_cpu_update_virq() and arm_cpu_update_vfiq() check that it is not a VINMI/VFNMI, so only set 1 bit in interrupt_request, not 2. - Adjust "hw/intc: Enable FEAT_GICv3_NMI Feature" to before "add irq non-maskable property". - superprio -> nmi, gicr_isuperprio -> gicr_inmir0, is_nmi -> nmi, is_hppi -> hppi, has_superprio -> nmi, superpriority -> non-maskable property. - Save NMI state in vmstate_gicv3_cpu and vmstate_gicv3. - Exchange the order of nmi and hppi parameters. - Handle APR and RPR NMI bits, rename ICH_AP1R_EL2_NMI to ICV_AP1R_EL1_NMI. - Set ICV_RPR_EL1.NMI according to the ICV_AP1R_EL1.NMI in ich_highest_active_virt_prio() - Update the commit message. Changes in v9: - Move nmi_reginfo and related functions inside an existing ifdef TARGET_AARCH64 to solve the --target-list=aarch64-softmmu,arm-softmmu compilation problem. - Check 'isread' when writing to ALLINT. - Update the GPIOs passed in the arm_cpu_kvm_set_irq, and update the comment. - Definitely not merge VINMI and VFNMI into EXCP_VNMI. - ARM_CPU_VNMI -> ARM_CPU_VINMI, CPU_INTERRUPT_VNMI -> CPU_INTERRUPT_VINMI. - Update VINMI and VFNMI when writing HCR_EL2 or HCRX_EL2. - Update the commit subject and message, VNMI -> VINMI. - Handle CPSR_F and ISR_FS according to CPU_INTERRUPT_VFNMI instead of CPU_INTERRUPT_VFIQ and HCRX_EL2.VFNMI. - Not check SCTLR_NMI in arm_cpu_do_interrupt_aarch64(). - Correct the INTID_NMI logic. - Declare cpu variable to reuse latter. Changes in v8: - Fix the rcu stall after sending a VNMI in qemu VM. - Fix an unexpected interrupt bug when sending VNMI by running qemu VM. - Test the VNMI interrupt. - Update the commit message. - Add Reviewed-by. Changes in v7: - env->cp15.hcrx_el2 -> arm_hcrx_el2_eff(). - Reorder the irqbetter() code for clarity. - Eliminate the has_superprio local variable for gicv3_get_priority(). - false -> cs->hpplpi.superprio in gicv3_redist_update_noirqset(). - 0x0 ->
[PATCH v10 06/23] target/arm: Add support for Non-maskable Interrupt
This only implements the external delivery method via the GICv3.
Signed-off-by: Jinjie Ruan
Reviewed-by: Richard Henderson
---
v10:
- In arm_cpu_exec_interrupt(), if SCTLR_ELx.NMI is 0, NMI -> IRQ,
VINMI -> VIRQ, VFNMI -> VFIQ.
- Make arm_cpu_update_virq() and arm_cpu_update_vfiq() check that it is not a
VINMI/VFNMI, so only set 1 bit in interrupt_request, not 2.
v9:
- Update the GPIOs passed in the arm_cpu_kvm_set_irq, and update the comment.
- Definitely not merge VINMI and VFNMI into EXCP_VNMI.
- Update VINMI and VFNMI when writing HCR_EL2 or HCRX_EL2.
v8:
- Fix the rcu stall after sending a VNMI in qemu VM.
v7:
- Add Reviewed-by.
v6:
- env->cp15.hcr_el2 -> arm_hcr_el2_eff().
- env->cp15.hcrx_el2 -> arm_hcrx_el2_eff().
- Not include VF && VFNMI in CPU_INTERRUPT_VNMI.
v4:
- Accept NMI unconditionally for arm_cpu_has_work() but add comment.
- Change from & to && for EXCP_IRQ or EXCP_FIQ.
- Refator nmi mask in arm_excp_unmasked().
- Also handle VNMI in arm_cpu_exec_interrupt() and arm_cpu_set_irq().
- Rename virtual to Virtual.
v3:
- Not include CPU_INTERRUPT_NMI when FEAT_NMI not enabled
- Add ARM_CPU_VNMI.
- Refator nmi mask in arm_excp_unmasked().
- Test SCTLR_ELx.NMI for ALLINT mask for NMI.
---
target/arm/cpu-qom.h | 5 +-
target/arm/cpu.c | 146 +
target/arm/cpu.h | 6 ++
target/arm/helper.c| 33 --
target/arm/internals.h | 18 +
5 files changed, 192 insertions(+), 16 deletions(-)
diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h
index 8e032691db..b497667d61 100644
--- a/target/arm/cpu-qom.h
+++ b/target/arm/cpu-qom.h
@@ -36,11 +36,14 @@ DECLARE_CLASS_CHECKERS(AArch64CPUClass, AARCH64_CPU,
#define ARM_CPU_TYPE_SUFFIX "-" TYPE_ARM_CPU
#define ARM_CPU_TYPE_NAME(name) (name ARM_CPU_TYPE_SUFFIX)
-/* Meanings of the ARMCPU object's four inbound GPIO lines */
+/* Meanings of the ARMCPU object's seven inbound GPIO lines */
#define ARM_CPU_IRQ 0
#define ARM_CPU_FIQ 1
#define ARM_CPU_VIRQ 2
#define ARM_CPU_VFIQ 3
+#define ARM_CPU_NMI 4
+#define ARM_CPU_VINMI 5
+#define ARM_CPU_VFNMI 6
/* For M profile, some registers are banked secure vs non-secure;
* these are represented as a 2-element array where the first element
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index ab8d007a86..74eb573aeb 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -122,6 +122,13 @@ void arm_restore_state_to_opc(CPUState *cs,
}
#endif /* CONFIG_TCG */
+/*
+ * With SCTLR_ELx.NMI == 0, IRQ with Superpriority is masked identically with
+ * IRQ without Superpriority. Moreover, if the GIC is configured so that
+ * FEAT_GICv3_NMI is only set if FEAT_NMI is set, then we won't ever see
+ * CPU_INTERRUPT_*NMI anyway. So we might as well accept NMI here
+ * unconditionally.
+ */
static bool arm_cpu_has_work(CPUState *cs)
{
ARMCPU *cpu = ARM_CPU(cs);
@@ -129,6 +136,7 @@ static bool arm_cpu_has_work(CPUState *cs)
return (cpu->power_state != PSCI_OFF)
&& cs->interrupt_request &
(CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD
+ | CPU_INTERRUPT_NMI | CPU_INTERRUPT_VINMI | CPU_INTERRUPT_VFNMI
| CPU_INTERRUPT_VFIQ | CPU_INTERRUPT_VIRQ | CPU_INTERRUPT_VSERR
| CPU_INTERRUPT_EXITTB);
}
@@ -668,6 +676,7 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned
int excp_idx,
CPUARMState *env = cpu_env(cs);
bool pstate_unmasked;
bool unmasked = false;
+bool allIntMask = false;
/*
* Don't take exceptions if they target a lower EL.
@@ -678,13 +687,36 @@ static inline bool arm_excp_unmasked(CPUState *cs,
unsigned int excp_idx,
return false;
}
+if (cpu_isar_feature(aa64_nmi, env_archcpu(env)) &&
+env->cp15.sctlr_el[target_el] & SCTLR_NMI && cur_el == target_el) {
+allIntMask = env->pstate & PSTATE_ALLINT ||
+ ((env->cp15.sctlr_el[target_el] & SCTLR_SPINTMASK) &&
+ (env->pstate & PSTATE_SP));
+}
+
switch (excp_idx) {
+case EXCP_NMI:
+pstate_unmasked = !allIntMask;
+break;
+
+case EXCP_VINMI:
+if (!(hcr_el2 & HCR_IMO) || (hcr_el2 & HCR_TGE)) {
+/* VINMIs are only taken when hypervized. */
+return false;
+}
+return !allIntMask;
+case EXCP_VFNMI:
+if (!(hcr_el2 & HCR_FMO) || (hcr_el2 & HCR_TGE)) {
+/* VFNMIs are only taken when hypervized. */
+return false;
+}
+return !allIntMask;
case EXCP_FIQ:
-pstate_unmasked = !(env->daif & PSTATE_F);
+pstate_unmasked = (!(env->daif & PSTATE_F)) && (!allIntMask);
break;
case EXCP_IRQ:
-pstate_unmasked = !(env->daif & PSTATE_I);
+pstate_unmasked = (!(env->daif & PSTATE_I)) && (!allIntMask);
break;
case EXCP_VFIQ:
@@ -692,13 +724,13 @@ static inline bool arm_excp_unmasked(CPUState *cs,
unsigned int excp_idx,
/* VFIQs are only taken
[PATCH v10 20/23] hw/intc/arm_gicv3: Report the NMI interrupt in gicv3_cpuif_update()
In CPU Interface, if the IRQ has the non-maskable property, report NMI to
the corresponding PE.
Signed-off-by: Jinjie Ruan
Reviewed-by: Richard Henderson
---
v10:
- superprio -> nmi.
- Update the commit message, superpriority -> non-maskable.
v6:
- Add Reviewed-by.
v4:
- Swap the ordering of the IFs.
v3:
- Remove handling nmi_is_irq flag.
---
hw/intc/arm_gicv3_cpuif.c | 4
1 file changed, 4 insertions(+)
diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index e0dc76df2f..4cd84b142e 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -1015,6 +1015,7 @@ void gicv3_cpuif_update(GICv3CPUState *cs)
/* Tell the CPU about its highest priority pending interrupt */
int irqlevel = 0;
int fiqlevel = 0;
+int nmilevel = 0;
ARMCPU *cpu = ARM_CPU(cs->cpu);
CPUARMState *env = >env;
@@ -1053,6 +1054,8 @@ void gicv3_cpuif_update(GICv3CPUState *cs)
if (isfiq) {
fiqlevel = 1;
+} else if (cs->hppi.nmi) {
+nmilevel = 1;
} else {
irqlevel = 1;
}
@@ -1062,6 +1065,7 @@ void gicv3_cpuif_update(GICv3CPUState *cs)
qemu_set_irq(cs->parent_fiq, fiqlevel);
qemu_set_irq(cs->parent_irq, irqlevel);
+qemu_set_irq(cs->parent_nmi, nmilevel);
}
static uint64_t icc_pmr_read(CPUARMState *env, const ARMCPRegInfo *ri)
--
2.34.1
[PATCH v10 05/23] target/arm: Support MSR access to ALLINT
Support ALLINT msr access as follow:
mrs , ALLINT// read allint
msr ALLINT, // write allint with imm
Signed-off-by: Jinjie Ruan
Reviewed-by: Richard Henderson
---
v9:
- Move nmi_reginfo and related functions inside an existing ifdef
TARGET_AARCH64 to solve the --target-list=aarch64-softmmu,arm-softmmu
compilation problem.
- Check 'isread' when writing to ALLINT.
v5:
- Add Reviewed-by.
v4:
- Remove arm_is_el2_enabled() check in allint_check().
- Change to env->pstate instead of env->allint.
v3:
- Remove EL0 check in aa64_allint_access() which alreay checks in .access
PL1_RW.
- Use arm_hcrx_el2_eff() in aa64_allint_access() instead of env->cp15.hcrx_el2.
- Make ALLINT msr access function controlled by aa64_nmi.
---
target/arm/helper.c | 35 +++
1 file changed, 35 insertions(+)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 7d6c6e9878..a65729af66 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -7497,6 +7497,37 @@ static const ARMCPRegInfo rme_mte_reginfo[] = {
.opc0 = 1, .opc1 = 6, .crn = 7, .crm = 14, .opc2 = 5,
.access = PL3_W, .type = ARM_CP_NOP },
};
+
+static void aa64_allint_write(CPUARMState *env, const ARMCPRegInfo *ri,
+ uint64_t value)
+{
+env->pstate = (env->pstate & ~PSTATE_ALLINT) | (value & PSTATE_ALLINT);
+}
+
+static uint64_t aa64_allint_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+return env->pstate & PSTATE_ALLINT;
+}
+
+static CPAccessResult aa64_allint_access(CPUARMState *env,
+ const ARMCPRegInfo *ri, bool isread)
+{
+if (!isread && arm_current_el(env) == 1 &&
+(arm_hcrx_el2_eff(env) & HCRX_TALLINT)) {
+return CP_ACCESS_TRAP_EL2;
+}
+return CP_ACCESS_OK;
+}
+
+static const ARMCPRegInfo nmi_reginfo[] = {
+{ .name = "ALLINT", .state = ARM_CP_STATE_AA64,
+ .opc0 = 3, .opc1 = 0, .opc2 = 0, .crn = 4, .crm = 3,
+ .type = ARM_CP_NO_RAW,
+ .access = PL1_RW, .accessfn = aa64_allint_access,
+ .fieldoffset = offsetof(CPUARMState, pstate),
+ .writefn = aa64_allint_write, .readfn = aa64_allint_read,
+ .resetfn = arm_cp_reset_ignore },
+};
#endif /* TARGET_AARCH64 */
static void define_pmu_regs(ARMCPU *cpu)
@@ -9891,6 +9922,10 @@ void register_cp_regs_for_features(ARMCPU *cpu)
if (cpu_isar_feature(aa64_nv2, cpu)) {
define_arm_cp_regs(cpu, nv2_reginfo);
}
+
+if (cpu_isar_feature(aa64_nmi, cpu)) {
+define_arm_cp_regs(cpu, nmi_reginfo);
+}
#endif
if (cpu_isar_feature(any_predinv, cpu)) {
--
2.34.1
[PATCH v10 02/23] target/arm: Add PSTATE.ALLINT
When PSTATE.ALLINT is set, an IRQ or FIQ interrupt that is targeted to ELx, with or without superpriority is masked. As Richard suggested, place ALLINT bit in PSTATE in env->pstate. With the change to pstate_read/write, exception entry and return are automatically handled. Signed-off-by: Jinjie Ruan Reviewed-by: Richard Henderson --- v5: - Remove the ALLINT comment, as it is covered by "all other bits". - Add Reviewed-by. v4: - Keep PSTATE.ALLINT in env->pstate but not env->allint. - Update the commit message. v3: - Remove ALLINT dump in aarch64_cpu_dump_state(). - Update the commit message. --- target/arm/cpu.h | 1 + 1 file changed, 1 insertion(+) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index bc0c84873f..de740d223f 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -1430,6 +1430,7 @@ void pmu_init(ARMCPU *cpu); #define PSTATE_D (1U << 9) #define PSTATE_BTYPE (3U << 10) #define PSTATE_SSBS (1U << 12) +#define PSTATE_ALLINT (1U << 13) #define PSTATE_IL (1U << 20) #define PSTATE_SS (1U << 21) #define PSTATE_PAN (1U << 22) -- 2.34.1
[PATCH v10 07/23] target/arm: Add support for NMI in arm_phys_excp_target_el()
According to Arm GIC section 4.6.3 Interrupt superpriority, the interrupt
with superpriority is always IRQ, never FIQ, so handle NMI same as IRQ in
arm_phys_excp_target_el().
Signed-off-by: Jinjie Ruan
Reviewed-by: Richard Henderson
---
v4:
- Add Reviewed-by.
v3:
- Remove nmi_is_irq flag in CPUARMState.
- Handle NMI same as IRQ in arm_phys_excp_target_el().
---
target/arm/helper.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 1868235499..077c9a6923 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -10760,6 +10760,7 @@ uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t
excp_idx,
hcr_el2 = arm_hcr_el2_eff(env);
switch (excp_idx) {
case EXCP_IRQ:
+case EXCP_NMI:
scr = ((env->cp15.scr_el3 & SCR_IRQ) == SCR_IRQ);
hcr = hcr_el2 & HCR_IMO;
break;
--
2.34.1
Re: [PATCH-for-9.0 1/2] hw/clock: Let clock_set_mul_div() return boolean value
On 16:58 Fri 22 Mar , Philippe Mathieu-Daudé wrote:
> Let clock_set_mul_div() return a boolean value whether the
> clock has been updated or not, similarly to clock_set().
>
> Signed-off-by: Philippe Mathieu-Daudé
Acked-by: Luc Michel
> ---
> include/hw/clock.h | 4 +++-
> hw/core/clock.c| 8 +++-
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/include/hw/clock.h b/include/hw/clock.h
> index bb12117f67..eb58599131 100644
> --- a/include/hw/clock.h
> +++ b/include/hw/clock.h
> @@ -357,6 +357,8 @@ char *clock_display_freq(Clock *clk);
> * @multiplier: multiplier value
> * @divider: divider value
> *
> + * @return: true if the clock is changed.
> + *
> * By default, a Clock's children will all run with the same period
> * as their parent. This function allows you to adjust the multiplier
> * and divider used to derive the child clock frequency.
> @@ -374,6 +376,6 @@ char *clock_display_freq(Clock *clk);
> * Note that this function does not call clock_propagate(); the
> * caller should do that if necessary.
> */
> -void clock_set_mul_div(Clock *clk, uint32_t multiplier, uint32_t divider);
> +bool clock_set_mul_div(Clock *clk, uint32_t multiplier, uint32_t divider);
>
> #endif /* QEMU_HW_CLOCK_H */
> diff --git a/hw/core/clock.c b/hw/core/clock.c
> index d82e44cd1a..a19c7db7df 100644
> --- a/hw/core/clock.c
> +++ b/hw/core/clock.c
> @@ -143,14 +143,20 @@ char *clock_display_freq(Clock *clk)
> return freq_to_str(clock_get_hz(clk));
> }
>
> -void clock_set_mul_div(Clock *clk, uint32_t multiplier, uint32_t divider)
> +bool clock_set_mul_div(Clock *clk, uint32_t multiplier, uint32_t divider)
> {
> assert(divider != 0);
>
> +if (clk->multiplier == multiplier && clk->divider == divider) {
> +return false;
> +}
> +
> trace_clock_set_mul_div(CLOCK_PATH(clk), clk->multiplier, multiplier,
> clk->divider, divider);
> clk->multiplier = multiplier;
> clk->divider = divider;
> +
> +return true;
> }
>
> static void clock_initfn(Object *obj)
> --
> 2.41.0
>
[PATCH v2 1/5] target/riscv: Add support for Zve32x extension
Add support for Zve32x extension and replace some checks for Zve32f with
Zve32x, since Zve32f depends on Zve32x.
Signed-off-by: Jason Chien
Reviewed-by: Frank Chang
Reviewed-by: Max Chou
---
target/riscv/cpu.c | 1 +
target/riscv/cpu_cfg.h | 1 +
target/riscv/cpu_helper.c | 2 +-
target/riscv/csr.c | 2 +-
target/riscv/insn_trans/trans_rvv.c.inc | 4 ++--
target/riscv/tcg/tcg-cpu.c | 16
6 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 36e3e5fdaf..851ac7372c 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -153,6 +153,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
ISA_EXT_DATA_ENTRY(zvbb, PRIV_VERSION_1_12_0, ext_zvbb),
ISA_EXT_DATA_ENTRY(zvbc, PRIV_VERSION_1_12_0, ext_zvbc),
ISA_EXT_DATA_ENTRY(zve32f, PRIV_VERSION_1_10_0, ext_zve32f),
+ISA_EXT_DATA_ENTRY(zve32x, PRIV_VERSION_1_10_0, ext_zve32x),
ISA_EXT_DATA_ENTRY(zve64f, PRIV_VERSION_1_10_0, ext_zve64f),
ISA_EXT_DATA_ENTRY(zve64d, PRIV_VERSION_1_10_0, ext_zve64d),
ISA_EXT_DATA_ENTRY(zvfbfmin, PRIV_VERSION_1_12_0, ext_zvfbfmin),
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index cb750154bd..dce49050c0 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -91,6 +91,7 @@ struct RISCVCPUConfig {
bool ext_zhinx;
bool ext_zhinxmin;
bool ext_zve32f;
+bool ext_zve32x;
bool ext_zve64f;
bool ext_zve64d;
bool ext_zvbb;
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index fc090d729a..b13a50a665 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -72,7 +72,7 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
*pc = env->xl == MXL_RV32 ? env->pc & UINT32_MAX : env->pc;
*cs_base = 0;
-if (cpu->cfg.ext_zve32f) {
+if (cpu->cfg.ext_zve32x) {
/*
* If env->vl equals to VLMAX, we can use generic vector operation
* expanders (GVEC) to accerlate the vector operations.
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 726096444f..d96feea5d3 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -93,7 +93,7 @@ static RISCVException fs(CPURISCVState *env, int csrno)
static RISCVException vs(CPURISCVState *env, int csrno)
{
-if (riscv_cpu_cfg(env)->ext_zve32f) {
+if (riscv_cpu_cfg(env)->ext_zve32x) {
#if !defined(CONFIG_USER_ONLY)
if (!env->debugger && !riscv_cpu_vector_enabled(env)) {
return RISCV_EXCP_ILLEGAL_INST;
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc
b/target/riscv/insn_trans/trans_rvv.c.inc
index 7d84e7d812..eec2939e23 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -149,7 +149,7 @@ static bool do_vsetvl(DisasContext *s, int rd, int rs1,
TCGv s2)
{
TCGv s1, dst;
-if (!require_rvv(s) || !s->cfg_ptr->ext_zve32f) {
+if (!require_rvv(s) || !s->cfg_ptr->ext_zve32x) {
return false;
}
@@ -179,7 +179,7 @@ static bool do_vsetivli(DisasContext *s, int rd, TCGv s1,
TCGv s2)
{
TCGv dst;
-if (!require_rvv(s) || !s->cfg_ptr->ext_zve32f) {
+if (!require_rvv(s) || !s->cfg_ptr->ext_zve32x) {
return false;
}
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index b5b95e052d..ff0d485e7f 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -511,9 +511,13 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu,
Error **errp)
return;
}
-if (cpu->cfg.ext_zve32f && !riscv_has_ext(env, RVF)) {
-error_setg(errp, "Zve32f/Zve64f extensions require F extension");
-return;
+/* The Zve32f extension depends on the Zve32x extension */
+if (cpu->cfg.ext_zve32f) {
+if (!riscv_has_ext(env, RVF)) {
+error_setg(errp, "Zve32f/Zve64f extensions require F extension");
+return;
+}
+cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zve32x), true);
}
if (cpu->cfg.ext_zvfh) {
@@ -658,13 +662,9 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu,
Error **errp)
cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zvbc), true);
}
-/*
- * In principle Zve*x would also suffice here, were they supported
- * in qemu
- */
if ((cpu->cfg.ext_zvbb || cpu->cfg.ext_zvkb || cpu->cfg.ext_zvkg ||
cpu->cfg.ext_zvkned || cpu->cfg.ext_zvknha || cpu->cfg.ext_zvksed ||
- cpu->cfg.ext_zvksh) && !cpu->cfg.ext_zve32f) {
+ cpu->cfg.ext_zvksh) && !cpu->cfg.ext_zve32x) {
error_setg(errp,
"Vector crypto extensions require V or Zve* extensions");
return;
--
2.43.2
[PATCH v2 4/5] target/riscv: Expose Zve64x extension to users
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2107
Signed-off-by: Jason Chien
Reviewed-by: Frank Chang
Reviewed-by: Max Chou
---
target/riscv/cpu.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index f6287bf892..18e1ae66f4 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1477,6 +1477,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
MULTI_EXT_CFG_BOOL("zve32x", ext_zve32x, false),
MULTI_EXT_CFG_BOOL("zve64f", ext_zve64f, false),
MULTI_EXT_CFG_BOOL("zve64d", ext_zve64d, false),
+MULTI_EXT_CFG_BOOL("zve64x", ext_zve64x, false),
MULTI_EXT_CFG_BOOL("zvfbfmin", ext_zvfbfmin, false),
MULTI_EXT_CFG_BOOL("zvfbfwma", ext_zvfbfwma, false),
MULTI_EXT_CFG_BOOL("zvfh", ext_zvfh, false),
--
2.43.2
[PATCH v2 2/5] target/riscv: Expose Zve32x extension to users
Signed-off-by: Jason Chien
Reviewed-by: Frank Chang
Reviewed-by: Max Chou
---
target/riscv/cpu.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 851ac7372c..6bd8798bb5 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1473,6 +1473,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
MULTI_EXT_CFG_BOOL("zfh", ext_zfh, false),
MULTI_EXT_CFG_BOOL("zfhmin", ext_zfhmin, false),
MULTI_EXT_CFG_BOOL("zve32f", ext_zve32f, false),
+MULTI_EXT_CFG_BOOL("zve32x", ext_zve32x, false),
MULTI_EXT_CFG_BOOL("zve64f", ext_zve64f, false),
MULTI_EXT_CFG_BOOL("zve64d", ext_zve64d, false),
MULTI_EXT_CFG_BOOL("zvfbfmin", ext_zvfbfmin, false),
--
2.43.2
[PATCH v2 5/5] target/riscv: Relax vector register check in RISCV gdbstub
In current implementation, the gdbstub allows reading vector registers
only if V extension is supported. However, all vector extensions and
vector crypto extensions have the vector registers and they all depend
on Zve32x. The gdbstub should check for Zve32x instead.
Signed-off-by: Jason Chien
Reviewed-by: Frank Chang
Reviewed-by: Max Chou
---
target/riscv/gdbstub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index be7a02cd90..d0cc5762c2 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -338,7 +338,7 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
gdb_find_static_feature("riscv-32bit-fpu.xml"),
0);
}
-if (env->misa_ext & RVV) {
+if (cpu->cfg.ext_zve32x) {
gdb_register_coprocessor(cs, riscv_gdb_get_vector,
riscv_gdb_set_vector,
ricsv_gen_dynamic_vector_feature(cs,
cs->gdb_num_regs),
--
2.43.2
[PATCH v2 3/5] target/riscv: Add support for Zve64x extension
Add support for Zve64x extension. Enabling Zve64f enables Zve64x and
enabling Zve64x enables Zve32x according to their dependency.
Signed-off-by: Jason Chien
Reviewed-by: Frank Chang
Reviewed-by: Max Chou
---
target/riscv/cpu.c | 1 +
target/riscv/cpu_cfg.h | 1 +
target/riscv/tcg/tcg-cpu.c | 17 +++--
3 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 6bd8798bb5..f6287bf892 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -156,6 +156,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
ISA_EXT_DATA_ENTRY(zve32x, PRIV_VERSION_1_10_0, ext_zve32x),
ISA_EXT_DATA_ENTRY(zve64f, PRIV_VERSION_1_10_0, ext_zve64f),
ISA_EXT_DATA_ENTRY(zve64d, PRIV_VERSION_1_10_0, ext_zve64d),
+ISA_EXT_DATA_ENTRY(zve64x, PRIV_VERSION_1_10_0, ext_zve64x),
ISA_EXT_DATA_ENTRY(zvfbfmin, PRIV_VERSION_1_12_0, ext_zvfbfmin),
ISA_EXT_DATA_ENTRY(zvfbfwma, PRIV_VERSION_1_12_0, ext_zvfbfwma),
ISA_EXT_DATA_ENTRY(zvfh, PRIV_VERSION_1_12_0, ext_zvfh),
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index dce49050c0..e1e4f32698 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -94,6 +94,7 @@ struct RISCVCPUConfig {
bool ext_zve32x;
bool ext_zve64f;
bool ext_zve64d;
+bool ext_zve64x;
bool ext_zvbb;
bool ext_zvbc;
bool ext_zvkb;
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index ff0d485e7f..4ebebebe09 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -498,17 +498,22 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu,
Error **errp)
/* The Zve64d extension depends on the Zve64f extension */
if (cpu->cfg.ext_zve64d) {
+if (!riscv_has_ext(env, RVD)) {
+error_setg(errp, "Zve64d/V extensions require D extension");
+return;
+}
cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zve64f), true);
}
-/* The Zve64f extension depends on the Zve32f extension */
+/* The Zve64f extension depends on the Zve64x and Zve32f extensions */
if (cpu->cfg.ext_zve64f) {
+cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zve64x), true);
cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zve32f), true);
}
-if (cpu->cfg.ext_zve64d && !riscv_has_ext(env, RVD)) {
-error_setg(errp, "Zve64d/V extensions require D extension");
-return;
+/* The Zve64x extension depends on the Zve32x extension */
+if (cpu->cfg.ext_zve64x) {
+cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zve32x), true);
}
/* The Zve32f extension depends on the Zve32x extension */
@@ -670,10 +675,10 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu,
Error **errp)
return;
}
-if ((cpu->cfg.ext_zvbc || cpu->cfg.ext_zvknhb) && !cpu->cfg.ext_zve64f) {
+if ((cpu->cfg.ext_zvbc || cpu->cfg.ext_zvknhb) && !cpu->cfg.ext_zve64x) {
error_setg(
errp,
-"Zvbc and Zvknhb extensions require V or Zve64{f,d} extensions");
+"Zvbc and Zvknhb extensions require V or Zve64x extensions");
return;
}
--
2.43.2
[PATCH v2 0/5] target/riscv: Support Zve32x and Zve64x extensions
This patch series adds the support for Zve32x and Zvx64x and makes vector registers visible in GDB if any of the V/Zve*/Zvk* extensions is enabled. v2: Rebase onto riscv-to-apply.next (commit 385e575). Jason Chien (5): target/riscv: Add support for Zve32x extension target/riscv: Expose Zve32x extension to users target/riscv: Add support for Zve64x extension target/riscv: Expose Zve64x extension to users target/riscv: Relax vector register check in RISCV gdbstub target/riscv/cpu.c | 4 +++ target/riscv/cpu_cfg.h | 2 ++ target/riscv/cpu_helper.c | 2 +- target/riscv/csr.c | 2 +- target/riscv/gdbstub.c | 2 +- target/riscv/insn_trans/trans_rvv.c.inc | 4 +-- target/riscv/tcg/tcg-cpu.c | 33 ++--- 7 files changed, 30 insertions(+), 19 deletions(-) -- 2.43.2
Re: [PATCH 03/26] confidential guest support: Add kvm_init() and kvm_reset() in class
Hi Xiaoyao,
On 22/3/24 19:10, Paolo Bonzini wrote:
From: Xiaoyao Li
Different confidential VMs in different architectures all have the same
needs to do their specific initialization (and maybe resetting) stuffs
with KVM. Currently each of them exposes individual *_kvm_init()
functions and let machine code or kvm code to call it.
To facilitate the introduction of confidential guest technology from
different x86 vendors, add two virtual functions, kvm_init() and kvm_reset()
in ConfidentialGuestSupportClass, and expose two helpers functions for
invodking them.
Signed-off-by: Xiaoyao Li
Message-Id: <20240229060038.606591-1-xiaoyao...@intel.com>
Signed-off-by: Paolo Bonzini
---
include/exec/confidential-guest-support.h | 34 ++-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/include/exec/confidential-guest-support.h
b/include/exec/confidential-guest-support.h
index ba2dd4b5dfc..e5b188cffbf 100644
--- a/include/exec/confidential-guest-support.h
+++ b/include/exec/confidential-guest-support.h
@@ -23,7 +23,10 @@
#include "qom/object.h"
#define TYPE_CONFIDENTIAL_GUEST_SUPPORT "confidential-guest-support"
-OBJECT_DECLARE_SIMPLE_TYPE(ConfidentialGuestSupport,
CONFIDENTIAL_GUEST_SUPPORT)
+OBJECT_DECLARE_TYPE(ConfidentialGuestSupport,
+ConfidentialGuestSupportClass,
+CONFIDENTIAL_GUEST_SUPPORT)
+
struct ConfidentialGuestSupport {
Object parent;
@@ -55,8 +58,37 @@ struct ConfidentialGuestSupport {
typedef struct ConfidentialGuestSupportClass {
ObjectClass parent;
+
+int (*kvm_init)(ConfidentialGuestSupport *cgs, Error **errp);
+int (*kvm_reset)(ConfidentialGuestSupport *cgs, Error **errp);
Can we get a docstring indicating what these functions return?
Looking at the next patch, the KVM specific return value doesn't
seem used, so can we return a boolean instead?
} ConfidentialGuestSupportClass;
I suppose it will be easy enough to refactor for future other
HW accelerators.
Regards,
Phil.
Re: [PATCH-for-9.1] target/ppc: Unify TYPE_POWERPC_CPU definition for 32/64-bit
On 22/03/2024 19.34, Philippe Mathieu-Daudé wrote: Apparently there is no wordsize special use with the QOM TYPE_POWERPC_CPU typename. Unify 32 and 64-bit with a single common definition. Signed-off-by: Philippe Mathieu-Daudé --- target/ppc/cpu-qom.h | 4 1 file changed, 4 deletions(-) diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h index 8247fa2336..ed75f1b690 100644 --- a/target/ppc/cpu-qom.h +++ b/target/ppc/cpu-qom.h @@ -23,11 +23,7 @@ #include "exec/gdbstub.h" #include "hw/core/cpu.h" -#ifdef TARGET_PPC64 -#define TYPE_POWERPC_CPU "powerpc64-cpu" -#else #define TYPE_POWERPC_CPU "powerpc-cpu" -#endif Have you tried a bunch of migrations with ppc machines from an older QEMU to a QEMU with this change and back, to make sure that there are no regressions? Thomas
[RFC QEMU PATCH v7 1/1] virtio-pci: implement No_Soft_Reset bit
In current code, when guest does S3, virtio devices are reset due to
the bit No_Soft_Reset is not set. After resetting, the display resources
of virtio-gpu are destroyed, then the display can't come back and only
show blank after resuming.
Implement No_Soft_Reset bit of PCI_PM_CTRL register, then guest can check
this bit, if this bit is set, the devices resetting will not be done, and
then the display can work after resuming.
Signed-off-by: Jiqian Chen
---
hw/virtio/virtio-pci.c | 38 +-
include/hw/virtio/virtio-pci.h | 5 +
2 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 1a7039fb0c68..daafda315f8c 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -2197,6 +2197,11 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error
**errp)
pcie_cap_lnkctl_init(pci_dev);
}
+if (proxy->flags & VIRTIO_PCI_FLAG_PM_NO_SOFT_RESET) {
+pci_set_word(pci_dev->config + pos + PCI_PM_CTRL,
+ PCI_PM_CTRL_NO_SOFT_RESET);
+}
+
if (proxy->flags & VIRTIO_PCI_FLAG_INIT_PM) {
/* Init Power Management Control Register */
pci_set_word(pci_dev->wmask + pos + PCI_PM_CTRL,
@@ -2259,18 +2264,47 @@ static void virtio_pci_reset(DeviceState *qdev)
}
}
+static bool device_no_need_reset(PCIDevice *dev)
+{
+if (pci_is_express(dev)) {
+uint16_t pmcsr;
+
+pmcsr = pci_get_word(dev->config + dev->exp.pm_cap + PCI_PM_CTRL);
+/*
+ * When No_Soft_Reset bit is set and the device
+ * is in D3hot state, don't reset device
+ */
+if ((pmcsr & PCI_PM_CTRL_NO_SOFT_RESET) &&
+(pmcsr & PCI_PM_CTRL_STATE_MASK) == 3) {
+return true;
+}
+}
+
+return false;
+}
+
static void virtio_pci_bus_reset_hold(Object *obj)
{
PCIDevice *dev = PCI_DEVICE(obj);
DeviceState *qdev = DEVICE(obj);
+if (device_no_need_reset(dev)) {
+return;
+}
+
virtio_pci_reset(qdev);
if (pci_is_express(dev)) {
+uint16_t val = 0;
+VirtIOPCIProxy *proxy = VIRTIO_PCI(dev);
+
pcie_cap_deverr_reset(dev);
pcie_cap_lnkctl_reset(dev);
-pci_set_word(dev->config + dev->exp.pm_cap + PCI_PM_CTRL, 0);
+if (proxy->flags & VIRTIO_PCI_FLAG_PM_NO_SOFT_RESET) {
+val |= PCI_PM_CTRL_NO_SOFT_RESET;
+}
+pci_set_word(dev->config + dev->exp.pm_cap + PCI_PM_CTRL, val);
}
}
@@ -2297,6 +2331,8 @@ static Property virtio_pci_properties[] = {
VIRTIO_PCI_FLAG_INIT_LNKCTL_BIT, true),
DEFINE_PROP_BIT("x-pcie-pm-init", VirtIOPCIProxy, flags,
VIRTIO_PCI_FLAG_INIT_PM_BIT, true),
+DEFINE_PROP_BIT("x-pcie-pm-no-soft-reset", VirtIOPCIProxy, flags,
+VIRTIO_PCI_FLAG_PM_NO_SOFT_RESET_BIT, false),
DEFINE_PROP_BIT("x-pcie-flr-init", VirtIOPCIProxy, flags,
VIRTIO_PCI_FLAG_INIT_FLR_BIT, true),
DEFINE_PROP_BIT("aer", VirtIOPCIProxy, flags,
diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
index 59d88018c16a..9e67ba38c748 100644
--- a/include/hw/virtio/virtio-pci.h
+++ b/include/hw/virtio/virtio-pci.h
@@ -43,6 +43,7 @@ enum {
VIRTIO_PCI_FLAG_INIT_FLR_BIT,
VIRTIO_PCI_FLAG_AER_BIT,
VIRTIO_PCI_FLAG_ATS_PAGE_ALIGNED_BIT,
+VIRTIO_PCI_FLAG_PM_NO_SOFT_RESET_BIT,
};
/* Need to activate work-arounds for buggy guests at vmstate load. */
@@ -79,6 +80,10 @@ enum {
/* Init Power Management */
#define VIRTIO_PCI_FLAG_INIT_PM (1 << VIRTIO_PCI_FLAG_INIT_PM_BIT)
+/* Init The No_Soft_Reset bit of Power Management */
+#define VIRTIO_PCI_FLAG_PM_NO_SOFT_RESET \
+ (1 << VIRTIO_PCI_FLAG_PM_NO_SOFT_RESET_BIT)
+
/* Init Function Level Reset capability */
#define VIRTIO_PCI_FLAG_INIT_FLR (1 << VIRTIO_PCI_FLAG_INIT_FLR_BIT)
--
2.34.1
[RFC QEMU PATCH v7 0/1] S3 support
Hi all, This is the v7 patch to support S3. v7 makes below changes: * Tested this patch with Qemu on Xen hypervisor. Depending on kernel patch (virtio: Add support for no-reset virtio PCI PM: https://lore.kernel.org/lkml/20231208070754.3132339-1-steve...@chromium.org/) * Changed the default value of flag VIRTIO_PCI_FLAG_PM_NO_SOFT_RESET_BIT to false * Fixed coding style violation * Modified the content of the comments. * Removed useless flag PCI_PM_CTRL_DATA_SCALE_MASK. Best regards, Jiqian Chen V6: In current code, when guest does S3, virtio devices are reset during that process, that causes the display resources of virtio-gpu are destroyed, then the display can't come back after resuming. This v6 patch implement the No_Soft_Reset bit of PCI_PM_CTRL register, when this bit is set, the resetting will not be done, so that the display can work after resuming. This version abandons all previous version implementations and is a new different solution according to the outcome of the discussion and suggestions in the mailing thread of virtio-spec. (https://lists.oasis-open.org/archives/virtio-comment/202401/msg00077.html) V5: Hi all, v5 makes below changes: * Since this series patches add a new mechanism that let virtgpu and Qemu can negotiate their reset behavior, and other guys hope me can improve this mechanism to virtio pci level, so that other virtio devices can also benefit from it. So instead of adding new feature flag VIRTIO_GPU_F_FREEZE_S3 only serves for virtgpu, v5 add a new parameter named freeze_mode to struct VirtIODevice, when guest begin suspending, set freeze_mode to VIRTIO_PCI_FREEZE_MODE_FREEZE_S3, and then all virtio devices can get this status, and notice that guest is suspending, then they can change their reset behavior . See the new commit "virtio-pci: Add freeze_mode case for virtio pci" * The second commit is just for virtgpu, when freeze_mode is VIRTIO_PCI_FREEZE_MODE_FREEZE_S3, prevent Qemu destroying render resources, so that the display can come back after resuming. V5 of kernel patch: https://lore.kernel.org/lkml/20230919104607.2282248-1-jiqian.c...@amd.com/T/#t The link to trace this issue: https://gitlab.com/qemu-project/qemu/-/issues/1860 v4: Hi all, Thanks for Gerd Hoffmann's advice. V4 makes below changes: * Use enum for freeze mode, so this can be extended with more modes in the future. * Rename functions and paratemers with "_S3" postfix. And no functional changes. Link: https://lore.kernel.org/qemu-devel/20230720120816.8751-1-jiqian.c...@amd.com/ No v4 patch on kernel side. v3: Hi all, Thanks for Michael S. Tsirkin's advice. V3 makes below changes: * Remove changes in file include/standard-headers/linux/virtio_gpu.h I am not supposed to edit this file and it will be imported after the patches of linux kernel was merged. Link: https://lore.kernel.org/qemu-devel/20230719074726.1613088-1-jiqian.c...@amd.com/T/#t V3 of kernel patch: https://lore.kernel.org/lkml/20230720115805.8206-1-jiqian.c...@amd.com/T/#t v2: makes below changes: * Change VIRTIO_CPU_CMD_STATUS_FREEZING to 0x0400 (<0x1000) * Add virtio_gpu_device_unrealize to destroy resources to solve potential memory leak problem. This also needs hot-plug support. * Add a new feature flag VIRTIO_GPU_F_FREEZING, so that guest and host can negotiate whenever freezing is supported or not. Link: https://lore.kernel.org/qemu-devel/20230630070016.841459-1-jiqian.c...@amd.com/T/#t V2 of kernel patch: https://lore.kernel.org/lkml/20230630073448.842767-1-jiqian.c...@amd.com/T/#t v1: Hi all, I am working to implement virtgpu S3 function on Xen. Currently on Xen, if we start a guest who enables virtgpu, and then run "echo mem > /sys/power/state" to suspend guest. And run "sudo xl trigger s3resume" to resume guest. We can find that the guest kernel comes back, but the display doesn't. It just shown a black screen. Through reading codes, I founded that when guest was during suspending, it called into Qemu to call virtio_gpu_gl_reset. In virtio_gpu_gl_reset, it destroyed all resources and reset renderer. This made the display gone after guest resumed. I think we should keep resources or prevent they being destroyed when guest is suspending. So, I add a new status named freezing to virtgpu, and add a new ctrl message VIRTIO_GPU_CMD_STATUS_FREEZING to get notification from guest. If freezing is set to true, and then Qemu will realize that guest is suspending, it will not destroy resources and will not reset renderer. If freezing is set to false, Qemu will do its origin actions, and has no other impaction. And now, display can come back and applications can continue their status after guest resumes. Link: https://lore.kernel.org/qemu-devel/20230608025655.1674357-1-jiqian.c...@amd.com/ V1 of kernel patch: https://lore.kernel.org/lkml/20230608063857.1677973-1-jiqian.c...@amd.com/ Jiqian Chen (1): virtio-pci: implement No_Soft_Reset bit hw/virtio/virtio-pci.c
Re: [PATCH] chardev/char-win-stdio: Fix keyboard input after exit Qemu on
Hi
On Sun, Mar 24, 2024 at 7:23 PM Irina Ryapolova
wrote:
>
> After exit Qemu need to return the terminal to the default state.
>
> Signed-off-by: Irina Ryapolova
> ---
> chardev/char-win-stdio.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/chardev/char-win-stdio.c b/chardev/char-win-stdio.c
> index 1a18999e78..4fa2c3de8b 100644
> --- a/chardev/char-win-stdio.c
> +++ b/chardev/char-win-stdio.c
> @@ -220,6 +220,7 @@ err1:
> static void char_win_stdio_finalize(Object *obj)
> {
> WinStdioChardev *stdio = WIN_STDIO_CHARDEV(obj);
> +DWORD dwMode;
>
> if (stdio->hInputReadyEvent != INVALID_HANDLE_VALUE) {
> CloseHandle(stdio->hInputReadyEvent);
> @@ -230,6 +231,10 @@ static void char_win_stdio_finalize(Object *obj)
> if (stdio->hInputThread != INVALID_HANDLE_VALUE) {
> TerminateThread(stdio->hInputThread, 0);
> }
> +
> +GetConsoleMode(stdio->hStdIn, );
> +dwMode &= ~ENABLE_VIRTUAL_TERMINAL_INPUT;
> +SetConsoleMode(stdio->hStdIn, dwMode);
I'd suggest saving the mode when opening instead, to make sure we
restore the same value.
thanks
--
Marc-André Lureau
Re: [PATCH v2] contrib/plugins/execlog: Fix compiler warning
On 3/25/24 10:06, Yao Xingtao wrote:
1. The g_pattern_match_string() is deprecated when glib2 version >= 2.70.
Use g_pattern_spec_match_string() instead to avoid this problem.
2. The type of second parameter in g_ptr_array_add() is
'gpointer' {aka 'void *'}, but the type of reg->name is 'const char*'.
Cast the type of reg->name to 'gpointer' to avoid this problem.
compiler warning message:
/root/qemu/contrib/plugins/execlog.c:330:17: warning: ‘g_pattern_match_string’
is deprecated: Use 'g_pattern_spec_match_string'
instead [-Wdeprecated-declarations]
330 | if (g_pattern_match_string(pat, rd->name) ||
| ^~
In file included from /usr/include/glib-2.0/glib.h:67,
from /root/qemu/contrib/plugins/execlog.c:9:
/usr/include/glib-2.0/glib/gpattern.h:57:15: note: declared here
57 | gboolean g_pattern_match_string (GPatternSpec *pspec,
| ^~
/root/qemu/contrib/plugins/execlog.c:331:21: warning: ‘g_pattern_match_string’
is deprecated: Use 'g_pattern_spec_match_string'
instead [-Wdeprecated-declarations]
331 | g_pattern_match_string(pat, rd_lower)) {
| ^~
/usr/include/glib-2.0/glib/gpattern.h:57:15: note: declared here
57 | gboolean g_pattern_match_string (GPatternSpec *pspec,
| ^~
/root/qemu/contrib/plugins/execlog.c:339:63: warning: passing argument 2 of
‘g_ptr_array_add’ discards ‘const’ qualifier from pointer target type
[-Wdiscarded-qualifiers]
339 | g_ptr_array_add(all_reg_names, reg->name);
|~~~^~
In file included from /usr/include/glib-2.0/glib.h:33:
/usr/include/glib-2.0/glib/garray.h:198:62: note: expected
‘gpointer’ {aka ‘void *’} but argument is of type ‘const char *’
198 |gpointer data);
|~~^~~~
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2210
Signed-off-by: Yao Xingtao
---
contrib/plugins/execlog.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
index a1dfd59ab7..09654910ee 100644
--- a/contrib/plugins/execlog.c
+++ b/contrib/plugins/execlog.c
@@ -327,8 +327,13 @@ static GPtrArray *registers_init(int vcpu_index)
for (int p = 0; p < rmatches->len; p++) {
g_autoptr(GPatternSpec) pat =
g_pattern_spec_new(rmatches->pdata[p]);
g_autofree gchar *rd_lower = g_utf8_strdown(rd->name, -1);
+#if GLIB_CHECK_VERSION(2, 70, 0)
+if (g_pattern_spec_match_string(pat, rd->name) ||
+g_pattern_spec_match_string(pat, rd_lower)) {
+#else
if (g_pattern_match_string(pat, rd->name) ||
g_pattern_match_string(pat, rd_lower)) {
+#endif
Register *reg = init_vcpu_register(rd);
g_ptr_array_add(registers, reg);
As suggested by Peter on previous version, you can declare a new
function `g_pattern_match_string_qemu` in include/glib-compat.h which
abstract this.
You'll need to add include/ to Makefile as well, so glib-compat.h will
be accessible to contrib plugins too.
@@ -336,7 +341,7 @@ static GPtrArray *registers_init(int vcpu_index)
if (disas_assist) {
g_mutex_lock(_reg_name_lock);
if (!g_ptr_array_find(all_reg_names, reg->name,
NULL)) {
-g_ptr_array_add(all_reg_names, reg->name);
+g_ptr_array_add(all_reg_names,
(gpointer)reg->name);
}
g_mutex_unlock(_reg_name_lock);
}
Re: [PATCH 2/3] target/hppa: mask offset bits in gva
Richard Henderson writes: > On 3/24/24 08:41, Sven Schnelle wrote: >> 7f09e0: val=000fffb0301f r2=110e0f01 r1=01fff600 >> phys=fffb 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW) >> 'val' is the value constructed from IOR/ISR, > > Is this byte swapped in some weird way? I do not see how 'val' > corresponds to any of the addresses we're talking about. From here, > the string "301f" appears to an out-of-context grep hit. It's just both values combined together, where the 301f is basically the ISR content. It's not a out of context grep - the real machines i have are constructing the same value, and the same offset into the pagetable. I verified that by patching the DTLB miss handler in HPUX to write the ISR/IOR and calulated pagetable offset/value to PAGE0 and looked with the kernel debugger at the values.
Re: [PATCH v2 6/6] tests/qtest: Add tests for the STM32L4x5 USART
Hi!
On 24/03/2024 17.55, Arnaud Minier wrote:
Test:
- read/write from/to the usart registers
- send/receive a character/string over the serial port
Signed-off-by: Arnaud Minier
Signed-off-by: Inès Varhol
---
tests/qtest/meson.build| 3 +-
tests/qtest/stm32l4x5_usart-test.c | 326 +
2 files changed, 328 insertions(+), 1 deletion(-)
create mode 100644 tests/qtest/stm32l4x5_usart-test.c
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 36c5c13a7b..e0d72ee91e 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -205,7 +205,8 @@ qtests_stm32l4x5 = \
['stm32l4x5_exti-test',
'stm32l4x5_syscfg-test',
'stm32l4x5_rcc-test',
- 'stm32l4x5_gpio-test']
+ 'stm32l4x5_gpio-test',
+ 'stm32l4x5_usart-test']
We are now using timeouts from the meson test harneess in meson.build, too,
see the slow_qtests[] at the beginning of that file.
You seem to be using a 10 minutes timeout in your test below
(usart_wait_for_flag() function), but you didn't adjust the meson timeout
setting in meson.build, so this does not quite match...
How long does your test take on a very loaded machine (with --enable-debug
used)? If it could take more than 30 seconds, you need to adjust the timeout
in meson.build, too. If it is running very fast, you should likely adjust
the 10 minutes timeout in usart_wait_for_flag() to < 30 seconds instead to
match the meson timeout setting.
qtests_arm = \
(config_all_devices.has_key('CONFIG_MPS2') ? ['sse-timer-test'] : []) + \
diff --git a/tests/qtest/stm32l4x5_usart-test.c
b/tests/qtest/stm32l4x5_usart-test.c
new file mode 100644
index 00..2d42f053f6
--- /dev/null
+++ b/tests/qtest/stm32l4x5_usart-test.c
@@ -0,0 +1,326 @@
+/*
+ * QTest testcase for STML4X5_USART
+ *
+ * Copyright (c) 2023 Arnaud Minier
+ * Copyright (c) 2023 Inès Varhol
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "hw/misc/stm32l4x5_rcc_internals.h"
+#include "hw/registerfields.h"
+
+#define RCC_BASE_ADDR 0x40021000
+/* Use USART 1 ADDR, assume the others work the same */
+#define USART1_BASE_ADDR 0x40013800
+
+/* See stm32l4x5_usart for definitions */
+REG32(CR1, 0x00)
+FIELD(CR1, M1, 28, 1)
+FIELD(CR1, OVER8, 15, 1)
+FIELD(CR1, M0, 12, 1)
+FIELD(CR1, PCE, 10, 1)
+FIELD(CR1, TXEIE, 7, 1)
+FIELD(CR1, RXNEIE, 5, 1)
+FIELD(CR1, TE, 3, 1)
+FIELD(CR1, RE, 2, 1)
+FIELD(CR1, UE, 0, 1)
+REG32(CR2, 0x04)
+REG32(CR3, 0x08)
+FIELD(CR3, OVRDIS, 12, 1)
+REG32(BRR, 0x0C)
+REG32(GTPR, 0x10)
+REG32(RTOR, 0x14)
+REG32(RQR, 0x18)
+REG32(ISR, 0x1C)
+FIELD(ISR, TXE, 7, 1)
+FIELD(ISR, RXNE, 5, 1)
+FIELD(ISR, ORE, 3, 1)
+REG32(ICR, 0x20)
+REG32(RDR, 0x24)
+REG32(TDR, 0x28)
+
+#define NVIC_ISPR1 0XE000E204
+#define NVIC_ICPR1 0xE000E284
+#define USART1_IRQ 37
+
+static bool check_nvic_pending(QTestState *qts, unsigned int n)
+{
+/* No USART interrupts are less than 32 */
+assert(n > 32);
+n -= 32;
+return qtest_readl(qts, NVIC_ISPR1) & (1 << n);
+}
+
+static bool clear_nvic_pending(QTestState *qts, unsigned int n)
+{
+/* No USART interrupts are less than 32 */
+assert(n > 32);
+n -= 32;
+qtest_writel(qts, NVIC_ICPR1, (1 << n));
+return true;
I'd suggest to change the return type to "void" and drop the "return true" here.
+}
+
+/*
+ Tests should never need to sleep(), because while it might be plenty of time
on a
+ fast development machine, it can cause intermittent failures due
+ to timeouts if the test is on some heavily-loaded slow CI runner.
+ */
+static bool usart_wait_for_flag(QTestState *qts, uint32_t event_addr, uint32_t
flag)
+{
+/* Wait at most 10 minutes */
+for (int i = 0; i < 60; i++) {
+if ((qtest_readl(qts, event_addr) & flag)) {
+return true;
+}
+g_usleep(1000);
As I recently learnt again, some systems (like some BSD kernels) still use a
time slice resolution of 10 ms, so it might be better to g_usleep(1) and
adjust the loop counter to a value that is 10 times less instead, otherwise
your loop might 100 minutes instead of 10 minutes in the worst case instead.
+}
+
+return false;
+}
Thomas
Re: [PATCH v4 2/2] vhost: Perform memory section dirty scans once per iteration
On Sat, Mar 23, 2024 at 5:14 AM Si-Wei Liu wrote:
>
>
>
> On 3/21/2024 10:08 PM, Jason Wang wrote:
> > On Fri, Mar 22, 2024 at 5:43 AM Si-Wei Liu wrote:
> >>
> >>
> >> On 3/20/2024 8:56 PM, Jason Wang wrote:
> >>> On Thu, Mar 21, 2024 at 5:03 AM Si-Wei Liu wrote:
>
> On 3/19/2024 8:27 PM, Jason Wang wrote:
> > On Tue, Mar 19, 2024 at 6:16 AM Si-Wei Liu
> > wrote:
> >> On 3/17/2024 8:22 PM, Jason Wang wrote:
> >>> On Sat, Mar 16, 2024 at 2:45 AM Si-Wei Liu
> >>> wrote:
> On 3/14/2024 9:03 PM, Jason Wang wrote:
> > On Fri, Mar 15, 2024 at 5:39 AM Si-Wei Liu
> > wrote:
> >> On setups with one or more virtio-net devices with vhost on,
> >> dirty tracking iteration increases cost the bigger the number
> >> amount of queues are set up e.g. on idle guests migration the
> >> following is observed with virtio-net with vhost=on:
> >>
> >> 48 queues -> 78.11% [.] vhost_dev_sync_region.isra.13
> >> 8 queues -> 40.50% [.] vhost_dev_sync_region.isra.13
> >> 1 queue -> 6.89% [.] vhost_dev_sync_region.isra.13
> >> 2 devices, 1 queue -> 18.60% [.] vhost_dev_sync_region.isra.14
> >>
> >> With high memory rates the symptom is lack of convergence as soon
> >> as it has a vhost device with a sufficiently high number of queues,
> >> the sufficient number of vhost devices.
> >>
> >> On every migration iteration (every 100msecs) it will redundantly
> >> query the *shared log* the number of queues configured with vhost
> >> that exist in the guest. For the virtqueue data, this is necessary,
> >> but not for the memory sections which are the same. So essentially
> >> we end up scanning the dirty log too often.
> >>
> >> To fix that, select a vhost device responsible for scanning the
> >> log with regards to memory sections dirty tracking. It is selected
> >> when we enable the logger (during migration) and cleared when we
> >> disable the logger. If the vhost logger device goes away for some
> >> reason, the logger will be re-selected from the rest of vhost
> >> devices.
> >>
> >> After making mem-section logger a singleton instance, constant cost
> >> of 7%-9% (like the 1 queue report) will be seen, no matter how many
> >> queues or how many vhost devices are configured:
> >>
> >> 48 queues -> 8.71%[.] vhost_dev_sync_region.isra.13
> >> 2 devices, 8 queues -> 7.97% [.] vhost_dev_sync_region.isra.14
> >>
> >> Co-developed-by: Joao Martins
> >> Signed-off-by: Joao Martins
> >> Signed-off-by: Si-Wei Liu
> >>
> >> ---
> >> v3 -> v4:
> >>- add comment to clarify effect on cache locality and
> >> performance
> >>
> >> v2 -> v3:
> >>- add after-fix benchmark to commit log
> >>- rename vhost_log_dev_enabled to vhost_dev_should_log
> >>- remove unneeded comparisons for backend_type
> >>- use QLIST array instead of single flat list to store vhost
> >> logger devices
> >>- simplify logger election logic
> >> ---
> >> hw/virtio/vhost.c | 67
> >> ++-
> >> include/hw/virtio/vhost.h | 1 +
> >> 2 files changed, 62 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> >> index 612f4db..58522f1 100644
> >> --- a/hw/virtio/vhost.c
> >> +++ b/hw/virtio/vhost.c
> >> @@ -45,6 +45,7 @@
> >>
> >> static struct vhost_log *vhost_log[VHOST_BACKEND_TYPE_MAX];
> >> static struct vhost_log
> >> *vhost_log_shm[VHOST_BACKEND_TYPE_MAX];
> >> +static QLIST_HEAD(, vhost_dev)
> >> vhost_log_devs[VHOST_BACKEND_TYPE_MAX];
> >>
> >> /* Memslots used by backends that support private memslots
> >> (without an fd). */
> >> static unsigned int used_memslots;
> >> @@ -149,6 +150,47 @@ bool vhost_dev_has_iommu(struct vhost_dev
> >> *dev)
> >> }
> >> }
> >>
> >> +static inline bool vhost_dev_should_log(struct vhost_dev *dev)
> >> +{
> >> +assert(dev->vhost_ops);
> >> +assert(dev->vhost_ops->backend_type >
> >> VHOST_BACKEND_TYPE_NONE);
> >> +assert(dev->vhost_ops->backend_type < VHOST_BACKEND_TYPE_MAX);
> >> +
> >> +return dev ==
> >> QLIST_FIRST(_log_devs[dev->vhost_ops->backend_type]);
> > A dumb question, why not simple check
> >
> > dev->log ==
[PATCH v2] contrib/plugins/execlog: Fix compiler warning
1. The g_pattern_match_string() is deprecated when glib2 version >= 2.70.
Use g_pattern_spec_match_string() instead to avoid this problem.
2. The type of second parameter in g_ptr_array_add() is
'gpointer' {aka 'void *'}, but the type of reg->name is 'const char*'.
Cast the type of reg->name to 'gpointer' to avoid this problem.
compiler warning message:
/root/qemu/contrib/plugins/execlog.c:330:17: warning: ‘g_pattern_match_string’
is deprecated: Use 'g_pattern_spec_match_string'
instead [-Wdeprecated-declarations]
330 | if (g_pattern_match_string(pat, rd->name) ||
| ^~
In file included from /usr/include/glib-2.0/glib.h:67,
from /root/qemu/contrib/plugins/execlog.c:9:
/usr/include/glib-2.0/glib/gpattern.h:57:15: note: declared here
57 | gboolean g_pattern_match_string (GPatternSpec *pspec,
| ^~
/root/qemu/contrib/plugins/execlog.c:331:21: warning: ‘g_pattern_match_string’
is deprecated: Use 'g_pattern_spec_match_string'
instead [-Wdeprecated-declarations]
331 | g_pattern_match_string(pat, rd_lower)) {
| ^~
/usr/include/glib-2.0/glib/gpattern.h:57:15: note: declared here
57 | gboolean g_pattern_match_string (GPatternSpec *pspec,
| ^~
/root/qemu/contrib/plugins/execlog.c:339:63: warning: passing argument 2 of
‘g_ptr_array_add’ discards ‘const’ qualifier from pointer target type
[-Wdiscarded-qualifiers]
339 | g_ptr_array_add(all_reg_names, reg->name);
|~~~^~
In file included from /usr/include/glib-2.0/glib.h:33:
/usr/include/glib-2.0/glib/garray.h:198:62: note: expected
‘gpointer’ {aka ‘void *’} but argument is of type ‘const char *’
198 |gpointer data);
|~~^~~~
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2210
Signed-off-by: Yao Xingtao
---
contrib/plugins/execlog.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
index a1dfd59ab7..09654910ee 100644
--- a/contrib/plugins/execlog.c
+++ b/contrib/plugins/execlog.c
@@ -327,8 +327,13 @@ static GPtrArray *registers_init(int vcpu_index)
for (int p = 0; p < rmatches->len; p++) {
g_autoptr(GPatternSpec) pat =
g_pattern_spec_new(rmatches->pdata[p]);
g_autofree gchar *rd_lower = g_utf8_strdown(rd->name, -1);
+#if GLIB_CHECK_VERSION(2, 70, 0)
+if (g_pattern_spec_match_string(pat, rd->name) ||
+g_pattern_spec_match_string(pat, rd_lower)) {
+#else
if (g_pattern_match_string(pat, rd->name) ||
g_pattern_match_string(pat, rd_lower)) {
+#endif
Register *reg = init_vcpu_register(rd);
g_ptr_array_add(registers, reg);
@@ -336,7 +341,7 @@ static GPtrArray *registers_init(int vcpu_index)
if (disas_assist) {
g_mutex_lock(_reg_name_lock);
if (!g_ptr_array_find(all_reg_names, reg->name, NULL))
{
-g_ptr_array_add(all_reg_names, reg->name);
+g_ptr_array_add(all_reg_names,
(gpointer)reg->name);
}
g_mutex_unlock(_reg_name_lock);
}
--
2.37.3
Re: [PATCH] virtio_net: Do not send RSS key if it is not supported
On Fri, 22 Mar 2024 03:21:21 -0700, Breno Leitao wrote:
> Hello Xuan,
>
> On Fri, Mar 22, 2024 at 10:00:22AM +0800, Xuan Zhuo wrote:
> > On Thu, 21 Mar 2024 09:54:30 -0700, Breno Leitao wrote:
>
> > > 4) Since the command above does not have a key, then the last
> > >scatter-gatter entry will be zeroed, since rss_key_size == 0.
> > > sg_buf_size = vi->rss_key_size;
> >
> >
> >
> > if (vi->has_rss || vi->has_rss_hash_report) {
> > vi->rss_indir_table_size =
> > virtio_cread16(vdev, offsetof(struct virtio_net_config,
> > rss_max_indirection_table_length));
> > vi->rss_key_size =
> > virtio_cread8(vdev, offsetof(struct virtio_net_config,
> > rss_max_key_size));
> >
> > vi->rss_hash_types_supported =
> > virtio_cread32(vdev, offsetof(struct virtio_net_config,
> > supported_hash_types));
> > vi->rss_hash_types_supported &=
> > ~(VIRTIO_NET_RSS_HASH_TYPE_IP_EX |
> > VIRTIO_NET_RSS_HASH_TYPE_TCP_EX |
> > VIRTIO_NET_RSS_HASH_TYPE_UDP_EX);
> >
> > dev->hw_features |= NETIF_F_RXHASH;
> > }
> >
> >
> > vi->rss_key_size is initiated here, I wonder if there is something wrong?
>
> Not really, the code above is never executed (in my machines). This is
> because `vi->has_rss` and `vi->has_rss_hash_report` are both unset.
>
> Looking further, vdev does not have the VIRTIO_NET_F_RSS and
> VIRTIO_NET_F_HASH_REPORT features.
>
> Also, when I run `ethtool -x`, I got:
>
> # ethtool -x eth0
> RX flow hash indirection table for eth0 with 1 RX ring(s):
> Operation not supported
> RSS hash key:
> Operation not supported
> RSS hash function:
> toeplitz: on
> xor: off
> crc32: off
The spec saies:
Note that if the device offers VIRTIO_NET_F_HASH_REPORT, even if it
supports only one pair of virtqueues, it MUST support at least one of
commands of VIRTIO_NET_CTRL_MQ class to configure reported hash
parameters:
If the device offers VIRTIO_NET_F_RSS, it MUST support
VIRTIO_NET_CTRL_MQ_RSS_CONFIG command per 5.1.6.5.7.1.
Otherwise the device MUST support VIRTIO_NET_CTRL_MQ_HASH_CONFIG command
per 5.1.6.5.6.4.
So if we have not anyone of `vi->has_rss` and `vi->has_rss_hash_report`,
we should return from virtnet_set_rxfh directly.
Thanks.
