Re: [PATCH 2/2] x86/boot: Add setup_indirect support in early_memremap_is_setup_data

From: Ross Philipson
Date: Tue Feb 15 2022 - 06:37:26 EST


On 2/11/22 13:41, Borislav Petkov wrote:
> On Thu, Jan 27, 2022 at 12:04:16PM -0500, Ross Philipson wrote:
>> The x86 boot documentation describes the setup_indirect structures and
>> how they are used. Only one of the two functions in ioremap.c that needed
>> to be modified to be aware of the introduction of setup_indirect
>> functionality was updated. This adds comparable support to the other
>
> s/This adds/Add/

Ack.

>
>> function where it was missing.
>>
>> Fixes: b3c72fc9a78e ("x86/boot: Introduce setup_indirect")
>>
>
> No need for a newline here.
>
>> Signed-off-by: Ross Philipson <ross.philipson@xxxxxxxxxx>
>> Reviewed-by: Daniel Kiper <daniel.kiper@xxxxxxxxxx>
>> ---
>> arch/x86/mm/ioremap.c | 21 +++++++++++++++++++--
>> 1 file changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
>> index b45e86e..9129b29 100644
>> --- a/arch/x86/mm/ioremap.c
>> +++ b/arch/x86/mm/ioremap.c
>> @@ -670,17 +670,34 @@ static bool __init early_memremap_is_setup_data(resource_size_t phys_addr,
>>
>> paddr = boot_params.hdr.setup_data;
>> while (paddr) {
>> - unsigned int len;
>> + unsigned int len, size;
>>
>> if (phys_addr == paddr)
>> return true;
>>
>> data = early_memremap_decrypted(paddr, sizeof(*data));
>> + size = sizeof(*data);
>>
>> paddr_next = data->next;
>> len = data->len;
>>
>> - early_memunmap(data, sizeof(*data));
>> + if ((phys_addr > paddr) && (phys_addr < (paddr + len))) {
>> + early_memunmap(data, sizeof(*data));
>> + return true;
>> + }
>> +
>> + if (data->type == SETUP_INDIRECT) {
>> + size += len;
>> + early_memunmap(data, sizeof(*data));
>> + data = early_memremap_decrypted(paddr, size);
>
> That can return NULL.

Although there is a bit more indirection here, it is the same reply with
questions in the first patch.

>
>> +
>> + if (((struct setup_indirect *)data->data)->type != SETUP_INDIRECT) {
>> + paddr = ((struct setup_indirect *)data->data)->addr;
>> + len = ((struct setup_indirect *)data->data)->len;
>
> As before, use a helper variable here pls.
>
> Thx.
>

Thanks,

Ross Philipson