From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.5 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 147CFC433DB for ; Fri, 22 Jan 2021 10:41:09 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 62BA322DCC for ; Fri, 22 Jan 2021 10:41:07 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 62BA322DCC Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 534606B0007; Fri, 22 Jan 2021 05:41:07 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 4F1156B000A; Fri, 22 Jan 2021 05:41:07 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3AB6A6B000C; Fri, 22 Jan 2021 05:41:07 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0034.hostedemail.com [216.40.44.34]) by kanga.kvack.org (Postfix) with ESMTP id 211F36B0007 for ; Fri, 22 Jan 2021 05:41:07 -0500 (EST) Received: from smtpin13.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id D26B7180CCE1A for ; Fri, 22 Jan 2021 10:41:06 +0000 (UTC) X-FDA: 77733068532.13.cars36_07182892756b Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin13.hostedemail.com (Postfix) with ESMTP id A8C8118140B67 for ; Fri, 22 Jan 2021 10:41:06 +0000 (UTC) X-HE-Tag: cars36_07182892756b X-Filterd-Recvd-Size: 4478 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf08.hostedemail.com (Postfix) with ESMTP for ; Fri, 22 Jan 2021 10:41:05 +0000 (UTC) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 2C5D411D4; Fri, 22 Jan 2021 02:41:05 -0800 (PST) Received: from [10.163.90.127] (unknown [10.163.90.127]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 81AA93F719; Fri, 22 Jan 2021 02:41:01 -0800 (PST) Subject: Re: [PATCH V3 1/3] mm/memory_hotplug: Prevalidate the address range being added with platform To: David Hildenbrand , linux-mm@kvack.org, akpm@linux-foundation.org, hca@linux.ibm.com, catalin.marinas@arm.com Cc: Oscar Salvador , Vasily Gorbik , Will Deacon , Ard Biesheuvel , Mark Rutland , linux-arm-kernel@lists.infradead.org, linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org References: <1610975582-12646-1-git-send-email-anshuman.khandual@arm.com> <1610975582-12646-2-git-send-email-anshuman.khandual@arm.com> <9916f217-ec29-33ff-a260-7a26792d23a1@redhat.com> From: Anshuman Khandual Message-ID: <897c31ba-d3bd-b694-8c87-82e784a60c51@arm.com> Date: Fri, 22 Jan 2021 16:11:25 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <9916f217-ec29-33ff-a260-7a26792d23a1@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Bogosity: Ham, tests=bogofilter, spamicity=0.000003, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On 1/22/21 2:48 PM, David Hildenbrand wrote: > >> +/* >> + * Platforms should define arch_get_mappable_range() that provides >> + * maximum possible addressable physical memory range for which the >> + * linear mapping could be created. The platform returned address >> + * range must adhere to these following semantics. >> + * >> + * - range.start <= range.end >> + * - Range includes both end points [range.start..range.end] >> + * >> + * There is also a fallback definition provided here, allowing the >> + * entire possible physical address range in case any platform does >> + * not define arch_get_mappable_range(). >> + */ >> +struct range __weak arch_get_mappable_range(void) >> +{ >> + struct range memhp_range = { >> + .start = 0UL, >> + .end = -1ULL, >> + }; >> + return memhp_range; >> +} >> + >> +struct range memhp_get_pluggable_range(bool need_mapping) >> +{ >> + const u64 max_phys = (1ULL << (MAX_PHYSMEM_BITS + 1)) - 1; > > Sorry, thought about that line a bit more, and I think this is just > wrong (took me longer to realize as it should). The old code used this > calculation to print the limit only (in a wrong way), let's recap: > > Assume MAX_PHYSMEM_BITS=32 > > max_phys = (1ULL << (32 + 1)) - 1 = 0x1ffffffffull; > > Ehm, these are 33 bit. > > OTOH, old code checked for > > if (max_addr >> MAX_PHYSMEM_BITS) { > > Which makes sense, because > > 0x1ffffffffull >> 32 = 1 > > results in "true", meaning it's to big, while > > 0xffffffffull >> 32 = 0 > > correctly results in "false", meaning the address is fine. > > > > So, this should just be > > const u64 max_phys = 1ULL << MAX_PHYSMEM_BITS; > > (similarly as calculated in virito-mem code, or in kernel/resource.c) Should this be 1ULL << MAX_PHYSMEM_BITS - 1 instead ? Currently there are three usage for this variable in the function. - if (mhp_range.start > max_phys) - mhp_range.end = min_t(u64, mhp_range.end, max_phys) - mhp_range.end = max_phys mhp_range.end being always inclusive on the higher end and could be maximum (1ULL << MAX_PHYSMEM_BITS - 1) which is 0xFFFFFFFF instead of 0x100000000 when (1ULL << MAX_PHYSMEM_BITS) is followed for a 32 bit system. This seems consistent with the default fallback (range.end = -1ULL) defined above. > > >> + struct range memhp_range; >> + >> + if (need_mapping) { >> + memhp_range = arch_get_mappable_range(); >> + if (memhp_range.start > max_phys) { >> + memhp_range.start = 0; >> + memhp_range.end = 0; >> + } >> + memhp_range.end = min_t(u64, memhp_range.end, max_phys); >> + } else { >> + memhp_range.start = 0; >> + memhp_range.end = max_phys; >> + } >> + return memhp_range; >> +} >> +EXPORT_SYMBOL_GPL(memhp_get_pluggable_range); > >