On 6/14/24 14:14, Don Porter wrote:
On 6/7/24 1:43 PM, Richard Henderson wrote:
On 6/6/24 07:02, Don Porter wrote:
+/**
+ * get_pte - Copy the contents of the page table entry at node[i] into
pt_entry.
+ * Optionally, add the relevant bits to the virtual address in
+ * vaddr_pte.
+ *
+ * @cs - CPU state
+ * @node - physical address of the current page table node
+ * @i - index (in page table entries, not bytes) of the page table
+ * entry, within node
+ * @height - height of node within the tree (leaves are 1, not 0)
+ * @pt_entry - Poiter to a PTE_t, stores the contents of the page table entry
+ * @vaddr_parent - The virtual address bits already translated in walking the
+ * page table to node. Optional: only used if vaddr_pte is
set.
+ * @vaddr_pte - Optional pointer to a variable storing the virtual address bits
+ * translated by node[i].
+ * @pte_paddr - Pointer to the physical address of the PTE within node.
+ * Optional parameter.
+ */
+void
+x86_get_pte(CPUState *cs, hwaddr node, int i, int height,
+ PTE_t *pt_entry, vaddr vaddr_parent, vaddr *vaddr_pte,
+ hwaddr *pte_paddr)
+
+{
+ X86CPU *cpu = X86_CPU(cs);
+ CPUX86State *env = &cpu->env;
+ int32_t a20_mask = x86_get_a20_mask(env);
+ hwaddr pte;
+
+ if (env->hflags & HF_LMA_MASK) {
+ /* 64 bit */
+ int pte_width = 8;
+ pte = (node + (i * pte_width)) & a20_mask;
+ pt_entry->pte64_t = address_space_ldq(cs->as, pte,
+ MEMTXATTRS_UNSPECIFIED, NULL);
+ } else {
+ /* 32 bit */
+ int pte_width = 4;
+ pte = (node + (i * pte_width)) & a20_mask;
+ pt_entry->pte32_t = address_space_ldl(cs->as, pte,
+ MEMTXATTRS_UNSPECIFIED, NULL);
+ }
+
+ if (vaddr_pte) {
+ int shift = 0;
+ _mmu_decode_va_parameters(cs, height, &shift, NULL);
+ *vaddr_pte = vaddr_parent | ((i & 0x1ffULL) << shift);
+ }
+
+ if (pte_paddr) {
+ *pte_paddr = pte;
+ }
+}
This fails to recurse with nested page tables, which definitely breaks the TCG
walker.
Hi Richard,
Thank you again for all of the advice and feedback.
What do you think the correct semantics should be for nested paging?
That it works like hardware does.
My understanding is that the current 'info mem' command on x86 does not recur on nested
page tables, but this does seem like a useful extension. Same for 'info tlb'.
My understanding is that the two existing hmp commands are broken, and that the *only*
correct page table walker is get_physical_address.
In the case of 'info pg', I might want to print each page table separately, rather than a
combined/shadow view.
Without doing the nested page translation for the page tables themselves, you're not
looking at the correct page from which to read the PTEs, and reading garbage.
My reading of the tcg code is that it also walks the guest page tables first, then uses
probe_access_full() to query the guest->host physical translation, but I may be
misunderstanding the code:
https://github.com/qemu/qemu/blob/046a64b9801343e2e89eef10c7a48eec8d8c0d4f/target/i386/tcg/sysemu/excp_helper.c#L432
I'm talking about this, for the page tables themselves:
https://github.com/qemu/qemu/blob/046a64b9801343e2e89eef10c7a48eec8d8c0d4f/target/i386/tcg/sysemu/excp_helper.c#L69
A better example is for ARM, which has had its debug page table walker folded
in:
https://github.com/qemu/qemu/blob/046a64b9801343e2e89eef10c7a48eec8d8c0d4f/target/arm/ptw.c#L553
If in_debug, don't use probe_access_full_mmu as a cache (and host page resolution), but
explicitly recurse on get_phys_addr_gpc.
r~
