Date: Tue, 17 Jun 2014 10:16:48 +0200 From: =?UTF-8?B?Um9nZXIgUGF1IE1vbm7DqQ==?= <royger@FreeBSD.org> To: Konstantin Belousov <kostikbel@gmail.com> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r267526 - in head/sys: amd64/amd64 amd64/include i386/i386 i386/include x86/include x86/x86 x86/xen Message-ID: <539FF970.2040504@FreeBSD.org> In-Reply-To: <20140617064646.GI3991@kib.kiev.ua> References: <201406160843.s5G8h3mk073933@svn.freebsd.org> <20140617064646.GI3991@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
-----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-----
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?539FF970.2040504>