lkml.org 
[lkml]   [2017]   [Sep]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v6 05/11] arm64/mm: Add support for XPFO
    From
    Date
    On 14/09/17 12:41, Julien Grall wrote:
    > Hi,
    >
    > CC Juergen, Boris and Stefano.
    >
    > On 08/09/17 18:24, Tycho Andersen wrote:
    >> On Fri, Sep 08, 2017 at 12:53:47AM -0700, Christoph Hellwig wrote:
    >>>> +/*
    >>>> + * Lookup the page table entry for a virtual address and return a
    >>>> pointer to
    >>>> + * the entry. Based on x86 tree.
    >>>> + */
    >>>> +static pte_t *lookup_address(unsigned long addr)
    >>>
    >>> Seems like this should be moved to common arm64 mm code and used by
    >>> kernel_page_present.
    >>
    >> Sounds good, I'll include something like the patch below in the next
    >> series.
    >>
    >> Unfortunately, adding an implementation of lookup_address seems to be
    >> slightly more complicated than necessary, because of the xen piece. We
    >> have to define lookup_address() with the level parameter, but it's not
    >> obvious to me to name the page levels. So for now I've just left it as
    >> a WARN() if someone supplies it.
    >>
    >> It seems like xen still does need this to be defined, because if I
    >> define it without level:
    >>
    >> drivers/xen/xenbus/xenbus_client.c: In function
    >> ‘xenbus_unmap_ring_vfree_pv’:
    >> drivers/xen/xenbus/xenbus_client.c:760:4: error: too many arguments to
    >> function ‘lookup_address’
    >>      lookup_address(addr, &level)).maddr;
    >>      ^~~~~~~~~~~~~~
    >> In file included from ./arch/arm64/include/asm/page.h:37:0,
    >>                   from ./include/linux/mmzone.h:20,
    >>                   from ./include/linux/gfp.h:5,
    >>                   from ./include/linux/mm.h:9,
    >>                   from drivers/xen/xenbus/xenbus_client.c:33:
    >> ./arch/arm64/include/asm/pgtable-types.h:67:15: note: declared here
    >>   extern pte_t *lookup_address(unsigned long addr);
    >>                 ^~~~~~~~~~~~~~
    >>
    >> I've cc-d the xen folks, maybe they can suggest a way to untangle it?
    >> Alternatively, if someone can suggest a good naming scheme for the
    >> page levels, I can just do that.
    >
    > The implementation of lookup_address(...) on ARM for Xen (see
    > include/xen/arm/page.h) is just a BUG(). This is because this code
    > should never be called (only used for x86 PV code).
    >
    > Furthermore, xenbus client does not use at all the level. It is just to
    > cope with the x86 version of lookup_address.
    >
    > So one way to solve the problem would be to introduce
    > xen_lookup_address(addr) that would be implemented as:
    >     - on x86
    >         unsigned int level;
    >
    >         return lookup_address(addr, &level).maddr;
    >     - on ARM
    >         BUG();
    >
    > With that there would be no prototype clash and avoid introducing a
    > level parameter.

    I'd rather add some #ifdef CONFIG_XEN_PV and remove the *_pv functions
    from ARM completely this way. I'm sending a patch soon...


    Juergen

    \
     
     \ /
      Last update: 2017-09-14 19:28    [W:2.268 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site