linux-kernel - Re: [patch 4/4] x86: apic_is_clustered_box to indicate unsynched TSC's on multiboard vSMP systems

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080321185225.GA23139@localdomain>
Date:	Fri, 21 Mar 2008 11:52:25 -0700
From:	Ravikiran G Thirumalai <kiran@...lex86.org>
To:	Ingo Molnar <mingo@...e.hu>
Cc:	Yinghai Lu <yhlu.kernel@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org,
	Glauber de Oliveira Costa <gcosta@...hat.com>,
	shai@...lex86.org
Subject: Re: [patch 4/4] x86: apic_is_clustered_box to indicate unsynched TSC's on multiboard vSMP systems

On Fri, Mar 21, 2008 at 10:15:42AM +0100, Ingo Molnar wrote:
>
>* Ravikiran G Thirumalai <kiran@...lex86.org> wrote:
>
>> >>  -       if (!is_vsmp_box() && (boot_cpu_data.x86_vendor == X86_VENDOR_AMD))
>> >>  +       if ((boot_cpu_data.x86_vendor == X86_VENDOR_AMD) && !is_vsmp_box())
>> >>                 return 0;
>
>> Yes.  The first call is to state that TSC's are synced if it is an AMD 
>> box and if it is _not_ a vSMPowered box (for the apic id lifting case 
>> on Sun boxes), the second call is for _any_ vSMPowered system with 
>> more than one board -- TSC's are not guaranteed to be synched in that 
>> case.  Both these calls are needed.
>
>i suspect there are two questions here: firstly, your change only flips 
>the condition around which should not have any effect _normally_. But 
>the thing is that is_vsmp_box() has side-effects: it reads the PCI 
>config space and sets a flag. It would be cleaner if there was a 
>separate, explicit detect_vsmp_box() call early during bootup which did 
>the PCI config space access, and is_vsmp_box() would only return the 
>current state of the vsmp flag. Then your above change would be 
>unnecessary.

The above change -- the flipping of conditions -- is not _really_ necessary.
The condition and call is needed however.  The flip was done while
experimenting with the patch, since most vSMPowered machines today are mostly
intel, the call to is_vsmp_box() could be avoided here on intel boxes.
However this is only a trivial micro-optimization, as, we call
is_vsmp_box() again even for intel boxes now.

As for the observation about probing the pci space early during the bootup,
we call vsmp_init() much earlier during the bootup, which calls is_vsmp_box(),
does the pci probing and caches the result in the flag,  as you suggest.
So the call in the above diff context does not access the pci config space
as is.

Thanks,
Kiran
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/