From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60591) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z4rvD-0000LR-64 for qemu-devel@nongnu.org; Tue, 16 Jun 2015 10:33:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z4rv4-0008Kw-Un for qemu-devel@nongnu.org; Tue, 16 Jun 2015 10:33:03 -0400 Received: from mail-pd0-f170.google.com ([209.85.192.170]:33703) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z4rv4-0008KK-PD for qemu-devel@nongnu.org; Tue, 16 Jun 2015 10:32:54 -0400 Received: by pdjn11 with SMTP id n11so15773939pdj.0 for ; Tue, 16 Jun 2015 07:32:54 -0700 (PDT) Message-ID: <5580338E.6090300@ozlabs.ru> Date: Wed, 17 Jun 2015 00:32:46 +1000 From: Alexey Kardashevskiy MIME-Version: 1.0 References: <20150615172804.22475.7018.stgit@bahia.huguette.org> <557FE41F.9020204@ozlabs.ru> <20150616112457.7f3dd7bb@bahia.local> In-Reply-To: <20150616112457.7f3dd7bb@bahia.local> Content-Type: text/plain; charset=koi8-r; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH] spapr_iommu: fix erroneous sanity check in h_put_tce_indirect() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Greg Kurz Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org, David Gibson On 06/16/2015 07:24 PM, Greg Kurz wrote: > On Tue, 16 Jun 2015 18:53:51 +1000 > Alexey Kardashevskiy wrote: > >> On 06/16/2015 03:28 AM, Greg Kurz wrote: >>> According to PAPR+ 3.2.2.2, the TCE is implemented as follows: >>> bits 0-51: real page number >>> bits 52-61: reserved for future use >>> bits 62-63: page protection (read-only, write-only, read-write) >> >> >> >> tce_list is not a TCE, it is "The logical address of a page of (4 K long on >> a 4 K boundary) of TCE contents to be stored in the TCE table (contains >> logical address of storage page to be mapped)" so we rather want to remove >> (tce_list & ~SPAPR_TCE_RW) below. >> > > Oops my bad for the confustion... :-\ > > Then it looks like SPAPR_TCE_RW can be dropped as well since put_tce_emu() uses > IOMMU_RW instead. Well, spapr_tce_translate_iommu() should actually translate SPAPR_TCE_xxx (PAPR defined flags) to IOMMU_xxx (QEMU IOMMUTLBEntry permission bits), not just rely on the fact that they match. > >> >>> Possible values for the protection bits are necessarly non-zero and thus >>> H_PUT_TCE_INDIRECT currently always fails and returns H_PARAMETER. >>> >>> The code explicitly clears the protection bits when computing the TCE offset, >>> a few lines below (tce_list & ~SPAPR_TCE_RW)... The sanity check is obviously >>> wrong for these bits. >>> >>> Moreover, I could find no indication in PAPR+ that using the other reserved >>> bits should error out with H_PARAMETER. >>> >>> This patch simply drops the offending check. >>> >>> Signed-off-by: Greg Kurz >>> --- >>> hw/ppc/spapr_iommu.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c >>> index 8cd9dba9ac4d..37a1110c9d87 100644 >>> --- a/hw/ppc/spapr_iommu.c >>> +++ b/hw/ppc/spapr_iommu.c >>> @@ -258,7 +258,7 @@ static target_ulong h_put_tce_indirect(PowerPCCPU *cpu, >>> return H_PARAMETER; >>> } >>> >>> - if ((npages > 512) || (tce_list & SPAPR_TCE_PAGE_MASK)) { >>> + if (npages > 512) { >>> return H_PARAMETER; >>> } >>> >>> >>> >> >> > -- Alexey