Re: [PATCH] x86/mm/sme: fix the kdump kernel breakage on SME system when CONFIG_IMA_KEXEC=y

[Tom Lendacky <thomas.lendacky@amd.com>]

On 8/26/24 22:19, Baoquan He wrote:
> On 08/26/24 at 09:24am, Tom Lendacky wrote:
>> On 8/25/24 21:44, Baoquan He wrote:
>>> Recently, it's reported that kdump kernel is broken during bootup on
>>> SME system when CONFIG_IMA_KEXEC=y. When debugging, I noticed this
>>> can be traced back to commit ("b69a2afd5afc x86/kexec: Carry forward
>>> IMA measurement log on kexec"). Just nobody ever tested it on SME
>>> system when enabling CONFIG_IMA_KEXEC.
>>>
>>>
>>> Here fix the code bug to make kexec/kdump kernel boot up successfully.
>>>
>>> Fixes: 8f716c9b5feb ("x86/mm: Add support to access boot related data in the clear")
>>
>> The check that was modified was added by:
>> 	b3c72fc9a78e ("x86/boot: Introduce setup_indirect")
>>
>> The SETUP_INDIRECT patches seem to be the issue here.
> 
> Hmm, I didn't check it carefully, thanks for addding this info. While
> after checking commit b3c72fc9a78e, I feel the adding code was trying to
> fix your original early_memremap_is_setup_data(). Even though
> SETUP_INDIRECT type of setup_data has been added, the original
> early_memremap_is_setup_data() only check the starting address and
> the content of struct setup_data, that's obviously wrong.

IIRC, when this function was created, the value of "len" in setup_data
included the length of "data", so the calculation was correct. Everything
was contiguous in a setup_data element.

> 
> arch/x86/include/uapi/asm/setup_data.h:
> /* extensible setup data list node */
> struct setup_data {
>         __u64 next;
>         __u32 type;
>         __u32 len;
>         __u8 data[];
> };
> 
> As you can see, the zero-length will embed the carried data which is
> actually expected and adjacent to its carrier, the struct setup_data.

Right, and "len" is the length of that data. So paddr + len goes to the
end of the overall setup_data.

Thanks,
Tom

> 
>>