From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex =?utf-8?Q?Benn=C3=A9e?= Subject: Re: [PATCH v10 13/18] arm64/sve: Move sve_pffr() to fpsimd.h and make inline Date: Thu, 24 May 2018 11:20:59 +0100 Message-ID: <8736yhtkvo.fsf@linaro.org> References: <1527005119-6842-1-git-send-email-Dave.Martin@arm.com> <1527005119-6842-14-git-send-email-Dave.Martin@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-reply-to: <1527005119-6842-14-git-send-email-Dave.Martin@arm.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Dave Martin Cc: Christoffer Dall , Ard Biesheuvel , Marc Zyngier , Catalin Marinas , Will Deacon , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org List-Id: kvmarm@lists.cs.columbia.edu CkRhdmUgTWFydGluIDxEYXZlLk1hcnRpbkBhcm0uY29tPiB3cml0ZXM6Cgo+IEluIG9yZGVyIHRv IG1ha2Ugc3ZlX3NhdmVfc3RhdGUoKS9zdmVfbG9hZF9zdGF0ZSgpIG1vcmUgZWFzaWx5Cj4gcmV1 c2FibGUgYW5kIHRvIGdldCByaWQgb2YgYSBwb3RlbnRpYWwgYnJhbmNoIG9uIGNvbnRleHQgc3dp dGNoCj4gY3JpdGljYWwgcGF0aHMsIHRoaXMgcGF0Y2ggbWFrZXMgc3ZlX3BmZnIoKSBpbmxpbmUg YW5kIG1vdmVzIGl0IHRvCj4gZnBzaW1kLmguCj4KPiA8YXNtL3Byb2Nlc3Nvci5oPiBtdXN0IGJl IGluY2x1ZGVkIGluIGZwc2ltZC5oIGluIG9yZGVyIHRvIG1ha2UKPiB0aGlzIHdvcmssIGFuZCB0 aGlzIGNyZWF0ZXMgYW4gI2luY2x1ZGUgY3ljbGUgdGhhdCBpcyB0cmlja3kgdG8KPiBhdm9pZCB3 aXRob3V0IG1vZGlmeWluZyBjb3JlIGNvZGUsIGR1ZSB0byB0aGUgd2F5IHRoZSBQUl9TVkVfKigp Cj4gcHJjdGwgaGVscGVycyBhcmUgaW5jbHVkZWQgaW4gdGhlIGNvcmUgcHJjdGwgaW1wbGVtZW50 YXRpb24uCj4KPiBJbnN0ZWFkIG9mIGJyZWFraW5nIHRoZSBjeWNsZSwgdGhpcyBwYXRjaCBkZWZl cnMgaW5jbHVzaW9uIG9mCj4gPGFzbS9mcHNpbWQuaD4gaW4gPGFzbS9wcm9jZXNzb3IuaD4gdW50 aWwgdGhlIHBvaW50IHdoZXJlIGl0IGlzCj4gYWN0dWFsbHkgbmVlZGVkOiBpLmUuLCBpbW1lZGlh dGVseSBiZWZvcmUgdGhlIHByY3RsIGRlZmluaXRpb25zLgo+Cj4gTm8gZnVuY3Rpb25hbCBjaGFu Z2UuCj4KPiBTaWduZWQtb2ZmLWJ5OiBEYXZlIE1hcnRpbiA8RGF2ZS5NYXJ0aW5AYXJtLmNvbT4K PiBBY2tlZC1ieTogQ2F0YWxpbiBNYXJpbmFzIDxjYXRhbGluLm1hcmluYXNAYXJtLmNvbT4KPiBB Y2tlZC1ieTogTWFyYyBaeW5naWVyIDxtYXJjLnp5bmdpZXJAYXJtLmNvbT4KPiAtLS0KPiAgYXJj aC9hcm02NC9pbmNsdWRlL2FzbS9mcHNpbWQuaCAgICB8IDEzICsrKysrKysrKysrKysKPiAgYXJj aC9hcm02NC9pbmNsdWRlL2FzbS9wcm9jZXNzb3IuaCB8ICAzICsrLQo+ICBhcmNoL2FybTY0L2tl cm5lbC9mcHNpbWQuYyAgICAgICAgIHwgMTIgLS0tLS0tLS0tLS0tCj4gIDMgZmlsZXMgY2hhbmdl ZCwgMTUgaW5zZXJ0aW9ucygrKSwgMTMgZGVsZXRpb25zKC0pCj4KPiBkaWZmIC0tZ2l0IGEvYXJj aC9hcm02NC9pbmNsdWRlL2FzbS9mcHNpbWQuaCBiL2FyY2gvYXJtNjQvaW5jbHVkZS9hc20vZnBz aW1kLmgKPiBpbmRleCBmYjYwYjIyLi5mYTkyNzQ3IDEwMDY0NAo+IC0tLSBhL2FyY2gvYXJtNjQv aW5jbHVkZS9hc20vZnBzaW1kLmgKPiArKysgYi9hcmNoL2FybTY0L2luY2x1ZGUvYXNtL2Zwc2lt ZC5oCj4gQEAgLTE4LDYgKzE4LDggQEAKPgo+ICAjaW5jbHVkZSA8YXNtL3B0cmFjZS5oPgo+ICAj aW5jbHVkZSA8YXNtL2Vycm5vLmg+Cj4gKyNpbmNsdWRlIDxhc20vcHJvY2Vzc29yLmg+Cj4gKyNp bmNsdWRlIDxhc20vc2lnY29udGV4dC5oPgo+Cj4gICNpZm5kZWYgX19BU1NFTUJMWV9fCj4KPiBA QCAtNjEsNiArNjMsMTcgQEAgZXh0ZXJuIHZvaWQgc3ZlX2ZsdXNoX2NwdV9zdGF0ZSh2b2lkKTsK PiAgLyogTWF4aW11bSBWTCB0aGF0IFNWRSBWTC1hZ25vc3RpYyBzb2Z0d2FyZSBjYW4gdHJhbnNw YXJlbnRseSBzdXBwb3J0ICovCj4gICNkZWZpbmUgU1ZFX1ZMX0FSQ0hfTUFYIDB4MTAwCj4KPiAr LyogT2Zmc2V0IG9mIEZGUiBpbiB0aGUgU1ZFIHJlZ2lzdGVyIGR1bXAgKi8KPiArc3RhdGljIGlu bGluZSBzaXplX3Qgc3ZlX2Zmcl9vZmZzZXQoaW50IHZsKQo+ICt7Cj4gKwlyZXR1cm4gU1ZFX1NJ R19GRlJfT0ZGU0VUKHN2ZV92cV9mcm9tX3ZsKHZsKSkgLSBTVkVfU0lHX1JFR1NfT0ZGU0VUOwo+ ICt9Cj4gKwo+ICtzdGF0aWMgaW5saW5lIHZvaWQgKnN2ZV9wZmZyKHN0cnVjdCB0aHJlYWRfc3Ry dWN0ICp0aHJlYWQpCj4gK3sKPiArCXJldHVybiAoY2hhciAqKXRocmVhZC0+c3ZlX3N0YXRlICsg c3ZlX2Zmcl9vZmZzZXQodGhyZWFkLT5zdmVfdmwpOwo+ICt9Cj4gKwo+ICBleHRlcm4gdm9pZCBz dmVfc2F2ZV9zdGF0ZSh2b2lkICpzdGF0ZSwgdTMyICpwZnBzcik7Cj4gIGV4dGVybiB2b2lkIHN2 ZV9sb2FkX3N0YXRlKHZvaWQgY29uc3QgKnN0YXRlLCB1MzIgY29uc3QgKnBmcHNyLAo+ICAJCQkg ICB1bnNpZ25lZCBsb25nIHZxX21pbnVzXzEpOwo+IGRpZmYgLS1naXQgYS9hcmNoL2FybTY0L2lu Y2x1ZGUvYXNtL3Byb2Nlc3Nvci5oIGIvYXJjaC9hcm02NC9pbmNsdWRlL2FzbS9wcm9jZXNzb3Iu aAo+IGluZGV4IGY5MDJiNmQuLmViYWFkYjEgMTAwNjQ0Cj4gLS0tIGEvYXJjaC9hcm02NC9pbmNs dWRlL2FzbS9wcm9jZXNzb3IuaAo+ICsrKyBiL2FyY2gvYXJtNjQvaW5jbHVkZS9hc20vcHJvY2Vz c29yLmgKPiBAQCAtNDAsNyArNDAsNiBAQAo+Cj4gICNpbmNsdWRlIDxhc20vYWx0ZXJuYXRpdmUu aD4KPiAgI2luY2x1ZGUgPGFzbS9jcHVmZWF0dXJlLmg+Cj4gLSNpbmNsdWRlIDxhc20vZnBzaW1k Lmg+Cj4gICNpbmNsdWRlIDxhc20vaHdfYnJlYWtwb2ludC5oPgo+ICAjaW5jbHVkZSA8YXNtL2xz ZS5oPgo+ICAjaW5jbHVkZSA8YXNtL3BndGFibGUtaHdkZWYuaD4KPiBAQCAtMjQ1LDYgKzI0NCw4 IEBAIHZvaWQgY3B1X2VuYWJsZV9wYW4oY29uc3Qgc3RydWN0IGFybTY0X2NwdV9jYXBhYmlsaXRp ZXMgKl9fdW51c2VkKTsKPiAgdm9pZCBjcHVfZW5hYmxlX2NhY2hlX21haW50X3RyYXAoY29uc3Qg c3RydWN0IGFybTY0X2NwdV9jYXBhYmlsaXRpZXMgKl9fdW51c2VkKTsKPiAgdm9pZCBjcHVfY2xl YXJfZGlzcihjb25zdCBzdHJ1Y3QgYXJtNjRfY3B1X2NhcGFiaWxpdGllcyAqX191bnVzZWQpOwo+ Cj4gKyNpbmNsdWRlIDxhc20vZnBzaW1kLmg+Cj4gKwoKWW91IHJlYWxseSBuZWVkIGEgb25lLWxp bmVyIGNvbW1lbnQgdG8gbm90ZSB3aHkgdGhlIGluY2x1ZGUgaXMgaW4gYQpmdW5ueSBwbGFjZSB0 byBzYXZlIHNvbWVvbmUganVzdCBtb3ZpbmcgaXQgYmFjayBhbmQgdGhlbiBnZXR0aW5nIHJlYWxs eQpjb25mdXNlZC4gTWF5YmU6CgogIC8qIGluY2x1ZGVkIGp1c3QgaW4gdGltZSB0byBhdm9pZCBj aXJjdWxhciBpbmNsdXNpb24gaXNzdWVzICovCiAgI2luY2x1ZGUgPGFzbS9mcHNpbWQuaD4KCkl0 IHN0aWxsIHNlZW1zIHdlaXJkIHRvIG1lIHRob3VnaCA6LS8KCk90aGVyd2lzZToKClJldmlld2Vk LWJ5OiBBbGV4IEJlbm7DqWUgPGFsZXguYmVubmVlQGxpbmFyby5vcmc+CgotLQpBbGV4IEJlbm7D qWUKCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmxpbnV4 LWFybS1rZXJuZWwgbWFpbGluZyBsaXN0CmxpbnV4LWFybS1rZXJuZWxAbGlzdHMuaW5mcmFkZWFk Lm9yZwpodHRwOi8vbGlzdHMuaW5mcmFkZWFkLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2xpbnV4LWFy bS1rZXJuZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 From: alex.bennee@linaro.org (Alex =?utf-8?Q?Benn=C3=A9e?=) Date: Thu, 24 May 2018 11:20:59 +0100 Subject: [PATCH v10 13/18] arm64/sve: Move sve_pffr() to fpsimd.h and make inline In-Reply-To: <1527005119-6842-14-git-send-email-Dave.Martin@arm.com> References: <1527005119-6842-1-git-send-email-Dave.Martin@arm.com> <1527005119-6842-14-git-send-email-Dave.Martin@arm.com> Message-ID: <8736yhtkvo.fsf@linaro.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Dave Martin writes: > In order to make sve_save_state()/sve_load_state() more easily > reusable and to get rid of a potential branch on context switch > critical paths, this patch makes sve_pffr() inline and moves it to > fpsimd.h. > > must be included in fpsimd.h in order to make > this work, and this creates an #include cycle that is tricky to > avoid without modifying core code, due to the way the PR_SVE_*() > prctl helpers are included in the core prctl implementation. > > Instead of breaking the cycle, this patch defers inclusion of > in until the point where it is > actually needed: i.e., immediately before the prctl definitions. > > No functional change. > > Signed-off-by: Dave Martin > Acked-by: Catalin Marinas > Acked-by: Marc Zyngier > --- > arch/arm64/include/asm/fpsimd.h | 13 +++++++++++++ > arch/arm64/include/asm/processor.h | 3 ++- > arch/arm64/kernel/fpsimd.c | 12 ------------ > 3 files changed, 15 insertions(+), 13 deletions(-) > > diff --git a/arch/arm64/include/asm/fpsimd.h b/arch/arm64/include/asm/fpsimd.h > index fb60b22..fa92747 100644 > --- a/arch/arm64/include/asm/fpsimd.h > +++ b/arch/arm64/include/asm/fpsimd.h > @@ -18,6 +18,8 @@ > > #include > #include > +#include > +#include > > #ifndef __ASSEMBLY__ > > @@ -61,6 +63,17 @@ extern void sve_flush_cpu_state(void); > /* Maximum VL that SVE VL-agnostic software can transparently support */ > #define SVE_VL_ARCH_MAX 0x100 > > +/* Offset of FFR in the SVE register dump */ > +static inline size_t sve_ffr_offset(int vl) > +{ > + return SVE_SIG_FFR_OFFSET(sve_vq_from_vl(vl)) - SVE_SIG_REGS_OFFSET; > +} > + > +static inline void *sve_pffr(struct thread_struct *thread) > +{ > + return (char *)thread->sve_state + sve_ffr_offset(thread->sve_vl); > +} > + > extern void sve_save_state(void *state, u32 *pfpsr); > extern void sve_load_state(void const *state, u32 const *pfpsr, > unsigned long vq_minus_1); > diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h > index f902b6d..ebaadb1 100644 > --- a/arch/arm64/include/asm/processor.h > +++ b/arch/arm64/include/asm/processor.h > @@ -40,7 +40,6 @@ > > #include > #include > -#include > #include > #include > #include > @@ -245,6 +244,8 @@ void cpu_enable_pan(const struct arm64_cpu_capabilities *__unused); > void cpu_enable_cache_maint_trap(const struct arm64_cpu_capabilities *__unused); > void cpu_clear_disr(const struct arm64_cpu_capabilities *__unused); > > +#include > + You really need a one-liner comment to note why the include is in a funny place to save someone just moving it back and then getting really confused. Maybe: /* included just in time to avoid circular inclusion issues */ #include It still seems weird to me though :-/ Otherwise: Reviewed-by: Alex Benn?e -- Alex Benn?e