> From: Tang Chen [mailto:tangchen@xxxxxxxxxxxxxx] > Sent: Friday, August 02, 2013 3:01 PM > > On 08/02/2013 01:25 PM, Zheng, Lv wrote: > ...... > >>> index ce3d5db..9d68ffc 100644 > >>> --- a/drivers/acpi/acpica/tbutils.c > >>> +++ b/drivers/acpi/acpica/tbutils.c > >>> @@ -766,9 +766,30 @@ > >> acpi_tb_parse_root_table(acpi_physical_address rsdp_address) > >>> */ > >>> acpi_os_unmap_memory(table, length); > >>> > >>> + return_ACPI_STATUS(AE_OK); > >>> +} > >>> + > >>> > > > > I don't think you can split the function here. > > ACPICA still need to continue to parse the table using the logic > implemented in the acpi_tb_install_table() and acpi_tb_parse_fadt(). > (for example, endianess of the signature). > > You'd better to keep them as is and split some codes from > 'acpi_tb_install_table' to form another function: > acpi_tb_override_table(). > > I'm sorry, I don't quite follow this. > > I split acpi_tb_parse_root_table(), not acpi_tb_install_table() and > acpi_tb_parse_fadt(). > If ACPICA wants to use these two functions somewhere else, I think it is > OK, isn't it? > > And the reason I did this, please see below. > > ...... > >>> + * > >>> + * FUNCTION: acpi_tb_install_root_table > > > > I think this function should be acpi_tb_override_tables, and call > acpi_tb_override_table() inside this function for each table. > > It is not just about acpi initrd table override. > > acpi_tb_parse_root_table() was split into two steps: > 1. initialize acpi_gbl_root_table_list > 2. install tables into acpi_gbl_root_table_list > > I need step1 earlier because I want to find SRAT at early time. > But I don't want step2 earlier because before install the tables in > firmware, > acpi initrd table override could happen. I want only SRAT, I don't want to > touch much existing code. According to what you've explained, what you didn’t want to be called earlier is exactly "acpi initrd table override", please split only this logic to the step 2 and leave the others remained. I think you should write a function named as acpi_override_tables() or likewise in tbxface.c to be executed as the OSPM entry of the step 2. Inside this function, acpi_tb_table_override() should be called. 268 void 269 acpi_tb_install_table(acpi_physical_address address, 270 char *signature, u32 table_index) 271 { I think you still need the following codes to be called at the early stage. 272 struct acpi_table_header *table; 273 struct acpi_table_header *final_table; 274 struct acpi_table_desc *table_desc; 275 276 if (!address) { 277 ACPI_ERROR((AE_INFO, 278 "Null physical address for ACPI table [%s]", 279 signature)); 280 return; 281 } 282 283 /* Map just the table header */ 284 285 table = acpi_os_map_memory(address, sizeof(struct acpi_table_header)); 286 if (!table) { 287 ACPI_ERROR((AE_INFO, 288 "Could not map memory for table [%s] at %p", 289 signature, ACPI_CAST_PTR(void, address))); 290 return; 291 } 292 293 /* If a particular signature is expected (DSDT/FACS), it must match */ 294 295 if (signature && !ACPI_COMPARE_NAME(table->signature, signature)) { 296 ACPI_BIOS_ERROR((AE_INFO, 297 "Invalid signature 0x%X for ACPI table, expected [%s]", 298 *ACPI_CAST_PTR(u32, table->signature), 299 signature)); 300 goto unmap_and_exit; 301 } 302 303 /* 304 * Initialize the table entry. Set the pointer to NULL, since the 305 * table is not fully mapped at this time. 306 */ 307 table_desc = &acpi_gbl_root_table_list.tables[table_index]; 308 309 table_desc->address = address; 310 table_desc->pointer = NULL; 311 table_desc->length = table->length; 312 table_desc->flags = ACPI_TABLE_ORIGIN_MAPPED; 313 ACPI_MOVE_32_TO_32(table_desc->signature.ascii, table->signature); 314 You should delete the following codes: 315 /* 316 * ACPI Table Override: 317 * 318 * Before we install the table, let the host OS override it with a new 319 * one if desired. Any table within the RSDT/XSDT can be replaced, 320 * including the DSDT which is pointed to by the FADT. 321 * 322 * NOTE: If the table is overridden, then final_table will contain a 323 * mapped pointer to the full new table. If the table is not overridden, 324 * or if there has been a physical override, then the table will be 325 * fully mapped later (in verify table). In any case, we must 326 * unmap the header that was mapped above. 327 */ 328 final_table = acpi_tb_table_override(table, table_desc); 329 if (!final_table) { 330 final_table = table; /* There was no override */ 331 } 332 You still need to keep the following logic. 333 acpi_tb_print_table_header(table_desc->address, final_table); 334 335 /* Set the global integer width (based upon revision of the DSDT) */ 336 337 if (table_index == ACPI_TABLE_INDEX_DSDT) { 338 acpi_ut_set_integer_width(final_table->revision); 339 } 340 You should delete the following codes: 341 /* 342 * If we have a physical override during this early loading of the ACPI 343 * tables, unmap the table for now. It will be mapped again later when 344 * it is actually used. This supports very early loading of ACPI tables, 345 * before virtual memory is fully initialized and running within the 346 * host OS. Note: A logical override has the ACPI_TABLE_ORIGIN_OVERRIDE 347 * flag set and will not be deleted below. 348 */ 349 if (final_table != table) { 350 acpi_tb_delete_table(table_desc); 351 } Keep the following. 352 353 unmap_and_exit: 354 355 /* Always unmap the table header that we mapped above */ 356 357 acpi_os_unmap_memory(table, sizeof(struct acpi_table_header)); 358 } I'm not sure if this can make my concerns clearer for you now. Thanks and best regards -Lv > > Would you please explain more about your comment ? I think maybe I > missed something > important to you guys. :) > > And all the other ACPICA rules will be followed in the next version. > > Thanks. ��.n������g��� �a��� �&ޖ)���)��h���&������梷���� �Ǟ�m������)������^�����������v���O��zf������