On Mon, Mar 4, 2019 at 11:31 AM Robin Murphy <[email protected]> wrote:
>
> On 04/03/2019 15:32, Rob Herring wrote:
> > On Fri, Mar 1, 2019 at 10:28 AM Robin Murphy <[email protected]> wrote:
> >>
> >> On 27/02/2019 23:22, Rob Herring wrote:
> >>> ARM Mali midgard GPU page tables are similar to standard 64-bit stage 1
> >>> page tables, but have a few differences. Add a new format type to
> >>> represent the format. The input address size is 48-bits and the output
> >>> address size is 40-bits (and possibly less?). Note that the later
> >>> bifrost GPUs follow the standard 64-bit stage 1 format.
> >>>
> >>> The differences in the format compared to 64-bit stage 1 format are:
> >>>
> >>> The 3rd level page entry bits are 0x1 instead of 0x3 for page entries.
> >>>
> >>> The access flags are not read-only and unprivileged, but read and write.
> >>> This is similar to stage 2 entries, but the memory attributes field 
> >>> matches
> >>> stage 1 being an index.
> >>>
> >>> The nG bit is not set by the vendor driver. This one didn't seem to 
> >>> matter,
> >>> but we'll keep it aligned to the vendor driver.
> >
> > [...]
> >
> >>> +     cfg->pgsize_bitmap &= (SZ_4K | SZ_2M | SZ_1G);
> >>> +     iop = arm_64_lpae_alloc_pgtable_s1(cfg, cookie);
> >>> +     if (iop)
> >>> +             cfg->arm_lpae_s1_cfg.tcr = 0;
> >>
> >> The general design intent is that we return ready-to-go register values
> >> here (much like mmu_get_as_setup() in mali_kbase, it seems), so I think
> >> it's worth adding an arm_mali_cfg to the io_pgtable_cfg union and
> >> transforming the VMSA output into final transtab/transcfg/memattr format
> >> at this point, rather then callers needing to care (e.g. some of those
> >> AS_TRANSTAB_LPAE_* bits look reminiscent of the walk attributes we set
> >> up for a VMSA TCR).
> >
> > I agree with returning ready-to-go values, but I'm not so sure the
> > bits are the same. Bits 0-1 are enable/mode bits which are pretty
> > specific to mali. Bit 2 is 'read inner' for whatever that means.
> > Perhaps it is read allocate cacheability, but that's a bit different
> > than RGN bits. Bit 4 is 'share outer'. Maybe it's the same as SH0 if
> > bit 3 is included, but I have no evidence of that. So I don't think
> > there's really anything to transform. We just set the bits needed. So
> > here's what I have in mind:
>
> Right, my Friday-afternoon wording perhaps wasn't the best - by
> "transform" I didn't mean to imply trying to reinterpret the default
> settings we configure for a VMSA TCR, merely applying some
> similarly-appropriate defaults to make a Mali TRANSTAB out of the VMSA TTBR.
>
> > iop = arm_64_lpae_alloc_pgtable_s1(cfg, cookie);
> > if (iop) {
> >    u64 mair, ttbr;
> >
> >    /* Copy values as union fields overlap */
> >    mair = cfg->arm_lpae_s1_cfg.mair[0];
> >    ttbr = cfg->arm_lpae_s1_cfg.ttbr[0];
> >
> >    cfg->arm_mali_lpae_cfg.mair = mair;
> >    cfg->arm_mali_lpae_cfg.ttbr = ttbr;
> >    cfg->arm_mali_lpae_cfg.ttbr |= ARM_MALI_LPAE_TTBR_READ_INNER |
> >      ARM_MALI_LPAE_TTBR_ADRMODE_TABLE;
> > }
>
> ...pretty much exactly like that (although I'd still prefer to use the
> Mali register names for clarity, and presumably you'll still explicitly
> initialise TRANSCFG too in the real patch).

No, TRANSCFG is only on Bifrost. While the page table format seems to
be standard stage 1 64-bit, the registers are still different from
anything else. So I guess we'll need yet another format definition
when we get there. Also, we may still have to massage the register
values outside of this code. It's not going to know the
'system_coherency' value the kbase driver uses (And I'm not sure how
we want to express that upstream either).

Rob
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to