On Tue, 13 Oct 2026 at 09:40, Peter Maydell <[email protected]> wrote:
>
> The IRS_VMAP_L2_VMTR register allows the guest to make a level 2 VM
> table valid. It's similar to what the IRS_MAP_L2_ISTR register does
> for the IST, but it's 64 bits, and so it is R/W and has an M bit for
> "actually do the operation" so that the guest can write it as two 32
> bit halves. Since we don't cache anything yet, all the "make valid"
> operation needs to do is write to the VALID bit in the L1 table entry
> in guest memory.
>
> Signed-off-by: Peter Maydell <[email protected]>
> +static void irs_vmap_l2_vmtr_write(GICv5 *s, GICv5Domain domain, uint64_t
> value)
> +{
> + GICv5Common *cs = ARM_GICV5_COMMON(s);
> + GICv5VMTConfig *cfg = &s->vmt_config[domain];
> + uint32_t vmid = FIELD_EX64(value, IRS_VMAP_L2_VMTR, VM_ID);
> + bool m = FIELD_EX64(value, IRS_VMAP_L2_VMTR, M);
> + hwaddr l1_addr;
> + uint64_t l1_vmte;
> + MemTxResult res;
> +
> + if (!gicv5_domain_has_virt(cs, domain) || !cfg->valid ||
> !cfg->structure) {
> + /*
> + * The whole register is RAZ/WI if no virt, or if the VM
> + * table is invalid or not 2 level.
> + */
> + return;
> + }
> +
> + /*
> + * Otherwise, write to the register. The M bit is WO/UNKNOWN,
> + * and we make sure it is 0, so that a 32-bit write to the low
> + * half of the register never appears to have the M bit set.
> + */
> + cs->irs_vmap_l2_vmtr[domain] = value & ~R_IRS_VMAP_L2_VMTR_M_MASK;
> +
> + if (!m || vmid >= (1 << cfg->vm_id_bits)) {
> + /*
> + * If guest didn't actually ask to make the L2 table valid, or
> + * the ID is out of range, do nothing except the register update.
> + */
> + return;
> + }
> +
> + /* Find the relevant L1 VMTE and set its VALID bit */
> + l1_addr = l1_vmte_addr(cs, cfg, vmid);
> +
> + l1_vmte = address_space_ldq_le(&cs->dma_as, l1_addr, cfg->txattrs, &res);
> + if (res != MEMTX_OK) {
> + goto txfail;
> + }
> +
> + l1_vmte = FIELD_DP64(l1_vmte, L1_VMTE, VALID, 1);
This is missing a check for "is the VALID bit already set in the L1_VMTE?"
(which per the spec is supposed to be a "has no effect" situation). In this
patch the lack of a check is harmless, but when we add the code later to
cache VM table entries we'll fall through into that and incorrectly
corrupt the cached VM table entries by re-initializing them.
> +
> + address_space_stq_le(&cs->dma_as, l1_addr, l1_vmte, cfg->txattrs, &res);
> + if (res != MEMTX_OK) {
> + goto txfail;
> + }
> +
> + return;
> +
> +txfail:
> + /* Reportable with EC=0x0 if sw error reporting implemented */
> + qemu_log_mask(LOG_GUEST_ERROR, "L1 VMTE update failed for VMID 0x%x at "
> + "physical address 0x" HWADDR_FMT_plx "\n", vmid, l1_addr);
> +}
> +
-- PMM
