linux-kernel - Re: [PATCH v3 3/6] iommu: add ARM short descriptor page table allocator.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150728134710.GM29209@arm.com>
Date:	Tue, 28 Jul 2015 14:47:10 +0100
From:	Will Deacon <will.deacon@....com>
To:	Yong Wu <yong.wu@...iatek.com>
Cc:	Robin Murphy <Robin.Murphy@....com>,
	Joerg Roedel <joro@...tes.org>,
	Thierry Reding <treding@...dia.com>,
	Mark Rutland <Mark.Rutland@....com>,
	Matthias Brugger <matthias.bgg@...il.com>,
	Daniel Kurtz <djkurtz@...gle.com>,
	Tomasz Figa <tfiga@...gle.com>,
	Lucas Stach <l.stach@...gutronix.de>,
	Rob Herring <robh+dt@...nel.org>,
	Catalin Marinas <Catalin.Marinas@....com>,
	"linux-mediatek@...ts.infradead.org" 
	<linux-mediatek@...ts.infradead.org>,
	Sasha Hauer <kernel@...gutronix.de>,
	"srv_heupstream@...iatek.com" <srv_heupstream@...iatek.com>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
	"pebolle@...cali.nl" <pebolle@...cali.nl>,
	"arnd@...db.de" <arnd@...db.de>,
	"mitchelh@...eaurora.org" <mitchelh@...eaurora.org>,
	"cloud.chou@...iatek.com" <cloud.chou@...iatek.com>,
	"frederic.chen@...iatek.com" <frederic.chen@...iatek.com>
Subject: Re: [PATCH v3 3/6] iommu: add ARM short descriptor page table
 allocator.

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/