diff mbox series

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

Message ID 1628527136-2478-3-git-send-email-ross.philipson@oracle.com (mailing list archive)
State New, archived
Headers show
Series x86: Trenchboot secure dynamic launch Linux kernel support | expand

Commit Message

Ross Philipson Aug. 9, 2021, 4:38 p.m. UTC
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 <ross.philipson@oracle.com>
---
 arch/x86/mm/ioremap.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

Comments

Jarkko Sakkinen Aug. 10, 2021, 4:19 p.m. UTC | #1
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 <ross.philipson@oracle.com>
> ---
>  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
Ross Philipson Aug. 16, 2021, 2:16 p.m. UTC | #2
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 <ross.philipson@oracle.com>
>> ---
>>   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
>
diff mbox series

Patch

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;