Re: [PATCH 2/2] drivers/misc: Add Aspeed P2A control driver
On Wed, 27 Feb 2019, at 15:35, Florian Fainelli wrote: > > > On 2/21/2019 2:25 PM, Patrick Venture wrote: > > The ASPEED AST2400, and AST2500 in some configurations include a > > PCI-to-AHB MMIO bridge. This bridge allows a server to read and write > > in the BMC's memory space. This feature is especially useful when using > > this bridge to send large files to the BMC. > > > > The host may use this to send down a firmware image by staging data at a > > specific memory address, and in a coordinated effort with the BMC's > > software stack and kernel, transmit the bytes. > > > > This driver enables the BMC to unlock the PCI bridge on demand, and > > configure it via ioctl to allow the host to write bytes to an agreed > > upon location. In the primary use-case, the region to use is known > > apriori on the BMC, and the host requests this information. Once this > > request is received, the BMC's software stack will enable the bridge and > > the region and then using some software flow control (possibly via IPMI > > packets), copy the bytes down. Once the process is complete, the BMC > > will disable the bridge and unset any region involved. > > > > The default behavior of this bridge when present is: enabled and all > > regions marked read-write. This driver will fix the regions to be > > read-only and then disable the bridge entirely. > > A complete drive by review, so I could be completely off here (most > likely am), but have you considered using virtio and doing some sort of > rudimentary features (regions here) negotiation over that interface? > > If I get your description right in premise maybe emulating the AHB side > on the BMC as a PCI end-point device driver, and using it as a seemingly > regular PCI EP from the host side with BARs and stuff might make sense > here and be less of a security hole than it currently looks like. It's not immediately clear to me that this is a win; it seems like a lot of complexity that only prevents the host from accidentally stomping on areas of BMC RAM; a malicious host would just ignore this constraint anyway as the hardware capability allows access anywhere in the BMC's address space. Most of the accidental case is taken care of by Patrick's strategy anyway, which is disabling the capability entirely when there's no need for it to be active. Andrew
Re: [PATCH 2/2] drivers/misc: Add Aspeed P2A control driver
On Tue, Feb 26, 2019 at 9:53 PM Andrew Jeffery wrote: > > > > On Wed, 27 Feb 2019, at 13:04, Patrick Venture wrote: > > On Tue, Feb 26, 2019 at 5:05 PM Andrew Jeffery wrote: > > > > > > > > > > > > On Wed, 27 Feb 2019, at 08:12, Patrick Venture wrote: > > > > On Sun, Feb 24, 2019 at 5:26 PM Andrew Jeffery wrote: > > > > > > > > > > On Fri, 22 Feb 2019, at 08:55, Patrick Venture wrote: > > > > > > The ASPEED AST2400, and AST2500 in some configurations include a > > > > > > PCI-to-AHB MMIO bridge. This bridge allows a server to read and > > > > > > write > > > > > > in the BMC's memory space. > > > > > > > > > > Bit of a nit, but I think s/memory space/physical address space/ makes > > > > > the power of the interface a bit clearer. > > > > > > > > > > > This feature is especially useful when using > > > > > > this bridge to send large files to the BMC. > > > > > > > > > > > > The host may use this to send down a firmware image by staging data > > > > > > at a > > > > > > specific memory address, and in a coordinated effort with the BMC's > > > > > > software stack and kernel, transmit the bytes. > > > > > > > > > > > > This driver enables the BMC to unlock the PCI bridge on demand, and > > > > > > configure it via ioctl to allow the host to write bytes to an agreed > > > > > > upon location. In the primary use-case, the region to use is known > > > > > > apriori on the BMC, and the host requests this information. Once > > > > > > this > > > > > > request is received, the BMC's software stack will enable the > > > > > > bridge and > > > > > > the region and then using some software flow control (possibly via > > > > > > IPMI > > > > > > packets), copy the bytes down. Once the process is complete, the > > > > > > BMC > > > > > > will disable the bridge and unset any region involved. > > > > > > > > > > I feel the description is a little subtle. You mention locations and > > > > > regions > > > > > without really defining their relationship. We have the means to > > > > > prevent > > > > > writes via the P2A to following regions in the BMC's physical address > > > > > space: > > > > > > > > > > * BMC flash MMIO window > > > > > * System flash MMIO windows > > > > > * SOC IO (peripheral MMIO) > > > > > * DRAM > > > > > > > > > > So what I think should be made clear is once we allow the host to > > > > > write > > > > > to e.g. DRAM, it can write to *all* of DRAM, regardless of what > > > > > location the > > > > > BMC recommended, i.e. the BMC is at the mercy of the host wrt > > > > > confidentiality once the P2A is enabled (host can always read > > > > > anywhere) > > > > > and integrity when the DRAM write filter is disabled. > > > > > > > > Ok, I can try to work that phrasing in. > > > > > > > > > > > > > > There is no way to specify and constrain P2A writes to specific > > > > > locations > > > > > in DRAM. > > > > > > > > > > > > > > > > > The default behavior of this bridge when present is: enabled and all > > > > > > regions marked read-write. This driver will fix the regions to be > > > > > > read-only and then disable the bridge entirely. > > > > > > > > > > > > Signed-off-by: Patrick Venture > > > > > > --- > > > > > > drivers/misc/Kconfig | 8 + > > > > > > drivers/misc/Makefile| 1 + > > > > > > drivers/misc/aspeed-p2a-ctrl.c | 498 > > > > > > +++ > > > > > > include/uapi/linux/aspeed-p2a-ctrl.h | 46 +++ > > > > > > 4 files changed, 553 insertions(+) > > > > > > create mode 100644 drivers/misc/aspeed-p2a-ctrl.c > > > > > > create mode 100644 include/uapi/linux/aspeed-p2a-ctrl.h > > > > > > > > > > > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig > > > > > > index f417b06e11c5..54ed265a26f0 100644 > > > > > > --- a/drivers/misc/Kconfig > > > > > > +++ b/drivers/misc/Kconfig > > > > > > @@ -485,6 +485,14 @@ config VEXPRESS_SYSCFG > > > > > > bus. System Configuration interface is one of the possible > > > > > > means > > > > > > of generating transactions on this bus. > > > > > > > > > > > > +config ASPEED_P2A_CTRL > > > > > > + depends on (ARCH_ASPEED || COMPILE_TEST) > > > > > > + tristate "Aspeed ast2400/2500 HOST P2A VGA MMIO to BMC bridge > > > > > > control" > > > > > > + help > > > > > > + Control Aspeed ast2400/2500 HOST P2A VGA MMIO to BMC > > > > > > mappings > > > > > > through > > > > > > + ioctl()s, the driver also provides an interface for > > > > > > userspace > > > > > > mappings to > > > > > > + a pre-defined region. > > > > > > + > > > > > > config ASPEED_LPC_CTRL > > > > > > depends on (ARCH_ASPEED || COMPILE_TEST) && REGMAP && > > > > > > MFD_SYSCON > > > > > > tristate "Aspeed ast2400/2500 HOST LPC to BMC bridge control" > > > > > > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile > > > > > > index e39ccbbc1b3a..57577aee354f 100644 > > > > > > --- a/drivers/misc/Makefile > > > > > > +++ b/drivers/misc/
Re: [PATCH 2/2] drivers/misc: Add Aspeed P2A control driver
On Tue, Feb 26, 2019 at 9:05 PM Florian Fainelli wrote: > > > > On 2/21/2019 2:25 PM, Patrick Venture wrote: > > The ASPEED AST2400, and AST2500 in some configurations include a > > PCI-to-AHB MMIO bridge. This bridge allows a server to read and write > > in the BMC's memory space. This feature is especially useful when using > > this bridge to send large files to the BMC. > > > > The host may use this to send down a firmware image by staging data at a > > specific memory address, and in a coordinated effort with the BMC's > > software stack and kernel, transmit the bytes. > > > > This driver enables the BMC to unlock the PCI bridge on demand, and > > configure it via ioctl to allow the host to write bytes to an agreed > > upon location. In the primary use-case, the region to use is known > > apriori on the BMC, and the host requests this information. Once this > > request is received, the BMC's software stack will enable the bridge and > > the region and then using some software flow control (possibly via IPMI > > packets), copy the bytes down. Once the process is complete, the BMC > > will disable the bridge and unset any region involved. > > > > The default behavior of this bridge when present is: enabled and all > > regions marked read-write. This driver will fix the regions to be > > read-only and then disable the bridge entirely. > > A complete drive by review, so I could be completely off here (most > likely am), but have you considered using virtio and doing some sort of > rudimentary features (regions here) negotiation over that interface? I have not. > > If I get your description right in premise maybe emulating the AHB side > on the BMC as a PCI end-point device driver, and using it as a seemingly > regular PCI EP from the host side with BARs and stuff might make sense > here and be less of a security hole than it currently looks like. In this case, what I"m trying to do is control access to regions of BMC physical address space. The ASPEED BMC is by default, completely open in this regard, see CVE-2019-6260. There are two sets of registers that control the host's ability to read or write the physical address space. The host needs to be able to write to the BMC's physical address space in some use-cases -- one of which is my firmware staging case. It could just as easily be used as a memory buffer region for a virtual nic. Part of the goal here though is that the host should not have control of what regions are on/off without the BMC allowing it. It's currently a security hole, and this driver is meant to open that hole on demand for specific purposes, whereas the default state is completely open. > -- > Florian
Re: [PATCH 2/2] drivers/misc: Add Aspeed P2A control driver
On Wed, 27 Feb 2019, at 13:04, Patrick Venture wrote: > On Tue, Feb 26, 2019 at 5:05 PM Andrew Jeffery wrote: > > > > > > > > On Wed, 27 Feb 2019, at 08:12, Patrick Venture wrote: > > > On Sun, Feb 24, 2019 at 5:26 PM Andrew Jeffery wrote: > > > > > > > > On Fri, 22 Feb 2019, at 08:55, Patrick Venture wrote: > > > > > The ASPEED AST2400, and AST2500 in some configurations include a > > > > > PCI-to-AHB MMIO bridge. This bridge allows a server to read and write > > > > > in the BMC's memory space. > > > > > > > > Bit of a nit, but I think s/memory space/physical address space/ makes > > > > the power of the interface a bit clearer. > > > > > > > > > This feature is especially useful when using > > > > > this bridge to send large files to the BMC. > > > > > > > > > > The host may use this to send down a firmware image by staging data > > > > > at a > > > > > specific memory address, and in a coordinated effort with the BMC's > > > > > software stack and kernel, transmit the bytes. > > > > > > > > > > This driver enables the BMC to unlock the PCI bridge on demand, and > > > > > configure it via ioctl to allow the host to write bytes to an agreed > > > > > upon location. In the primary use-case, the region to use is known > > > > > apriori on the BMC, and the host requests this information. Once this > > > > > request is received, the BMC's software stack will enable the bridge > > > > > and > > > > > the region and then using some software flow control (possibly via > > > > > IPMI > > > > > packets), copy the bytes down. Once the process is complete, the BMC > > > > > will disable the bridge and unset any region involved. > > > > > > > > I feel the description is a little subtle. You mention locations and > > > > regions > > > > without really defining their relationship. We have the means to prevent > > > > writes via the P2A to following regions in the BMC's physical address > > > > space: > > > > > > > > * BMC flash MMIO window > > > > * System flash MMIO windows > > > > * SOC IO (peripheral MMIO) > > > > * DRAM > > > > > > > > So what I think should be made clear is once we allow the host to write > > > > to e.g. DRAM, it can write to *all* of DRAM, regardless of what > > > > location the > > > > BMC recommended, i.e. the BMC is at the mercy of the host wrt > > > > confidentiality once the P2A is enabled (host can always read anywhere) > > > > and integrity when the DRAM write filter is disabled. > > > > > > Ok, I can try to work that phrasing in. > > > > > > > > > > > There is no way to specify and constrain P2A writes to specific > > > > locations > > > > in DRAM. > > > > > > > > > > > > > > The default behavior of this bridge when present is: enabled and all > > > > > regions marked read-write. This driver will fix the regions to be > > > > > read-only and then disable the bridge entirely. > > > > > > > > > > Signed-off-by: Patrick Venture > > > > > --- > > > > > drivers/misc/Kconfig | 8 + > > > > > drivers/misc/Makefile| 1 + > > > > > drivers/misc/aspeed-p2a-ctrl.c | 498 > > > > > +++ > > > > > include/uapi/linux/aspeed-p2a-ctrl.h | 46 +++ > > > > > 4 files changed, 553 insertions(+) > > > > > create mode 100644 drivers/misc/aspeed-p2a-ctrl.c > > > > > create mode 100644 include/uapi/linux/aspeed-p2a-ctrl.h > > > > > > > > > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig > > > > > index f417b06e11c5..54ed265a26f0 100644 > > > > > --- a/drivers/misc/Kconfig > > > > > +++ b/drivers/misc/Kconfig > > > > > @@ -485,6 +485,14 @@ config VEXPRESS_SYSCFG > > > > > bus. System Configuration interface is one of the possible > > > > > means > > > > > of generating transactions on this bus. > > > > > > > > > > +config ASPEED_P2A_CTRL > > > > > + depends on (ARCH_ASPEED || COMPILE_TEST) > > > > > + tristate "Aspeed ast2400/2500 HOST P2A VGA MMIO to BMC bridge > > > > > control" > > > > > + help > > > > > + Control Aspeed ast2400/2500 HOST P2A VGA MMIO to BMC mappings > > > > > through > > > > > + ioctl()s, the driver also provides an interface for userspace > > > > > mappings to > > > > > + a pre-defined region. > > > > > + > > > > > config ASPEED_LPC_CTRL > > > > > depends on (ARCH_ASPEED || COMPILE_TEST) && REGMAP && MFD_SYSCON > > > > > tristate "Aspeed ast2400/2500 HOST LPC to BMC bridge control" > > > > > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile > > > > > index e39ccbbc1b3a..57577aee354f 100644 > > > > > --- a/drivers/misc/Makefile > > > > > +++ b/drivers/misc/Makefile > > > > > @@ -55,6 +55,7 @@ obj-$(CONFIG_VEXPRESS_SYSCFG) += > > > > > vexpress-syscfg.o > > > > > obj-$(CONFIG_CXL_BASE) += cxl/ > > > > > obj-$(CONFIG_ASPEED_LPC_CTRL)+= aspeed-lpc-ctrl.o > > > > > obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o > > > > > +obj-$(CONFIG_ASPEED_P2A_CTRL)+= aspeed-p2a-ctrl.o > > >
Re: [PATCH 2/2] drivers/misc: Add Aspeed P2A control driver
On 2/21/2019 2:25 PM, Patrick Venture wrote: > The ASPEED AST2400, and AST2500 in some configurations include a > PCI-to-AHB MMIO bridge. This bridge allows a server to read and write > in the BMC's memory space. This feature is especially useful when using > this bridge to send large files to the BMC. > > The host may use this to send down a firmware image by staging data at a > specific memory address, and in a coordinated effort with the BMC's > software stack and kernel, transmit the bytes. > > This driver enables the BMC to unlock the PCI bridge on demand, and > configure it via ioctl to allow the host to write bytes to an agreed > upon location. In the primary use-case, the region to use is known > apriori on the BMC, and the host requests this information. Once this > request is received, the BMC's software stack will enable the bridge and > the region and then using some software flow control (possibly via IPMI > packets), copy the bytes down. Once the process is complete, the BMC > will disable the bridge and unset any region involved. > > The default behavior of this bridge when present is: enabled and all > regions marked read-write. This driver will fix the regions to be > read-only and then disable the bridge entirely. A complete drive by review, so I could be completely off here (most likely am), but have you considered using virtio and doing some sort of rudimentary features (regions here) negotiation over that interface? If I get your description right in premise maybe emulating the AHB side on the BMC as a PCI end-point device driver, and using it as a seemingly regular PCI EP from the host side with BARs and stuff might make sense here and be less of a security hole than it currently looks like. -- Florian
Re: [PATCH 2/2] drivers/misc: Add Aspeed P2A control driver
On Tue, Feb 26, 2019 at 5:05 PM Andrew Jeffery wrote: > > > > On Wed, 27 Feb 2019, at 08:12, Patrick Venture wrote: > > On Sun, Feb 24, 2019 at 5:26 PM Andrew Jeffery wrote: > > > > > > On Fri, 22 Feb 2019, at 08:55, Patrick Venture wrote: > > > > The ASPEED AST2400, and AST2500 in some configurations include a > > > > PCI-to-AHB MMIO bridge. This bridge allows a server to read and write > > > > in the BMC's memory space. > > > > > > Bit of a nit, but I think s/memory space/physical address space/ makes > > > the power of the interface a bit clearer. > > > > > > > This feature is especially useful when using > > > > this bridge to send large files to the BMC. > > > > > > > > The host may use this to send down a firmware image by staging data at a > > > > specific memory address, and in a coordinated effort with the BMC's > > > > software stack and kernel, transmit the bytes. > > > > > > > > This driver enables the BMC to unlock the PCI bridge on demand, and > > > > configure it via ioctl to allow the host to write bytes to an agreed > > > > upon location. In the primary use-case, the region to use is known > > > > apriori on the BMC, and the host requests this information. Once this > > > > request is received, the BMC's software stack will enable the bridge and > > > > the region and then using some software flow control (possibly via IPMI > > > > packets), copy the bytes down. Once the process is complete, the BMC > > > > will disable the bridge and unset any region involved. > > > > > > I feel the description is a little subtle. You mention locations and > > > regions > > > without really defining their relationship. We have the means to prevent > > > writes via the P2A to following regions in the BMC's physical address > > > space: > > > > > > * BMC flash MMIO window > > > * System flash MMIO windows > > > * SOC IO (peripheral MMIO) > > > * DRAM > > > > > > So what I think should be made clear is once we allow the host to write > > > to e.g. DRAM, it can write to *all* of DRAM, regardless of what location > > > the > > > BMC recommended, i.e. the BMC is at the mercy of the host wrt > > > confidentiality once the P2A is enabled (host can always read anywhere) > > > and integrity when the DRAM write filter is disabled. > > > > Ok, I can try to work that phrasing in. > > > > > > > > There is no way to specify and constrain P2A writes to specific locations > > > in DRAM. > > > > > > > > > > > The default behavior of this bridge when present is: enabled and all > > > > regions marked read-write. This driver will fix the regions to be > > > > read-only and then disable the bridge entirely. > > > > > > > > Signed-off-by: Patrick Venture > > > > --- > > > > drivers/misc/Kconfig | 8 + > > > > drivers/misc/Makefile| 1 + > > > > drivers/misc/aspeed-p2a-ctrl.c | 498 +++ > > > > include/uapi/linux/aspeed-p2a-ctrl.h | 46 +++ > > > > 4 files changed, 553 insertions(+) > > > > create mode 100644 drivers/misc/aspeed-p2a-ctrl.c > > > > create mode 100644 include/uapi/linux/aspeed-p2a-ctrl.h > > > > > > > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig > > > > index f417b06e11c5..54ed265a26f0 100644 > > > > --- a/drivers/misc/Kconfig > > > > +++ b/drivers/misc/Kconfig > > > > @@ -485,6 +485,14 @@ config VEXPRESS_SYSCFG > > > > bus. System Configuration interface is one of the possible means > > > > of generating transactions on this bus. > > > > > > > > +config ASPEED_P2A_CTRL > > > > + depends on (ARCH_ASPEED || COMPILE_TEST) > > > > + tristate "Aspeed ast2400/2500 HOST P2A VGA MMIO to BMC bridge > > > > control" > > > > + help > > > > + Control Aspeed ast2400/2500 HOST P2A VGA MMIO to BMC mappings > > > > through > > > > + ioctl()s, the driver also provides an interface for userspace > > > > mappings to > > > > + a pre-defined region. > > > > + > > > > config ASPEED_LPC_CTRL > > > > depends on (ARCH_ASPEED || COMPILE_TEST) && REGMAP && MFD_SYSCON > > > > tristate "Aspeed ast2400/2500 HOST LPC to BMC bridge control" > > > > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile > > > > index e39ccbbc1b3a..57577aee354f 100644 > > > > --- a/drivers/misc/Makefile > > > > +++ b/drivers/misc/Makefile > > > > @@ -55,6 +55,7 @@ obj-$(CONFIG_VEXPRESS_SYSCFG) += > > > > vexpress-syscfg.o > > > > obj-$(CONFIG_CXL_BASE) += cxl/ > > > > obj-$(CONFIG_ASPEED_LPC_CTRL)+= aspeed-lpc-ctrl.o > > > > obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o > > > > +obj-$(CONFIG_ASPEED_P2A_CTRL)+= aspeed-p2a-ctrl.o > > > > obj-$(CONFIG_PCI_ENDPOINT_TEST) += pci_endpoint_test.o > > > > obj-$(CONFIG_OCXL) += ocxl/ > > > > obj-y+= cardreader/ > > > > diff --git a/drivers/misc/aspeed-p2a-ctrl.c > > > > b/drivers/misc/aspeed-p2a-ctrl.c > > > > new file mode 100644 > > > > index ..a3cf
Re: [PATCH 2/2] drivers/misc: Add Aspeed P2A control driver
On Wed, 27 Feb 2019, at 08:12, Patrick Venture wrote: > On Sun, Feb 24, 2019 at 5:26 PM Andrew Jeffery wrote: > > > > On Fri, 22 Feb 2019, at 08:55, Patrick Venture wrote: > > > The ASPEED AST2400, and AST2500 in some configurations include a > > > PCI-to-AHB MMIO bridge. This bridge allows a server to read and write > > > in the BMC's memory space. > > > > Bit of a nit, but I think s/memory space/physical address space/ makes > > the power of the interface a bit clearer. > > > > > This feature is especially useful when using > > > this bridge to send large files to the BMC. > > > > > > The host may use this to send down a firmware image by staging data at a > > > specific memory address, and in a coordinated effort with the BMC's > > > software stack and kernel, transmit the bytes. > > > > > > This driver enables the BMC to unlock the PCI bridge on demand, and > > > configure it via ioctl to allow the host to write bytes to an agreed > > > upon location. In the primary use-case, the region to use is known > > > apriori on the BMC, and the host requests this information. Once this > > > request is received, the BMC's software stack will enable the bridge and > > > the region and then using some software flow control (possibly via IPMI > > > packets), copy the bytes down. Once the process is complete, the BMC > > > will disable the bridge and unset any region involved. > > > > I feel the description is a little subtle. You mention locations and regions > > without really defining their relationship. We have the means to prevent > > writes via the P2A to following regions in the BMC's physical address space: > > > > * BMC flash MMIO window > > * System flash MMIO windows > > * SOC IO (peripheral MMIO) > > * DRAM > > > > So what I think should be made clear is once we allow the host to write > > to e.g. DRAM, it can write to *all* of DRAM, regardless of what location the > > BMC recommended, i.e. the BMC is at the mercy of the host wrt > > confidentiality once the P2A is enabled (host can always read anywhere) > > and integrity when the DRAM write filter is disabled. > > Ok, I can try to work that phrasing in. > > > > > There is no way to specify and constrain P2A writes to specific locations > > in DRAM. > > > > > > > > The default behavior of this bridge when present is: enabled and all > > > regions marked read-write. This driver will fix the regions to be > > > read-only and then disable the bridge entirely. > > > > > > Signed-off-by: Patrick Venture > > > --- > > > drivers/misc/Kconfig | 8 + > > > drivers/misc/Makefile| 1 + > > > drivers/misc/aspeed-p2a-ctrl.c | 498 +++ > > > include/uapi/linux/aspeed-p2a-ctrl.h | 46 +++ > > > 4 files changed, 553 insertions(+) > > > create mode 100644 drivers/misc/aspeed-p2a-ctrl.c > > > create mode 100644 include/uapi/linux/aspeed-p2a-ctrl.h > > > > > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig > > > index f417b06e11c5..54ed265a26f0 100644 > > > --- a/drivers/misc/Kconfig > > > +++ b/drivers/misc/Kconfig > > > @@ -485,6 +485,14 @@ config VEXPRESS_SYSCFG > > > bus. System Configuration interface is one of the possible means > > > of generating transactions on this bus. > > > > > > +config ASPEED_P2A_CTRL > > > + depends on (ARCH_ASPEED || COMPILE_TEST) > > > + tristate "Aspeed ast2400/2500 HOST P2A VGA MMIO to BMC bridge > > > control" > > > + help > > > + Control Aspeed ast2400/2500 HOST P2A VGA MMIO to BMC mappings > > > through > > > + ioctl()s, the driver also provides an interface for userspace > > > mappings to > > > + a pre-defined region. > > > + > > > config ASPEED_LPC_CTRL > > > depends on (ARCH_ASPEED || COMPILE_TEST) && REGMAP && MFD_SYSCON > > > tristate "Aspeed ast2400/2500 HOST LPC to BMC bridge control" > > > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile > > > index e39ccbbc1b3a..57577aee354f 100644 > > > --- a/drivers/misc/Makefile > > > +++ b/drivers/misc/Makefile > > > @@ -55,6 +55,7 @@ obj-$(CONFIG_VEXPRESS_SYSCFG) += vexpress-syscfg.o > > > obj-$(CONFIG_CXL_BASE) += cxl/ > > > obj-$(CONFIG_ASPEED_LPC_CTRL)+= aspeed-lpc-ctrl.o > > > obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o > > > +obj-$(CONFIG_ASPEED_P2A_CTRL)+= aspeed-p2a-ctrl.o > > > obj-$(CONFIG_PCI_ENDPOINT_TEST) += pci_endpoint_test.o > > > obj-$(CONFIG_OCXL) += ocxl/ > > > obj-y+= cardreader/ > > > diff --git a/drivers/misc/aspeed-p2a-ctrl.c > > > b/drivers/misc/aspeed-p2a-ctrl.c > > > new file mode 100644 > > > index ..a3cf00de9038 > > > --- /dev/null > > > +++ b/drivers/misc/aspeed-p2a-ctrl.c > > > @@ -0,0 +1,498 @@ > > > +/* > > > + * Copyright 2019 Google Inc > > > + * > > > + * This program is free software; you can redistribute it and/or > > > + * modify it under the terms of the GNU General Public Li
Re: [PATCH 2/2] drivers/misc: Add Aspeed P2A control driver
On Tue, Feb 26, 2019 at 2:20 PM Andrew Jeffery wrote: > > > > On Wed, 27 Feb 2019, at 08:26, Patrick Venture wrote: > > On Tue, Feb 26, 2019 at 1:42 PM Patrick Venture wrote: > > > > > > On Sun, Feb 24, 2019 at 5:26 PM Andrew Jeffery wrote: > > > > > > > > > > > > > > > > + > > > > > + /* Access to these needs to be locked, held via probe, mapping > > > > > ioctl, > > > > > + * and release, remove. > > > > > + */ > > > > > + struct mutex tracking; > > > > > + u32 readers; > > > > > + u32 readerwriters[P2A_REGION_COUNT]; > > > > > > > > Might be better to use refcount_t here instead of u32? > > > > > > Ack > > > > refcount requires atomic, is that something we have with the ast2400, 2500? > > Ah, we do on the 2500 (armv6) but not the 2400 (armv5) Ok, so to avoid that complexity I'm not going to swap in refcounts. but good to know for future reference since it's basically what I was doing manually.
Re: [PATCH 2/2] drivers/misc: Add Aspeed P2A control driver
On Wed, 27 Feb 2019, at 08:26, Patrick Venture wrote: > On Tue, Feb 26, 2019 at 1:42 PM Patrick Venture wrote: > > > > On Sun, Feb 24, 2019 at 5:26 PM Andrew Jeffery wrote: > > > > > > > > > > > > + > > > > + /* Access to these needs to be locked, held via probe, mapping > > > > ioctl, > > > > + * and release, remove. > > > > + */ > > > > + struct mutex tracking; > > > > + u32 readers; > > > > + u32 readerwriters[P2A_REGION_COUNT]; > > > > > > Might be better to use refcount_t here instead of u32? > > > > Ack > > refcount requires atomic, is that something we have with the ast2400, 2500? Ah, we do on the 2500 (armv6) but not the 2400 (armv5)
Re: [PATCH 2/2] drivers/misc: Add Aspeed P2A control driver
On Tue, Feb 26, 2019 at 1:42 PM Patrick Venture wrote: > > On Sun, Feb 24, 2019 at 5:26 PM Andrew Jeffery wrote: > > > > On Fri, 22 Feb 2019, at 08:55, Patrick Venture wrote: > > > The ASPEED AST2400, and AST2500 in some configurations include a > > > PCI-to-AHB MMIO bridge. This bridge allows a server to read and write > > > in the BMC's memory space. > > > > Bit of a nit, but I think s/memory space/physical address space/ makes > > the power of the interface a bit clearer. > > > > > This feature is especially useful when using > > > this bridge to send large files to the BMC. > > > > > > The host may use this to send down a firmware image by staging data at a > > > specific memory address, and in a coordinated effort with the BMC's > > > software stack and kernel, transmit the bytes. > > > > > > This driver enables the BMC to unlock the PCI bridge on demand, and > > > configure it via ioctl to allow the host to write bytes to an agreed > > > upon location. In the primary use-case, the region to use is known > > > apriori on the BMC, and the host requests this information. Once this > > > request is received, the BMC's software stack will enable the bridge and > > > the region and then using some software flow control (possibly via IPMI > > > packets), copy the bytes down. Once the process is complete, the BMC > > > will disable the bridge and unset any region involved. > > > > I feel the description is a little subtle. You mention locations and regions > > without really defining their relationship. We have the means to prevent > > writes via the P2A to following regions in the BMC's physical address space: > > > > * BMC flash MMIO window > > * System flash MMIO windows > > * SOC IO (peripheral MMIO) > > * DRAM > > > > So what I think should be made clear is once we allow the host to write > > to e.g. DRAM, it can write to *all* of DRAM, regardless of what location the > > BMC recommended, i.e. the BMC is at the mercy of the host wrt > > confidentiality once the P2A is enabled (host can always read anywhere) > > and integrity when the DRAM write filter is disabled. > > Ok, I can try to work that phrasing in. > > > > > There is no way to specify and constrain P2A writes to specific locations > > in DRAM. > > > > > > > > The default behavior of this bridge when present is: enabled and all > > > regions marked read-write. This driver will fix the regions to be > > > read-only and then disable the bridge entirely. > > > > > > Signed-off-by: Patrick Venture > > > --- > > > drivers/misc/Kconfig | 8 + > > > drivers/misc/Makefile| 1 + > > > drivers/misc/aspeed-p2a-ctrl.c | 498 +++ > > > include/uapi/linux/aspeed-p2a-ctrl.h | 46 +++ > > > 4 files changed, 553 insertions(+) > > > create mode 100644 drivers/misc/aspeed-p2a-ctrl.c > > > create mode 100644 include/uapi/linux/aspeed-p2a-ctrl.h > > > > > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig > > > index f417b06e11c5..54ed265a26f0 100644 > > > --- a/drivers/misc/Kconfig > > > +++ b/drivers/misc/Kconfig > > > @@ -485,6 +485,14 @@ config VEXPRESS_SYSCFG > > > bus. System Configuration interface is one of the possible means > > > of generating transactions on this bus. > > > > > > +config ASPEED_P2A_CTRL > > > + depends on (ARCH_ASPEED || COMPILE_TEST) > > > + tristate "Aspeed ast2400/2500 HOST P2A VGA MMIO to BMC bridge > > > control" > > > + help > > > + Control Aspeed ast2400/2500 HOST P2A VGA MMIO to BMC mappings > > > through > > > + ioctl()s, the driver also provides an interface for userspace > > > mappings to > > > + a pre-defined region. > > > + > > > config ASPEED_LPC_CTRL > > > depends on (ARCH_ASPEED || COMPILE_TEST) && REGMAP && MFD_SYSCON > > > tristate "Aspeed ast2400/2500 HOST LPC to BMC bridge control" > > > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile > > > index e39ccbbc1b3a..57577aee354f 100644 > > > --- a/drivers/misc/Makefile > > > +++ b/drivers/misc/Makefile > > > @@ -55,6 +55,7 @@ obj-$(CONFIG_VEXPRESS_SYSCFG) += vexpress-syscfg.o > > > obj-$(CONFIG_CXL_BASE) += cxl/ > > > obj-$(CONFIG_ASPEED_LPC_CTRL)+= aspeed-lpc-ctrl.o > > > obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o > > > +obj-$(CONFIG_ASPEED_P2A_CTRL)+= aspeed-p2a-ctrl.o > > > obj-$(CONFIG_PCI_ENDPOINT_TEST) += pci_endpoint_test.o > > > obj-$(CONFIG_OCXL) += ocxl/ > > > obj-y+= cardreader/ > > > diff --git a/drivers/misc/aspeed-p2a-ctrl.c > > > b/drivers/misc/aspeed-p2a-ctrl.c > > > new file mode 100644 > > > index ..a3cf00de9038 > > > --- /dev/null > > > +++ b/drivers/misc/aspeed-p2a-ctrl.c > > > @@ -0,0 +1,498 @@ > > > +/* > > > + * Copyright 2019 Google Inc > > > + * > > > + * This program is free software; you can redistribute it and/or > > > + * modify it under the terms of the GNU General Public Lic
Re: [PATCH 2/2] drivers/misc: Add Aspeed P2A control driver
On Sun, Feb 24, 2019 at 5:26 PM Andrew Jeffery wrote: > > On Fri, 22 Feb 2019, at 08:55, Patrick Venture wrote: > > The ASPEED AST2400, and AST2500 in some configurations include a > > PCI-to-AHB MMIO bridge. This bridge allows a server to read and write > > in the BMC's memory space. > > Bit of a nit, but I think s/memory space/physical address space/ makes > the power of the interface a bit clearer. > > > This feature is especially useful when using > > this bridge to send large files to the BMC. > > > > The host may use this to send down a firmware image by staging data at a > > specific memory address, and in a coordinated effort with the BMC's > > software stack and kernel, transmit the bytes. > > > > This driver enables the BMC to unlock the PCI bridge on demand, and > > configure it via ioctl to allow the host to write bytes to an agreed > > upon location. In the primary use-case, the region to use is known > > apriori on the BMC, and the host requests this information. Once this > > request is received, the BMC's software stack will enable the bridge and > > the region and then using some software flow control (possibly via IPMI > > packets), copy the bytes down. Once the process is complete, the BMC > > will disable the bridge and unset any region involved. > > I feel the description is a little subtle. You mention locations and regions > without really defining their relationship. We have the means to prevent > writes via the P2A to following regions in the BMC's physical address space: > > * BMC flash MMIO window > * System flash MMIO windows > * SOC IO (peripheral MMIO) > * DRAM > > So what I think should be made clear is once we allow the host to write > to e.g. DRAM, it can write to *all* of DRAM, regardless of what location the > BMC recommended, i.e. the BMC is at the mercy of the host wrt > confidentiality once the P2A is enabled (host can always read anywhere) > and integrity when the DRAM write filter is disabled. Ok, I can try to work that phrasing in. > > There is no way to specify and constrain P2A writes to specific locations > in DRAM. > > > > > The default behavior of this bridge when present is: enabled and all > > regions marked read-write. This driver will fix the regions to be > > read-only and then disable the bridge entirely. > > > > Signed-off-by: Patrick Venture > > --- > > drivers/misc/Kconfig | 8 + > > drivers/misc/Makefile| 1 + > > drivers/misc/aspeed-p2a-ctrl.c | 498 +++ > > include/uapi/linux/aspeed-p2a-ctrl.h | 46 +++ > > 4 files changed, 553 insertions(+) > > create mode 100644 drivers/misc/aspeed-p2a-ctrl.c > > create mode 100644 include/uapi/linux/aspeed-p2a-ctrl.h > > > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig > > index f417b06e11c5..54ed265a26f0 100644 > > --- a/drivers/misc/Kconfig > > +++ b/drivers/misc/Kconfig > > @@ -485,6 +485,14 @@ config VEXPRESS_SYSCFG > > bus. System Configuration interface is one of the possible means > > of generating transactions on this bus. > > > > +config ASPEED_P2A_CTRL > > + depends on (ARCH_ASPEED || COMPILE_TEST) > > + tristate "Aspeed ast2400/2500 HOST P2A VGA MMIO to BMC bridge control" > > + help > > + Control Aspeed ast2400/2500 HOST P2A VGA MMIO to BMC mappings > > through > > + ioctl()s, the driver also provides an interface for userspace > > mappings to > > + a pre-defined region. > > + > > config ASPEED_LPC_CTRL > > depends on (ARCH_ASPEED || COMPILE_TEST) && REGMAP && MFD_SYSCON > > tristate "Aspeed ast2400/2500 HOST LPC to BMC bridge control" > > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile > > index e39ccbbc1b3a..57577aee354f 100644 > > --- a/drivers/misc/Makefile > > +++ b/drivers/misc/Makefile > > @@ -55,6 +55,7 @@ obj-$(CONFIG_VEXPRESS_SYSCFG) += vexpress-syscfg.o > > obj-$(CONFIG_CXL_BASE) += cxl/ > > obj-$(CONFIG_ASPEED_LPC_CTRL)+= aspeed-lpc-ctrl.o > > obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o > > +obj-$(CONFIG_ASPEED_P2A_CTRL)+= aspeed-p2a-ctrl.o > > obj-$(CONFIG_PCI_ENDPOINT_TEST) += pci_endpoint_test.o > > obj-$(CONFIG_OCXL) += ocxl/ > > obj-y+= cardreader/ > > diff --git a/drivers/misc/aspeed-p2a-ctrl.c > > b/drivers/misc/aspeed-p2a-ctrl.c > > new file mode 100644 > > index ..a3cf00de9038 > > --- /dev/null > > +++ b/drivers/misc/aspeed-p2a-ctrl.c > > @@ -0,0 +1,498 @@ > > +/* > > + * Copyright 2019 Google Inc > > + * > > + * This program is free software; you can redistribute it and/or > > + * modify it under the terms of the GNU General Public License > > + * as published by the Free Software Foundation; either version > > + * 2 of the License, or (at your option) any later version. > > + * > > + * Provides a simple driver to control the ASPEED P2A interface which > > allows > > + * the host to read and write to variou
Re: [PATCH 2/2] drivers/misc: Add Aspeed P2A control driver
On Fri, 22 Feb 2019, at 08:55, Patrick Venture wrote: > The ASPEED AST2400, and AST2500 in some configurations include a > PCI-to-AHB MMIO bridge. This bridge allows a server to read and write > in the BMC's memory space. Bit of a nit, but I think s/memory space/physical address space/ makes the power of the interface a bit clearer. > This feature is especially useful when using > this bridge to send large files to the BMC. > > The host may use this to send down a firmware image by staging data at a > specific memory address, and in a coordinated effort with the BMC's > software stack and kernel, transmit the bytes. > > This driver enables the BMC to unlock the PCI bridge on demand, and > configure it via ioctl to allow the host to write bytes to an agreed > upon location. In the primary use-case, the region to use is known > apriori on the BMC, and the host requests this information. Once this > request is received, the BMC's software stack will enable the bridge and > the region and then using some software flow control (possibly via IPMI > packets), copy the bytes down. Once the process is complete, the BMC > will disable the bridge and unset any region involved. I feel the description is a little subtle. You mention locations and regions without really defining their relationship. We have the means to prevent writes via the P2A to following regions in the BMC's physical address space: * BMC flash MMIO window * System flash MMIO windows * SOC IO (peripheral MMIO) * DRAM So what I think should be made clear is once we allow the host to write to e.g. DRAM, it can write to *all* of DRAM, regardless of what location the BMC recommended, i.e. the BMC is at the mercy of the host wrt confidentiality once the P2A is enabled (host can always read anywhere) and integrity when the DRAM write filter is disabled. There is no way to specify and constrain P2A writes to specific locations in DRAM. > > The default behavior of this bridge when present is: enabled and all > regions marked read-write. This driver will fix the regions to be > read-only and then disable the bridge entirely. > > Signed-off-by: Patrick Venture > --- > drivers/misc/Kconfig | 8 + > drivers/misc/Makefile| 1 + > drivers/misc/aspeed-p2a-ctrl.c | 498 +++ > include/uapi/linux/aspeed-p2a-ctrl.h | 46 +++ > 4 files changed, 553 insertions(+) > create mode 100644 drivers/misc/aspeed-p2a-ctrl.c > create mode 100644 include/uapi/linux/aspeed-p2a-ctrl.h > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig > index f417b06e11c5..54ed265a26f0 100644 > --- a/drivers/misc/Kconfig > +++ b/drivers/misc/Kconfig > @@ -485,6 +485,14 @@ config VEXPRESS_SYSCFG > bus. System Configuration interface is one of the possible means > of generating transactions on this bus. > > +config ASPEED_P2A_CTRL > + depends on (ARCH_ASPEED || COMPILE_TEST) > + tristate "Aspeed ast2400/2500 HOST P2A VGA MMIO to BMC bridge control" > + help > + Control Aspeed ast2400/2500 HOST P2A VGA MMIO to BMC mappings > through > + ioctl()s, the driver also provides an interface for userspace > mappings to > + a pre-defined region. > + > config ASPEED_LPC_CTRL > depends on (ARCH_ASPEED || COMPILE_TEST) && REGMAP && MFD_SYSCON > tristate "Aspeed ast2400/2500 HOST LPC to BMC bridge control" > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile > index e39ccbbc1b3a..57577aee354f 100644 > --- a/drivers/misc/Makefile > +++ b/drivers/misc/Makefile > @@ -55,6 +55,7 @@ obj-$(CONFIG_VEXPRESS_SYSCFG) += vexpress-syscfg.o > obj-$(CONFIG_CXL_BASE) += cxl/ > obj-$(CONFIG_ASPEED_LPC_CTRL)+= aspeed-lpc-ctrl.o > obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o > +obj-$(CONFIG_ASPEED_P2A_CTRL)+= aspeed-p2a-ctrl.o > obj-$(CONFIG_PCI_ENDPOINT_TEST) += pci_endpoint_test.o > obj-$(CONFIG_OCXL) += ocxl/ > obj-y+= cardreader/ > diff --git a/drivers/misc/aspeed-p2a-ctrl.c > b/drivers/misc/aspeed-p2a-ctrl.c > new file mode 100644 > index ..a3cf00de9038 > --- /dev/null > +++ b/drivers/misc/aspeed-p2a-ctrl.c > @@ -0,0 +1,498 @@ > +/* > + * Copyright 2019 Google Inc > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * as published by the Free Software Foundation; either version > + * 2 of the License, or (at your option) any later version. > + * > + * Provides a simple driver to control the ASPEED P2A interface which > allows > + * the host to read and write to various regions of the BMC's memory. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +#define DEVICE_NAME "aspeed-p2a-ctrl" > + > +#define SCU180_ENP2A BIT(0) > + > +/* The
[PATCH 2/2] drivers/misc: Add Aspeed P2A control driver
The ASPEED AST2400, and AST2500 in some configurations include a PCI-to-AHB MMIO bridge. This bridge allows a server to read and write in the BMC's memory space. This feature is especially useful when using this bridge to send large files to the BMC. The host may use this to send down a firmware image by staging data at a specific memory address, and in a coordinated effort with the BMC's software stack and kernel, transmit the bytes. This driver enables the BMC to unlock the PCI bridge on demand, and configure it via ioctl to allow the host to write bytes to an agreed upon location. In the primary use-case, the region to use is known apriori on the BMC, and the host requests this information. Once this request is received, the BMC's software stack will enable the bridge and the region and then using some software flow control (possibly via IPMI packets), copy the bytes down. Once the process is complete, the BMC will disable the bridge and unset any region involved. The default behavior of this bridge when present is: enabled and all regions marked read-write. This driver will fix the regions to be read-only and then disable the bridge entirely. Signed-off-by: Patrick Venture --- drivers/misc/Kconfig | 8 + drivers/misc/Makefile| 1 + drivers/misc/aspeed-p2a-ctrl.c | 498 +++ include/uapi/linux/aspeed-p2a-ctrl.h | 46 +++ 4 files changed, 553 insertions(+) create mode 100644 drivers/misc/aspeed-p2a-ctrl.c create mode 100644 include/uapi/linux/aspeed-p2a-ctrl.h diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index f417b06e11c5..54ed265a26f0 100644 --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -485,6 +485,14 @@ config VEXPRESS_SYSCFG bus. System Configuration interface is one of the possible means of generating transactions on this bus. +config ASPEED_P2A_CTRL + depends on (ARCH_ASPEED || COMPILE_TEST) + tristate "Aspeed ast2400/2500 HOST P2A VGA MMIO to BMC bridge control" + help + Control Aspeed ast2400/2500 HOST P2A VGA MMIO to BMC mappings through + ioctl()s, the driver also provides an interface for userspace mappings to + a pre-defined region. + config ASPEED_LPC_CTRL depends on (ARCH_ASPEED || COMPILE_TEST) && REGMAP && MFD_SYSCON tristate "Aspeed ast2400/2500 HOST LPC to BMC bridge control" diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index e39ccbbc1b3a..57577aee354f 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -55,6 +55,7 @@ obj-$(CONFIG_VEXPRESS_SYSCFG) += vexpress-syscfg.o obj-$(CONFIG_CXL_BASE) += cxl/ obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o +obj-$(CONFIG_ASPEED_P2A_CTRL) += aspeed-p2a-ctrl.o obj-$(CONFIG_PCI_ENDPOINT_TEST)+= pci_endpoint_test.o obj-$(CONFIG_OCXL) += ocxl/ obj-y += cardreader/ diff --git a/drivers/misc/aspeed-p2a-ctrl.c b/drivers/misc/aspeed-p2a-ctrl.c new file mode 100644 index ..a3cf00de9038 --- /dev/null +++ b/drivers/misc/aspeed-p2a-ctrl.c @@ -0,0 +1,498 @@ +/* + * Copyright 2019 Google Inc + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version + * 2 of the License, or (at your option) any later version. + * + * Provides a simple driver to control the ASPEED P2A interface which allows + * the host to read and write to various regions of the BMC's memory. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#define DEVICE_NAME"aspeed-p2a-ctrl" + +#define SCU180_ENP2A BIT(0) + +/* The ast2400/2500 both have six ranges. */ +#define P2A_REGION_COUNT 6 + +struct region { + u32 min; + u32 max; + u32 bit; +}; + +struct aspeed_p2a_model_data { + /* min, max, bit */ + struct region regions[P2A_REGION_COUNT]; +}; + +struct aspeed_p2a_ctrl { + struct miscdevice miscdev; + + /* Lock access to the registers so they're is consistent. */ + struct mutex lo_mutex; + void __iomem *loregs; + struct mutex hi_mutex; + void __iomem *hiregs; + + const struct aspeed_p2a_model_data *config; + + /* Access to these needs to be locked, held via probe, mapping ioctl, +* and release, remove. +*/ + struct mutex tracking; + u32 readers; + u32 readerwriters[P2A_REGION_COUNT]; + + phys_addr_t mem_base; + resource_size_t mem_size; + /* Because the memory_region is optional, this tracks whether it was +* set. +*/ + bool region_specified; +}; + +static inline u32 aspeed_p2a_read(void __iomem *base) +{ + return readl((void *)base); +} + +static inline void aspeed_p2a_write(