Re: [PATCH v8 1/4] powerpc: Refactor kexec functions to move arch independent code to drivers/of

From: Mimi Zohar <zohar@linux.ibm.com>
To: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>,
	bauerman@linux.ibm.com, robh@kernel.org,
	gregkh@linuxfoundation.org, james.morse@arm.com,
	catalin.marinas@arm.com, sashal@kernel.org, will@kernel.org,
	mpe@ellerman.id.au, benh@kernel.crashing.org, paulus@samba.org,
	robh+dt@kernel.org, frowand.list@gmail.com,
	vincenzo.frascino@arm.com, mark.rutland@arm.com,
	dmitry.kasatkin@gmail.com, jmorris@namei.org, serge@hallyn.com,
	pasha.tatashin@soleen.com, allison@lohutok.net,
	kstewart@linuxfoundation.org, takahiro.akashi@linaro.org,
	tglx@linutronix.de, masahiroy@kernel.org, bhsharma@redhat.com,
	mbrugger@suse.com, hsinyi@chromium.org, tao.li@vivo.com,
	christophe.leroy@c-s.fr
Cc: linux-integrity@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, prsriva@linux.microsoft.com,
	balajib@linux.microsoft.com
Subject: Re: [PATCH v8 1/4] powerpc: Refactor kexec functions to move arch independent code to drivers/of
Date: Tue, 03 Nov 2020 09:55:03 -0500	[thread overview]
Message-ID: <506372c01decf59bc2ef39c92a86ba7f6f2ae81b.camel@linux.ibm.com> (raw)
In-Reply-To: <20201030174429.29893-2-nramas@linux.microsoft.com>

Hi Lakshmi,

On Fri, 2020-10-30 at 10:44 -0700, Lakshmi Ramasubramanian wrote:
> The functions remove_ima_buffer() and delete_fdt_mem_rsv() that handle
> carrying forward the IMA measurement logs on kexec for powerpc do not
> have architecture specific code, but they are currently defined for
> powerpc only.

^ ... logs on kexec, do not have architecture specific code, but are
currently limited to powerpc.

> 
> remove_ima_buffer() and delete_fdt_mem_rsv() are used to remove
> the IMA log entry from the device tree and free the memory reserved
> for the log. These functions need to be defined even if the current
> kernel does not support carrying forward IMA log across kexec since
> the previous kernel could have supported that and therefore the current
> kernel needs to free the allocation.

The first paragraph describes these function as "handle carrying
forward the IMA measurement logs on kexec", while this paragraph says
"are used to remove the IMA log entry".  Consider listing all of the
functions being moved in the first paragrah, then "handle carrying
forward" could be expanded to "carrying ... and removing".

> 
> Rename remove_ima_buffer() to remove_ima_kexec_buffer().
> Define remove_ima_kexec_buffer() and delete_fdt_mem_rsv() in
> drivers/of/fdt.c. A later patch in this series will use these functions
> to free the allocation, if any, made by the previous kernel for ARM64.

- ^Define -> Move
- Three functions are being moved, but only two are listed. 
"do_get_kexec_buffer" is not mentioned.
- Don't refer to a later patch, but explain the purpose here.  For
example, "Move ... , making them accessible to other archs."

> 
> Define FDT_PROP_IMA_KEXEC_BUFFER for the chosen node, namely
> "linux,ima-kexec-buffer", that is added to the DTB to hold
> the address and the size of the memory reserved to carry
> the IMA measurement log.

The above two paragraphs describe renaming a function and defining a
chosen node.  These two preparatory changes should be made,
independently of each other, prior to this patch.  This patch should be
limited to moving code, with the subject line truncated to "move arch
independent code to drivers/of".

thanks,

Mimi