KVM ARM Archive on lore.kernel.org
 help / color / Atom feed
From: Alexandru Elisei <alexandru.elisei@arm.com>
To: Mark Rutland <mark.rutland@arm.com>
Cc: maz@kernel.org, pbonzini@redhat.com,
	kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org,
	andre.przywara@arm.com
Subject: Re: [kvm-unit-tests RFC PATCH 04/16] arm/arm64: selftest: Add prefetch abort test
Date: Thu, 29 Aug 2019 09:18:35 +0100
Message-ID: <e6b8a3c9-2e11-c806-da5b-8b66d8f63ce3@arm.com> (raw)
In-Reply-To: <20190828140925.GC41023@lakrids.cambridge.arm.com>

On 8/28/19 3:09 PM, Mark Rutland wrote:
> On Wed, Aug 28, 2019 at 02:38:19PM +0100, Alexandru Elisei wrote:
>> When a guest tries to execute code from MMIO memory, KVM injects an
>> external abort into that guest. We have now fixed the psci test to not
>> fetch instructions from the I/O region, and it's not that often that a
>> guest misbehaves in such a way. Let's expand our coverage by adding a
>> proper test targetting this corner case.
>>
>> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
>> ---
>> The fault injection path is broken for nested guests [1]. You can use the
>> last patch from the thread [2] to successfully run the test at EL2.
>>
>> [1] https://www.spinics.net/lists/arm-kernel/msg745391.html
>> [2] https://www.spinics.net/lists/arm-kernel/msg750310.html
>>
>>  lib/arm64/asm/esr.h |  3 ++
>>  arm/selftest.c      | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++--
>>  2 files changed, 96 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/arm64/asm/esr.h b/lib/arm64/asm/esr.h
>> index 8e5af4d90767..8c351631b0a0 100644
>> --- a/lib/arm64/asm/esr.h
>> +++ b/lib/arm64/asm/esr.h
>> @@ -44,4 +44,7 @@
>>  #define ESR_EL1_EC_BKPT32	(0x38)
>>  #define ESR_EL1_EC_BRK64	(0x3C)
>>  
>> +#define ESR_EL1_FSC_MASK	(0x3F)
>> +#define ESR_EL1_FSC_EXTABT	(0x10)
>> +
>>  #endif /* _ASMARM64_ESR_H_ */
>> diff --git a/arm/selftest.c b/arm/selftest.c
>> index 176231f32ee1..18cc0ad8f729 100644
>> --- a/arm/selftest.c
>> +++ b/arm/selftest.c
>> @@ -16,6 +16,8 @@
>>  #include <asm/psci.h>
>>  #include <asm/smp.h>
>>  #include <asm/barrier.h>
>> +#include <asm/mmu.h>
>> +#include <asm/pgtable.h>
>>  
>>  static void __user_psci_system_off(void)
>>  {
>> @@ -60,9 +62,38 @@ static void check_setup(int argc, char **argv)
>>  		report_abort("missing input");
>>  }
>>  
>> +extern pgd_t *mmu_idmap;
>> +static void prep_io_exec(void)
>> +{
>> +	pgd_t *pgd = pgd_offset(mmu_idmap, 0);
>> +	unsigned long sctlr;
>> +
>> +	/*
>> +	 * AArch64 treats all regions writable at EL0 as PXN.
> I didn't think that was the case, and I can't find wording to that
> effect in the ARM ARM (looking at ARM DDI 0487E.a). Where is that
> stated?

It's in ARM DDI 0487E.a, table D5-33, footnote c: "Not executable, because
AArch64 execution treats all regions writable at EL0 as being PXN". I'll update
the comment to include the quote.

>
>> Clear the user bit
>> +	 * so we can execute code from the bottom I/O space (0G-1G) to simulate
>> +	 * a misbehaved guest.
>> +	 */
>> +	pgd_val(*pgd) &= ~PMD_SECT_USER;
>> +	flush_dcache_addr((unsigned long)pgd);
> The virtualization extensions imply coherent page table walks, so I
> don't think the cache maintenance is necessary (provided
> TCR_EL1.{SH*,ORGN*,IRGN*} are configured appropriately.

I was following the pattern from lib/arm/mmu.c. You are correct, and Linux
doesn't do any dcache maintenance either (judging by looking at both set_pte
(for arm64) and various implementations for set_pte_ext (for armv7)).

For future reference, ARM DDI 0487E.a, in section D13.2.72, states about the
ID_MMFR3_EL1 register:

"CohWalk, bits [23:20]

Coherent Walk. Indicates whether Translation table updates require a clean to
the Point of Unification. Defined values are:
0b0000 Updates to the translation tables require a clean to the Point of
Unification to ensure visibility by subsequent translation table walks.
0b0001 Updates to the translation tables do not require a clean to the Point of
Unification to ensure visibility by subsequent translation table walks.

In Armv8-A the only permitted value is 0b0001."

For armv7, ARM DDI 0406C.d states in section B3.3.1 Translation table walks:

"If an implementation includes the Multiprocessing Extensions, translation table
walks must access data or unified caches, or data and unified caches, of other
agents participating in the coherency protocol, according to the shareability
attributes described in the  TTBR. These shareability attributes must be
consistent with the shareability attributes for the translation tables themselves."

and in section B1.7 that virtualization extensions require the multiprocessing
extensions.

So the dcache maintenance operations are not needed, I'll remove them, thank you
for pointing this out.

Thanks,
Alex
>
>> +	flush_tlb_page(0);
>> +
>> +	/* Make sure we can actually execute from a writable region */
>> +#ifdef __arm__
>> +	asm volatile("mrc p15, 0, %0, c1, c0, 0": "=r" (sctlr));
>> +	sctlr &= ~CR_ST;
>> +	asm volatile("mcr p15, 0, %0, c1, c0, 0" :: "r" (sctlr));
>> +#else
>> +	sctlr = read_sysreg(sctlr_el1);
>> +	sctlr &= ~SCTLR_EL1_WXN;
>> +	write_sysreg(sctlr, sctlr_el1);
>> +#endif
>> +	isb();
>> +}
> Thanks,
> Mark.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

  reply index

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-28 13:38 [kvm-unit-tests RFC PATCH 00/16] arm64: Run at EL2 Alexandru Elisei
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 01/16] arm: selftest.c: Remove redundant check for Exception Level Alexandru Elisei
2019-08-28 14:32   ` Andrew Jones
2019-08-28 15:39     ` Alexandru Elisei
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 02/16] arm/arm64: psci: Don't run C code without stack or vectors Alexandru Elisei
2019-08-28 14:45   ` Andrew Jones
2019-08-28 15:14     ` Alexandru Elisei
2019-09-02 14:55       ` Alexandru Elisei
2019-09-03  6:37         ` Andrew Jones
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 03/16] lib: arm/arm64: Add missing include for alloc_page.h in pgtable.h Alexandru Elisei
2019-08-28 14:47   ` Andrew Jones
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 04/16] arm/arm64: selftest: Add prefetch abort test Alexandru Elisei
2019-08-28 14:09   ` Mark Rutland
2019-08-29  8:18     ` Alexandru Elisei [this message]
2019-08-29 10:19       ` Mark Rutland
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 05/16] arm64: timer: Write to ICENABLER to disable timer IRQ Alexandru Elisei
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 06/16] arm64: timer: EOIR the interrupt after masking the timer Alexandru Elisei
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 07/16] arm64: timer: Test behavior when timer disabled or masked Alexandru Elisei
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 08/16] lib: arm/arm64: Refuse to disable the MMU with non-identity stack pointer Alexandru Elisei
2019-08-28 14:55   ` Andrew Jones
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 09/16] lib: arm/arm64: Invalidate TLB before enabling MMU Alexandru Elisei
2019-08-28 14:59   ` Andrew Jones
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 10/16] lib: Add UL and ULL definitions to linux/const.h Alexandru Elisei
2019-08-28 15:10   ` Andrew Jones
2019-08-28 15:46     ` Alexandru Elisei
2019-08-28 16:19       ` Andrew Jones
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 11/16] lib: arm64: Run existing tests at EL2 Alexandru Elisei
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 12/16] arm64: timer: Add test for EL2 timers Alexandru Elisei
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 13/16] arm64: selftest: Add basic test for EL2 Alexandru Elisei
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 14/16] lib: arm64: Add support for disabling and re-enabling VHE Alexandru Elisei
2019-08-28 14:19   ` Mark Rutland
2019-08-29  8:36     ` Alexandru Elisei
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 15/16] arm64: selftest: Expand EL2 test to disable and re-enable VHE Alexandru Elisei
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 16/16] arm64: timer: Run tests with VHE disabled Alexandru Elisei

Reply instructions:

You may reply publically 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=e6b8a3c9-2e11-c806-da5b-8b66d8f63ce3@arm.com \
    --to=alexandru.elisei@arm.com \
    --cc=andre.przywara@arm.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=mark.rutland@arm.com \
    --cc=maz@kernel.org \
    --cc=pbonzini@redhat.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

KVM ARM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kvmarm/0 kvmarm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kvmarm kvmarm/ https://lore.kernel.org/kvmarm \
		kvmarm@lists.cs.columbia.edu kvmarm@archiver.kernel.org
	public-inbox-index kvmarm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/edu.columbia.cs.lists.kvmarm


AGPL code for this site: git clone https://public-inbox.org/ public-inbox