From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-11.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 67299C433ED for ; Thu, 13 May 2021 17:19:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 45D536143B for ; Thu, 13 May 2021 17:19:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231260AbhEMRUZ (ORCPT ); Thu, 13 May 2021 13:20:25 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:23618 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230352AbhEMRUF (ORCPT ); Thu, 13 May 2021 13:20:05 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1620926332; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=xHVcXYbINWFy4D5xh98QUhowHYCyGVQ4fQqW66RPW/k=; b=GtHK6luBoBUjZKxHr71C/otZK8NXCoKfxzYzBSLlvuw7ki+v+2+Reg77vmsue1D2slO+E8 4+10j5I+DWJ3XGKUNyQxFEj3piZ+0Db7PcyR6pCgkZ119/dPbaHBreNhRr3auZT1bHv6ze bjiOqh+558urSuOWmVfQ+O5lTht7seA= Received: from mail-ed1-f70.google.com (mail-ed1-f70.google.com [209.85.208.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-129-eooPx9hyPdaOhWXC61b7pg-1; Thu, 13 May 2021 13:18:48 -0400 X-MC-Unique: eooPx9hyPdaOhWXC61b7pg-1 Received: by mail-ed1-f70.google.com with SMTP id k10-20020a50cb8a0000b0290387e0173bf7so15102195edi.8 for ; Thu, 13 May 2021 10:18:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to; bh=xHVcXYbINWFy4D5xh98QUhowHYCyGVQ4fQqW66RPW/k=; b=dHldgImEHq+mXEDHUVHQ5oi7CBMLR5BRHsTJvad3UsDlQAG1l0ouBFKcIOBnIOCuAR +gHWxNfp8vu/JL95SpnwFDDdhkK21WKXsOJ5ApcXZBXKXC/SgJjzFs2N/9B38T2P8dr5 B9nNAYe9WpyRP5nzibWpOp/XXHJgx/n2guadawuUKA3bZA8PpZMz4touE9+yBfYn9Ug9 ZxZVsMq0HixMdlKMAsQ+Bk15XbGYr9ZfZmWldtAm4QNQIc83WbDsKBQDVActB4lo8L6k Lo+j9rzUopSxtsznS4OV9jKzLCRV9z97cXoOTGQ0OQwH8li/U5JnAUr2s7JdVcgAi02+ wAFw== X-Gm-Message-State: AOAM5334BMxObZsnc5evF0qLqjon0DCzKkqvj0oBK3GSAk8jICDy+1WV IR9JwU5XMV1uVtm1tqkyGBtOvO+mQDpfpKuCdl+BpGZBm73tXHkiOngYnmdgTNBCIgnsiWMMpPd fv/UIsJifGA4x X-Received: by 2002:a17:906:e294:: with SMTP id gg20mr5924921ejb.227.1620926327261; Thu, 13 May 2021 10:18:47 -0700 (PDT) X-Google-Smtp-Source: ABdhPJx7AS+Aqh+3g/PPYMa28VNtg9E4HDWisH6i92ywBu6FXcFhKWTxD1p5Swvg6s7pkqBtu0pV8g== X-Received: by 2002:a17:906:e294:: with SMTP id gg20mr5924901ejb.227.1620926327029; Thu, 13 May 2021 10:18:47 -0700 (PDT) Received: from gator (cst2-174-132.cust.vodafone.cz. [31.30.174.132]) by smtp.gmail.com with ESMTPSA id r17sm2705027edt.33.2021.05.13.10.18.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 May 2021 10:18:46 -0700 (PDT) Date: Thu, 13 May 2021 19:18:44 +0200 From: Andrew Jones To: Alexandru Elisei Cc: kvm@vger.kernel.org, nikos.nikoleris@arm.com, andre.przywara@arm.com, eric.auger@redhat.com Subject: Re: [PATCH kvm-unit-tests v3 4/8] arm/arm64: mmu: Stop mapping an assumed IO region Message-ID: <20210513171844.n3h3c7l5srhuriyy@gator> References: <20210429164130.405198-1-drjones@redhat.com> <20210429164130.405198-5-drjones@redhat.com> <94288c5b-8894-5f8b-2477-6e45e087c4b5@arm.com> <0ca20ae5-d797-1c9f-9414-1d162d86f1b5@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <0ca20ae5-d797-1c9f-9414-1d162d86f1b5@arm.com> Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On Thu, May 13, 2021 at 04:48:16PM +0100, Alexandru Elisei wrote: > Hi Drew, > > On 5/10/21 4:45 PM, Alexandru Elisei wrote: > > Hi Drew, > > > > On 4/29/21 5:41 PM, Andrew Jones wrote: > >> By providing a proper ioremap function, we can just rely on devices > >> calling it for each region they need (as they already do) instead of > >> mapping a big assumed I/O range. We don't require the MMU to be > >> enabled at the time of the ioremap. In that case, we add the mapping > >> to the identity map anyway. This allows us to call setup_vm after > >> io_init. Why don't we just call setup_vm before io_init, I hear you > >> ask? Well, that's because tests like sieve want to start with the MMU > >> off, later call setup_vm, and all the while have working I/O. Some > >> unit tests are just really demanding... > >> > >> While at it, ensure we map the I/O regions with XN (execute never), > >> as suggested by Alexandru Elisei. > > I got to thinking why this wasn't an issue before. Under KVM, device memory is not > > usually mapped at stage 2, so any speculated reads wouldn't have reached memory at > > all. The only way I imagine that happening if the user was running kvm-unit-tests > > with a passthrough PCI device, which I don't think happens too often. > > > > But we cannot rely on devices not being mapped at stage 2 when running under EFI > > (we're mapping them ourselves with ioremap), so I believe this is a good fix. > > > > Had another look at the patch, looks good to me: > > While testing the series I discovered that this patch introduces a bug when > running under kvmtool. > > Here's the splat: > > $ ./configure --vmm=kvmtool --earlycon=uart,mmio,0x1000000 --page-size=4K && make > clean && make -j6 && ./vm run -c2 -m128 -f arm/micro-bench.flat > [..] >   # lkvm run --firmware arm/micro-bench.flat -m 128 -c 2 --name guest-6986 >   Info: Placing fdt at 0x80200000 - 0x80210000 > chr_testdev_init: chr-testdev: can't find a virtio-console > Timer Frequency 24000000 Hz (Output in microseconds) > > name                                    total ns                         avg > ns             > -------------------------------------------------------------------------------------------- > hvc                                 168674516.0                         > 2573.0              > Load address: 80000000 > PC: 80000128 PC offset: 128 > Unhandled exception ec=0x25 (DABT_EL1) > Vector: 4 (el1h_sync) > ESR_EL1:         96000006, ec=0x25 (DABT_EL1) > FAR_EL1: 000000000a000008 (valid) > Exception frame registers: > pc : [<0000000080000128>] lr : [<000000008000cac8>] pstate: 800003c5 > sp : 000000008003ff90 > x29: 0000000000000000 x28: 0000000000000000 > x27: 00000011ada4d0c2 x26: 0000000000000000 > x25: 0000000080015978 x24: 0000000080015a90 > x23: 0000048c27394fff x22: 20c49ba5e353f7cf > x21: 28f5c28f5c28f5c3 x20: 0000000080016af0 > x19: 000000e8d4a51000 x18: 0000000080040000 > x17: 0000000000000000 x16: 0000000080008210 > x15: 000000008003fe5c x14: 0000000000000260 > x13: 00000000000002a4 x12: 0000000080040000 > x11: 0000000000000001 x10: 0000000000000060 > x9 : 0000000000000000 x8 : 0000000000000039 > x7 : 0000000000000040 x6 : 0000000080013983 > x5 : 000000008003f74e x4 : 000000008003f69c > x3 : 0000000000000000 x2 : 0000000000000000 > x1 : 0000000000000000 x0 : 000000000a000008 > > The issue is caused by the mmio_read_user_exec() function from arm/micro-bench.c. > kvmtool doesn't have a chr-testdev device, and because probing fails, the address > 0x0a000008 isn't ioremap'ed. The 0-1G memory region is not automatically mapped > anymore, and the access triggers a data abort at stage 1. > > I fixed the splat with this change: > > diff --git a/arm/micro-bench.c b/arm/micro-bench.c > index 95c418c10eb4..ad9e44d71d8d 100644 > --- a/arm/micro-bench.c > +++ b/arm/micro-bench.c > @@ -281,7 +281,7 @@ static void mmio_read_user_exec(void) >          * updated in the future if any relevant changes in QEMU >          * test-dev are made. >          */ > -       void *userspace_emulated_addr = (void*)0x0a000008; > +       void *userspace_emulated_addr = (void*)ioremap(0x0a000008, 8); >   >         readl(userspace_emulated_addr); >  } > > kvmtool ignores the MMIO exit reason if no device owns the IPA, that's why it also > works on kvmtool. > > The micro-bench test with the diff passes under qemu and kvmtool, tested with 4K, > 16K and 64K pages on an odroid-c4. > Thanks Alex, I think a better fix is this untested one below, though. If you can test it out and confirm it also resolves the issue, then I'll add this patch to the series. Thanks, drew diff --git a/arm/micro-bench.c b/arm/micro-bench.c index 95c418c10eb4..deafd5695c33 100644 --- a/arm/micro-bench.c +++ b/arm/micro-bench.c @@ -273,16 +273,22 @@ static void hvc_exec(void) asm volatile("mov w0, #0x4b000000; hvc #0" ::: "w0"); } -static void mmio_read_user_exec(void) +/* + * FIXME: Read device-id in virtio mmio here in order to + * force an exit to userspace. This address needs to be + * updated in the future if any relevant changes in QEMU + * test-dev are made. + */ +static void *userspace_emulated_addr; + +static bool mmio_read_user_prep(void) { - /* - * FIXME: Read device-id in virtio mmio here in order to - * force an exit to userspace. This address needs to be - * updated in the future if any relevant changes in QEMU - * test-dev are made. - */ - void *userspace_emulated_addr = (void*)0x0a000008; + userspace_emulated_addr = (void*)ioremap(0x0a000008, 8); + return true; +} +static void mmio_read_user_exec(void) +{ readl(userspace_emulated_addr); } @@ -309,14 +315,14 @@ struct exit_test { }; static struct exit_test tests[] = { - {"hvc", NULL, hvc_exec, NULL, 65536, true}, - {"mmio_read_user", NULL, mmio_read_user_exec, NULL, 65536, true}, - {"mmio_read_vgic", NULL, mmio_read_vgic_exec, NULL, 65536, true}, - {"eoi", NULL, eoi_exec, NULL, 65536, true}, - {"ipi", ipi_prep, ipi_exec, NULL, 65536, true}, - {"ipi_hw", ipi_hw_prep, ipi_exec, NULL, 65536, true}, - {"lpi", lpi_prep, lpi_exec, NULL, 65536, true}, - {"timer_10ms", timer_prep, timer_exec, timer_post, 256, true}, + {"hvc", NULL, hvc_exec, NULL, 65536, true}, + {"mmio_read_user", mmio_read_user_prep, mmio_read_user_exec, NULL, 65536, true}, + {"mmio_read_vgic", NULL, mmio_read_vgic_exec, NULL, 65536, true}, + {"eoi", NULL, eoi_exec, NULL, 65536, true}, + {"ipi", ipi_prep, ipi_exec, NULL, 65536, true}, + {"ipi_hw", ipi_hw_prep, ipi_exec, NULL, 65536, true}, + {"lpi", lpi_prep, lpi_exec, NULL, 65536, true}, + {"timer_10ms", timer_prep, timer_exec, timer_post, 256, true}, }; struct ns_time {