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 CF671C3DA42 for ; Wed, 10 Jul 2024 15:35:31 +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-Transfer-Encoding: Content-Type:In-Reply-To:From:References:Cc:To:Subject:MIME-Version:Date: Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=U1xRPGDkZg4NIFB2S8/9/pCSk9vOyUnPD/7XcljEqLk=; b=g6ODmH0L0AWuTeIgskRZPll5BR pEwuzMzzutshljI5LIq+S8lyfNR5Z8DyPFKIw6oPy+M1EOYwugpd5EG/OstHYkWmdr+5UhTaltIzj zFUgMx1Aza11HR1+avZZweJkZtHxQwwzPeWa7oIP9cLkx0U+hfQ16imsACM2PbEf89BWRWN//ovoW 5ltBEA/WznAj6FHvifcl1aETHyoIXAktcpxTCQI++9+tdQVmR36sVhENEB1HlAoCuow0rLcMVuApL KzzQpZYMRP9SEJWzuYYPrX6Vrt7Gfqh8lOvvfQXmjeEgKZDj9YWlq0wXbQo1/at9i1vX+KDrLzi4p FFyiz4Xg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sRZLl-0000000AyWs-3pAV; Wed, 10 Jul 2024 15:35:21 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sRZL3-0000000AyD9-0IRV for linux-arm-kernel@lists.infradead.org; Wed, 10 Jul 2024 15:34:38 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 65615106F; Wed, 10 Jul 2024 08:35:01 -0700 (PDT) Received: from [10.57.8.115] (unknown [10.57.8.115]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C4DCC3F766; Wed, 10 Jul 2024 08:34:29 -0700 (PDT) Message-ID: Date: Wed, 10 Jul 2024 16:34:17 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v4 05/15] arm64: Mark all I/O as non-secure shared To: Suzuki K Poulose , Will Deacon Cc: kvm@vger.kernel.org, kvmarm@lists.linux.dev, Catalin Marinas , Marc Zyngier , James Morse , Oliver Upton , Zenghui Yu , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Joey Gouly , Alexandru Elisei , Christoffer Dall , Fuad Tabba , linux-coco@lists.linux.dev, Ganapatrao Kulkarni References: <20240701095505.165383-1-steven.price@arm.com> <20240701095505.165383-6-steven.price@arm.com> <20240709113925.GA13242@willie-the-truck> From: Steven Price Content-Language: en-GB In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240710_083437_368045_094BA20C X-CRM114-Status: GOOD ( 27.43 ) 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 09/07/2024 13:54, Suzuki K Poulose wrote: > Hi Will > > On 09/07/2024 12:39, Will Deacon wrote: >> On Mon, Jul 01, 2024 at 10:54:55AM +0100, Steven Price wrote: >>> All I/O is by default considered non-secure for realms. As such >>> mark them as shared with the host. >>> >>> Co-developed-by: Suzuki K Poulose >>> Signed-off-by: Suzuki K Poulose >>> Signed-off-by: Steven Price >>> --- >>> Changes since v3: >>>   * Add PROT_NS_SHARED to FIXMAP_PAGE_IO rather than overriding >>>     set_fixmap_io() with a custom function. >>>   * Modify ioreamp_cache() to specify PROT_NS_SHARED too. >>> --- >>>   arch/arm64/include/asm/fixmap.h | 2 +- >>>   arch/arm64/include/asm/io.h     | 8 ++++---- >>>   2 files changed, 5 insertions(+), 5 deletions(-) >>> >>> diff --git a/arch/arm64/include/asm/fixmap.h >>> b/arch/arm64/include/asm/fixmap.h >>> index 87e307804b99..f2c5e653562e 100644 >>> --- a/arch/arm64/include/asm/fixmap.h >>> +++ b/arch/arm64/include/asm/fixmap.h >>> @@ -98,7 +98,7 @@ enum fixed_addresses { >>>   #define FIXADDR_TOT_SIZE    (__end_of_fixed_addresses << PAGE_SHIFT) >>>   #define FIXADDR_TOT_START    (FIXADDR_TOP - FIXADDR_TOT_SIZE) >>>   -#define FIXMAP_PAGE_IO     __pgprot(PROT_DEVICE_nGnRE) >>> +#define FIXMAP_PAGE_IO     __pgprot(PROT_DEVICE_nGnRE | PROT_NS_SHARED) >>>     void __init early_fixmap_init(void); >>>   diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h >>> index 4ff0ae3f6d66..07fc1801c6ad 100644 >>> --- a/arch/arm64/include/asm/io.h >>> +++ b/arch/arm64/include/asm/io.h >>> @@ -277,12 +277,12 @@ static inline void __const_iowrite64_copy(void >>> __iomem *to, const void *from, >>>     #define ioremap_prot ioremap_prot >>>   -#define _PAGE_IOREMAP PROT_DEVICE_nGnRE >>> +#define _PAGE_IOREMAP (PROT_DEVICE_nGnRE | PROT_NS_SHARED) >>>     #define ioremap_wc(addr, size)    \ >>> -    ioremap_prot((addr), (size), PROT_NORMAL_NC) >>> +    ioremap_prot((addr), (size), (PROT_NORMAL_NC | PROT_NS_SHARED)) >>>   #define ioremap_np(addr, size)    \ >>> -    ioremap_prot((addr), (size), PROT_DEVICE_nGnRnE) >>> +    ioremap_prot((addr), (size), (PROT_DEVICE_nGnRnE | PROT_NS_SHARED)) >> >> Hmm. I do wonder whether you've pushed the PROT_NS_SHARED too far here. >> >> There's nothing _architecturally_ special about the top address bit. >> Even if the RSI divides the IPA space in half, the CPU doesn't give two >> hoots about it in the hardware. In which case, it feels wrong to bake >> PROT_NS_SHARED into ioremap_prot -- it feels much better to me if the >> ioremap() code OR'd that into the physical address when passing it down This is really just a simplification given we don't (yet) have device assignment. > Actually we would like to push the decision of applying the > "pgprot_decrypted" vs pgprot_encrypted into ioremap_prot(), rather > than sprinkling every user of ioremap_prot(). > > This could be made depending on the address that is passed on to the > ioremap_prot(). I guess we would need explicit requests from the callers > to add "encrypted vs decrypted". Is that what you guys are looking at ? There's a missing piece at the moment in terms of how the guest is going to identify whether a particular device is protected or shared (i.e. a real assigned device, or emulated by the VMM). When that's added then I was expecting ioremap_prot() to provide that flag based on discovering whether the address range passed in is for an assigned device or not. >> >> There's a selfish side of that argument, in that we need to hook >> ioremap() for pKVM protected guests, but I do genuinely feel that >> treating address bits as protection bits is arbitrary and doesn't belong >> in these low-level definitions. In a similar vein, AMD has its I'd be interested to see how pKVM will handle both protected and emulated (by the VMM) devices. Although we have the 'top bit' flag it's actually a pain to pass that down to the guest as a flag to use for this purpose (e.g. 32 bit PCI BARs are too small). So our current thought is an out-of-band request to identify whether a particular address corresponds to a protected device or not. We'd then set the top bit appropriately. >> sme_{set,clr}() macros that operate on the PA (e.g. via dma_to_phys()), >> which feels like a more accurate abstraction to me. > > I believe that doesn't solve all the problems. They do have a hook in > __ioremap_caller() that implicitly applies pgprot_{en,de}crypted > depending on other info. This is the other option - which pushes the knowledge down to the individual drivers to decide whether a region is 'encrypted' (i.e. protected) or not. It's more flexible, but potentially requires 'fixing' many drivers to understand this. Thanks, Steve > Cheers > Suzuki > >> >> Will >