All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Brown <broonie@kernel.org>
To: Dave Martin <Dave.Martin@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Shuah Khan <skhan@linuxfoundation.org>,
	linux-arm-kernel@lists.infradead.org,
	linux-kselftest@vger.kernel.org
Subject: Re: [PATCH v2 1/4] kselftest/arm64: Provide a helper binary and "library" for SVE RDVL
Date: Thu, 29 Jul 2021 12:22:17 +0100	[thread overview]
Message-ID: <20210729112217.GK4670@sirena.org.uk> (raw)
In-Reply-To: <20210729095222.GH1724@arm.com>

[-- Attachment #1: Type: text/plain, Size: 1777 bytes --]

On Thu, Jul 29, 2021 at 10:52:24AM +0100, Dave Martin wrote:
> On Wed, Jul 28, 2021 at 05:33:15PM +0100, Mark Brown wrote:

> > +	return 0;

> For consistency with the changes in vec-syscfg, we could use
> EXIT_SUCCESS here.

0 and EXIT_SUCCESS are defined as equivalent (though they need not be
equal!) and 0 is much more idiomatic.

> Although it's hard to see what could go wrong I/O-wise that doesn't
> involve vec-syscfg itself having gone wrong, it's probably good
> practice to do the final error check:

> 	if (ferror(stdout) || fclose(stdout))
> 		return EXIT_FAILURE;

> 	return EXIT_SUCCESS;

> (In reality, people rarely seem to bother with this, so I'm not going
> to lose sleep if we don't do it...)

Yeah, I think this is one of those raising more questions than it
answers kind of things.

> > +.globl rdvl_sve

> Should we stick a

> 	.type rdvl_sve, @function

> here?  This may avoid surprises with future toolchain behaviours.
> Probably doesn't matter, but I have bad memories of Thumb-2...

> Lacking this annotation is widespread though, as well as being de facto
> standard before awkward architectures came along.

Yeah, it doesn't seem to be in the slightest bit idiomatic for the arm64
asm code the kernel has.  I don't know if you think it's worth adding
that to SYM_FUNC_START now we have it though?

> If the selftests have access to the ENTRY() macro we could use that, but
> I'm guessing that isn't exported for userspace.

We don't use that any more anyway, it's SYM_FUNC_START() and friends not
that those are outside the kernel either.  We will have to do something
like that if anyone starts building userspace with BTI though (or I
might just shove a BTI C in there unconditionally, I'm sure we'll cope
with the overhead on older systems).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Mark Brown <broonie@kernel.org>
To: Dave Martin <Dave.Martin@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Shuah Khan <skhan@linuxfoundation.org>,
	linux-arm-kernel@lists.infradead.org,
	linux-kselftest@vger.kernel.org
Subject: Re: [PATCH v2 1/4] kselftest/arm64: Provide a helper binary and "library" for SVE RDVL
Date: Thu, 29 Jul 2021 12:22:17 +0100	[thread overview]
Message-ID: <20210729112217.GK4670@sirena.org.uk> (raw)
In-Reply-To: <20210729095222.GH1724@arm.com>


[-- Attachment #1.1: Type: text/plain, Size: 1777 bytes --]

On Thu, Jul 29, 2021 at 10:52:24AM +0100, Dave Martin wrote:
> On Wed, Jul 28, 2021 at 05:33:15PM +0100, Mark Brown wrote:

> > +	return 0;

> For consistency with the changes in vec-syscfg, we could use
> EXIT_SUCCESS here.

0 and EXIT_SUCCESS are defined as equivalent (though they need not be
equal!) and 0 is much more idiomatic.

> Although it's hard to see what could go wrong I/O-wise that doesn't
> involve vec-syscfg itself having gone wrong, it's probably good
> practice to do the final error check:

> 	if (ferror(stdout) || fclose(stdout))
> 		return EXIT_FAILURE;

> 	return EXIT_SUCCESS;

> (In reality, people rarely seem to bother with this, so I'm not going
> to lose sleep if we don't do it...)

Yeah, I think this is one of those raising more questions than it
answers kind of things.

> > +.globl rdvl_sve

> Should we stick a

> 	.type rdvl_sve, @function

> here?  This may avoid surprises with future toolchain behaviours.
> Probably doesn't matter, but I have bad memories of Thumb-2...

> Lacking this annotation is widespread though, as well as being de facto
> standard before awkward architectures came along.

Yeah, it doesn't seem to be in the slightest bit idiomatic for the arm64
asm code the kernel has.  I don't know if you think it's worth adding
that to SYM_FUNC_START now we have it though?

> If the selftests have access to the ENTRY() macro we could use that, but
> I'm guessing that isn't exported for userspace.

We don't use that any more anyway, it's SYM_FUNC_START() and friends not
that those are outside the kernel either.  We will have to do something
like that if anyone starts building userspace with BTI though (or I
might just shove a BTI C in there unconditionally, I'm sure we'll cope
with the overhead on older systems).

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-07-29 11:22 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-28 16:33 [PATCH v2 0/4] kselftest/arm64: Vector length configuration tests Mark Brown
2021-07-28 16:33 ` Mark Brown
2021-07-28 16:33 ` [PATCH v2 1/4] kselftest/arm64: Provide a helper binary and "library" for SVE RDVL Mark Brown
2021-07-28 16:33   ` Mark Brown
2021-07-29  9:52   ` Dave Martin
2021-07-29  9:52     ` Dave Martin
2021-07-29 11:22     ` Mark Brown [this message]
2021-07-29 11:22       ` Mark Brown
2021-07-29 13:27       ` Dave P Martin
2021-07-29 13:27         ` Dave P Martin
2021-07-29 16:03         ` Mark Brown
2021-07-29 16:03           ` Mark Brown
2021-07-29 16:17           ` Dave Martin
2021-07-29 16:17             ` Dave Martin
2021-07-28 16:33 ` [PATCH v2 2/4] kselftest/arm64: Validate vector lengths are set in sve-probe-vls Mark Brown
2021-07-28 16:33   ` Mark Brown
2021-07-29  9:52   ` Dave Martin
2021-07-29  9:52     ` Dave Martin
2021-07-28 16:33 ` [PATCH v2 3/4] kselftest/arm64: Add tests for SVE vector configuration Mark Brown
2021-07-28 16:33   ` Mark Brown
2021-07-29  9:52   ` Dave Martin
2021-07-29  9:52     ` Dave Martin
2021-07-28 16:33 ` [PATCH v2 4/4] kselftest/arm64: Add a TODO list for floating point tests Mark Brown
2021-07-28 16:33   ` Mark Brown
2021-07-29  9:52   ` Dave Martin
2021-07-29  9:52     ` Dave Martin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210729112217.GK4670@sirena.org.uk \
    --to=broonie@kernel.org \
    --cc=Dave.Martin@arm.com \
    --cc=catalin.marinas@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.