All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland@arm.com>
To: Alexandru Elisei <alexandru.elisei@arm.com>
Cc: Andre Przywara <andre.przywara@arm.com>,
	kvm@vger.kernel.org, pbonzini@redhat.com, drjones@redhat.com,
	maz@kernel.org, vladimir.murzin@arm.com
Subject: Re: [kvm-unit-tests PATCH v3 06/18] arm/arm64: psci: Don't run C code without stack or vectors
Date: Mon, 6 Jan 2020 11:41:49 +0000	[thread overview]
Message-ID: <20200106114149.GB9630@lakrids.cambridge.arm.com> (raw)
In-Reply-To: <61ea7391-7e65-4548-17b6-7dbd977fa394@arm.com>

On Mon, Jan 06, 2020 at 10:41:55AM +0000, Alexandru Elisei wrote:
> On 1/2/20 6:11 PM, Andre Przywara wrote:
> > On Tue, 31 Dec 2019 16:09:37 +0000
> > Alexandru Elisei <alexandru.elisei@arm.com> wrote:
> >> +.global asm_cpu_psci_cpu_die
> >> +asm_cpu_psci_cpu_die:
> >> +	ldr	r0, =PSCI_0_2_FN_CPU_OFF
> >> +	hvc	#0
> >> +	b	.
> > I am wondering if this implementation is actually too simple. Both
> > the current implementation and the kernel clear at least the first
> > three arguments to 0.
> > I failed to find a requirement for doing this (nothing in the SMCCC
> > or the PSCI spec), but I guess it would make sense when looking at
> > forward compatibility.
> 
> The SMC calling convention only specifies the values for the arguments that are
> used by a function, not the values for all possible arguments. kvm-unit-tests sets
> the other arguments to 0 because the function prototype that does the actual SMC
> call takes 4 arguments. The value 0 is a random value that was chosen for those
> unused parameters. For example, it could have been a random number on each call.

That's correct.

A caller can leave arbitrary values in non-argument registers, in the
same manner as a caller of an AAPCS function. The callee should not
consume those values as they are not arguments.

> Let me put it another way. Suggesting that unused arguments should be set to 0 is
> the same as suggesting that normal C function that adheres to procedure call
> standard for arm64 should always have 8 arguments, and for a particular function
> that doesn't use all of them, they should be set to 0 by the caller.

Heh, same rationale. :)

> @Mark Rutland has worked on the SMC implementation for the Linux kernel, if he
> wants to chime in on this.
> 
> Thanks,
> Alex
> > At the very least it's a change in behaviour (ignoring the missing printf).
> > So shall we just clear r1, r2 and r3 here? (Same for arm64 below)

There's no need to zero non-argument registers, and it could potentially
mask bugs in callees, so I don't think it's a good idea to do so.

If you really want to test that the callee is robust and correct, it
would be better to randomize the content of non-argument regsiters to
fuzz the callee.

Thanks,
Mark.

  parent reply	other threads:[~2020-01-06 11:41 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-31 16:09 [kvm-unit-tests PATCH v3 00/18] arm/arm64: Various fixes Alexandru Elisei
2019-12-31 16:09 ` [kvm-unit-tests PATCH v3 01/18] lib: arm/arm64: Remove unnecessary dcache maintenance operations Alexandru Elisei
2019-12-31 16:09 ` [kvm-unit-tests PATCH v3 02/18] lib: arm: Add proper data synchronization barriers for TLBIs Alexandru Elisei
2019-12-31 16:09 ` [kvm-unit-tests PATCH v3 03/18] lib: Add WRITE_ONCE and READ_ONCE implementations in compiler.h Alexandru Elisei
2020-01-02 18:03   ` Andre Przywara
2019-12-31 16:09 ` [kvm-unit-tests PATCH v3 04/18] lib: arm/arm64: Use WRITE_ONCE to update the translation tables Alexandru Elisei
2020-01-02 18:06   ` Andre Przywara
2019-12-31 16:09 ` [kvm-unit-tests PATCH v3 05/18] lib: arm/arm64: Remove unused CPU_OFF parameter Alexandru Elisei
2020-01-02 18:11   ` Andre Przywara
2019-12-31 16:09 ` [kvm-unit-tests PATCH v3 06/18] arm/arm64: psci: Don't run C code without stack or vectors Alexandru Elisei
2020-01-02 18:11   ` Andre Przywara
2020-01-03 15:31     ` Andrew Jones
2020-01-06 11:02       ` Alexandru Elisei
2020-01-06 13:03         ` Andrew Jones
2020-01-06 14:03           ` Alexandru Elisei
2020-01-06 10:41     ` Alexandru Elisei
2020-01-06 11:17       ` Andre Przywara
2020-01-06 11:28         ` Alexandru Elisei
2020-01-06 11:36         ` Mark Rutland
2020-01-06 11:41       ` Mark Rutland [this message]
2020-01-06 13:17         ` Andrew Jones
2020-01-06 14:12           ` Alexandru Elisei
2020-01-06 15:20             ` Andrew Jones
2019-12-31 16:09 ` [kvm-unit-tests PATCH v3 07/18] lib: arm/arm64: Add missing include for alloc_page.h in pgtable.h Alexandru Elisei
2019-12-31 16:09 ` [kvm-unit-tests PATCH v3 08/18] lib: arm: Implement flush_tlb_all Alexandru Elisei
2019-12-31 16:09 ` [kvm-unit-tests PATCH v3 09/18] lib: arm/arm64: Teach mmu_clear_user about block mappings Alexandru Elisei
2019-12-31 16:09 ` [kvm-unit-tests PATCH v3 10/18] arm/arm64: selftest: Add prefetch abort test Alexandru Elisei
2020-01-06  9:24   ` Andrew Jones
2020-01-06 11:03     ` Alexandru Elisei
2019-12-31 16:09 ` [kvm-unit-tests PATCH v3 11/18] arm64: timer: Write to ICENABLER to disable timer IRQ Alexandru Elisei
2020-01-03 13:36   ` Andre Przywara
2019-12-31 16:09 ` [kvm-unit-tests PATCH v3 12/18] arm64: timer: EOIR the interrupt after masking the timer Alexandru Elisei
2020-01-03 13:36   ` Andre Przywara
2020-01-06 11:35     ` Alexandru Elisei
2019-12-31 16:09 ` [kvm-unit-tests PATCH v3 13/18] arm64: timer: Test behavior when timer disabled or masked Alexandru Elisei
2020-01-03 13:37   ` Andre Przywara
2020-01-06 13:22     ` Alexandru Elisei
2019-12-31 16:09 ` [kvm-unit-tests PATCH v3 14/18] lib: arm/arm64: Refuse to disable the MMU with non-identity stack pointer Alexandru Elisei
2019-12-31 16:09 ` [kvm-unit-tests PATCH v3 15/18] arm/arm64: Perform dcache clean + invalidate after turning MMU off Alexandru Elisei
2020-01-03 16:49   ` Andre Przywara
2020-01-06 14:27     ` Alexandru Elisei
2020-01-06 16:28       ` Andrew Jones
2019-12-31 16:09 ` [kvm-unit-tests PATCH v3 16/18] arm: cstart64.S: Downgrade TLBI to non-shareable in asm_mmu_enable Alexandru Elisei
2019-12-31 16:09 ` [kvm-unit-tests PATCH v3 17/18] arm/arm64: Invalidate TLB before enabling MMU Alexandru Elisei
2019-12-31 16:09 ` [kvm-unit-tests PATCH v3 18/18] arm: cstart64.S: Remove icache invalidation from asm_mmu_enable Alexandru Elisei
2020-01-06  9:28 ` [kvm-unit-tests PATCH v3 00/18] arm/arm64: Various fixes Andrew Jones
2020-01-09 10:01   ` Alexandru Elisei

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=20200106114149.GB9630@lakrids.cambridge.arm.com \
    --to=mark.rutland@arm.com \
    --cc=alexandru.elisei@arm.com \
    --cc=andre.przywara@arm.com \
    --cc=drjones@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=maz@kernel.org \
    --cc=pbonzini@redhat.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.