From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50374) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZC1pK-0002YB-6A for qemu-devel@nongnu.org; Mon, 06 Jul 2015 04:32:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZC1pI-00041j-Ur for qemu-devel@nongnu.org; Mon, 06 Jul 2015 04:32:34 -0400 Received: from hall.aurel32.net ([2001:bc8:30d7:100::1]:47089) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZC1pI-00041e-PM for qemu-devel@nongnu.org; Mon, 06 Jul 2015 04:32:32 -0400 Date: Mon, 6 Jul 2015 10:32:31 +0200 From: Aurelien Jarno Message-ID: <20150706083231.GT931@aurel32.net> References: <478545ddab5b50a804bed4eea1c8f38e155335fa.1435723168.git.serge.vakulenko@gmail.com> <20150701101107.GB11361@aurel32.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH pic32 v2 2/5] Fixed random index generation for TLBWR instruction. It was not quite random and did not skip Wired entries. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Serge Vakulenko Cc: Leon Alrae , qemu-devel@nongnu.org On 2015-07-05 17:03, Serge Vakulenko wrote: > On Wed, Jul 1, 2015 at 3:11 AM, Aurelien Jarno wrote: > > On 2015-06-30 21:12, Serge Vakulenko wrote: > >> Signed-off-by: Serge Vakulenko > >> --- > >> hw/mips/cputimer.c | 18 +++++------------- > >> 1 file changed, 5 insertions(+), 13 deletions(-) > >> > >> diff --git a/hw/mips/cputimer.c b/hw/mips/cputimer.c > >> index 4f02a9f..94a29df 100644 > >> --- a/hw/mips/cputimer.c > >> +++ b/hw/mips/cputimer.c > >> @@ -25,21 +25,13 @@ > >> #include "qemu/timer.h" > >> #include "sysemu/kvm.h" > >> > >> -#define TIMER_FREQ 100 * 1000 * 1000 > >> - > >> -/* XXX: do not use a global */ > >> +/* Generate a random TLB index. > >> + * Skip wired entries. */ > >> uint32_t cpu_mips_get_random (CPUMIPSState *env) > >> { > >> - static uint32_t lfsr = 1; > >> - static uint32_t prev_idx = 0; > >> - uint32_t idx; > >> - /* Don't return same value twice, so get another value */ > >> - do { > >> - lfsr = (lfsr >> 1) ^ (-(lfsr & 1u) & 0xd0000001u); > >> - idx = lfsr % (env->tlb->nb_tlb - env->CP0_Wired) + env->CP0_Wired; > >> - } while (idx == prev_idx); > >> - prev_idx = idx; > >> - return idx; > >> + env->CP0_Random = env->CP0_Wired + > >> + random() % (env->tlb->nb_tlb - env->CP0_Wired); > >> + return env->CP0_Random; > >> } > >> > >> /* MIPS R4K timer */ > > > > Can you please give us more details about what issue you are trying to > > fix there? Especially I don't understand about the "skip wired entries" > > part. It seems the original code handles the wired entries correctly, > > and at least your patch doesn't seem to change anything regarded that > > part. > > The original code looks fine by itself. But when you try to run in for > nb_tlb=16 and CP0_Wired=1, you get a sequence: > > 15, 6, 7, 2, 7, 2, 7, 2, 7, 2, 7, 2, 7, 2, 7, 2, 7, 2, 7, 2, 7, 2, 7, > 2, 7, 2, 7, 2... > > This is what happens when 4.4bsd kernel starts on pic32mz processor. > It makes the VM subsystem a bit crazy. Later the sequence becomes > better, but I think it makes sense to improve it somehow. > Thanks for the explanation, I know understand the issue and I agree it should be fixed. -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurelien@aurel32.net http://www.aurel32.net