Re: [PATCH 2/2] drivers/misc: Add Aspeed P2A control driver

2019-02-27 Thread Andrew Jeffery
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

2019-02-27 Thread Patrick Venture
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

2019-02-27 Thread Patrick Venture
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

2019-02-26 Thread Andrew Jeffery



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

2019-02-26 Thread Florian Fainelli



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

2019-02-26 Thread Patrick Venture
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

2019-02-26 Thread Andrew Jeffery



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

2019-02-26 Thread Patrick Venture
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

2019-02-26 Thread Andrew Jeffery



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

2019-02-26 Thread Patrick Venture
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

2019-02-26 Thread Patrick Venture
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

2019-02-24 Thread Andrew Jeffery
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

2019-02-21 Thread Patrick Venture
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(