[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linuxbios
Subject:    Re: [coreboot] MP table multicore patch
From:       Peter Stuge <peter () stuge ! se>
Date:       2010-02-12 9:37:33
Message-ID: 20100212093733.13338.qmail () stuge ! se
[Download RAW message or body]

tpearson@raptorengineeringinc.com wrote:
> I have patched src/arch/i386/smp/mpspec.c to write a correct, multi-core
> MP table under amdfam10.

I think this is very desirable and a great functionality improvement!


> +	// First, scan the root node for APIC clusters and APICs
> +	for(root_tree_iter=0;root_tree_iter<(all_devices->links);root_tree_iter++) {
> +		for(child_tree_iter=0;child_tree_iter<(all_devices->links);child_tree_iter++) {
> +			for (child=all_devices->link[child_tree_iter].children; child; \
> child=child->sibling) { +				// Is this an APIC?
> +				if (child->path.type == DEVICE_PATH_APIC) {
> +					// Found an APIC, add it to the MP table
> +					if (child->enabled) {
> +						unsigned long cpu_flag;
> +						cpu_flag = MPC_CPU_ENABLED;
> +						if (boot_apic_id == child->path.apic.apic_id) {
> +							cpu_flag = MPC_CPU_ENABLED | MPC_CPU_BOOTPROCESSOR;
> +						}
> +						smp_write_processor(mc, 
> +							child->path.apic.apic_id, apic_version,
> +							cpu_flag, cpu_features, cpu_feature_flags
> +						);
> +					}
> +				}
> +				// Or an APIC cluster?
> +				if (child->path.type == DEVICE_PATH_APIC_CLUSTER) {
> +					// Found an APIC cluster, scan it for APICs
> +					for(apicc_child_tree_iter=0;apicc_child_tree_iter<(child->links);apicc_child_tree_iter++) \
> { +						for (apicc_child=child->link[apicc_child_tree_iter].children; apicc_child; \
> apicc_child=apicc_child->sibling) { +							// Is this an APIC?
> +							if (apicc_child->path.type == DEVICE_PATH_APIC) {
> +								// Found an APIC, add it to the MP table
> +								if (apicc_child->enabled) {
> +									unsigned long cpu_flag;
> +									cpu_flag = MPC_CPU_ENABLED;
> +									if (boot_apic_id == apicc_child->path.apic.apic_id) {
> +										cpu_flag = MPC_CPU_ENABLED | MPC_CPU_BOOTPROCESSOR;
> +									}
> +									smp_write_processor(mc, 
> +										apicc_child->path.apic.apic_id, apic_version,
> +										cpu_flag, cpu_features, cpu_feature_flags
> +									);
> +								}
> +							}
> +						}
> +					}
> +				}
> +			}
> 		}
> 	}

But this code is not nice at all.

Could you shift it around so that it uses continue aggressively, and
has shorter variable names? It looks like that could reduce
indentation two or three levels, and then the code might actually be
visible in my terminal...

Is this romcc code? If not, maybe it could even be recursive..


//Peter

-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot


[prev in list] [next in list] [prev in thread] [next in thread]