From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37278) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VeQI0-0005s6-Tb for qemu-devel@nongnu.org; Thu, 07 Nov 2013 09:10:36 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VeQHs-0000AQ-Oa for qemu-devel@nongnu.org; Thu, 07 Nov 2013 09:10:28 -0500 Received: from e28smtp06.in.ibm.com ([122.248.162.6]:44647) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VeQHs-00009X-49 for qemu-devel@nongnu.org; Thu, 07 Nov 2013 09:10:20 -0500 Received: from /spool/local by e28smtp06.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 7 Nov 2013 19:40:16 +0530 From: "Aneesh Kumar K.V" In-Reply-To: References: <1381827527-24475-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com> <1381827527-24475-3-git-send-email-aneesh.kumar@linux.vnet.ibm.com> Date: Thu, 07 Nov 2013 19:34:13 +0530 Message-ID: <87ppqcs3z6.fsf@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH -V6 3/3] target-ppc: Fix htab_mask calculation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: Paul Mackerras , "open list:PReP" , QEMU Developers Alexander Graf writes: > On 15.10.2013, at 01:58, Aneesh Kumar K.V wrote: > >> From: "Aneesh Kumar K.V" >> >> Correctly update the htab_mask using the return value of >> KVM_PPC_ALLOCATE_HTAB ioctl. Also we don't update sdr1 >> on GET_SREGS for HV. So don't update htab_mask if sdr1 >> is found to be zero. Fix the pte index calculation to be >> same as that found in the kernel >> >> Signed-off-by: Aneesh Kumar K.V >> --- >> hw/ppc/spapr.c | 3 ++- >> target-ppc/mmu-hash64.c | 2 +- >> target-ppc/mmu_helper.c | 4 +++- >> 3 files changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >> index 22f2a8a..d4f3502 100644 >> --- a/hw/ppc/spapr.c >> +++ b/hw/ppc/spapr.c >> @@ -724,7 +724,8 @@ static void spapr_cpu_reset(void *opaque) >> env->external_htab = (void *)1; >> } >> env->htab_base = -1; >> - env->htab_mask = HTAB_SIZE(spapr) - 1; >> + /* 128 (2**7) bytes in each HPTEG */ >> + env->htab_mask = (1ULL << ((spapr)->htab_shift - 7)) - 1; > > HTAB_SIZE(spapr) / 128? The compiler should be smart enough to produce > the same code out of that. (HTAB_SIZE(spapr) - 1) / 128. I am not sure that is any better ? > > However, could you please explain why it's better to have the mask be > on the PTEG rather than the offset? Is this something you missed in > the previous patch? If so, please change the semantics on what > htab_mask means before you break the code as that makes bisecting > hard. That is how kernel does the masking. In kvmppc_alloc_hpt() we have /* 128 (2**7) bytes in each HPTEG */ kvm->arch.hpt_mask = (1ul << (order - 7)) - 1; If we don't have it same, we would end up with wrong hpte index > > Furthermore, since you are changing the semantics of htab_mask, have > you checked all other users of it? Most notably the hash32 code. I can verify the code, but i don't have setup -aneesh