* [Qemu-devel] [PATCH 12/25] target-sparc: Add MMU_REAL_IDX
2015-12-17 20:54 [Qemu-devel] [PATCH 00/25] target-sparc improvements Richard Henderson
@ 2015-12-17 20:57 ` Richard Henderson
2016-01-11 11:15 ` Artyom Tarasenko
0 siblings, 1 reply; 7+ messages in thread
From: Richard Henderson @ 2015-12-17 20:57 UTC (permalink / raw)
To: qemu-devel; +Cc: mark.cave-ayland, aurelien
This gives us a trivial way to access physical addresses
(aka "real addresses", in sun4v terminology) directly from
qemu_ld/st, without having to go through another helper.
This also fixes a bug in get_physical_address_code where
it inferred NUCLEUS from env->tl instead of from mmu_idx.
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
target-sparc/cpu.h | 18 +++++++---
target-sparc/mmu_helper.c | 90 +++++++++++++++++++++++++++++------------------
2 files changed, 69 insertions(+), 39 deletions(-)
diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
index 7f4d47f..b1222a1 100644
--- a/target-sparc/cpu.h
+++ b/target-sparc/cpu.h
@@ -220,9 +220,9 @@ enum {
#define MAX_NWINDOWS 32
#if !defined(TARGET_SPARC64)
-#define NB_MMU_MODES 2
+#define NB_MMU_MODES 3
#else
-#define NB_MMU_MODES 6
+#define NB_MMU_MODES 7
typedef struct trap_state {
uint64_t tpc;
uint64_t tnpc;
@@ -612,11 +612,13 @@ int cpu_sparc_signal_handler(int host_signum, void *pinfo, void *puc);
#define MMU_MODE4_SUFFIX _nucleus
#define MMU_HYPV_IDX 5
#define MMU_MODE5_SUFFIX _hypv
+#define MMU_REAL_IDX 6
#else
#define MMU_USER_IDX 0
#define MMU_MODE0_SUFFIX _user
#define MMU_KERNEL_IDX 1
#define MMU_MODE1_SUFFIX _kernel
+#define MMU_REAL_IDX 2
#endif
#if defined (TARGET_SPARC64)
@@ -641,9 +643,17 @@ static inline int cpu_mmu_index(CPUSPARCState *env1, bool ifetch)
#if defined(CONFIG_USER_ONLY)
return MMU_USER_IDX;
#elif !defined(TARGET_SPARC64)
- return env1->psrs;
+ if (!(env1->mmuregs[0] & MMU_E)) {
+ return MMU_REAL_IDX; /* MMU disabled */
+ } else {
+ return env1->psrs;
+ }
#else
- if (env1->tl > 0) {
+ if (ifetch
+ ? !(env1->lsu & IMMU_E) || (env1->pstate & PS_RED)
+ : !(env1->lsu & DMMU_E)) {
+ return MMU_REAL_IDX; /* MMU disabled */
+ } else if (env1->tl > 0) {
return MMU_NUCLEUS_IDX;
} else if (cpu_hypervisor_mode(env1)) {
return MMU_HYPV_IDX;
diff --git a/target-sparc/mmu_helper.c b/target-sparc/mmu_helper.c
index 7495406..105f00d 100644
--- a/target-sparc/mmu_helper.c
+++ b/target-sparc/mmu_helper.c
@@ -90,7 +90,7 @@ static int get_physical_address(CPUSPARCState *env, hwaddr *physical,
is_user = mmu_idx == MMU_USER_IDX;
- if ((env->mmuregs[0] & MMU_E) == 0) { /* MMU disabled */
+ if (mmu_idx == MMU_REAL_IDX) { /* MMU bypass access */
*page_size = TARGET_PAGE_SIZE;
/* Boot mode: instruction fetches are taken from PROM */
if (rw == 2 && (env->mmuregs[0] & env->def->mmu_bm)) {
@@ -492,33 +492,40 @@ static int get_physical_address_data(CPUSPARCState *env,
unsigned int i;
uint64_t context;
uint64_t sfsr = 0;
+ bool is_user = false;
- int is_user = (mmu_idx == MMU_USER_IDX ||
- mmu_idx == MMU_USER_SECONDARY_IDX);
-
- if ((env->lsu & DMMU_E) == 0) { /* DMMU disabled */
+ switch (mmu_idx) {
+ case MMU_REAL_IDX:
+ /* MMU bypass access */
*physical = ultrasparc_truncate_physical(address);
- *prot = PAGE_READ | PAGE_WRITE;
+ *prot = PAGE_EXEC | PAGE_READ | PAGE_WRITE;
return 0;
- }
- switch (mmu_idx) {
+ case MMU_NUCLEUS_IDX:
+ sfsr |= SFSR_CT_NUCLEUS;
+ /* fallthru */
+ case MMU_HYPV_IDX:
+ /* No context. */
+ context = 0;
+ break;
case MMU_USER_IDX:
+ is_user = true;
+ /* fallthru */
case MMU_KERNEL_IDX:
+ /* PRIMARY context */
context = env->dmmu.mmu_primary_context & 0x1fff;
sfsr |= SFSR_CT_PRIMARY;
break;
case MMU_USER_SECONDARY_IDX:
+ is_user = true;
+ /* fallthru */
case MMU_KERNEL_SECONDARY_IDX:
+ /* PRIMARY context */
context = env->dmmu.mmu_secondary_context & 0x1fff;
sfsr |= SFSR_CT_SECONDARY;
break;
- case MMU_NUCLEUS_IDX:
- sfsr |= SFSR_CT_NUCLEUS;
- /* FALLTHRU */
default:
- context = 0;
- break;
+ g_assert_not_reached();
}
if (rw == 1) {
@@ -573,8 +580,8 @@ static int get_physical_address_data(CPUSPARCState *env,
}
if (env->dmmu.sfsr & SFSR_VALID_BIT) { /* Fault status register */
- sfsr |= SFSR_OW_BIT; /* overflow (not read before
- another fault) */
+ /* overflow (not read before another fault) */
+ sfsr |= SFSR_OW_BIT;
}
if (env->pstate & PS_PRIV) {
@@ -611,23 +618,41 @@ static int get_physical_address_code(CPUSPARCState *env,
CPUState *cs = CPU(sparc_env_get_cpu(env));
unsigned int i;
uint64_t context;
+ uint64_t sfsr = 0;
+ bool is_user = false;
- int is_user = (mmu_idx == MMU_USER_IDX ||
- mmu_idx == MMU_USER_SECONDARY_IDX);
-
- if ((env->lsu & IMMU_E) == 0 || (env->pstate & PS_RED) != 0) {
- /* IMMU disabled */
+ switch (mmu_idx) {
+ case MMU_REAL_IDX:
+ /* MMU bypass access */
*physical = ultrasparc_truncate_physical(address);
- *prot = PAGE_EXEC;
+ *prot = PAGE_EXEC | PAGE_READ | PAGE_WRITE;
return 0;
- }
- if (env->tl == 0) {
+ case MMU_NUCLEUS_IDX:
+ sfsr |= SFSR_CT_NUCLEUS;
+ /* fallthru */
+ case MMU_HYPV_IDX:
+ /* No context. */
+ context = 0;
+ break;
+ case MMU_USER_IDX:
+ is_user = true;
+ /* fallthru */
+ case MMU_KERNEL_IDX:
/* PRIMARY context */
context = env->dmmu.mmu_primary_context & 0x1fff;
- } else {
- /* NUCLEUS context */
- context = 0;
+ sfsr |= SFSR_CT_PRIMARY;
+ break;
+ case MMU_USER_SECONDARY_IDX:
+ is_user = true;
+ /* fallthru */
+ case MMU_KERNEL_SECONDARY_IDX:
+ /* PRIMARY context */
+ context = env->dmmu.mmu_secondary_context & 0x1fff;
+ sfsr |= SFSR_CT_SECONDARY;
+ break;
+ default:
+ g_assert_not_reached();
}
for (i = 0; i < 64; i++) {
@@ -638,20 +663,15 @@ static int get_physical_address_code(CPUSPARCState *env,
if (TTE_IS_PRIV(env->itlb[i].tte) && is_user) {
/* Fault status register */
if (env->immu.sfsr & SFSR_VALID_BIT) {
- env->immu.sfsr = SFSR_OW_BIT; /* overflow (not read before
- another fault) */
- } else {
- env->immu.sfsr = 0;
+ /* overflow (not read before another fault) */
+ sfsr |= SFSR_OW_BIT;
}
if (env->pstate & PS_PRIV) {
- env->immu.sfsr |= SFSR_PR_BIT;
- }
- if (env->tl > 0) {
- env->immu.sfsr |= SFSR_CT_NUCLEUS;
+ sfsr |= SFSR_PR_BIT;
}
/* FIXME: ASI field in SFSR must be set */
- env->immu.sfsr |= SFSR_FT_PRIV_BIT | SFSR_VALID_BIT;
+ env->immu.sfsr |= sfsr | SFSR_FT_PRIV_BIT | SFSR_VALID_BIT;
cs->exception_index = TT_TFAULT;
env->immu.tag_access = (address & ~0x1fffULL) | context;
--
2.5.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 12/25] target-sparc: Add MMU_REAL_IDX
2015-12-17 20:57 ` [Qemu-devel] [PATCH 12/25] target-sparc: Add MMU_REAL_IDX Richard Henderson
@ 2016-01-11 11:15 ` Artyom Tarasenko
2016-01-11 12:01 ` Artyom Tarasenko
0 siblings, 1 reply; 7+ messages in thread
From: Artyom Tarasenko @ 2016-01-11 11:15 UTC (permalink / raw)
To: Richard Henderson; +Cc: Mark Cave-Ayland, qemu-devel, Aurelien Jarno
Hi Richard,
first of all, this is a very nice series.
I really enjoy reading it, thank you very much.
It makes the code is more readable and likely to be more performant.
A nitpick below.
On Thu, Dec 17, 2015 at 9:57 PM, Richard Henderson <rth@twiddle.net> wrote:
> This gives us a trivial way to access physical addresses
> (aka "real addresses", in sun4v terminology) directly from
> qemu_ld/st, without having to go through another helper.
>
> This also fixes a bug in get_physical_address_code where
> it inferred NUCLEUS from env->tl instead of from mmu_idx.
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
> target-sparc/cpu.h | 18 +++++++---
> target-sparc/mmu_helper.c | 90 +++++++++++++++++++++++++++++------------------
> 2 files changed, 69 insertions(+), 39 deletions(-)
>
> diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
> index 7f4d47f..b1222a1 100644
> --- a/target-sparc/cpu.h
> +++ b/target-sparc/cpu.h
> @@ -220,9 +220,9 @@ enum {
> #define MAX_NWINDOWS 32
>
> #if !defined(TARGET_SPARC64)
> -#define NB_MMU_MODES 2
> +#define NB_MMU_MODES 3
> #else
> -#define NB_MMU_MODES 6
> +#define NB_MMU_MODES 7
> typedef struct trap_state {
> uint64_t tpc;
> uint64_t tnpc;
> @@ -612,11 +612,13 @@ int cpu_sparc_signal_handler(int host_signum, void *pinfo, void *puc);
> #define MMU_MODE4_SUFFIX _nucleus
> #define MMU_HYPV_IDX 5
> #define MMU_MODE5_SUFFIX _hypv
> +#define MMU_REAL_IDX 6
> #else
> #define MMU_USER_IDX 0
> #define MMU_MODE0_SUFFIX _user
> #define MMU_KERNEL_IDX 1
> #define MMU_MODE1_SUFFIX _kernel
> +#define MMU_REAL_IDX 2
> #endif
>
> #if defined (TARGET_SPARC64)
> @@ -641,9 +643,17 @@ static inline int cpu_mmu_index(CPUSPARCState *env1, bool ifetch)
> #if defined(CONFIG_USER_ONLY)
> return MMU_USER_IDX;
> #elif !defined(TARGET_SPARC64)
> - return env1->psrs;
> + if (!(env1->mmuregs[0] & MMU_E)) {
> + return MMU_REAL_IDX; /* MMU disabled */
> + } else {
> + return env1->psrs;
> + }
> #else
> - if (env1->tl > 0) {
> + if (ifetch
> + ? !(env1->lsu & IMMU_E) || (env1->pstate & PS_RED)
> + : !(env1->lsu & DMMU_E)) {
> + return MMU_REAL_IDX; /* MMU disabled */
> + } else if (env1->tl > 0) {
> return MMU_NUCLEUS_IDX;
> } else if (cpu_hypervisor_mode(env1)) {
> return MMU_HYPV_IDX;
> diff --git a/target-sparc/mmu_helper.c b/target-sparc/mmu_helper.c
> index 7495406..105f00d 100644
> --- a/target-sparc/mmu_helper.c
> +++ b/target-sparc/mmu_helper.c
> @@ -90,7 +90,7 @@ static int get_physical_address(CPUSPARCState *env, hwaddr *physical,
>
> is_user = mmu_idx == MMU_USER_IDX;
>
> - if ((env->mmuregs[0] & MMU_E) == 0) { /* MMU disabled */
> + if (mmu_idx == MMU_REAL_IDX) { /* MMU bypass access */
> *page_size = TARGET_PAGE_SIZE;
> /* Boot mode: instruction fetches are taken from PROM */
> if (rw == 2 && (env->mmuregs[0] & env->def->mmu_bm)) {
> @@ -492,33 +492,40 @@ static int get_physical_address_data(CPUSPARCState *env,
> unsigned int i;
> uint64_t context;
> uint64_t sfsr = 0;
> + bool is_user = false;
>
> - int is_user = (mmu_idx == MMU_USER_IDX ||
> - mmu_idx == MMU_USER_SECONDARY_IDX);
> -
> - if ((env->lsu & DMMU_E) == 0) { /* DMMU disabled */
^^^^
in case of ASI instructions, mmu_idx can come from dc->mem_idx, right?
So the check may be still necessary here,
> + switch (mmu_idx) {
> + case MMU_REAL_IDX:
> + /* MMU bypass access */
> *physical = ultrasparc_truncate_physical(address);
> - *prot = PAGE_READ | PAGE_WRITE;
> + *prot = PAGE_EXEC | PAGE_READ | PAGE_WRITE;
> return 0;
> - }
>
> - switch (mmu_idx) {
> + case MMU_NUCLEUS_IDX:
> + sfsr |= SFSR_CT_NUCLEUS;
> + /* fallthru */
> + case MMU_HYPV_IDX:
> + /* No context. */
> + context = 0;
> + break;
> case MMU_USER_IDX:
> + is_user = true;
> + /* fallthru */
> case MMU_KERNEL_IDX:
> + /* PRIMARY context */
> context = env->dmmu.mmu_primary_context & 0x1fff;
> sfsr |= SFSR_CT_PRIMARY;
> break;
> case MMU_USER_SECONDARY_IDX:
> + is_user = true;
> + /* fallthru */
> case MMU_KERNEL_SECONDARY_IDX:
> + /* PRIMARY context */
> context = env->dmmu.mmu_secondary_context & 0x1fff;
> sfsr |= SFSR_CT_SECONDARY;
> break;
> - case MMU_NUCLEUS_IDX:
> - sfsr |= SFSR_CT_NUCLEUS;
> - /* FALLTHRU */
> default:
> - context = 0;
> - break;
> + g_assert_not_reached();
> }
>
> if (rw == 1) {
> @@ -573,8 +580,8 @@ static int get_physical_address_data(CPUSPARCState *env,
> }
>
> if (env->dmmu.sfsr & SFSR_VALID_BIT) { /* Fault status register */
> - sfsr |= SFSR_OW_BIT; /* overflow (not read before
> - another fault) */
> + /* overflow (not read before another fault) */
> + sfsr |= SFSR_OW_BIT;
> }
>
> if (env->pstate & PS_PRIV) {
> @@ -611,23 +618,41 @@ static int get_physical_address_code(CPUSPARCState *env,
> CPUState *cs = CPU(sparc_env_get_cpu(env));
> unsigned int i;
> uint64_t context;
> + uint64_t sfsr = 0;
> + bool is_user = false;
>
> - int is_user = (mmu_idx == MMU_USER_IDX ||
> - mmu_idx == MMU_USER_SECONDARY_IDX);
> -
> - if ((env->lsu & IMMU_E) == 0 || (env->pstate & PS_RED) != 0) {
> - /* IMMU disabled */
and here.
Artyom
> + switch (mmu_idx) {
> + case MMU_REAL_IDX:
> + /* MMU bypass access */
> *physical = ultrasparc_truncate_physical(address);
> - *prot = PAGE_EXEC;
> + *prot = PAGE_EXEC | PAGE_READ | PAGE_WRITE;
> return 0;
> - }
>
> - if (env->tl == 0) {
> + case MMU_NUCLEUS_IDX:
> + sfsr |= SFSR_CT_NUCLEUS;
> + /* fallthru */
> + case MMU_HYPV_IDX:
> + /* No context. */
> + context = 0;
> + break;
> + case MMU_USER_IDX:
> + is_user = true;
> + /* fallthru */
> + case MMU_KERNEL_IDX:
> /* PRIMARY context */
> context = env->dmmu.mmu_primary_context & 0x1fff;
> - } else {
> - /* NUCLEUS context */
> - context = 0;
> + sfsr |= SFSR_CT_PRIMARY;
> + break;
> + case MMU_USER_SECONDARY_IDX:
> + is_user = true;
> + /* fallthru */
> + case MMU_KERNEL_SECONDARY_IDX:
> + /* PRIMARY context */
> + context = env->dmmu.mmu_secondary_context & 0x1fff;
> + sfsr |= SFSR_CT_SECONDARY;
> + break;
> + default:
> + g_assert_not_reached();
> }
>
> for (i = 0; i < 64; i++) {
> @@ -638,20 +663,15 @@ static int get_physical_address_code(CPUSPARCState *env,
> if (TTE_IS_PRIV(env->itlb[i].tte) && is_user) {
> /* Fault status register */
> if (env->immu.sfsr & SFSR_VALID_BIT) {
> - env->immu.sfsr = SFSR_OW_BIT; /* overflow (not read before
> - another fault) */
> - } else {
> - env->immu.sfsr = 0;
> + /* overflow (not read before another fault) */
> + sfsr |= SFSR_OW_BIT;
> }
> if (env->pstate & PS_PRIV) {
> - env->immu.sfsr |= SFSR_PR_BIT;
> - }
> - if (env->tl > 0) {
> - env->immu.sfsr |= SFSR_CT_NUCLEUS;
> + sfsr |= SFSR_PR_BIT;
> }
>
> /* FIXME: ASI field in SFSR must be set */
> - env->immu.sfsr |= SFSR_FT_PRIV_BIT | SFSR_VALID_BIT;
> + env->immu.sfsr |= sfsr | SFSR_FT_PRIV_BIT | SFSR_VALID_BIT;
> cs->exception_index = TT_TFAULT;
>
> env->immu.tag_access = (address & ~0x1fffULL) | context;
> --
> 2.5.0
>
>
--
Regards,
Artyom Tarasenko
SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 12/25] target-sparc: Add MMU_REAL_IDX
2016-01-11 11:15 ` Artyom Tarasenko
@ 2016-01-11 12:01 ` Artyom Tarasenko
0 siblings, 0 replies; 7+ messages in thread
From: Artyom Tarasenko @ 2016-01-11 12:01 UTC (permalink / raw)
To: Richard Henderson; +Cc: Mark Cave-Ayland, qemu-devel, Aurelien Jarno
On Mon, Jan 11, 2016 at 12:15 PM, Artyom Tarasenko <atar4qemu@gmail.com> wrote:
> Hi Richard,
>
> first of all, this is a very nice series.
> I really enjoy reading it, thank you very much.
> It makes the code is more readable and likely to be more performant.
> A nitpick below.
>
> On Thu, Dec 17, 2015 at 9:57 PM, Richard Henderson <rth@twiddle.net> wrote:
>> This gives us a trivial way to access physical addresses
>> (aka "real addresses", in sun4v terminology) directly from
>> qemu_ld/st, without having to go through another helper.
>>
>> This also fixes a bug in get_physical_address_code where
>> it inferred NUCLEUS from env->tl instead of from mmu_idx.
>>
>> Signed-off-by: Richard Henderson <rth@twiddle.net>
>> ---
>> target-sparc/cpu.h | 18 +++++++---
>> target-sparc/mmu_helper.c | 90 +++++++++++++++++++++++++++++------------------
>> 2 files changed, 69 insertions(+), 39 deletions(-)
>>
>> diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
>> index 7f4d47f..b1222a1 100644
>> --- a/target-sparc/cpu.h
>> +++ b/target-sparc/cpu.h
>> @@ -220,9 +220,9 @@ enum {
>> #define MAX_NWINDOWS 32
>>
>> #if !defined(TARGET_SPARC64)
>> -#define NB_MMU_MODES 2
>> +#define NB_MMU_MODES 3
>> #else
>> -#define NB_MMU_MODES 6
>> +#define NB_MMU_MODES 7
>> typedef struct trap_state {
>> uint64_t tpc;
>> uint64_t tnpc;
>> @@ -612,11 +612,13 @@ int cpu_sparc_signal_handler(int host_signum, void *pinfo, void *puc);
>> #define MMU_MODE4_SUFFIX _nucleus
>> #define MMU_HYPV_IDX 5
>> #define MMU_MODE5_SUFFIX _hypv
>> +#define MMU_REAL_IDX 6
>> #else
>> #define MMU_USER_IDX 0
>> #define MMU_MODE0_SUFFIX _user
>> #define MMU_KERNEL_IDX 1
>> #define MMU_MODE1_SUFFIX _kernel
>> +#define MMU_REAL_IDX 2
>> #endif
>>
>> #if defined (TARGET_SPARC64)
>> @@ -641,9 +643,17 @@ static inline int cpu_mmu_index(CPUSPARCState *env1, bool ifetch)
>> #if defined(CONFIG_USER_ONLY)
>> return MMU_USER_IDX;
>> #elif !defined(TARGET_SPARC64)
>> - return env1->psrs;
>> + if (!(env1->mmuregs[0] & MMU_E)) {
>> + return MMU_REAL_IDX; /* MMU disabled */
>> + } else {
>> + return env1->psrs;
>> + }
>> #else
>> - if (env1->tl > 0) {
>> + if (ifetch
>> + ? !(env1->lsu & IMMU_E) || (env1->pstate & PS_RED)
>> + : !(env1->lsu & DMMU_E)) {
>> + return MMU_REAL_IDX; /* MMU disabled */
>> + } else if (env1->tl > 0) {
>> return MMU_NUCLEUS_IDX;
>> } else if (cpu_hypervisor_mode(env1)) {
>> return MMU_HYPV_IDX;
>> diff --git a/target-sparc/mmu_helper.c b/target-sparc/mmu_helper.c
>> index 7495406..105f00d 100644
>> --- a/target-sparc/mmu_helper.c
>> +++ b/target-sparc/mmu_helper.c
>> @@ -90,7 +90,7 @@ static int get_physical_address(CPUSPARCState *env, hwaddr *physical,
>>
>> is_user = mmu_idx == MMU_USER_IDX;
>>
>> - if ((env->mmuregs[0] & MMU_E) == 0) { /* MMU disabled */
>> + if (mmu_idx == MMU_REAL_IDX) { /* MMU bypass access */
>> *page_size = TARGET_PAGE_SIZE;
>> /* Boot mode: instruction fetches are taken from PROM */
>> if (rw == 2 && (env->mmuregs[0] & env->def->mmu_bm)) {
>> @@ -492,33 +492,40 @@ static int get_physical_address_data(CPUSPARCState *env,
>> unsigned int i;
>> uint64_t context;
>> uint64_t sfsr = 0;
>> + bool is_user = false;
>>
>> - int is_user = (mmu_idx == MMU_USER_IDX ||
>> - mmu_idx == MMU_USER_SECONDARY_IDX);
>> -
>> - if ((env->lsu & DMMU_E) == 0) { /* DMMU disabled */
>
> ^^^^
>
> in case of ASI instructions, mmu_idx can come from dc->mem_idx, right?
> So the check may be still necessary here,
>
>> + switch (mmu_idx) {
>> + case MMU_REAL_IDX:
>> + /* MMU bypass access */
>> *physical = ultrasparc_truncate_physical(address);
>> - *prot = PAGE_READ | PAGE_WRITE;
>> + *prot = PAGE_EXEC | PAGE_READ | PAGE_WRITE;
>> return 0;
>> - }
>>
>> - switch (mmu_idx) {
>> + case MMU_NUCLEUS_IDX:
>> + sfsr |= SFSR_CT_NUCLEUS;
>> + /* fallthru */
>> + case MMU_HYPV_IDX:
>> + /* No context. */
>> + context = 0;
>> + break;
>> case MMU_USER_IDX:
>> + is_user = true;
>> + /* fallthru */
>> case MMU_KERNEL_IDX:
>> + /* PRIMARY context */
>> context = env->dmmu.mmu_primary_context & 0x1fff;
>> sfsr |= SFSR_CT_PRIMARY;
>> break;
>> case MMU_USER_SECONDARY_IDX:
>> + is_user = true;
>> + /* fallthru */
>> case MMU_KERNEL_SECONDARY_IDX:
>> + /* PRIMARY context */
>> context = env->dmmu.mmu_secondary_context & 0x1fff;
>> sfsr |= SFSR_CT_SECONDARY;
>> break;
>> - case MMU_NUCLEUS_IDX:
>> - sfsr |= SFSR_CT_NUCLEUS;
>> - /* FALLTHRU */
>> default:
>> - context = 0;
>> - break;
>> + g_assert_not_reached();
>> }
>>
>> if (rw == 1) {
>> @@ -573,8 +580,8 @@ static int get_physical_address_data(CPUSPARCState *env,
>> }
>>
>> if (env->dmmu.sfsr & SFSR_VALID_BIT) { /* Fault status register */
>> - sfsr |= SFSR_OW_BIT; /* overflow (not read before
>> - another fault) */
>> + /* overflow (not read before another fault) */
>> + sfsr |= SFSR_OW_BIT;
>> }
>>
>> if (env->pstate & PS_PRIV) {
>> @@ -611,23 +618,41 @@ static int get_physical_address_code(CPUSPARCState *env,
>> CPUState *cs = CPU(sparc_env_get_cpu(env));
>> unsigned int i;
>> uint64_t context;
>> + uint64_t sfsr = 0;
>> + bool is_user = false;
>>
>> - int is_user = (mmu_idx == MMU_USER_IDX ||
>> - mmu_idx == MMU_USER_SECONDARY_IDX);
>> -
>> - if ((env->lsu & IMMU_E) == 0 || (env->pstate & PS_RED) != 0) {
>> - /* IMMU disabled */
>
> and here.
On a second thought, IMMU check is not necessary, because there is no
ASI access for instructions.
So, here no check is necessary, only above.
Artyom
>
>> + switch (mmu_idx) {
>> + case MMU_REAL_IDX:
>> + /* MMU bypass access */
>> *physical = ultrasparc_truncate_physical(address);
>> - *prot = PAGE_EXEC;
>> + *prot = PAGE_EXEC | PAGE_READ | PAGE_WRITE;
>> return 0;
>> - }
>>
>> - if (env->tl == 0) {
>> + case MMU_NUCLEUS_IDX:
>> + sfsr |= SFSR_CT_NUCLEUS;
>> + /* fallthru */
>> + case MMU_HYPV_IDX:
>> + /* No context. */
>> + context = 0;
>> + break;
>> + case MMU_USER_IDX:
>> + is_user = true;
>> + /* fallthru */
>> + case MMU_KERNEL_IDX:
>> /* PRIMARY context */
>> context = env->dmmu.mmu_primary_context & 0x1fff;
>> - } else {
>> - /* NUCLEUS context */
>> - context = 0;
>> + sfsr |= SFSR_CT_PRIMARY;
>> + break;
>> + case MMU_USER_SECONDARY_IDX:
>> + is_user = true;
>> + /* fallthru */
>> + case MMU_KERNEL_SECONDARY_IDX:
>> + /* PRIMARY context */
>> + context = env->dmmu.mmu_secondary_context & 0x1fff;
>> + sfsr |= SFSR_CT_SECONDARY;
>> + break;
>> + default:
>> + g_assert_not_reached();
>> }
>>
>> for (i = 0; i < 64; i++) {
>> @@ -638,20 +663,15 @@ static int get_physical_address_code(CPUSPARCState *env,
>> if (TTE_IS_PRIV(env->itlb[i].tte) && is_user) {
>> /* Fault status register */
>> if (env->immu.sfsr & SFSR_VALID_BIT) {
>> - env->immu.sfsr = SFSR_OW_BIT; /* overflow (not read before
>> - another fault) */
>> - } else {
>> - env->immu.sfsr = 0;
>> + /* overflow (not read before another fault) */
>> + sfsr |= SFSR_OW_BIT;
>> }
>> if (env->pstate & PS_PRIV) {
>> - env->immu.sfsr |= SFSR_PR_BIT;
>> - }
>> - if (env->tl > 0) {
>> - env->immu.sfsr |= SFSR_CT_NUCLEUS;
>> + sfsr |= SFSR_PR_BIT;
>> }
>>
>> /* FIXME: ASI field in SFSR must be set */
>> - env->immu.sfsr |= SFSR_FT_PRIV_BIT | SFSR_VALID_BIT;
>> + env->immu.sfsr |= sfsr | SFSR_FT_PRIV_BIT | SFSR_VALID_BIT;
>> cs->exception_index = TT_TFAULT;
>>
>> env->immu.tag_access = (address & ~0x1fffULL) | context;
>> --
>> 2.5.0
>>
>>
>
>
>
> --
> Regards,
> Artyom Tarasenko
>
> SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 12/25] target-sparc: Add MMU_REAL_IDX
@ 2016-01-15 13:17 Artyom Tarasenko
2016-01-15 18:03 ` Richard Henderson
0 siblings, 1 reply; 7+ messages in thread
From: Artyom Tarasenko @ 2016-01-15 13:17 UTC (permalink / raw)
To: Richard Henderson, Mark Cave-Ayland; +Cc: qemu-devel, Aurelien Jarno
Hi Richard,
please ignore my 2 previous mails: I've misread the commit message.
The actual problem and a possible solution below.
On Thu, Dec 17, 2015 at 9:57 PM, Richard Henderson <rth@twiddle.net> wrote:
> This gives us a trivial way to access physical addresses
> (aka "real addresses", in sun4v terminology)
In sun4v terminology "real address" is not "physical addresses".
There is just one more level of translation:
VA->RA->PA
which it's only visible from the hypervisor mode.
But I see where the confusion is coming from:
to make the hypervisor presence transparent for the existing sun4u drivers,
Sun mapped all sun4u ASIs that used to access physical space, to
access real space in sun4v instead.
This way every virtual machine (a "partition" in sun4v terminology)
may have it's own pseudo-physical ("real") space without overlapping
with other virtual machines.
I.e. "ASI_PHYS_USE_EC_L" became "ASI_REAL_L" which triggers one more
translation and so on.
The support of sun4v in current QEMU is very rudimentary, so if we
just ignore it for the moment and focus on sun4u, the approach of this
patch is correct, except for the naming.
With MMU_REAL_IDX renamed to MMU_PHYS_IDX, I think we are fine.
I suggest we keep sun4v names also for the sun4u ASIs (as in this
series) because having different names for sun4u and sun4v would make
the code ugly: it's not possible to #ifdef them because the
sun4u/sun4v cpu/mmu type is a runtime information [1].
Luckily I'm not a maintainer, so it's up to you guys to decide.
As for the sun4v support, I think, we can add one more index, this
time the real MMU_REAL_IDX, which can only be used by sun4v MMU(s).
(As I was reading this patch the last time, I thought the plan was to
use the MMU_REAL_IDX for both real and physical accesses, hence the
confusion. But using one index for two modes would have been a bad
idea because we'd have to flush the translations every time we switch
to/from hypervisor mode which is too often).
Kind regards,
Artyom
1. https://lists.gnu.org/archive/html/qemu-devel/2012-04/msg04281.html
> directly from
> qemu_ld/st, without having to go through another helper.
>
> This also fixes a bug in get_physical_address_code where
> it inferred NUCLEUS from env->tl instead of from mmu_idx.
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
> target-sparc/cpu.h | 18 +++++++---
> target-sparc/mmu_helper.c | 90 +++++++++++++++++++++++++++++------------------
> 2 files changed, 69 insertions(+), 39 deletions(-)
>
> diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
> index 7f4d47f..b1222a1 100644
> --- a/target-sparc/cpu.h
> +++ b/target-sparc/cpu.h
> @@ -220,9 +220,9 @@ enum {
> #define MAX_NWINDOWS 32
>
> #if !defined(TARGET_SPARC64)
> -#define NB_MMU_MODES 2
> +#define NB_MMU_MODES 3
> #else
> -#define NB_MMU_MODES 6
> +#define NB_MMU_MODES 7
> typedef struct trap_state {
> uint64_t tpc;
> uint64_t tnpc;
> @@ -612,11 +612,13 @@ int cpu_sparc_signal_handler(int host_signum, void *pinfo, void *puc);
> #define MMU_MODE4_SUFFIX _nucleus
> #define MMU_HYPV_IDX 5
> #define MMU_MODE5_SUFFIX _hypv
> +#define MMU_REAL_IDX 6
> #else
> #define MMU_USER_IDX 0
> #define MMU_MODE0_SUFFIX _user
> #define MMU_KERNEL_IDX 1
> #define MMU_MODE1_SUFFIX _kernel
> +#define MMU_REAL_IDX 2
> #endif
>
> #if defined (TARGET_SPARC64)
> @@ -641,9 +643,17 @@ static inline int cpu_mmu_index(CPUSPARCState *env1, bool ifetch)
> #if defined(CONFIG_USER_ONLY)
> return MMU_USER_IDX;
> #elif !defined(TARGET_SPARC64)
> - return env1->psrs;
> + if (!(env1->mmuregs[0] & MMU_E)) {
> + return MMU_REAL_IDX; /* MMU disabled */
> + } else {
> + return env1->psrs;
> + }
> #else
> - if (env1->tl > 0) {
> + if (ifetch
> + ? !(env1->lsu & IMMU_E) || (env1->pstate & PS_RED)
> + : !(env1->lsu & DMMU_E)) {
> + return MMU_REAL_IDX; /* MMU disabled */
> + } else if (env1->tl > 0) {
> return MMU_NUCLEUS_IDX;
> } else if (cpu_hypervisor_mode(env1)) {
> return MMU_HYPV_IDX;
> diff --git a/target-sparc/mmu_helper.c b/target-sparc/mmu_helper.c
> index 7495406..105f00d 100644
> --- a/target-sparc/mmu_helper.c
> +++ b/target-sparc/mmu_helper.c
> @@ -90,7 +90,7 @@ static int get_physical_address(CPUSPARCState *env, hwaddr *physical,
>
> is_user = mmu_idx == MMU_USER_IDX;
>
> - if ((env->mmuregs[0] & MMU_E) == 0) { /* MMU disabled */
> + if (mmu_idx == MMU_REAL_IDX) { /* MMU bypass access */
> *page_size = TARGET_PAGE_SIZE;
> /* Boot mode: instruction fetches are taken from PROM */
> if (rw == 2 && (env->mmuregs[0] & env->def->mmu_bm)) {
> @@ -492,33 +492,40 @@ static int get_physical_address_data(CPUSPARCState *env,
> unsigned int i;
> uint64_t context;
> uint64_t sfsr = 0;
> + bool is_user = false;
>
> - int is_user = (mmu_idx == MMU_USER_IDX ||
> - mmu_idx == MMU_USER_SECONDARY_IDX);
> -
> - if ((env->lsu & DMMU_E) == 0) { /* DMMU disabled */
> + switch (mmu_idx) {
> + case MMU_REAL_IDX:
> + /* MMU bypass access */
> *physical = ultrasparc_truncate_physical(address);
> - *prot = PAGE_READ | PAGE_WRITE;
> + *prot = PAGE_EXEC | PAGE_READ | PAGE_WRITE;
> return 0;
> - }
>
> - switch (mmu_idx) {
> + case MMU_NUCLEUS_IDX:
> + sfsr |= SFSR_CT_NUCLEUS;
> + /* fallthru */
> + case MMU_HYPV_IDX:
> + /* No context. */
> + context = 0;
> + break;
> case MMU_USER_IDX:
> + is_user = true;
> + /* fallthru */
> case MMU_KERNEL_IDX:
> + /* PRIMARY context */
> context = env->dmmu.mmu_primary_context & 0x1fff;
> sfsr |= SFSR_CT_PRIMARY;
> break;
> case MMU_USER_SECONDARY_IDX:
> + is_user = true;
> + /* fallthru */
> case MMU_KERNEL_SECONDARY_IDX:
> + /* PRIMARY context */
> context = env->dmmu.mmu_secondary_context & 0x1fff;
> sfsr |= SFSR_CT_SECONDARY;
> break;
> - case MMU_NUCLEUS_IDX:
> - sfsr |= SFSR_CT_NUCLEUS;
> - /* FALLTHRU */
> default:
> - context = 0;
> - break;
> + g_assert_not_reached();
> }
>
> if (rw == 1) {
> @@ -573,8 +580,8 @@ static int get_physical_address_data(CPUSPARCState *env,
> }
>
> if (env->dmmu.sfsr & SFSR_VALID_BIT) { /* Fault status register */
> - sfsr |= SFSR_OW_BIT; /* overflow (not read before
> - another fault) */
> + /* overflow (not read before another fault) */
> + sfsr |= SFSR_OW_BIT;
> }
>
> if (env->pstate & PS_PRIV) {
> @@ -611,23 +618,41 @@ static int get_physical_address_code(CPUSPARCState *env,
> CPUState *cs = CPU(sparc_env_get_cpu(env));
> unsigned int i;
> uint64_t context;
> + uint64_t sfsr = 0;
> + bool is_user = false;
>
> - int is_user = (mmu_idx == MMU_USER_IDX ||
> - mmu_idx == MMU_USER_SECONDARY_IDX);
> -
> - if ((env->lsu & IMMU_E) == 0 || (env->pstate & PS_RED) != 0) {
> - /* IMMU disabled */
> + switch (mmu_idx) {
> + case MMU_REAL_IDX:
> + /* MMU bypass access */
> *physical = ultrasparc_truncate_physical(address);
> - *prot = PAGE_EXEC;
> + *prot = PAGE_EXEC | PAGE_READ | PAGE_WRITE;
> return 0;
> - }
>
> - if (env->tl == 0) {
> + case MMU_NUCLEUS_IDX:
> + sfsr |= SFSR_CT_NUCLEUS;
> + /* fallthru */
> + case MMU_HYPV_IDX:
> + /* No context. */
> + context = 0;
> + break;
> + case MMU_USER_IDX:
> + is_user = true;
> + /* fallthru */
> + case MMU_KERNEL_IDX:
> /* PRIMARY context */
> context = env->dmmu.mmu_primary_context & 0x1fff;
> - } else {
> - /* NUCLEUS context */
> - context = 0;
> + sfsr |= SFSR_CT_PRIMARY;
> + break;
> + case MMU_USER_SECONDARY_IDX:
> + is_user = true;
> + /* fallthru */
> + case MMU_KERNEL_SECONDARY_IDX:
> + /* PRIMARY context */
> + context = env->dmmu.mmu_secondary_context & 0x1fff;
> + sfsr |= SFSR_CT_SECONDARY;
> + break;
> + default:
> + g_assert_not_reached();
> }
>
> for (i = 0; i < 64; i++) {
> @@ -638,20 +663,15 @@ static int get_physical_address_code(CPUSPARCState *env,
> if (TTE_IS_PRIV(env->itlb[i].tte) && is_user) {
> /* Fault status register */
> if (env->immu.sfsr & SFSR_VALID_BIT) {
> - env->immu.sfsr = SFSR_OW_BIT; /* overflow (not read before
> - another fault) */
> - } else {
> - env->immu.sfsr = 0;
> + /* overflow (not read before another fault) */
> + sfsr |= SFSR_OW_BIT;
> }
> if (env->pstate & PS_PRIV) {
> - env->immu.sfsr |= SFSR_PR_BIT;
> - }
> - if (env->tl > 0) {
> - env->immu.sfsr |= SFSR_CT_NUCLEUS;
> + sfsr |= SFSR_PR_BIT;
> }
>
> /* FIXME: ASI field in SFSR must be set */
> - env->immu.sfsr |= SFSR_FT_PRIV_BIT | SFSR_VALID_BIT;
> + env->immu.sfsr |= sfsr | SFSR_FT_PRIV_BIT | SFSR_VALID_BIT;
> cs->exception_index = TT_TFAULT;
>
> env->immu.tag_access = (address & ~0x1fffULL) | context;
> --
> 2.5.0
>
>
--
Regards,
Artyom Tarasenko
SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 12/25] target-sparc: Add MMU_REAL_IDX
2016-01-15 13:17 [Qemu-devel] [PATCH 12/25] target-sparc: Add MMU_REAL_IDX Artyom Tarasenko
@ 2016-01-15 18:03 ` Richard Henderson
2016-01-15 20:32 ` Artyom Tarasenko
0 siblings, 1 reply; 7+ messages in thread
From: Richard Henderson @ 2016-01-15 18:03 UTC (permalink / raw)
To: Artyom Tarasenko, Mark Cave-Ayland; +Cc: qemu-devel, Aurelien Jarno
On 01/15/2016 05:17 AM, Artyom Tarasenko wrote:
> Hi Richard,
>
> please ignore my 2 previous mails: I've misread the commit message.
> The actual problem and a possible solution below.
>
> On Thu, Dec 17, 2015 at 9:57 PM, Richard Henderson <rth@twiddle.net> wrote:
>> This gives us a trivial way to access physical addresses
>> (aka "real addresses", in sun4v terminology)
>
> In sun4v terminology "real address" is not "physical addresses".
> There is just one more level of translation:
> VA->RA->PA
> which it's only visible from the hypervisor mode.
Correct, but...
> With MMU_REAL_IDX renamed to MMU_PHYS_IDX, I think we are fine.
... no. REAL is currently implemented as PHYS, true, but that's only because
we don't actually implemnet sun4v.
If we ever properly implement a sun4v platform, we will implement the bulk of
the hypervisor within qemu itself, for speed. At which point REAL will in fact
undergo that final layer of translation exactly as expected.
I think the naming is exactly correct, for the current sun4u implementation.
> (As I was reading this patch the last time, I thought the plan was to
> use the MMU_REAL_IDX for both real and physical accesses, hence the
> confusion. But using one index for two modes would have been a bad
> idea because we'd have to flush the translations every time we switch
> to/from hypervisor mode which is too often).
And that is exactly why hypervisor mode would not be implemented like you
think, but within qemu itself.
r~
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 12/25] target-sparc: Add MMU_REAL_IDX
2016-01-15 18:03 ` Richard Henderson
@ 2016-01-15 20:32 ` Artyom Tarasenko
2016-01-15 21:47 ` Richard Henderson
0 siblings, 1 reply; 7+ messages in thread
From: Artyom Tarasenko @ 2016-01-15 20:32 UTC (permalink / raw)
To: Richard Henderson; +Cc: Mark Cave-Ayland, qemu-devel, Aurelien Jarno
On Fri, Jan 15, 2016 at 7:03 PM, Richard Henderson <rth@twiddle.net> wrote:
> On 01/15/2016 05:17 AM, Artyom Tarasenko wrote:
>> Hi Richard,
>>
>> please ignore my 2 previous mails: I've misread the commit message.
>> The actual problem and a possible solution below.
>>
>> On Thu, Dec 17, 2015 at 9:57 PM, Richard Henderson <rth@twiddle.net> wrote:
>>> This gives us a trivial way to access physical addresses
>>> (aka "real addresses", in sun4v terminology)
>>
>> In sun4v terminology "real address" is not "physical addresses".
>> There is just one more level of translation:
>> VA->RA->PA
>> which it's only visible from the hypervisor mode.
>
> Correct, but...
>
>> With MMU_REAL_IDX renamed to MMU_PHYS_IDX, I think we are fine.
>
> ... no. REAL is currently implemented as PHYS, true, but that's only because
> we don't actually implemnet sun4v.
>
> If we ever properly implement a sun4v platform, we will implement the bulk of
> the hypervisor within qemu itself, for speed. At which point REAL will in fact
> undergo that final layer of translation exactly as expected.
>
> I think the naming is exactly correct, for the current sun4u implementation.
But sun4u has neither real mode nor any ASI having "real" as a part of
its name, no?
>> (As I was reading this patch the last time, I thought the plan was to
>> use the MMU_REAL_IDX for both real and physical accesses, hence the
>> confusion. But using one index for two modes would have been a bad
>> idea because we'd have to flush the translations every time we switch
>> to/from hypervisor mode which is too often).
>
> And that is exactly why hypervisor mode would not be implemented like you
> think, but within qemu itself.
In my case it is sort of implemented it like I think: few years back
I've implemented
its subset (incomplete but capable to boot the OpenSPARC firmware)
and have been trying to merge it with your work. :-)
I thought it is much easier to implement the RA translation than the hypervisor,
but maybe implementing the hypervisor is not as complex as I thought.
Indeed, your variant would be more performant.
In any case it looks like an interesting challenge.
I'd still prefer to call it MMU_PHYS_IDX for now and rename it once
the hypervisor layer is implemented.
But I'm not insisting on the naming. The naming issues are always hard. :-)
--
Regards,
Artyom Tarasenko
SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 12/25] target-sparc: Add MMU_REAL_IDX
2016-01-15 20:32 ` Artyom Tarasenko
@ 2016-01-15 21:47 ` Richard Henderson
0 siblings, 0 replies; 7+ messages in thread
From: Richard Henderson @ 2016-01-15 21:47 UTC (permalink / raw)
To: Artyom Tarasenko; +Cc: Mark Cave-Ayland, qemu-devel, Aurelien Jarno
On 01/15/2016 12:32 PM, Artyom Tarasenko wrote:
>> If we ever properly implement a sun4v platform, we will implement the bulk of
>> the hypervisor within qemu itself, for speed. At which point REAL will in fact
>> undergo that final layer of translation exactly as expected.
>>
>> I think the naming is exactly correct, for the current sun4u implementation.
>
> But sun4u has neither real mode nor any ASI having "real" as a part of
> its name, no?
Correct-ish. The ASI is called ASI_PHYS_USE_EC. But it has the same number as
ASI_REAL. When there's a conflict I'm preferring the newer names. I think
that it will cause less confusion in the end.
> I thought it is much easier to implement the RA translation than the hypervisor,
> but maybe implementing the hypervisor is not as complex as I thought.
>
> Indeed, your variant would be more performant.
> In any case it looks like an interesting challenge.
The Alpha PALcode layer (equivalent to the sun4v hypervisor) is 99% implemented
in qemu, and performs significantly better because of it.
There are a few stubs in the qemu bios calling into some special helpers; I'd
expect OpenBIOS for qemu to be the same.
r~
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-01-15 21:47 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-15 13:17 [Qemu-devel] [PATCH 12/25] target-sparc: Add MMU_REAL_IDX Artyom Tarasenko
2016-01-15 18:03 ` Richard Henderson
2016-01-15 20:32 ` Artyom Tarasenko
2016-01-15 21:47 ` Richard Henderson
-- strict thread matches above, loose matches on Subject: below --
2015-12-17 20:54 [Qemu-devel] [PATCH 00/25] target-sparc improvements Richard Henderson
2015-12-17 20:57 ` [Qemu-devel] [PATCH 12/25] target-sparc: Add MMU_REAL_IDX Richard Henderson
2016-01-11 11:15 ` Artyom Tarasenko
2016-01-11 12:01 ` Artyom Tarasenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).