Message-ID: <87h8qtfdvj.fsf@xmission.com>
Date:   Wed, 07 Feb 2018 01:38:40 -0600
From:   ebiederm@...ssion.com (Eric W. Biederman)
To:     Khalid Aziz <khalid.aziz@...cle.com>
Cc:     davem@...emloft.net, dave.hansen@...ux.intel.com,
        aarcange@...hat.com, akpm@...ux-foundation.org,
        allen.pais@...cle.com, anthony.yznaga@...cle.com, arnd@...db.de,
        babu.moger@...cle.com, benh@...nel.crashing.org,
        bob.picco@...cle.com, bsingharora@...il.com, corbet@....net,
        dan.j.williams@...el.com, dave.jiang@...el.com,
        david.j.aldridge@...cle.com, elena.reshetova@...el.com,
        glx@...utronix.de, gregkh@...uxfoundation.org, hannes@...xchg.org,
        hillf.zj@...baba-inc.com, hpa@...or.com, hughd@...gle.com,
        imbrenda@...ux.vnet.ibm.com, jack@...e.cz, jag.raman@...cle.com,
        jane.chu@...cle.com, jglisse@...hat.com, jroedel@...e.de,
        khalid@...ehiking.org, khandual@...ux.vnet.ibm.com,
        kirill.shutemov@...ux.intel.com, kstewart@...uxfoundation.org,
        ktkhai@...tuozzo.com, liam.merwick@...cle.com,
        linux-arch@...r.kernel.org, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        linuxppc-dev@...ts.ozlabs.org, linux@...ck-us.net, me@...in.cc,
        mgorman@...e.de, mgorman@...hsingularity.net, mhocko@...e.com,
        mike.kravetz@...cle.com, minchan@...nel.org, mingo@...nel.org,
        mingo@...hat.com, mpe@...erman.id.au, nadav.amit@...il.com,
        nagarathnam.muthusamy@...cle.com, nborisov@...e.com,
        n-horiguchi@...jp.nec.com, nick.alcock@...cle.com,
        nitin.m.gupta@...cle.com, ombredanne@...b.com,
        pasha.tatashin@...cle.com, paulus@...ba.org, pombredanne@...b.com,
        punit.agrawal@....com, rob.gardner@...cle.com,
        ross.zwisler@...ux.intel.com, shannon.nelson@...cle.com,
        shli@...com, sparclinux@...r.kernel.org, steven.sistare@...cle.com,
        tglx@...utronix.de, thomas.tai@...cle.com, tklauser@...tanz.ch,
        tom.hromatka@...cle.com, vegard.nossum@...cle.com,
        vijay.ac.kumar@...cle.com, willy@...radead.org, x86@...nel.org,
        zi.yan@...rutgers.edu
Subject: Re: [PATCH v11 00/10] Application Data Integrity feature introduced by SPARC M7

Khalid Aziz <khalid.aziz@...cle.com> writes:

> On 02/01/2018 07:29 PM, ebiederm@...ssion.com wrote:
>> Khalid Aziz <khalid.aziz@...cle.com> writes:
>>
>>> V11 changes:
>>> This series is same as v10 and was simply rebased on 4.15 kernel. Can
>>> mm maintainers please review patches 2, 7, 8 and 9 which are arch
>>> independent, and include/linux/mm.h and mm/ksm.c changes in patch 10
>>> and ack these if everything looks good?
>>
>> I am a bit puzzled how this differs from the pkey's that other
>> architectures are implementing to achieve a similar result.
>>
>> I am a bit mystified why you don't store the tag in a vma
>> instead of inventing a new way to store data on page out.
>
> Hello Eric,
>
> As Steven pointed out, sparc sets tags per cacheline unlike pkey. This results
> in much finer granularity for tags that pkey and hence requires larger tag
> storage than what we can do in a vma.

*Nod*   I am a bit mystified where you keep the information in memory.
I would think the tags would need to be stored per cacheline or per
tlb entry, in some kind of cache that could overflow.  So I would be
surprised if swapping is the only time this information needs stored
in memory.  Which makes me wonder if you have the proper data
structures.

I would think an array per vma or something in the page tables would
tend to make sense.

But perhaps I am missing something.

>> Can you please use force_sig_fault to send these signals instead
>> of force_sig_info.  Emperically I have found that it is very
>> error prone to generate siginfo's by hand, especially on code
>> paths where several different si_codes may apply.  So it helps
>> to go through a helper function to ensure the fiddly bits are
>> all correct.  AKA the unused bits all need to be set to zero before
>> struct siginfo is copied to userspace.
>>
>
> What you say makes sense. I followed the same code as other fault handlers for
> sparc. I could change just the fault handlers for ADI related faults. Would it
> make more sense to change all the fault handlers in a separate patch and keep
> the code in arch/sparc/kernel/traps_64.c consistent? Dave M, do you have a
> preference?

It is my intention post -rc1 to start sending out patches to get the
rest of not just sparc but all of the architectures using the new
helpers.  I have the code I just ran out of time befor the merge
window opened to ensure everything had a good thorough review.

So if you can handle the your new changes I expect I will handle the
rest.

Eric

Powered by blists - more mailing lists