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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id D201FCFA771 for ; Fri, 4 Oct 2024 12:12:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Type:Cc:To:Subject: Message-ID:Date:From:In-Reply-To:References:MIME-Version:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=nLRWxfYoK6Cafgt/qUAdxdQ9UmXyz/QoFGn6MQbA2BY=; b=ax9NONWIhJKI4o4ppxkDO5bK0o 2nc1jp2QwbJGB0PyolaKNDVdJcHs49l3zIe2+LVU+zo9jG+P8dRo7RXTL3LFXob7yEqnTwJlzqkfh wUGd/HibaRfM2Q6+PjctlrIsDmBCxol/OgPqaO8XUieBrcQot88V9vVEldv5CQUCApFeWUNh9szHi LHYu1fMQV5iXkuFmOLxSYmiCKBZFkXZZuMRiCIsw4LsEiCJCyaattYkHD0ndexGEezbD2vxYFA+im Fq+2YJnpG+PWcrOdBRPP45IoQa/mBpaOy62ouvSbBu1W0wjQ4sbJbP+VYwqX/CgUOgR8I3P9bs9CU c1NmpKCQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1swhAd-0000000CG6Z-3u5Y; Fri, 04 Oct 2024 12:12:32 +0000 Received: from mail-ed1-x52d.google.com ([2a00:1450:4864:20::52d]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1swh9E-0000000CFoa-1LAF for linux-arm-kernel@lists.infradead.org; Fri, 04 Oct 2024 12:11:05 +0000 Received: by mail-ed1-x52d.google.com with SMTP id 4fb4d7f45d1cf-5c89668464cso2613449a12.1 for ; Fri, 04 Oct 2024 05:11:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1728043860; x=1728648660; darn=lists.infradead.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=nLRWxfYoK6Cafgt/qUAdxdQ9UmXyz/QoFGn6MQbA2BY=; b=ioBdyVNySOMTlbFYZSeULwH7FRQCDZDBuhR0mzC/DSM92Got1y4wvjJaKJAT8frQWN jCxlT22cwVXQPMAKtu6lO03Ybi4oHhRRpHuaNqSg71LaVvzKhmxHvp3qLMrqYYbC3+Lz YPSOnEr9eizTlIYcufM+Tgz43Q2c9H+EVm4EiuSVe71H4BEfTwIJnKUSDtAlkFDoTtQo Rn9fhEdR61WjHHh7TBjJijubsy/oGPy6F9Fp3NDd1rI8M8EYDpOhYXHMLJaXtj3KM9hY lyRGXuo3Msecc/9zKdWte1jDB+NfjnOYVFXpwOJppB1rEUQiYYt6MDQFEjBJLLZw+gku Rdkg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1728043860; x=1728648660; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=nLRWxfYoK6Cafgt/qUAdxdQ9UmXyz/QoFGn6MQbA2BY=; b=pLvxAvpzld76AQxQlvxksnSptDE42rTP9rEcqICJJFzIWKT8f752YOPdtfWonTOqXe T+3m6T4tgl4+37gmvBhM92gatT/hj4UJckQHj1KvFVtxIqzWEFCDve+vgu+dWPJeQMCQ Owaop5TolTXZa+ZR/vIfT4sEG3+NpSi7L2sGln+yGSX++nDNzB7tYAr0nJnWI5OtosEL jI8olHgSPrdAKqa0YJtMflPF1YAfAPmVA2t9Ls++RNq5i0uBVR9XL33YLAF8mE1GzuyL YYIfWHCEjISKTA4kywZrbQDYyY0yY1jPi9VVYAZ0Gn/wdGUaskTF19ghr2a5H+jGi2pn +vAA== X-Forwarded-Encrypted: i=1; AJvYcCVdHrO8vH8AiqZVatbPlRTO91GDyifjp4z32cq4PY5y1SLCVbz4YNBa+GSsPoHd2cbhQlmiBDr5KBHdamusBJzq@lists.infradead.org X-Gm-Message-State: AOJu0YxCdwERkHKSrOHY4a2/YgAruVs+dWShmUc51QcM3d+PFZ30xAeo XAZzOH/DnaoUPCFCQiSWkTTsTDImEP65VxGzsdZvl+HZWipkev0kIhzyaHavaZI3DqQJ+tVIE0Z ApmeXF1tzaEAHacpp/qI3Fz11y21kn7KJ7zWYhA== X-Google-Smtp-Source: AGHT+IHMRD9ZbIfy5bdEm2JLsi5cSDroxj58zllMuaymeta2yhL1+Kk1fvT9teC49Lco6qfh+Pen4M9kMGDH3ABd5E8= X-Received: by 2002:a05:6402:26c8:b0:5c8:ae9e:baba with SMTP id 4fb4d7f45d1cf-5c8d2dfcf50mr1510339a12.2.1728043860273; Fri, 04 Oct 2024 05:11:00 -0700 (PDT) MIME-Version: 1.0 References: <89f184d6-5b61-4c77-9f3b-c0a8f6a75d60@pengutronix.de> In-Reply-To: From: Peter Maydell Date: Fri, 4 Oct 2024 13:10:48 +0100 Message-ID: Subject: Re: [BUG] ARM64 KVM: Data abort executing post-indexed LDR on MMIO address To: Ahmad Fatoum Cc: qemu-arm@nongnu.org, kvmarm@lists.linux.dev, kvm@vger.kernel.org, Pengutronix Kernel Team , "linux-arm-kernel@lists.infradead.org" , Enrico Joerns Content-Type: text/plain; charset="UTF-8" X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20241004_051104_406774_7F978268 X-CRM114-Status: GOOD ( 35.76 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, 4 Oct 2024 at 12:51, Ahmad Fatoum wrote: > > Hello Peter, > > Thanks for your quick response. > > On 04.10.24 12:40, Peter Maydell wrote: > > On Fri, 4 Oct 2024 at 10:47, Ahmad Fatoum wrote: > >> I am investigating a data abort affecting the barebox bootloader built for aarch64 > >> that only manifests with qemu-system-aarch64 -enable-kvm. > >> > >> The issue happens when using the post-indexed form of LDR on a MMIO address: > >> > >> ldr x0, =0x9000fe0 // MMIO address > >> ldr w1, [x0], #4 // data abort, but only with -enable-kvm > > > > Don't do this -- KVM doesn't support it. For access to MMIO, > > stick to instructions which will set the ISV bit in ESR_EL1. > > > > That is: > > > > * AArch64 loads and stores of a single general-purpose register > > (including the register specified with 0b11111, including those > > with Acquire/Release semantics, but excluding Load Exclusive > > or Store Exclusive and excluding those with writeback). > > * AArch32 instructions where the instruction: > > - Is an LDR, LDA, LDRT, LDRSH, LDRSHT, LDRH, LDAH, LDRHT, > > LDRSB, LDRSBT, LDRB, LDAB, LDRBT, STR, STL, STRT, STRH, > > STLH, STRHT, STRB, STLB, or STRBT instruction. > > - Is not performing register writeback. > > - Is not using R15 as a source or destination register. > > > > Your instruction is doing writeback. Do the address update > > as a separate instruction. > > This was enlightening, thanks. I will prepare patches to implement > readl/writel in barebox in ARM assembly, like Linux does > instead of the current fallback to the generic C version > that just does volatile accesses. > > > Strictly speaking this is a missing feature in KVM (in an > > ideal world it would let you do MMIO with any instruction > > that you could use on real hardware). > > I assume that's because KVM doesn't want to handle interruptions > in the middle of such "composite" instructions? It's because with the ISV=1 information in the ESR_EL2, KVM has everything it needs to emulate the load/store: it has the affected register number, the data width, etc. When ISV is 0, simulating the load/store would require KVM to load the actual instruction word, decode it to figure out what kind of load/store it was, and then emulate its behaviour. The instruction decode would be complicated and if done in the kernel would increase the attack surface exposed to the guest. (In practice KVM will these days bounce the ISV=0 failure out to the userspace VMM, but no VMM that I know of implements the decode of load/store instructions in userspace either, so that just changes which part prints the failure message.) > > In practice it is not > > a major issue because you don't typically want to do odd > > things when you're doing MMIO, you just want to load or > > store a single data item. If you're running into this then > > your guest software is usually doing something a bit strange. > > The AMBA Peripheral ID consists of 4 bytes with some padding > in-between. The barebox code to read it out looks is this: > > static inline u32 amba_device_get_pid(void *base, u32 size) > { > int i; > u32 pid; > > for (pid = 0, i = 0; i < 4; i++) > pid |= (readl(base + size - 0x20 + 4 * i) & 255) << > (i * 8); > > return pid; > } > > static inline u32 __raw_readl(const volatile void __iomem *addr) > { > return *(const volatile u32 __force *)addr; > } > > I wouldn't necessarily characterize this as strange, we just erroneously > assumed that with strongly ordered memory for MMIO regions and volatile > accesses we had our bases covered and indeed we did until the bases > shifted to include hardware-assisted virtualization. :-) I'm not a fan of doing MMIO access via 'volatile' in C code, personally -- I think the compiler has a tendency to do more clever recombination than you might actually want, because it doesn't know that the thing you're accessing isn't RAM-like. thanks -- PMM