Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <bfb76327-0054-a4be-51d6-f6bf3cc2e7ee@oracle.com>
Date:   Tue, 15 Feb 2022 06:36:49 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [PATCH 2/2] x86/boot: Add setup_indirect support in
 early_memremap_is_setup_data
Content-Language: en-US
To:     Borislav Petkov <bp@alien8.de>
Cc:     linux-kernel@vger.kernel.org, x86@kernel.org,
        daniel.kiper@oracle.com, dpsmith@apertussolutions.com,
        tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com,
        luto@amacapital.net, kanth.ghatraju@oracle.com,
        trenchboot-devel@googlegroups.com
References: <1643303056-22272-1-git-send-email-ross.philipson@oracle.com>
 <1643303056-22272-3-git-send-email-ross.philipson@oracle.com>
 <Ygat0FvdP3bWlJbx@zn.tnic>
From:   Ross Philipson <ross.philipson@oracle.com>
In-Reply-To: <Ygat0FvdP3bWlJbx@zn.tnic>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?cm1EbGRyZ2tQZVJKRDRSSS9scGdYV3dEaEYyM0piMktlRlp0TmhSRFhJMXUv?=
 =?utf-8?B?cG51ZVV2RHgxd1l5VHhKVmhxVVF0bXFRTERmQjRJQjFoYnRlTVZLUitDMG5F?=
 =?utf-8?B?aGxUUzdLUFJ6d1lZQmVwYzRIY3NralA0ci91U1VFUW1BaTRFL3J6VVVaUFMr?=
 =?utf-8?B?aUhmWTdqQ2tmeFo2QzRQUmd4V01vQlVHZ0xLNDQxNGVHTklKNWZNZDA0Z3ZC?=
 =?utf-8?B?NUovdkZPN0R5bUNISHlXdmt0MzVjNXA1TGU2dVFqak1FRFVqUVNlWU9zdk9I?=
 =?utf-8?B?UWtRWm5aNlh4WGdhNGhuZmhtV3YvQnFyUkVLNWw0MTZNTzQrQWY3VU9nNUZQ?=
 =?utf-8?B?RlJTZm5rTmhLVXZ0MUJCYWlIdDQ4VzNOLzZweGtad0t1THhYd2haY1FSb1ZN?=
 =?utf-8?B?MDZxdzVtM04vY0JFd3BxRERlaWRCQk1IazVmUEhoZndHK0NRRjVDVUtKc1V1?=
 =?utf-8?B?RklMajJzai83ZFZrZkN4R0VyRytsUGxkZG1INmE4Y3hNSlZVL0pyN1c3Vldw?=
 =?utf-8?B?MDN4VmpKTS9HMm5MbnVYamExd1FVWXhSUExMclRhaEJMdVdCRkpaNG5PZk9W?=
 =?utf-8?B?bHUvTU9US2JHcGdsV29HWWs0MDBoalhjUlIzQ2d2UWVzNkZkbnU2STVnYXNC?=
 =?utf-8?B?bjlZbDFldVR5T1pBRWprTE1TUEdMNFpKRHRlWWo3Sk4veWc1RDVCTDZHYk9m?=
 =?utf-8?B?M1NqQnBMbEJZdjROZ1J0Z2dqZmFZa0ExclFYNGRIRXZ3KzE0cWZkVWVyUUtj?=
 =?utf-8?B?QUhCY0Jma2hUYmpVUHUvb1ZTMC9KZ2p0WjZXMmsxVDNnTFNxaVN5MDM0WHFa?=
 =?utf-8?B?R2dqSFF0NmdObjI5cnUzbnA1dXRBQjdOWnRYS2lOelpSZVQxQmJCYnZPcVhs?=
 =?utf-8?B?aW9sVlZXWHVGbEVCTkRaRlMwcGdrRUdjSE54UEFQcTlnV2RZd1pUZTVTbmVq?=
 =?utf-8?B?S2hDcUxOaDVjeGd4MUZBeVRIVDYzMEUxWkk5eXRIeDJ1NFpMOWlzam9qWGR5?=
 =?utf-8?B?bUwwSXZVZGlTREhRTHpLeGdCNXo2UFg5Sm56cXVZbnBVazdKejZIeitvbU9S?=
 =?utf-8?B?TmNRWDBoL2F2S2NXR2FYKy83eW4vODFRNm9IS1dCRnY1eGNaNDdSaEhYaHFu?=
 =?utf-8?B?RXpkY0dsRWgyRVljYWNJWk1uWkdUUUtTOUYybi9mWlI5MlhnT1p6MUJqb1VS?=
 =?utf-8?B?N3hiSmxBZzBYOE5BT3E0cnlQbldra2J2ckJyVlNUSDB6WFY2ODk3dnA0UXk2?=
 =?utf-8?B?aTQrZXpoVEt3NkJmSHVXMkhHT3lVcThON1NDOHFyS0NUWXlKbkxiUGZwMGc5?=
 =?utf-8?B?OXl6a1FsTWJMRnJHTWZ3NDlmQ1JqcWo3aEk5ZVdWdjNJYlVwMHpzR2U3OVBV?=
 =?utf-8?B?dW9OL2d2S0NrbVBqcXphb0FBcnUveHVrYWs1RUNyTHlhZHpBU3dydFV6Mjd5?=
 =?utf-8?B?S2hUV2RsZmwxaWR1YzB0SlVWRENkTnc2NVpFR1VnM256RndmKzhwWnA2Wkw0?=
 =?utf-8?B?L2NGRHV1WG1aVVVrejZ3YWFlTG41UmJESnBNcWRyS3NPcVNIN2c0Zm50S1Iv?=
 =?utf-8?B?OUJXTFhPNzVoOUt6bmk3a01KeGhRN3Zmcmc2NElQSEZqWGczZjBuRUNGTGxt?=
 =?utf-8?B?RmVvUHczcWtEc2pEYzdpQzcyckNUNGRlTlU5OFQwcE1pRGg4MmNldGRob0ho?=
 =?utf-8?B?eDZnWFF1aHVLSGFabTVJdHk0aEtrUU84d0t4K3FoY2ljYTFrWUhrc3A3eE4r?=
 =?utf-8?B?dFdNRGwvQ2dNMi9LMlZsc2RtTW9ONzNWV1Mrd1Iyc2phYVhsdWVxZExWaTJh?=
 =?utf-8?B?Z3BFRnF2bnQxQnFpSzI3Q254SkxtaXc1eDBlbUZpNEFOVjV1bFoyei9Hd2Mr?=
 =?utf-8?B?bjNVVHk0VGhIemlyenRrQmZQczNZdW1RQ3UvTVJDWk1WS1BETWFtRmhTNjI3?=
 =?utf-8?B?RiszVEZLbjFuNFpweXdyMFZPeGVZTVo3cUhzOENVa1YzWmZOalBaaTFHQjhK?=
 =?utf-8?B?SjdoV3NOZVJJL0J1c0o3K2JHblZNRHVtc3o1Q01CMEk5UTVSNlM3Mk1GVE5Z?=
 =?utf-8?B?QXROeE5VNjMvSjUzdkNubzBZSC9VMjlEWmJsbEU1S0FLd3pTSHRwbTBvamJ2?=
 =?utf-8?B?bDEzMURETXZXSTQ4VzA3aU5yWGtoVzAzbnJrTUdzb00rd2NuT21raGlZWnM4?=
 =?utf-8?B?cEdYRHFNclJEMFROQ29hd1dpUGtTbEpxUEtpSnYwME5lMWR0Z1lPekFiT3d6?=
 =?utf-8?B?d1dhNTlMSTR4UzRIMnRJbHRSQU9XSkdDMmdxeU5xV3c4OEs4SWxYaGFKUVJt?=
 =?utf-8?Q?GYwu8+LtVjkobnRGAx?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d1898b8a-24f4-4d80-317f-08d9f077766e
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB3793.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Feb 2022 11:36:53.0374
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: ScBp6CIvhB6xBQnw+5e36xKh7AX2qTVkAtQI3o8uVVLO9HQxP5F4h7DqmWkgFvWDb+iJZhqyLdnBPVKD3pXHqUT9znmoFE1bLXpxw2A3jWY=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN7PR10MB2673
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10258 signatures=673431
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 bulkscore=0 suspectscore=0
 phishscore=0 malwarescore=0 mlxscore=0 mlxlogscore=999 adultscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2201110000
 definitions=main-2202150065
X-Proofpoint-ORIG-GUID: ft-lX3OMqVd3HkfpxOYaPpSM9TrRQnG_
X-Proofpoint-GUID: ft-lX3OMqVd3HkfpxOYaPpSM9TrRQnG_
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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@oracle.com>
>> Reviewed-by: Daniel Kiper <daniel.kiper@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 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