On 3/9/21 6:29 PM, Rebecca Cran wrote:
+void tlb_flush_page_range_by_mmuidx(CPUState *cpu, target_ulong addr,
+ unsigned int num_pages, uint16_t
idxmap)
I am not keen on this interface. I think you should take either
start+end addresses (inclusive) or start+length (in bytes).
Using num_pages, and as an unsigned int, seems too easy to fail when
applied to a different guest.
+{
+ /*
+ * We currently do a full flush, but for performance this should be
+ * updated to only flush the requested pages, taking TBI into account.
+ */
+ tlb_flush_by_mmuidx(cpu, idxmap);
+}
And if you're going to cop-out like this, you might as well just do it
in target/arm and not add these new interfaces at all.
+#ifdef TARGET_AARCH64
+static unsigned int tlbi_aa64_range_get_num_pages(CPUARMState *env,
+ uint64_t value,
+ uint64_t addr)
+{
+ unsigned int page_size;
+ unsigned int page_shift;
+ unsigned int page_size_granule;
+ uint64_t num;
+ uint64_t scale;
+ uint64_t exponent;
+ uint64_t high_addr;
+
+ num = (value >> 39) & 0xF;
+ scale = (value >> 44) & 0x3;
+ page_size_granule = (value >> 46) & 0x3;
extract64()
+
+ switch (page_size_granule) {
+ case 1:
+ page_size = 4096;
+ page_shift = 12;
+ break;
+ case 2:
+ page_size = 16384;
+ page_shift = 14;
+ break;
+ case 3:
+ page_size = 65536;
+ page_shift = 16;
+ break;
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR, "Invalid page size granule %d\n",
+ page_size_granule);
+
+ raise_exception(env, EXCP_UDEF, syn_uncategorized(),
+ exception_target_el(env));
You can't raise an exception from here, because you don't have all of
the context for unwinding the tcg state. Which you cannot access from
within the callback of an ARMCPRegInfo.
The manual says that if TG does not correspond to the granule size of
the actual translation then "the architecture does not require that the
instruction invalidates any entries". "Reserved" can be safely assumed
to "not correspond", so I think you could just as easily return 0 here,
after logging the guest error.
+ high_addr = addr + (((num + 1) << exponent) * page_size);
+
+ return (high_addr - addr) >> page_shift;
I'll note that it would be much easier for you to return a byte value
for the length, and that you don't actually need to pass in addr at all.
+ uint64_t addr = (value & 0xFFFFFFFFFUL) << TARGET_PAGE_BITS;
The manual does not explicitly say, but I'm certain that this should be
a signed address, when regime_has_2_ranges(). Otherwise it would be
impossible to flush a range of kernel addresses.
But all of this is moot if we're just going to flush all pages. At
which point you might as well simply re-use tlbi_aa64_vmalle1_write et
al. Place your TODO comment in front of tlbirange_reginfo[] instead of
buried n-levels further down.