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 4BC43C3DA41 for ; Tue, 9 Jul 2024 12:54:48 +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=dLYN3c7pMU+8L0RoJZ5ODCaASkg4bOujJP4l2jIPsnI=; b=o8ihM5+HYL2oNnhsxPLhwxTCQg q0CuIpWI8G1HgKPgUuCgloqrA4WktXiPNV94Ulq/vzUviScBbjN2DKaHAFbQ6liV1FiqQ2FCSXDpE HdFGPZ2R0wVmlHIhwJWeD19cxywUV8jCu2NxioH6ZYioG8RBzvpseDUg0BWEc5CSw+H0uI8SACyrV /ffHivKCmSnDgbOHx/NwZEih9bWPEH4YV5uWTROlIj51lbGmeAQcg9MC1anejMk58r0Iq8q5tbdSp 3hc2cFfxQPB+1tfrzATp1/2NS8F3ZWRPyZPaijVu2ne6RV0vp2VgIiAann68rj1yy+4D0iYorV34L hD+oNkag==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sRAMg-00000007Cio-0Aqa; Tue, 09 Jul 2024 12:54:38 +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 1sRAMP-00000007CfO-2niT for linux-arm-kernel@lists.infradead.org; Tue, 09 Jul 2024 12:54:23 +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 AC626153B; Tue, 9 Jul 2024 05:54:44 -0700 (PDT) Received: from [10.57.74.191] (unknown [10.57.74.191]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 817EB3F766; Tue, 9 Jul 2024 05:54:16 -0700 (PDT) Message-ID: Date: Tue, 9 Jul 2024 13:54:14 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v4 05/15] arm64: Mark all I/O as non-secure shared Content-Language: en-GB To: Will Deacon , Steven Price 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: Suzuki K Poulose In-Reply-To: <20240709113925.GA13242@willie-the-truck> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240709_055421_823716_861EDF7F X-CRM114-Status: GOOD ( 21.05 ) 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 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 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 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 > 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. Cheers Suzuki > > Will