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:

quoted

On Tue, Jul 28, 2015 at 06:08:14AM +0100, Yong Wu wrote:

quoted

On Mon, 2015-07-27 at 15:11 +0100, Will Deacon wrote:

quoted

On Mon, Jul 27, 2015 at 03:05:38PM +0100, Robin Murphy wrote:

quoted

On 27/07/15 05:21, Yong Wu wrote:

quoted

quoted

quoted

quoted

quoted

+       } 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