Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 17 Jun 2014 09:46:46 +0300
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        Roger Pau =?koi8-r?B?TW9ubsOp?= <royger@FreeBSD.org>
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:  <20140617064646.GI3991@kib.kiev.ua>
In-Reply-To: <201406160843.s5G8h3mk073933@svn.freebsd.org>
References:  <201406160843.s5G8h3mk073933@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

--D8796BilH3OdfdWD
Content-Type: text/plain; charset=koi8-r
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Mon, Jun 16, 2014 at 08:43:03AM +0000, Roger Pau Monn=C3=A9 wrote:
> Author: royger
> Date: Mon Jun 16 08:43:03 2014
> New Revision: 267526
> URL: http://svnweb.freebsd.org/changeset/base/267526
>=20
> Log:
>   amd64/i386: introduce APIC hooks for different APIC implementations.
>  =20
>   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.
>  =20
>   Sponsored by: Citrix Systems R&D
>   Reviewed by: jhb
>   Approved by: gibbs
>  =20
>   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.
>  =20
>   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.
>  =20
>   x86/x86/local_apic.c:
>    - Prefix bare metal public lapic functions with native_ and mark them
>      as static.
>    - Define default implementation of apic_ops.
>  =20
>   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.
>  =20
>   x86/xen/hvm.c:
>    - Switch to use the new apic_ops.
>=20
> 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
>=20
> Modified: head/sys/x86/x86/local_apic.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- 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 =3D {
> +	.create			=3D native_lapic_create,
> +	.init			=3D native_lapic_init,
> +	.setup			=3D native_lapic_setup,
> +	.dump			=3D native_lapic_dump,
> +	.disable		=3D native_lapic_disable,
> +	.eoi			=3D native_lapic_eoi,
> +	.id			=3D native_lapic_id,
> +	.intr_pending		=3D native_lapic_intr_pending,
> +	.set_logical_id		=3D native_lapic_set_logical_id,
> +	.cpuid			=3D native_apic_cpuid,
> +	.alloc_vector		=3D native_apic_alloc_vector,
> +	.alloc_vectors		=3D native_apic_alloc_vectors,
> +	.enable_vector		=3D native_apic_enable_vector,
> +	.disable_vector		=3D native_apic_disable_vector,
> +	.free_vector		=3D native_apic_free_vector,
> +	.enable_pmc		=3D native_lapic_enable_pmc,
> +	.disable_pmc		=3D native_lapic_disable_pmc,
> +	.reenable_pmc		=3D native_lapic_reenable_pmc,
> +	.enable_cmc		=3D native_lapic_enable_cmc,
> +	.ipi_raw		=3D native_lapic_ipi_raw,
> +	.ipi_vectored		=3D native_lapic_ipi_vectored,
> +	.ipi_wait		=3D native_lapic_ipi_wait,
> +	.set_lvt_mask		=3D native_lapic_set_lvt_mask,
> +	.set_lvt_mode		=3D native_lapic_set_lvt_mode,
> +	.set_lvt_polarity	=3D native_lapic_set_lvt_polarity,
> +	.set_lvt_triggermode	=3D 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 ?

diff --git a/sys/x86/x86/local_apic.c b/sys/x86/x86/local_apic.c
index 802168e..b15a02c 100644
--- a/sys/x86/x86/local_apic.c
+++ b/sys/x86/x86/local_apic.c
@@ -230,9 +230,11 @@ struct apic_ops apic_ops =3D {
 	.disable_pmc		=3D native_lapic_disable_pmc,
 	.reenable_pmc		=3D native_lapic_reenable_pmc,
 	.enable_cmc		=3D native_lapic_enable_cmc,
+#ifdef SMP
 	.ipi_raw		=3D native_lapic_ipi_raw,
 	.ipi_vectored		=3D native_lapic_ipi_vectored,
 	.ipi_wait		=3D native_lapic_ipi_wait,
+#endif
 	.set_lvt_mask		=3D native_lapic_set_lvt_mask,
 	.set_lvt_mode		=3D native_lapic_set_lvt_mode,
 	.set_lvt_polarity	=3D native_lapic_set_lvt_polarity,

--D8796BilH3OdfdWD
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJTn+RVAAoJEJDCuSvBvK1BapMP/A2LIdpvlwWk7FyjOVcEDrvl
RfbqLjMmV4/2gb7CaHyAwJtXQhN3X39/L7jm8I2cazdHqE+MqMNH+paXqSd88Ll2
j3qbqXO0YJ0ypv5YrMQ09shcVmbmb3B1k8RTGFtF9J6ezGzQxVCLrE9tBUpHykiC
Ii96A9JbgB63w4jYTrsi75972ccSJWBX39yDfnxf1C7xQZhz/Wt+/+KTj+jbRpjB
2KAoPTgQkbC3C7Cw6bnBVpRcgUkSUZglj0v1AhD73vgL5EJs9I7BgcIYOf29rbab
Ioedtiy1SgGiarorqe/eKv3niOnv3D4go1uJYtd3KRujb/RPigiE40b0lpw1WPNd
JP+pqjmCeNdl3xDJ36Qy3HSP1g+huKrCha+IrPGoWUz2QI19iCTqxltKPbzcYRkJ
YzdtMFDs2FojE/TKbdmj1h4Zsfmp+8RIPf7COyq/y16tJB02VhSQTo/0nYYdSN8F
OdkgSQmjCHJ+/k+kmfPT01X+ak8Mf3RwFRo7FuI/Z1u7v5qhTWzPIcGfQfBnp8CW
oHutDIT2h6e1/6TYfNtEjTcl8XBATKPs0zDYN73JOyJ1+7ieQ1bs3Qw6lMvMJU6y
qvKmty61tipIGb4f64QSR4k1nNeliCui5ZVcqqTE0COf4Tw9a7ktZ998JpEA0BIl
lyEanGxg4l2QLn/+W+Mt
=rzI5
-----END PGP SIGNATURE-----

--D8796BilH3OdfdWD--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140617064646.GI3991>