lkml.org 
[lkml]   [2022]   [Apr]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCHv4 6/8] x86/mm: Provide helpers for unaccepted memory
    On 4/5/22 16:43, Kirill A. Shutemov wrote:
    > Core-mm requires few helpers to support unaccepted memory:
    >
    > - accept_memory() checks the range of addresses against the bitmap and
    > accept memory if needed.
    >
    > - memory_is_unaccepted() check if anything within the range requires
    > acceptance.
    >
    > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
    > ---
    > arch/x86/include/asm/page.h | 5 +++
    > arch/x86/include/asm/unaccepted_memory.h | 1 +
    > arch/x86/mm/Makefile | 2 +
    > arch/x86/mm/unaccepted_memory.c | 53 ++++++++++++++++++++++++
    > 4 files changed, 61 insertions(+)
    > create mode 100644 arch/x86/mm/unaccepted_memory.c
    >
    > diff --git a/arch/x86/include/asm/page.h b/arch/x86/include/asm/page.h
    > index 9cc82f305f4b..9ae0064f97e5 100644
    > --- a/arch/x86/include/asm/page.h
    > +++ b/arch/x86/include/asm/page.h
    > @@ -19,6 +19,11 @@
    > struct page;
    >
    > #include <linux/range.h>
    > +
    > +#ifdef CONFIG_UNACCEPTED_MEMORY
    > +#include <asm/unaccepted_memory.h>
    > +#endif

    It's a lot nicer to just to the #ifdefs inside the header. Is there a
    specific reason to do it this way?

    > extern struct range pfn_mapped[];
    > extern int nr_pfn_mapped;
    >
    > diff --git a/arch/x86/include/asm/unaccepted_memory.h b/arch/x86/include/asm/unaccepted_memory.h
    > index f1f835d3cd78..a8d12ef1bda8 100644
    > --- a/arch/x86/include/asm/unaccepted_memory.h
    > +++ b/arch/x86/include/asm/unaccepted_memory.h
    > @@ -10,5 +10,6 @@ struct boot_params;
    > void mark_unaccepted(struct boot_params *params, u64 start, u64 num);
    >
    > void accept_memory(phys_addr_t start, phys_addr_t end);
    > +bool memory_is_unaccepted(phys_addr_t start, phys_addr_t end);
    >
    > #endif
    > diff --git a/arch/x86/mm/Makefile b/arch/x86/mm/Makefile
    > index fe3d3061fc11..e327f83e6bbf 100644
    > --- a/arch/x86/mm/Makefile
    > +++ b/arch/x86/mm/Makefile
    > @@ -60,3 +60,5 @@ obj-$(CONFIG_AMD_MEM_ENCRYPT) += mem_encrypt_amd.o
    >
    > obj-$(CONFIG_AMD_MEM_ENCRYPT) += mem_encrypt_identity.o
    > obj-$(CONFIG_AMD_MEM_ENCRYPT) += mem_encrypt_boot.o
    > +
    > +obj-$(CONFIG_UNACCEPTED_MEMORY) += unaccepted_memory.o
    > diff --git a/arch/x86/mm/unaccepted_memory.c b/arch/x86/mm/unaccepted_memory.c
    > new file mode 100644
    > index 000000000000..3588a7cb954c
    > --- /dev/null
    > +++ b/arch/x86/mm/unaccepted_memory.c
    > @@ -0,0 +1,53 @@
    > +// SPDX-License-Identifier: GPL-2.0-only
    > +#include <linux/memblock.h>
    > +#include <linux/mm.h>
    > +#include <linux/pfn.h>
    > +#include <linux/spinlock.h>
    > +
    > +#include <asm/io.h>
    > +#include <asm/setup.h>
    > +#include <asm/unaccepted_memory.h>
    > +
    > +static DEFINE_SPINLOCK(unaccepted_memory_lock);

    We need some documentation on what the lock does, either here or in the
    changelog.

    > +void accept_memory(phys_addr_t start, phys_addr_t end)
    > +{
    > + unsigned long *unaccepted_memory;
    > + unsigned long flags;
    > + unsigned int rs, re;
    > +
    > + if (!boot_params.unaccepted_memory)
    > + return;
    > +
    > + unaccepted_memory = __va(boot_params.unaccepted_memory);
    > + rs = start / PMD_SIZE;
    > +
    > + spin_lock_irqsave(&unaccepted_memory_lock, flags);
    > + for_each_set_bitrange_from(rs, re, unaccepted_memory,
    > + DIV_ROUND_UP(end, PMD_SIZE)) {
    > + /* Platform-specific memory-acceptance call goes here */
    > + panic("Cannot accept memory");
    > + bitmap_clear(unaccepted_memory, rs, re - rs);
    > + }
    > + spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
    > +}

    That panic() is making me nervous. Is this bisect-safe? Is it safe
    because there are no callers of this function yet?

    > +bool memory_is_unaccepted(phys_addr_t start, phys_addr_t end)
    > +{
    > + unsigned long *unaccepted_memory = __va(boot_params.unaccepted_memory);
    > + unsigned long flags;
    > + bool ret = false;
    > +
    > + spin_lock_irqsave(&unaccepted_memory_lock, flags);
    > + while (start < end) {
    > + if (test_bit(start / PMD_SIZE, unaccepted_memory)) {
    > + ret = true;
    > + break;
    > + }
    > +
    > + start += PMD_SIZE;
    > + }
    > + spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
    > +
    > + return ret;
    > +}

    \
     
     \ /
      Last update: 2022-04-08 20:15    [W:2.885 / U:0.304 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site