> From: Zheng, Lv > Sent: Friday, August 02, 2013 4:11 PM > > > 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. You might have concerns about how to handle FADT. In acpi_override_tables, your codes might be looking like: 584 for (i = 0; i < acpi_gbl_root_table_list.current_table_count; i++) { Just change the I from 2 to 0. 585 acpi_tb_table_override (...); 595 } You don't need to call acpi_tb_parse_table, the tables pointed by the FADT are DSDT and FACS, their index is 0 and 1. Thanks and best regards -Lv > > 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������