openbmc.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Ryan Chen <ryan_chen@aspeedtech.com>
To: Zev Weiss <zev@bewilderbeest.net>, Joel Stanley <joel@jms.id.au>,
	"openbmc@lists.ozlabs.org" <openbmc@lists.ozlabs.org>
Cc: Andrew Jeffery <andrew@aj.id.au>, Lei Yu <yulei.sh@bytedance.com>,
	Ian Woloschin <ian.woloschin@akamai.com>
Subject: RE: [PATCH u-boot v2019.04-aspeed-openbmc v3] aspeed: Disable backdoor interfaces
Date: Wed, 20 Apr 2022 03:06:02 +0000	[thread overview]
Message-ID: <HK0PR06MB3380C8FDEE1588E4BAE945B3F2F59@HK0PR06MB3380.apcprd06.prod.outlook.com> (raw)
In-Reply-To: <20220419234202.8895-1-zev@bewilderbeest.net>


> -----Original Message-----
> From: Zev Weiss <zev@bewilderbeest.net>
> Sent: Wednesday, April 20, 2022 7:42 AM
> To: Joel Stanley <joel@jms.id.au>; openbmc@lists.ozlabs.org
> Cc: Zev Weiss <zev@bewilderbeest.net>; Andrew Jeffery <andrew@aj.id.au>;
> Ryan Chen <ryan_chen@aspeedtech.com>; Ian Woloschin
> <ian.woloschin@akamai.com>; Lei Yu <yulei.sh@bytedance.com>
> Subject: [PATCH u-boot v2019.04-aspeed-openbmc v3] aspeed: Disable
> backdoor interfaces
> 
> On ast2400 and ast2500 we now disable the various hardware backdoor
> interfaces as is done on ast2600.  Two Kconfig options can selectively
> re-enable some of these interfaces: CONFIG_ASPEED_ENABLE_SUPERIO leaves
> the ast2x00 built-in Super I/O device enabled, as it is required for some
> systems, and CONFIG_ASPEED_ENABLE_DEBUG_UART leaves the hardware
> debug UART enabled, since it provides a relatively high ratio of utility to
> security risk during development.
> 
> This patch is based on a patch by Andrew Jeffery for an older u-boot branch in
> the OpenBMC tree for the df-isolate-bmc distro feature flag.
> 
> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> ---
> 
> Tested on ast2500 and (hostless, BMC-only) ast2400.
> 
> Ryan, are you OK with having an option (off by default) to leave the debug
> UART enabled as in this version of the patch?
> 
Thanks your submit.
Again, my opinion still keep the direct patch to disable it.
Not have config to enable it.

Ryan
> Ian, if you could test this out with CONFIG_ASPEED_ENABLE_SUPERIO=y on
> one of your systems and confirm that that setting works as intended that
> would be great.
> 
> Changes since v2 [1]:
>  - made most of the changes unconditional/unconfigurable, but added
>    Kconfig options to leave Super I/O and debug UART enabled
> 
> Changes since v1 [0]:
>  - extended to cover ast2400
>  - inverted sense of Kconfig option, default (n) is now secure mode
>  - renamed some register/bit macros more appropriately
> 
> [0]
> https://lore.kernel.org/openbmc/20220414040448.27100-1-zev@bewilderbees
> t.net/
> [1]
> https://lore.kernel.org/openbmc/20220414224004.29703-1-zev@bewilderbees
> t.net/
> 
>  arch/arm/include/asm/arch-aspeed/platform.h   |  7 ++
>  .../arm/include/asm/arch-aspeed/scu_ast2400.h |  7
> ++  .../arm/include/asm/arch-aspeed/scu_ast2500.h |  8 ++
>  arch/arm/mach-aspeed/Kconfig                  | 22 ++++++
>  arch/arm/mach-aspeed/ast2400/board_common.c   | 66
> +++++++++++++++++
>  arch/arm/mach-aspeed/ast2500/board_common.c   | 73
> +++++++++++++++++++
>  6 files changed, 183 insertions(+)
> 
> diff --git a/arch/arm/include/asm/arch-aspeed/platform.h
> b/arch/arm/include/asm/arch-aspeed/platform.h
> index f016bdaba3e7..f05747642f38 100644
> --- a/arch/arm/include/asm/arch-aspeed/platform.h
> +++ b/arch/arm/include/asm/arch-aspeed/platform.h
> @@ -15,24 +15,31 @@
> 
> /***************************************************************
> ******************/
>  #if defined(CONFIG_ASPEED_AST2400)
>  #define ASPEED_MAC_COUNT	2
> +#define ASPEED_SDRAM_CTRL	0x1e6e0000
>  #define ASPEED_HW_STRAP1	0x1e6e2070
>  #define ASPEED_REVISION_ID	0x1e6e207C
>  #define ASPEED_SYS_RESET_CTRL	0x1e6e203C
>  #define ASPEED_VGA_HANDSHAKE0	0x1e6e2040	/*	VGA fuction
> handshake register */
> +#define ASPEED_PCIE_CONFIG_SET	0x1e6e2180
>  #define ASPEED_DRAM_BASE	0x40000000
>  #define ASPEED_SRAM_BASE	0x1E720000
> +#define ASPEED_LPC_CTRL		0x1e789000
>  #define ASPEED_SRAM_SIZE	0x8000
>  #define ASPEED_FMC_CS0_BASE	0x20000000
>  #elif defined(CONFIG_ASPEED_AST2500)
>  #define ASPEED_MAC_COUNT	2
> +#define ASPEED_SDRAM_CTRL	0x1e6e0000
> +#define ASPEED_MISC1_CTRL	0x1e6e202C
>  #define ASPEED_HW_STRAP1	0x1e6e2070
>  #define ASPEED_HW_STRAP2	0x1e6e20D0
>  #define ASPEED_REVISION_ID	0x1e6e207C
>  #define ASPEED_SYS_RESET_CTRL	0x1e6e203C
>  #define ASPEED_VGA_HANDSHAKE0	0x1e6e2040	/*	VGA fuction
> handshake register */
> +#define ASPEED_PCIE_CONFIG_SET	0x1e6e2180
>  #define ASPEED_MAC_COUNT	2
>  #define ASPEED_DRAM_BASE	0x80000000
>  #define ASPEED_SRAM_BASE	0x1E720000
> +#define ASPEED_LPC_CTRL		0x1e789000
>  #define ASPEED_SRAM_SIZE	0x9000
>  #define ASPEED_FMC_CS0_BASE	0x20000000
>  #elif defined(CONFIG_ASPEED_AST2600)
> diff --git a/arch/arm/include/asm/arch-aspeed/scu_ast2400.h
> b/arch/arm/include/asm/arch-aspeed/scu_ast2400.h
> index 9c5d96ae84b9..55875fd8312f 100644
> --- a/arch/arm/include/asm/arch-aspeed/scu_ast2400.h
> +++ b/arch/arm/include/asm/arch-aspeed/scu_ast2400.h
> @@ -8,6 +8,7 @@
>  #define SCU_HWSTRAP_VGAMEM_MASK		(3 <<
> SCU_HWSTRAP_VGAMEM_SHIFT)
>  #define SCU_HWSTRAP_MAC1_RGMII		(1 << 6)
>  #define SCU_HWSTRAP_MAC2_RGMII		(1 << 7)
> +#define SCU_HWSTRAP_LPC_SIO_DEC_DIS	(1 << 20)
>  #define SCU_HWSTRAP_DDR4		(1 << 24)
>  #define SCU_HWSTRAP_CLKIN_25MHZ		(1 << 23)
> 
> @@ -104,6 +105,12 @@
>  #define SCU_CLKDUTY_RGMII2TXCK_SHIFT	16
>  #define SCU_CLKDUTY_RGMII2TXCK_MASK	(0x7f <<
> SCU_CLKDUTY_RGMII2TXCK_SHIFT)
> 
> +#define SCU_PCIE_CONFIG_SET_VGA_MMIO	(1 << 1)
> +#define SCU_PCIE_CONFIG_SET_BMC_EN	(1 << 8)
> +#define SCU_PCIE_CONFIG_SET_BMC_MMIO	(1 << 9)
> +#define SCU_PCIE_CONFIG_SET_BMC_DMA	(1 << 14)
> +
> +
>  struct ast2400_clk_priv {
>  	struct ast2400_scu *scu;
>  };
> diff --git a/arch/arm/include/asm/arch-aspeed/scu_ast2500.h
> b/arch/arm/include/asm/arch-aspeed/scu_ast2500.h
> index 8fe4028e4ff0..06dc998afaa8 100644
> --- a/arch/arm/include/asm/arch-aspeed/scu_ast2500.h
> +++ b/arch/arm/include/asm/arch-aspeed/scu_ast2500.h
> @@ -11,6 +11,7 @@
>  #define SCU_HWSTRAP_VGAMEM_MASK		(3 <<
> SCU_HWSTRAP_VGAMEM_SHIFT)
>  #define SCU_HWSTRAP_MAC1_RGMII		(1 << 6)
>  #define SCU_HWSTRAP_MAC2_RGMII		(1 << 7)
> +#define SCU_HWSTRAP_LPC_SIO_DEC_DIS	(1 << 20)
>  #define SCU_HWSTRAP_DDR4		(1 << 24)
>  #define SCU_HWSTRAP_CLKIN_25MHZ		(1 << 23)
> 
> @@ -107,6 +108,13 @@
>  #define SCU_CLKDUTY_RGMII2TXCK_SHIFT	16
>  #define SCU_CLKDUTY_RGMII2TXCK_MASK	(0x7f <<
> SCU_CLKDUTY_RGMII2TXCK_SHIFT)
> 
> +#define SCU_PCIE_CONFIG_SET_VGA_MMIO	(1 << 1)
> +#define SCU_PCIE_CONFIG_SET_BMC_EN	(1 << 8)
> +#define SCU_PCIE_CONFIG_SET_BMC_MMIO	(1 << 9)
> +#define SCU_PCIE_CONFIG_SET_BMC_DMA	(1 << 14)
> +
> +#define SCU_MISC_DEBUG_UART_DISABLE	(1 << 10)
> +
>  struct ast2500_clk_priv {
>  	struct ast2500_scu *scu;
>  };
> diff --git a/arch/arm/mach-aspeed/Kconfig b/arch/arm/mach-aspeed/Kconfig
> index 579a547df61e..fc565e0da830 100644
> --- a/arch/arm/mach-aspeed/Kconfig
> +++ b/arch/arm/mach-aspeed/Kconfig
> @@ -45,6 +45,28 @@ config ASPEED_AST2600
>  	  which is enabled by support of LPC and eSPI peripherals.
>  endchoice
> 
> +config ASPEED_ENABLE_SUPERIO
> +	bool "Enable built-in AST2x00 Super I/O hardware"
> +	depends on ASPEED_AST2400 || ASPEED_AST2500
> +	help
> +	  The Aspeed AST2400 and AST2500 include a built-in Super I/O
> +	  device that is normally disabled; say Y here to enable it.
> +	  Note that this has security implications: it grants the host
> +	  read access to the BMC's entire address space.  This should
> +	  thus be left disabled unless required by a specific system.
> +
> +config ASPEED_ENABLE_DEBUG_UART
> +	bool "Enable AST2500 hardware debug UART"
> +	depends on ASPEED_AST2500
> +	help
> +	  The Aspeed AST2500 include a hardware-supported, UART-based
> +	  debug interface that is normally disabled; say Y here to
> +	  enable it.  Note that this has security implications: the
> +	  debug UART provide read/write access to the BMC's entire
> +	  address space.  This should thus be left disabled on
> +	  production systems, but may be useful to enable for
> +	  debugging during development.
> +
>  config ASPEED_PALLADIUM
>  	bool "Aspeed palladium for simulation"
>  	default n
> diff --git a/arch/arm/mach-aspeed/ast2400/board_common.c
> b/arch/arm/mach-aspeed/ast2400/board_common.c
> index 3829b069342e..7134105232cb 100644
> --- a/arch/arm/mach-aspeed/ast2400/board_common.c
> +++ b/arch/arm/mach-aspeed/ast2400/board_common.c
> @@ -4,14 +4,80 @@
>  #include <ram.h>
>  #include <timer.h>
>  #include <asm/io.h>
> +#include <asm/arch/platform.h>
> +#include <asm/arch/scu_ast2400.h>
>  #include <asm/arch/timer.h>
>  #include <linux/err.h>
>  #include <dm/uclass.h>
> 
>  DECLARE_GLOBAL_DATA_PTR;
> 
> +#define AST_LPC_HICR5 0x080
> +# define LPC_HICR5_ENFWH BIT(10)
> +#define AST_LPC_HICRB 0x100
> +# define LPC_HICRB_SIO_ILPC2AHB_DIS BIT(6)
> +
> +#define AST_SDMC_PROTECT 0x00
> +# define SDRAM_UNLOCK_KEY 0xfc600309
> +#define AST_SDMC_GFX_PROT 0x08
> +# define SDMC_GFX_PROT_VGA_CURSOR BIT(0) # define
> +SDMC_GFX_PROT_VGA_CG_READ BIT(1) # define
> SDMC_GFX_PROT_VGA_ASCII_READ
> +BIT(2) # define SDMC_GFX_PROT_VGA_CRT BIT(3) # define
> +SDMC_GFX_PROT_PCIE BIT(16) # define SDMC_GFX_PROT_XDMA BIT(17)
> +
> +static void isolate_bmc(void)
> +{
> +	bool sdmc_unlocked;
> +	u32 val;
> +
> +	/* iLPC2AHB */
> +#if !defined(CONFIG_ASPEED_ENABLE_SUPERIO)
> +	val = readl(ASPEED_HW_STRAP1);
> +	val |= SCU_HWSTRAP_LPC_SIO_DEC_DIS;
> +	writel(val, ASPEED_HW_STRAP1);
> +#endif
> +
> +	val = readl(ASPEED_LPC_CTRL + AST_LPC_HICRB);
> +	val |= LPC_HICRB_SIO_ILPC2AHB_DIS;
> +	writel(val, ASPEED_LPC_CTRL + AST_LPC_HICRB);
> +
> +	/* P2A, PCIe BMC */
> +	val = readl(ASPEED_PCIE_CONFIG_SET);
> +	val &= ~(SCU_PCIE_CONFIG_SET_BMC_DMA
> +	         | SCU_PCIE_CONFIG_SET_BMC_MMIO
> +	         | SCU_PCIE_CONFIG_SET_BMC_EN
> +	         | SCU_PCIE_CONFIG_SET_VGA_MMIO);
> +	writel(val, ASPEED_PCIE_CONFIG_SET);
> +
> +	/* X-DMA */
> +	sdmc_unlocked = readl(ASPEED_SDRAM_CTRL + AST_SDMC_PROTECT);
> +	if (!sdmc_unlocked)
> +		writel(SDRAM_UNLOCK_KEY, ASPEED_SDRAM_CTRL +
> AST_SDMC_PROTECT);
> +
> +	val = readl(ASPEED_SDRAM_CTRL + AST_SDMC_GFX_PROT);
> +	val |= (SDMC_GFX_PROT_VGA_CURSOR
> +	        | SDMC_GFX_PROT_VGA_CG_READ
> +	        | SDMC_GFX_PROT_VGA_ASCII_READ
> +	        | SDMC_GFX_PROT_VGA_CRT
> +	        | SDMC_GFX_PROT_PCIE
> +	        | SDMC_GFX_PROT_XDMA);
> +	writel(val, ASPEED_SDRAM_CTRL + AST_SDMC_GFX_PROT);
> +
> +	if (!sdmc_unlocked)
> +		writel(~SDRAM_UNLOCK_KEY, ASPEED_SDRAM_CTRL +
> AST_SDMC_PROTECT);
> +
> +	/* LPC2AHB */
> +	val = readl(ASPEED_LPC_CTRL + AST_LPC_HICR5);
> +	val &= ~LPC_HICR5_ENFWH;
> +	writel(val, ASPEED_LPC_CTRL + AST_LPC_HICR5); }
> +
>  __weak int board_init(void)
>  {
> +	isolate_bmc();
> +
>  	gd->bd->bi_boot_params = CONFIG_SYS_SDRAM_BASE + 0x100;
> 
>  	return 0;
> diff --git a/arch/arm/mach-aspeed/ast2500/board_common.c
> b/arch/arm/mach-aspeed/ast2500/board_common.c
> index ce541e88fb8e..c63fe466eb4b 100644
> --- a/arch/arm/mach-aspeed/ast2500/board_common.c
> +++ b/arch/arm/mach-aspeed/ast2500/board_common.c
> @@ -7,18 +7,91 @@
>  #include <ram.h>
>  #include <timer.h>
>  #include <asm/io.h>
> +#include <asm/arch/platform.h>
> +#include <asm/arch/scu_ast2500.h>
> +#include <asm/arch/sdram_ast2500.h>
>  #include <asm/arch/timer.h>
>  #include <linux/err.h>
>  #include <dm/uclass.h>
> 
>  DECLARE_GLOBAL_DATA_PTR;
> 
> +#define AST_LPC_HICR5 0x080
> +# define LPC_HICR5_ENFWH BIT(10)
> +#define AST_LPC_HICRB 0x100
> +# define LPC_HICRB_SIO_ILPC2AHB_DIS BIT(6)
> +
> +# define AST_SDMC_PROTECT 0x00
> +# define AST_SDMC_GFX_PROT 0x08
> +#  define SDMC_GFX_PROT_VGA_CURSOR BIT(0) #  define
> +SDMC_GFX_PROT_VGA_CG_READ BIT(1) #  define
> SDMC_GFX_PROT_VGA_ASCII_READ
> +BIT(2) #  define SDMC_GFX_PROT_VGA_CRT BIT(3) #  define
> +SDMC_GFX_PROT_PCIE BIT(16) #  define SDMC_GFX_PROT_XDMA BIT(17)
> +
> +static void isolate_bmc(void)
> +{
> +	bool sdmc_unlocked;
> +	u32 val;
> +
> +	/* iLPC2AHB */
> +#if !defined(CONFIG_ASPEED_ENABLE_SUPERIO)
> +	val = readl(ASPEED_HW_STRAP1);
> +	val |= SCU_HWSTRAP_LPC_SIO_DEC_DIS;
> +	writel(val, ASPEED_HW_STRAP1);
> +#endif
> +
> +	val = readl(ASPEED_LPC_CTRL + AST_LPC_HICRB);
> +	val |= LPC_HICRB_SIO_ILPC2AHB_DIS;
> +	writel(val, ASPEED_LPC_CTRL + AST_LPC_HICRB);
> +
> +	/* P2A, PCIe BMC */
> +	val = readl(ASPEED_PCIE_CONFIG_SET);
> +	val &= ~(SCU_PCIE_CONFIG_SET_BMC_DMA
> +	         | SCU_PCIE_CONFIG_SET_BMC_MMIO
> +	         | SCU_PCIE_CONFIG_SET_BMC_EN
> +	         | SCU_PCIE_CONFIG_SET_VGA_MMIO);
> +	writel(val, ASPEED_PCIE_CONFIG_SET);
> +
> +	/* Debug UART */
> +#if !defined(CONFIG_ASPEED_ENABLE_DEBUG_UART)
> +	val = readl(ASPEED_MISC1_CTRL);
> +	val |= SCU_MISC_DEBUG_UART_DISABLE;
> +	writel(val, ASPEED_MISC1_CTRL);
> +#endif
> +
> +	/* X-DMA */
> +	sdmc_unlocked = readl(ASPEED_SDRAM_CTRL + AST_SDMC_PROTECT);
> +	if (!sdmc_unlocked)
> +		writel(SDRAM_UNLOCK_KEY, ASPEED_SDRAM_CTRL +
> AST_SDMC_PROTECT);
> +
> +	val = readl(ASPEED_SDRAM_CTRL + AST_SDMC_GFX_PROT);
> +	val |= (SDMC_GFX_PROT_VGA_CURSOR
> +	        | SDMC_GFX_PROT_VGA_CG_READ
> +	        | SDMC_GFX_PROT_VGA_ASCII_READ
> +	        | SDMC_GFX_PROT_VGA_CRT
> +	        | SDMC_GFX_PROT_PCIE
> +	        | SDMC_GFX_PROT_XDMA);
> +	writel(val, ASPEED_SDRAM_CTRL + AST_SDMC_GFX_PROT);
> +
> +	if (!sdmc_unlocked)
> +		writel(~SDRAM_UNLOCK_KEY, ASPEED_SDRAM_CTRL +
> AST_SDMC_PROTECT);
> +
> +	/* LPC2AHB */
> +	val = readl(ASPEED_LPC_CTRL + AST_LPC_HICR5);
> +	val &= ~LPC_HICR5_ENFWH;
> +	writel(val, ASPEED_LPC_CTRL + AST_LPC_HICR5); }
> +
>  __weak int board_init(void)
>  {
>  	struct udevice *dev;
>  	int i;
>  	int ret;
> 
> +	isolate_bmc();
> +
>  	gd->bd->bi_boot_params = CONFIG_SYS_SDRAM_BASE + 0x100;
> 
>  	/*
> --
> 2.35.1


  reply	other threads:[~2022-04-20  3:07 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-19 23:42 [PATCH u-boot v2019.04-aspeed-openbmc v3] aspeed: Disable backdoor interfaces Zev Weiss
2022-04-20  3:06 ` Ryan Chen [this message]
2022-04-20  3:34   ` Andrew Jeffery
2022-04-21  6:20     ` Joel Stanley
2022-05-02  8:11 ` Joel Stanley
2022-05-02 14:37   ` Woloschin, Ian
2022-05-04  0:21   ` Zev Weiss

Reply instructions:

You may reply publicly 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=HK0PR06MB3380C8FDEE1588E4BAE945B3F2F59@HK0PR06MB3380.apcprd06.prod.outlook.com \
    --to=ryan_chen@aspeedtech.com \
    --cc=andrew@aj.id.au \
    --cc=ian.woloschin@akamai.com \
    --cc=joel@jms.id.au \
    --cc=openbmc@lists.ozlabs.org \
    --cc=yulei.sh@bytedance.com \
    --cc=zev@bewilderbeest.net \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).