All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexandru Elisei <alexandru.elisei@arm.com>
To: Andrew Jones <andrew.jones@linux.dev>
Cc: Vladimir Murzin <vladimir.murzin@arm.com>,
	kvmarm@lists.linux.dev, nikos.nikoleris@arm.com,
	eric.auger@redhat.com
Subject: Re: [kvm-unit-tests PATCH v2] arm64: Add basic MTE test
Date: Thu, 2 Jan 2025 12:27:43 +0000	[thread overview]
Message-ID: <Z3aGP00bYI9fspi_@raptor> (raw)
In-Reply-To: <20241230-bccb6213d3a71a09a7d32834@orel>

Hi Drew,

On Mon, Dec 30, 2024 at 05:52:54PM +0100, Andrew Jones wrote:
> On Mon, Dec 30, 2024 at 04:28:30PM +0000, Alexandru Elisei wrote:
> > Hi Drew,
> > 
> > On Mon, Dec 30, 2024 at 04:45:01PM +0100, Andrew Jones wrote:
> > > On Mon, Dec 30, 2024 at 03:19:48PM +0000, Alexandru Elisei wrote:
> > > ...
> > > > Interestingly though, when I try to run a test (either with ./run_tests.sh
> > > > mte-sync, or by trying out your example), I get this error:
> > > > 
> > > > qemu-system-aarch64: MTE requested, but not supported by the guest CPU
> > > > 
> > > > My guess is that's caused by kvm-unit-tests defaulting to cortex-a57 as the
> > > > model CPU. Changing the cpu to 'max' makes the test run. How are you getting
> > > > around this on your end?
> > > >
> > > 
> > > We can (and I think should) change to 'max' for the default model.
> > 
> > I think so too, something like this maybe?
> > 
> > diff --git a/configure b/configure
> > index 86cf1da36467..4babbd5f5e74 100755
> > --- a/configure
> > +++ b/configure
> > @@ -286,7 +286,7 @@ fi
> >  [ -z "$processor" ] && processor="$arch"
> > 
> >  if [ "$processor" = "arm64" ]; then
> > -    processor="cortex-a57"
> > +    processor="max"
> >  elif [ "$processor" = "arm" ]; then
> >      processor="cortex-a15"
> >  fi
> > 
> > Can't do the same for arm, because Makefile.arm passes the processor to
> > -mcpu and that causes a build error.
> 
> I wonder if we can improve the mcpu thing by just selecting some generic
> cpu type at this point? Or just dropping it and counting on the compiler
> to use generic cpu type? It'd be nice to use max for both arm and arm64,

I tried setting the default to 'max' for both arm and arm64. If I pass
'max' as the cpu type to arm, I get the build error above. If I don't pass
-mcpu to the arm compiler when $PROCESSOR=max, I get different errors
because of unsupported instructions:

/tmp/ccOHIrg7.s: Assembler messages:
/tmp/ccOHIrg7.s:46: Error: selected processor does not support `yield' in ARM mode
/tmp/ccGCB0zP.s: Assembler messages:
/tmp/ccGCB0zP.s:55: Error: selected processor does not support `ldrex r2,[r4]' in ARM mode
/tmp/ccGCB0zP.s:59: Error: selected processor does not support `strex r3,r2,[r4]' in ARM mode
/tmp/ccGCB0zP.s:71: Error: selected processor does not support `dmb ish' in ARM mode
/tmp/ccGCB0zP.s:99: Error: selected processor does not support `dmb ish' in ARM mode
/tmp/ccGCB0zP.s:144: Error: selected processor does not support `dmb ish' in ARM mode
[..]

and the errors go on. This means that the default cannot be 'max' for arm.

I also realized that the configure --processor option for arm64:

   --processor=PROCESSOR  processor to compile for (aarch64)

does not work as advertised, because the $PROCESSOR variable is not passed
down to the compiler.

So what I'm proposing is this:

- Patch #1: pass $PROCESSOR to the arm64 compiler, to reconcile the help
  text for --processor and to achieve parity with arm.

- Patch #2: have arm64 ignore $PROCESSOR when compiling if it's 'max.  By
  ignore I mean don't add a -mcpu argument to the compiler invocation.
  Change configure to raise an error if --processor=max is used for arm
  (users might think that if it works for arm64, it also works for arm).

- Patch #3: change the default for arm64 to 'max', but don't touch the
  default for arm.

What do you think?

Thanks,
Alex

> but if nobody has time to do the arm work / testing to be sure we can,
> then I'm fine with just a patch changing arm64.
> 
> Thanks,
> drew

  reply	other threads:[~2025-01-02 12:27 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-12 10:34 [kvm-unit-tests PATCH v2] arm64: Add basic MTE test Vladimir Murzin
2024-12-23 12:03 ` Alexandru Elisei
2024-12-23 14:37   ` Vladimir Murzin
2024-12-30 15:19     ` Alexandru Elisei
2024-12-30 15:45       ` Andrew Jones
2024-12-30 16:28         ` Alexandru Elisei
2024-12-30 16:52           ` Andrew Jones
2025-01-02 12:27             ` Alexandru Elisei [this message]
2025-01-02 12:34               ` Andrew Jones
2025-01-02 10:04       ` Vladimir Murzin
2025-01-02 11:45         ` Alexandru Elisei
2025-01-02 12:10           ` Vladimir Murzin
2025-01-02 13:23             ` Alexandru Elisei
2024-12-30 17:01   ` Nikos Nikoleris
2025-01-02 10:49     ` Vladimir Murzin

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=Z3aGP00bYI9fspi_@raptor \
    --to=alexandru.elisei@arm.com \
    --cc=andrew.jones@linux.dev \
    --cc=eric.auger@redhat.com \
    --cc=kvmarm@lists.linux.dev \
    --cc=nikos.nikoleris@arm.com \
    --cc=vladimir.murzin@arm.com \
    /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.