svn commit: r267526 - in head/sys: amd64/amd64 amd64/include i386/i386 i386/include x86/include x86/x86 x86/xen

[Roger Pau Monné]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 17/06/14 08:46, Konstantin Belousov wrote:
> On Mon, Jun 16, 2014 at 08:43:03AM +0000, Roger Pau Monnц╘ wrote:
>> Author: royger Date: Mon Jun 16 08:43:03 2014 New Revision: 
>> 267526 URL: http://svnweb.freebsd.org/changeset/base/267526
>> 
>> Log: amd64/i386: introduce APIC hooks for different APIC 
>> implementations.
>> 
>> This is needed for Xen PV(H) guests, since there's no hardware 
>> lapic available on this kind of domains. This commit should not 
>> change functionality.
>> 
>> Sponsored by: Citrix Systems R&D Reviewed by: jhb Approved by: 
>> gibbs
>> 
>> amd64/include/cpu.h: amd64/amd64/mp_machdep.c: 
>> i386/include/cpu.h: i386/i386/mp_machdep.c: - Remove 
>> lapic_ipi_vectored hook from cpu_ops, since it's now implemented
>>  in the lapic hooks.
>> 
>> amd64/amd64/mp_machdep.c: i386/i386/mp_machdep.c: - Use 
>> lapic_ipi_vectored directly, since it's now an inline function 
>> that will call the appropiate hook.
>> 
>> x86/x86/local_apic.c: - Prefix bare metal public lapic functions
>>  with native_ and mark them as static. - Define default 
>> implementation of apic_ops.
>> 
>> x86/include/apicvar.h: - Declare the apic_ops structure and 
>> create inline functions to access the hooks, so the change is 
>> transparent to existing users of the lapic_ functions.
>> 
>> x86/xen/hvm.c: - Switch to use the new apic_ops.
>> 
>> Modified: head/sys/amd64/amd64/mp_machdep.c 
>> head/sys/amd64/include/cpu.h head/sys/i386/i386/mp_machdep.c 
>> head/sys/i386/include/cpu.h head/sys/x86/include/apicvar.h 
>> head/sys/x86/x86/local_apic.c head/sys/x86/xen/hvm.c
>> 
>> Modified: head/sys/x86/x86/local_apic.c 
>> ==============================================================================
>>
>>
>>
>>
>>
>> 
- --- head/sys/x86/x86/local_apic.c	Mon Jun 16 08:41:57 2014	(r267525)
>> +++ head/sys/x86/x86/local_apic.c	Mon Jun 16 08:43:03 2014 
>> (r267526) @@ -169,11 +169,76 @@ static void 
>> lapic_timer_stop(struct lapi
> 
>> +struct apic_ops apic_ops = { +	.create			= native_lapic_create, 
>> +	.init			= native_lapic_init, +	.setup			= native_lapic_setup, +
>> .dump			= native_lapic_dump, +	.disable		= native_lapic_disable,
>> +	.eoi			= native_lapic_eoi, +	.id			= native_lapic_id, +
>> .intr_pending		= native_lapic_intr_pending, + .set_logical_id		=
>> native_lapic_set_logical_id, +	.cpuid			= native_apic_cpuid, +
>> .alloc_vector		= native_apic_alloc_vector, + .alloc_vectors		=
>> native_apic_alloc_vectors, +	.enable_vector		= 
>> native_apic_enable_vector, +	.disable_vector		= 
>> native_apic_disable_vector, +	.free_vector		= 
>> native_apic_free_vector, +	.enable_pmc		= 
>> native_lapic_enable_pmc, +	.disable_pmc		= 
>> native_lapic_disable_pmc, +	.reenable_pmc		= 
>> native_lapic_reenable_pmc, +	.enable_cmc		= 
>> native_lapic_enable_cmc, +	.ipi_raw		= native_lapic_ipi_raw, + 
>> .ipi_vectored		= native_lapic_ipi_vectored, +	.ipi_wait		= 
>> native_lapic_ipi_wait, +	.set_lvt_mask		= 
>> native_lapic_set_lvt_mask, +	.set_lvt_mode		= 
>> native_lapic_set_lvt_mode, +	.set_lvt_polarity	= 
>> native_lapic_set_lvt_polarity, +	.set_lvt_triggermode	= 
>> native_lapic_set_lvt_triggermode, +}; + static uint32_t 
>> lvt_mode(struct lapic *la, u_int pin, uint32_t value) {
> 
> This breaks UP compilation, since native_lapic_ipi_* methods are 
> conditionalized on SMP. The patch below worked for me. I think that
> this way is better than removing #ifdef SMP braces around 
> native_lapic_ipi_* functions definitons, because it catches 
> attempts to call IPIs on the UP machine, if such error ever made.
> 
> Do you agree with the fix ?

Yes, I think it would also be good to gate the lapic_ipi_* inline
functions in x86/include/apicvar.h on SMP being defined, so that we
get a build error instead of a run time dereference if someone tries
to use them on !SMP:

diff --git a/sys/x86/include/apicvar.h b/sys/x86/include/apicvar.h
index 35603e8..44cfae1 100644
- --- a/sys/x86/include/apicvar.h
+++ b/sys/x86/include/apicvar.h
@@ -362,6 +362,7 @@ lapic_enable_cmc(void)
 	apic_ops.enable_cmc();
 }

+#ifdef SMP
 static inline void
 lapic_ipi_raw(register_t icrlo, u_int dest)
 {
@@ -382,6 +383,7 @@ lapic_ipi_wait(int delay)

 	return (apic_ops.ipi_wait(delay));
 }
+#endif

 static inline int
 lapic_set_lvt_mask(u_int apic_id, u_int lvt, u_char masked)

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (Darwin)

iQEcBAEBAgAGBQJTn/lwAAoJEKXZdqUyumTAHs0H/Au9u9tJoFuyf3ALVMDvuR4f
3KuA9hRehVicRUptI8ZElHMNZF3Ey/RClz44CGp11Tdr6Aqn3jY2Q3pZC297opfe
m6+4Lk3EZHwVQ9tjYMxiOCBW8aWnazw5gqM/JLfxTYullKQyPddc4Vnighmffd72
KAt3TqruzynBdXc4JZlAj/jqKNiPPsAj5gel5roGVMWTYtpeeH5qpJicqrPdb6aQ
fNwbFhJ37LFvQ5nC0gJiHnPb5G4Koz/4VFeh0ZFeMA+PfH6r5T9/TV0eB3tPzlUm
Q6adtzGGHs2XkSyZKlBlkDTDwusQwrB6TQz/Ej6tPt3wVd+Q7JhHR1vcXAUiUJQ=
=ug7Z
-----END PGP SIGNATURE-----