2021-08-09 16:33:37

by Ross Philipson

Subject: [PATCH v3 02/14] x86/boot: Add missing handling of setup_indirect structures

One of the two functions in ioremap.c that handles setup_data was
missing the correct handling of setup_indirect structures.

Functionality missing from original commit:

commit b3c72fc9a78e (x86/boot: Introduce setup_indirect)

Signed-off-by: Ross Philipson <[email protected]>
---
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 ab74e4f..f2b34c5 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -669,17 +669,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);
+
+ if (((struct setup_indirect *)data->data)->type != SETUP_INDIRECT) {
+ paddr = ((struct setup_indirect *)data->data)->addr;
+ len = ((struct setup_indirect *)data->data)->len;
+ }
+ }
+
+ early_memunmap(data, size);

if ((phys_addr > paddr) && (phys_addr < (paddr + len)))
return true;
--
1.8.3.1

2021-08-10 16:49:00

by Jarkko Sakkinen

Subject: Re: [PATCH v3 02/14] x86/boot: Add missing handling of setup_indirect structures

On Mon, Aug 09, 2021 at 12:38:44PM -0400, Ross Philipson wrote:
> One of the two functions in ioremap.c that handles setup_data was
> missing the correct handling of setup_indirect structures.

What is "correct handling", and how was it broken?

What is 'setup_indirect'?

> Functionality missing from original commit:

Remove this sentence.

> commit b3c72fc9a78e (x86/boot: Introduce setup_indirect)

Should be.

Fixes: b3c72fc9a78e ("x86/boot: Introduce setup_indirect")


> Signed-off-by: Ross Philipson <[email protected]>
> ---
> 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 ab74e4f..f2b34c5 100644
> --- a/arch/x86/mm/ioremap.c
> +++ b/arch/x86/mm/ioremap.c
> @@ -669,17 +669,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);
> +
> + if (((struct setup_indirect *)data->data)->type != SETUP_INDIRECT) {
> + paddr = ((struct setup_indirect *)data->data)->addr;
> + len = ((struct setup_indirect *)data->data)->len;
> + }
> + }
> +
> + early_memunmap(data, size);
>
> if ((phys_addr > paddr) && (phys_addr < (paddr + len)))
> return true;
> --
> 1.8.3.1
>
>

/Jarkko

2021-08-16 14:18:50

by Ross Philipson

Subject: Re: [PATCH v3 02/14] x86/boot: Add missing handling of setup_indirect structures

On 8/10/21 12:19 PM, Jarkko Sakkinen wrote:
> On Mon, Aug 09, 2021 at 12:38:44PM -0400, Ross Philipson wrote:
>> One of the two functions in ioremap.c that handles setup_data was
>> missing the correct handling of setup_indirect structures.
>
> What is "correct handling", and how was it broken?
>
> What is 'setup_indirect'?
>
>> Functionality missing from original commit:
>
> Remove this sentence.
>
>> commit b3c72fc9a78e (x86/boot: Introduce setup_indirect)
>
> Should be.
>
> Fixes: b3c72fc9a78e ("x86/boot: Introduce setup_indirect")

I will fix these things and make the commit message clearer.

Thanks,
Ross

>
>
>> Signed-off-by: Ross Philipson <[email protected]>
>> ---
>> 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 ab74e4f..f2b34c5 100644
>> --- a/arch/x86/mm/ioremap.c
>> +++ b/arch/x86/mm/ioremap.c
>> @@ -669,17 +669,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);
>> +
>> + if (((struct setup_indirect *)data->data)->type != SETUP_INDIRECT) {
>> + paddr = ((struct setup_indirect *)data->data)->addr;
>> + len = ((struct setup_indirect *)data->data)->len;
>> + }
>> + }
>> +
>> + early_memunmap(data, size);
>>
>> if ((phys_addr > paddr) && (phys_addr < (paddr + len)))
>> return true;
>> --
>> 1.8.3.1
>>
>>
>
> /Jarkko
>