Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755675AbbG1Ns0 (ORCPT ); Tue, 28 Jul 2015 09:48:26 -0400 Received: from foss.arm.com ([217.140.101.70]:59208 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932543AbbG1NrK (ORCPT ); Tue, 28 Jul 2015 09:47:10 -0400 Date: Tue, 28 Jul 2015 14:47:10 +0100 From: Will Deacon To: Yong Wu Cc: Robin Murphy , Joerg Roedel , Thierry Reding , Mark Rutland , Matthias Brugger , Daniel Kurtz , Tomasz Figa , Lucas Stach , Rob Herring , Catalin Marinas , "linux-mediatek@lists.infradead.org" , Sasha Hauer , "srv_heupstream@mediatek.com" , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "iommu@lists.linux-foundation.org" , "pebolle@tiscali.nl" , "arnd@arndb.de" , "mitchelh@codeaurora.org" , "cloud.chou@mediatek.com" , "frederic.chen@mediatek.com" Subject: Re: [PATCH v3 3/6] iommu: add ARM short descriptor page table allocator. Message-ID: <20150728134710.GM29209@arm.com> References: <1437037475-9065-4-git-send-email-yong.wu@mediatek.com> <20150721171101.GN31095@arm.com> <1437715466.23932.68.camel@mhfsdcap03> <20150724165325.GC21177@arm.com> <1437970868.25925.20.camel@mhfsdcap03> <55B63AB2.4050006@arm.com> <20150727141102.GJ3358@arm.com> <1438060094.25925.69.camel@mhfsdcap03> <20150728110023.GH29209@arm.com> <1438090663.25925.75.camel@mhfsdcap03> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1438090663.25925.75.camel@mhfsdcap03> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3324 Lines: 66 On Tue, Jul 28, 2015 at 02:37:43PM +0100, Yong Wu wrote: > On Tue, 2015-07-28 at 12:00 +0100, Will Deacon wrote: > > On Tue, Jul 28, 2015 at 06:08:14AM +0100, Yong Wu wrote: > > > On Mon, 2015-07-27 at 15:11 +0100, Will Deacon wrote: > > > > On Mon, Jul 27, 2015 at 03:05:38PM +0100, Robin Murphy wrote: > > > > > On 27/07/15 05:21, Yong Wu wrote: > > > > > >>>>> + } else { /* page or largepage */ > > > > > >>>>> + if (quirk & IO_PGTABLE_QUIRK_SHORT_MTK) { > > > > > >>>>> + if (large) { /* special Bit */ > > > > > >>>> > > > > > >>>> This definitely needs a better comment! What exactly are you doing here > > > > > >>>> and what is that quirk all about? > > > > > >>> > > > > > >>> I use this quirk is for MTK Special Bit as we don't have the XN bit in > > > > > >>> pagetable. > > > > > >> > > > > > >> I'm still not really clear about what this is. > > > > > > > > > > > > There is some difference between the standard spec and MTK HW, > > > > > > Our hw don't implement some bits, like XN and AP. > > > > > > So I add a quirk for MTK special. > > > > > > > > > > When you say it doesn't implement these bits, do you mean that having > > > > > them set will lead to Bad Things happening in the hardware, or that it > > > > > will simply ignore them and not enforce any of the protections they > > > > > imply? The former case would definitely want clearly documenting > > > > > somewhere, whereas for the latter case I'm not sure it's even worth the > > > > > complication of having a quirk - if the value doesn't matter there seems > > > > > little point in doing a special dance just for the sake of semantic > > > > > correctness of the in-memory PTEs, in my opinion. > > > > > > > > Agreed. We should only use quirks if the current (architecturally > > > > compliant) code causes real issues with the hardware. Then the quirk can > > > > be used to either avoid the problematic routines or to take extra steps > > > > to make things work as the architecture intended. > > > > > > > > I've asked how this IOMMU differs from the architecture on a number of > > > > occasions, but I'm still yet to receive a response other than "it's special". > > > > > > > > > > After check further with DE, Our pagetable is refer to ARM-v7's > > > short-descriptor which is a little different from ARM-v8. like bit0(PXN) > > > in section and supersection, I didn't read ARM-v7 spec before, so I add > > > a MTK quirk to disable PXN bit in section and supersection.(if the PXN > > > bit is wrote in ARM-v7 spec, HW will page fault.) > > > > I've been reviewing this using the ARMv7 ARM (Rev.C of DDI0406C) the whole > > time. PXN is there as an optional field in non-LPAE implementations. That's > > fine and doesn't require any quirks. > > Thanks for your confirm. > Then I delete all the PXN bit in here? > > Take a example, > #define ARM_SHORT_PGD_SECTION_XN (BIT(0) | BIT(4)) > I will change it to "BIT(4)". Yes. Then the PXN bit can be added later as a quirk when we have an implementation that supports it. Will -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/