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@xxxxxxxxxxxxxxx>
> ---
> 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;
> +}
