Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <1c9a1b86-efef-fa56-4451-ac3348b678bf@intel.com>
Date:   Fri, 4 Nov 2022 07:38:17 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.2
Subject: Re: [RFC v2 4/5] x86/crash: make the page that stores the LUKS volume
 key inaccessible
Content-Language: en-US
To:     Coiby Xu <coxu@redhat.com>, kexec@lists.infradead.org
Cc:     Milan Broz <gmazyland@gmail.com>,
        Thomas Staudt <tstaudt@de.ibm.com>,
        Kairui Song <ryncsn@gmail.com>, dm-devel@redhat.com,
        Jan Pazdziora <jpazdziora@redhat.com>,
        Pingfan Liu <kernelfans@gmail.com>,
        Baoquan He <bhe@redhat.com>, Dave Young <dyoung@redhat.com>,
        linux-kernel@vger.kernel.org, Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@kernel.org>,
        "H. Peter Anvin" <hpa@zytor.com>
References: <20221104113000.487098-1-coxu@redhat.com>
 <20221104113000.487098-5-coxu@redhat.com>
From:   Dave Hansen <dave.hansen@intel.com>
In-Reply-To: <20221104113000.487098-5-coxu@redhat.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 11/4/22 04:29, Coiby Xu wrote:
> +	if (kexec_crash_image->luks_volume_key_addr) {
> +		start = kexec_crash_image->luks_volume_key_addr;
> +		end = start + kexec_crash_image->luks_volume_key_sz - 1;
> +		page = pfn_to_page(start >> PAGE_SHIFT);
> +		nr_pages = (end >> PAGE_SHIFT) - (start >> PAGE_SHIFT) + 1;
> +		set_memory_np((unsigned long)page_address(page), nr_pages);
> +	}

Why does this go pfn -> page -> vaddr?  What good does having the page
do?  Can you just do phys_to_virt() on the start address?  Maybe:

 	start_paddr = kexec_crash_image->luks_volume_key_addr;
 	end_paddr   = start_paddr + kexec_crash_image->luks_volume_key_sz - 1;
 	nr_pages = (PAGE_ALIGN(end_paddr) - PAGE_ALIGN_DOWN(start_paddr))/
PAGE_SIZE;
 	set_memory_np((unsigned long)phys_to_virt(start_paddr), nr_pages);

Also, if you resend this, please just cc the x86 folks on the series.
The other patches and cover letter have desperately needed context
around this.