Re: [Qemu-devel] SH: Fix movca.l/ocbi emulation.

[Shin-ichiro KAWASAKI <kawasaki@juno.dti.ne.jp>]

Hi, Volodya-san.

Thank you for the patch.  I encountered the movca.l/ocbi
problem when I added SE7750 board support, and tried to 
create a patch like yours by mistake.  But same two patches
can never be good idea.
I hope your patch to be authorized, and want to provide
two comments on it from my point of view.

> diff --git a/cpu-exec.c b/cpu-exec.c
> index 9a35a59..64b0845 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -174,7 +174,7 @@ static inline TranslationBlock *tb_find_fast(void)
>      /* we record a subset of the CPU state. It will
>         always be the same before a given translated block
>         is executed. */
> -    cpu_get_tb_cpu_state(env, &pc, &cs_base, &flags);
> +    cpu_get_tb_cpu_state(env, &pc, &cs_base, &flags);  
I'm not sure if this modification should be in this patch.

(snip)

> +void helper_movcal(uint32_t address, uint32_t value)
> +{
> +  store_request_t *r = (store_request_t *)malloc (sizeof(store_request_t));
> +  r->address = address;
> +  r->value = value;
> +  r->next = NULL;
> +
> +  *(env->store_request_tail) = r;
> +  env->store_request_tail = &(r->next);
> +}
> +
> +void helper_do_stores(void)
> +{
> +  store_request_t *current = env->store_requests;
> +
> +  while(current)
> +    {
> +      uint32_t a = current->address, v = current->value;
> +      store_request_t *next = current->next;
> +      free (current);
> +      env->store_requests = current = next;
> +      if (current == 0)
> +	env->store_request_tail = &(env->store_requests);
> +
> +      stl_data(a, v);
> +    } 
> +}
> +
> +void helper_ocbi(uint32_t address)
> +{
> +  store_request_t **current = &(env->store_requests);
> +  while (*current)
> +    {
> +      if ((*current)->address == address)
> +	{
> +	  store_request_t *next = (*current)->next;
> +
> +	  if (next == 0)
> +	    {
> +	      env->store_request_tail = current;
> +	    }
> +
> +	  free (*current);
> +	  *current = next;
> +	  break;
> +	}
> +    }
> +}
> +
>  uint32_t helper_addc(uint32_t arg0, uint32_t arg1)
>  {
>      uint32_t tmp0, tmp1;

These movca.l/ocbi helpers should change their behavior
depending on cache status : area type, privilege mode,
cache enabling, and tlb entries' cache flags (Paul Mundt
and Edgar Iglesias pointed out it when they reviewed my patch) .

I added the cache mode checking routine to the end of this
mail.  Could you evaluate it and take into your patch?
That is_cached() function is to be placed in target-sh4/helper.c,
and to be invoked by helper_movca() or helper_ocbi().

Regards,
Shin-ichiro KAWASAKI


-------------------------------------------------------

int is_cached(CPUSH4State * env, target_ulong addr)
{
    int n;
    int use_asid = (env->mmucr & MMUCR_SV) == 0 || (env->sr & SR_MD) == 0;

    /* check area */
    if (env->sr & SR_MD) {
	/* For previledged mode, P2 and P4 area is not cachable. */
	if ((0xA0000000 <= addr && addr < 0xC0000000) || 0xE0000000 <= addr)
	    return 0;
    } else {
	/* For user mode, only U0 area is cachable. */
	if (0x80000000 <= addr)
	    return 0;
    }

    /*
     * TODO : Evaluate CCR and check if the cache is on or off.
     *        Now CCR is not in CPUSH4State, but in SH7750State.
     *        When you move the ccr inot CPUSH4State, the code will be
     *        as follows.
     */
#if 0
    /* check if operand cache is enabled or not. */
    if (!(env->ccr & 1))
	return 0;
#endif

    /* if MMU is off, no check for TLB. */
    if (env->mmucr & MMUCR_AT)
	return 1;

    /* check TLB */
    n = find_tlb_entry(env, addr, env->itlb, ITLB_SIZE, use_asid);
    if (n >= 0)
	return env->itlb[n].c;

    n = find_tlb_entry(env, addr, env->utlb, UTLB_SIZE, use_asid);
    if (n >= 0)
	return env->utlb[n].c;

    return 0;
}