Mailing List Archive: [Xen-merge] Odd diff in mpparse.c

[Xen-merge] Odd diff in mpparse.c

Aug 4, 2005, 10:27 PM

Post #1 of 5 (1138 views)

Maybe it's just too late to be piddling around with this still .. but ...

I don't understand this diff section:

@@ -690,7 +697,7 @@ void __init get_smp_config (void)
* Read the physical hardware table. Anything here will
* override the defaults.
*/
- if (!smp_read_mpc((void *)mpf->mpf_physptr)) {
+ if (!smp_read_mpc(isa_bus_to_virt(mpf->mpf_physptr))) {
smp_found_config = 0;
printk(KERN_ERR "BIOS bug, MP table errors detected!...\
n");
printk(KERN_ERR "... disabling SMP support. (tell your h
w vendor)\n");

That's going from the standard to the xen base. Obvious thing is
to just abstract out isa_bus_to_virt, etc. But ...How the hell was
that working in teh first plae, seems like we're passing a phys ptr
into something that's expecting a virtual address ???? The xen version
looks fine, it's the original that worries me.

Later down there's this:

- unsigned long *bp = phys_to_virt(base);
+ unsigned long *bp = isa_bus_to_virt(base);

which makes rather more sense to me. If anyone can make more sense of that
bit than me, would help to slap me with it.

Thanks,

M.

_______________________________________________
Xen-merge mailing list
Xen-merge@lists.xensource.com
http://lists.xensource.com/xen-merge

Re: [Xen-merge] Odd diff in mpparse.c [ In reply to ]

arun.sharma at intel

Aug 5, 2005, 10:54 AM

Post #2 of 5 (1124 views)

Permalink

Martin J. Bligh wrote:
> Maybe it's just too late to be piddling around with this still .. but ...
>
> I don't understand this diff section:
>
>
> @@ -690,7 +697,7 @@ void __init get_smp_config (void)
> * Read the physical hardware table. Anything here will
> * override the defaults.
> */
> - if (!smp_read_mpc((void *)mpf->mpf_physptr)) {
> + if (!smp_read_mpc(isa_bus_to_virt(mpf->mpf_physptr))) {
> smp_found_config = 0;
> printk(KERN_ERR "BIOS bug, MP table errors detected!...\
> n");
> printk(KERN_ERR "... disabling SMP support. (tell your h
> w vendor)\n");
>
>
> That's going from the standard to the xen base. Obvious thing is
> to just abstract out isa_bus_to_virt, etc. But ...How the hell was
> that working in teh first plae, seems like we're passing a phys ptr
> into something that's expecting a virtual address ???? The xen version
> looks fine, it's the original that worries me.

The original code takes advantage of a direct mapping (phys = virtual)
that goes away in later stages of booting.

-Arun

_______________________________________________
Xen-merge mailing list
Xen-merge@lists.xensource.com
http://lists.xensource.com/xen-merge

Re: [Xen-merge] Odd diff in mpparse.c [ In reply to ]

mbligh at mbligh

Aug 5, 2005, 10:57 AM

Post #3 of 5 (1117 views)

Permalink

--On Friday, August 05, 2005 10:54:34 -0700 Arun Sharma <arun.sharma@intel.com> wrote:

> Martin J. Bligh wrote:
>> Maybe it's just too late to be piddling around with this still .. but ...
>>
>> I don't understand this diff section:
>>
>>
>> @@ -690,7 +697,7 @@ void __init get_smp_config (void)
>> * Read the physical hardware table. Anything here will
>> * override the defaults.
>> */
>> - if (!smp_read_mpc((void *)mpf->mpf_physptr)) {
>> + if (!smp_read_mpc(isa_bus_to_virt(mpf->mpf_physptr))) {
>> smp_found_config = 0;
>> printk(KERN_ERR "BIOS bug, MP table errors detected!...\
>> n");
>> printk(KERN_ERR "... disabling SMP support. (tell your h
>> w vendor)\n");
>>
>>
>> That's going from the standard to the xen base. Obvious thing is
>> to just abstract out isa_bus_to_virt, etc. But ...How the hell was
>> that working in teh first plae, seems like we're passing a phys ptr
>> into something that's expecting a virtual address ???? The xen version
>> looks fine, it's the original that worries me.
>
> The original code takes advantage of a direct mapping (phys = virtual) that goes away in later stages of booting.

Are we really still that early in the process? Maybe we are ... I know
we used to direct map the first 8MB ... I had to move the NUMA-Q tables
down for that to get it to boot, so I guess that makes sense. Pah, is
going to make the merging a mess. oh well ;-(

Thanks,

M.

_______________________________________________
Xen-merge mailing list
Xen-merge@lists.xensource.com
http://lists.xensource.com/xen-merge

Re: [Xen-merge] Odd diff in mpparse.c [ In reply to ]

ak at suse

Aug 5, 2005, 2:32 PM

Post #4 of 5 (1127 views)

Permalink

> Later down there's this:
>
> - unsigned long *bp = phys_to_virt(base);
> + unsigned long *bp = isa_bus_to_virt(base);
>
>
> which makes rather more sense to me. If anyone can make more sense of that
> bit than me, would help to slap me with it.

It's still reading it from the direct mapping.

You could always switch over to ioremap(). Or just use ACPI which
handles this properly.

-Andi

_______________________________________________
Xen-merge mailing list
Xen-merge@lists.xensource.com
http://lists.xensource.com/xen-merge

Re: [Xen-merge] Odd diff in mpparse.c [ In reply to ]

mbligh at mbligh

Aug 5, 2005, 3:04 PM

Post #5 of 5 (1126 views)

Permalink

--On Friday, August 05, 2005 23:32:25 +0200 Andi Kleen <ak@suse.de> wrote:

>> Later down there's this:
>>
>> - unsigned long *bp = phys_to_virt(base);
>> + unsigned long *bp = isa_bus_to_virt(base);
>>
>>
>> which makes rather more sense to me. If anyone can make more sense of that
>> bit than me, would help to slap me with it.
>
> It's still reading it from the direct mapping.
>
> You could always switch over to ioremap(). Or just use ACPI which
> handles this properly.

Using ACPI on boxes that don't support it is tricky ;-)

M.

_______________________________________________
Xen-merge mailing list
Xen-merge@lists.xensource.com
http://lists.xensource.com/xen-merge