qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* 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
* [Qemu-devel] [PATCH 00/25] target-sparc improvements
@ 2015-12-17 20:54 Richard Henderson
  2015-12-17 20:57 ` [Qemu-devel] [PATCH 12/25] target-sparc: Add MMU_REAL_IDX Richard Henderson
  0 siblings, 1 reply; 7+ messages in thread
From: Richard Henderson @ 2015-12-17 20:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: mark.cave-ayland, aurelien

The primary focus of this patch set is to reduce the number of
helpers that modify TCG globals, and thus increase the lifetime
of those globals within each TB, and thus decrease the number
of times that tcg must spill and fill them from backing store.

This patch set is relative to the "Improve sparc register windows"
patches, which turned all of the integer registers into TCG globals.
Thus this patch set improves the effect of the previous patch set.

As a byproduct, I also implement the bulk of the interesting v9 ASIs
inline, thus exposing e.g. the little-endian loads and stores as
simple tcg operations.


r~


Richard Henderson (25):
  target-sparc: Mark more flags for helpers
  target-sparc: Remove softint as a TCG global
  target-sparc: Store mmu index in TB flags
  target-sparc: Create gen_exception
  target-sparc: Unify asi handling between 32 and 64-bit
  target-sparc: Store %asi in TB flags
  target-sparc: Introduce get_asi
  target-sparc: Pass TCGMemOp to gen_ld/st_asi
  target-sparc: Import linux/arch/sparc/include/uapi/asm/asi.h
  target-sparc: Add UA2011 defines to asi.h
  target-sparc: Use defines from asi.h
  target-sparc: Add MMU_REAL_IDX
  target-sparc: Directly implement easy ld/st asis
  target-sparc: Use QT0 to return results from ldda
  target-sparc: Introduce gen_check_align
  target-sparc: Directly implement easy ldd/std asis
  target-sparc: Fix obvious error in ASI_M_BFILL
  target-sparc: Pass TCGMemOp constants to helper_ld/st_asi
  target-sparc: Directly implement easy ldf/stf asis
  target-sparc: Directly implement block and short ldf/stf asis
  target-sparc: Remove helper_ldf_asi, helper_stf_asi
  target-sparc: Use explicit writes to cpu_fsr
  target-sparc: Use cpu_fsr in stfsr
  target-sparc: Use cpu_loop_exit_restore from
    helper_check_ieee_exceptions
  target-sparc: Elide duplicate updates to fprs

 target-sparc/asi.h         |  311 +++++++++++
 target-sparc/cpu.h         |   46 +-
 target-sparc/fop_helper.c  |  229 +++-----
 target-sparc/helper.h      |  168 +++---
 target-sparc/ldst_helper.c |  696 +++++++++++-------------
 target-sparc/mmu_helper.c  |   90 ++--
 target-sparc/translate.c   | 1264 ++++++++++++++++++++++++++++----------------
 7 files changed, 1667 insertions(+), 1137 deletions(-)
 create mode 100644 target-sparc/asi.h

-- 
2.5.0

^ 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).