Re: [PATCH 3/3] target/ppc: Check page dir/table base alignment
On 6/23/22 18:34, Richard Henderson wrote:
[E-MAIL EXTERNO] Não clique em links ou abra anexos, a menos que você
possa confirmar o remetente e saber que o conteúdo é seguro. Em caso de
e-mail suspeito entre imediatamente em contato com o DTI.
On 6/23/22 07:26, Leandro Lupori wrote:
On 6/21/22 18:26, Fabiano Rosas wrote:
[E-MAIL EXTERNO] Não clique em links ou abra anexos, a menos que você
possa confirmar o
remetente e saber que o conteúdo é seguro. Em caso de e-mail suspeito
entre
imediatamente em contato com o DTI.
Leandro Lupori writes:
Check if each page dir/table base address is properly aligned and
log a guest error if not, as real hardware behave incorrectly in
this case.
These checks are only performed when DEBUG_MMU is defined, to avoid
hurting the performance.
Signed-off-by: Leandro Lupori
---
target/ppc/mmu-radix64.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
index 2f0bcbfe2e..80d945a7c3 100644
--- a/target/ppc/mmu-radix64.c
+++ b/target/ppc/mmu-radix64.c
@@ -28,6 +28,8 @@
#include "mmu-radix64.h"
#include "mmu-book3s-v3.h"
+/* #define DEBUG_MMU */
+
static bool ppc_radix64_get_fully_qualified_addr(const CPUPPCState
*env,
vaddr eaddr,
uint64_t *lpid,
uint64_t *pid)
@@ -277,6 +279,16 @@ static int ppc_radix64_next_level(AddressSpace
*as, vaddr eaddr,
if (!(pde & R_PTE_LEAF)) { /* Prepare for next iteration */
++*level;
*nls = pde & R_PDE_NLS;
+
+#ifdef DEBUG_MMU
+ if ((pde & R_PDE_NLB) & MAKE_64BIT_MASK(0, *nls + 3)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: misaligned page dir/table base: 0x%"VADDR_PRIx
+ " page dir size: 0x%"PRIx64" level: %d\n",
+ __func__, (pde & R_PDE_NLB), BIT(*nls + 3), *level);
+ }
+#endif
Maybe use qemu_log_enabled() instead of the define? I think it is a
little more useful and has less chance to rot.
Ok. I wanted to avoid introducing any extra overhead, but I guess just
checking
qemu_log_enabled() should be ok.
No, qemu_log_enabled is *already* taken into account by qemu_log_mask.
Just drop the ifdefs.
Do you in fact need to raise an exception here?
Not in this case. I've tested it with KVM and it doesn't raise an
exception. It seems to just ignore PDE_NLB's bits lower than nls + 3.
Thanks,
Leandro
r~
Re: [PATCH 3/3] target/ppc: Check page dir/table base alignment
On 6/23/22 07:26, Leandro Lupori wrote:
On 6/21/22 18:26, Fabiano Rosas wrote:
[E-MAIL EXTERNO] Não clique em links ou abra anexos, a menos que você possa confirmar o
remetente e saber que o conteúdo é seguro. Em caso de e-mail suspeito entre
imediatamente em contato com o DTI.
Leandro Lupori writes:
Check if each page dir/table base address is properly aligned and
log a guest error if not, as real hardware behave incorrectly in
this case.
These checks are only performed when DEBUG_MMU is defined, to avoid
hurting the performance.
Signed-off-by: Leandro Lupori
---
target/ppc/mmu-radix64.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
index 2f0bcbfe2e..80d945a7c3 100644
--- a/target/ppc/mmu-radix64.c
+++ b/target/ppc/mmu-radix64.c
@@ -28,6 +28,8 @@
#include "mmu-radix64.h"
#include "mmu-book3s-v3.h"
+/* #define DEBUG_MMU */
+
static bool ppc_radix64_get_fully_qualified_addr(const CPUPPCState *env,
vaddr eaddr,
uint64_t *lpid, uint64_t
*pid)
@@ -277,6 +279,16 @@ static int ppc_radix64_next_level(AddressSpace *as, vaddr
eaddr,
if (!(pde & R_PTE_LEAF)) { /* Prepare for next iteration */
++*level;
*nls = pde & R_PDE_NLS;
+
+#ifdef DEBUG_MMU
+ if ((pde & R_PDE_NLB) & MAKE_64BIT_MASK(0, *nls + 3)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: misaligned page dir/table base: 0x%"VADDR_PRIx
+ " page dir size: 0x%"PRIx64" level: %d\n",
+ __func__, (pde & R_PDE_NLB), BIT(*nls + 3), *level);
+ }
+#endif
Maybe use qemu_log_enabled() instead of the define? I think it is a
little more useful and has less chance to rot.
Ok. I wanted to avoid introducing any extra overhead, but I guess just checking
qemu_log_enabled() should be ok.
No, qemu_log_enabled is *already* taken into account by qemu_log_mask.
Just drop the ifdefs.
Do you in fact need to raise an exception here?
r~
Re: [PATCH 3/3] target/ppc: Check page dir/table base alignment
On 6/21/22 18:26, Fabiano Rosas wrote:
[E-MAIL EXTERNO] Não clique em links ou abra anexos, a menos que você possa
confirmar o remetente e saber que o conteúdo é seguro. Em caso de e-mail
suspeito entre imediatamente em contato com o DTI.
Leandro Lupori writes:
Check if each page dir/table base address is properly aligned and
log a guest error if not, as real hardware behave incorrectly in
this case.
These checks are only performed when DEBUG_MMU is defined, to avoid
hurting the performance.
Signed-off-by: Leandro Lupori
---
target/ppc/mmu-radix64.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
index 2f0bcbfe2e..80d945a7c3 100644
--- a/target/ppc/mmu-radix64.c
+++ b/target/ppc/mmu-radix64.c
@@ -28,6 +28,8 @@
#include "mmu-radix64.h"
#include "mmu-book3s-v3.h"
+/* #define DEBUG_MMU */
+
static bool ppc_radix64_get_fully_qualified_addr(const CPUPPCState *env,
vaddr eaddr,
uint64_t *lpid, uint64_t
*pid)
@@ -277,6 +279,16 @@ static int ppc_radix64_next_level(AddressSpace *as, vaddr
eaddr,
if (!(pde & R_PTE_LEAF)) { /* Prepare for next iteration */
++*level;
*nls = pde & R_PDE_NLS;
+
+#ifdef DEBUG_MMU
+if ((pde & R_PDE_NLB) & MAKE_64BIT_MASK(0, *nls + 3)) {
+qemu_log_mask(LOG_GUEST_ERROR,
+"%s: misaligned page dir/table base: 0x%"VADDR_PRIx
+" page dir size: 0x%"PRIx64" level: %d\n",
+__func__, (pde & R_PDE_NLB), BIT(*nls + 3), *level);
+}
+#endif
Maybe use qemu_log_enabled() instead of the define? I think it is a
little more useful and has less chance to rot.
Ok. I wanted to avoid introducing any extra overhead, but I guess just
checking qemu_log_enabled() should be ok.
+
index = eaddr >> (*psize - *nls); /* Shift */
index &= ((1UL << *nls) - 1); /* Mask */
*pte_addr = (pde & R_PDE_NLB) + (index * sizeof(pde));
@@ -297,6 +309,15 @@ static int ppc_radix64_walk_tree(AddressSpace *as, vaddr
eaddr,
return 1;
}
+#ifdef DEBUG_MMU
+if (base_addr & MAKE_64BIT_MASK(0, nls + 3)) {
+qemu_log_mask(LOG_GUEST_ERROR,
+"%s: misaligned page dir base: 0x%"VADDR_PRIx
+" page dir size: 0x%"PRIx64"\n",
+__func__, base_addr, BIT(nls + 3));
+}
+#endif
+
index = eaddr >> (*psize - nls);/* Shift */
index &= ((1UL << nls) - 1); /* Mask */
*pte_addr = base_addr + (index * sizeof(pde));
Re: [PATCH 3/3] target/ppc: Check page dir/table base alignment
Leandro Lupori writes:
> Check if each page dir/table base address is properly aligned and
> log a guest error if not, as real hardware behave incorrectly in
> this case.
>
> These checks are only performed when DEBUG_MMU is defined, to avoid
> hurting the performance.
>
> Signed-off-by: Leandro Lupori
> ---
> target/ppc/mmu-radix64.c | 21 +
> 1 file changed, 21 insertions(+)
>
> diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
> index 2f0bcbfe2e..80d945a7c3 100644
> --- a/target/ppc/mmu-radix64.c
> +++ b/target/ppc/mmu-radix64.c
> @@ -28,6 +28,8 @@
> #include "mmu-radix64.h"
> #include "mmu-book3s-v3.h"
>
> +/* #define DEBUG_MMU */
> +
> static bool ppc_radix64_get_fully_qualified_addr(const CPUPPCState *env,
> vaddr eaddr,
> uint64_t *lpid, uint64_t
> *pid)
> @@ -277,6 +279,16 @@ static int ppc_radix64_next_level(AddressSpace *as,
> vaddr eaddr,
> if (!(pde & R_PTE_LEAF)) { /* Prepare for next iteration */
> ++*level;
> *nls = pde & R_PDE_NLS;
> +
> +#ifdef DEBUG_MMU
> +if ((pde & R_PDE_NLB) & MAKE_64BIT_MASK(0, *nls + 3)) {
> +qemu_log_mask(LOG_GUEST_ERROR,
> +"%s: misaligned page dir/table base: 0x%"VADDR_PRIx
> +" page dir size: 0x%"PRIx64" level: %d\n",
> +__func__, (pde & R_PDE_NLB), BIT(*nls + 3), *level);
> +}
> +#endif
Maybe use qemu_log_enabled() instead of the define? I think it is a
little more useful and has less chance to rot.
> +
> index = eaddr >> (*psize - *nls); /* Shift */
> index &= ((1UL << *nls) - 1); /* Mask */
> *pte_addr = (pde & R_PDE_NLB) + (index * sizeof(pde));
> @@ -297,6 +309,15 @@ static int ppc_radix64_walk_tree(AddressSpace *as, vaddr
> eaddr,
> return 1;
> }
>
> +#ifdef DEBUG_MMU
> +if (base_addr & MAKE_64BIT_MASK(0, nls + 3)) {
> +qemu_log_mask(LOG_GUEST_ERROR,
> +"%s: misaligned page dir base: 0x%"VADDR_PRIx
> +" page dir size: 0x%"PRIx64"\n",
> +__func__, base_addr, BIT(nls + 3));
> +}
> +#endif
> +
> index = eaddr >> (*psize - nls);/* Shift */
> index &= ((1UL << nls) - 1); /* Mask */
> *pte_addr = base_addr + (index * sizeof(pde));
[PATCH 3/3] target/ppc: Check page dir/table base alignment
Check if each page dir/table base address is properly aligned and
log a guest error if not, as real hardware behave incorrectly in
this case.
These checks are only performed when DEBUG_MMU is defined, to avoid
hurting the performance.
Signed-off-by: Leandro Lupori
---
target/ppc/mmu-radix64.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
index 2f0bcbfe2e..80d945a7c3 100644
--- a/target/ppc/mmu-radix64.c
+++ b/target/ppc/mmu-radix64.c
@@ -28,6 +28,8 @@
#include "mmu-radix64.h"
#include "mmu-book3s-v3.h"
+/* #define DEBUG_MMU */
+
static bool ppc_radix64_get_fully_qualified_addr(const CPUPPCState *env,
vaddr eaddr,
uint64_t *lpid, uint64_t *pid)
@@ -277,6 +279,16 @@ static int ppc_radix64_next_level(AddressSpace *as, vaddr
eaddr,
if (!(pde & R_PTE_LEAF)) { /* Prepare for next iteration */
++*level;
*nls = pde & R_PDE_NLS;
+
+#ifdef DEBUG_MMU
+if ((pde & R_PDE_NLB) & MAKE_64BIT_MASK(0, *nls + 3)) {
+qemu_log_mask(LOG_GUEST_ERROR,
+"%s: misaligned page dir/table base: 0x%"VADDR_PRIx
+" page dir size: 0x%"PRIx64" level: %d\n",
+__func__, (pde & R_PDE_NLB), BIT(*nls + 3), *level);
+}
+#endif
+
index = eaddr >> (*psize - *nls); /* Shift */
index &= ((1UL << *nls) - 1); /* Mask */
*pte_addr = (pde & R_PDE_NLB) + (index * sizeof(pde));
@@ -297,6 +309,15 @@ static int ppc_radix64_walk_tree(AddressSpace *as, vaddr
eaddr,
return 1;
}
+#ifdef DEBUG_MMU
+if (base_addr & MAKE_64BIT_MASK(0, nls + 3)) {
+qemu_log_mask(LOG_GUEST_ERROR,
+"%s: misaligned page dir base: 0x%"VADDR_PRIx
+" page dir size: 0x%"PRIx64"\n",
+__func__, base_addr, BIT(nls + 3));
+}
+#endif
+
index = eaddr >> (*psize - nls);/* Shift */
index &= ((1UL << nls) - 1); /* Mask */
*pte_addr = base_addr + (index * sizeof(pde));
--
2.25.1
