[Skiboot] [PATCH v2 2/2] pci/quirk: Populate device tree for AST2400 VGA

[Joel Stanley]

On Fri, Feb 24, 2017 at 3:33 PM, Russell Currey <ruscur at russell.cc> wrote:
> Adding these properties enables the kernel to function in the same way
> that it would if it could no longer access BMC configuration registers
> through a backdoor, which may become the default in future.
>
> The comments describe how isolating the host from the BMC could be
> achieved in skiboot, assuming all kernels that the system boots
> support this.  Isolating the BMC and the host from each other is
> important if they are owned by different parties; for example, a cloud
> provider renting machines "bare metal".
>
> Also add a "aspeed,must-init-vga" property to indicate that the VGA
> portion of the chip hasn't been initialized by a VBIOS and needs
> to be cleaned up first.
>
> Signed-off-by: Russell Currey <ruscur at russell.cc>
> Signed-off-by: Benjamin Herrenschmidt <benh at kernel.crashing.org>
> ---
> v2 [ruscur]: Drop dt_node from function and rename "ast,..." to "aspeed,..."
> ---
>  core/pci-quirk.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/ast.h    |  7 +++++++
>  2 files changed, 55 insertions(+)
>
> diff --git a/core/pci-quirk.c b/core/pci-quirk.c
> index 93f613eb..7e3f1a6a 100644
> --- a/core/pci-quirk.c
> +++ b/core/pci-quirk.c
> @@ -19,8 +19,56 @@
>  #include <pci-quirk.h>
>  #include <ast.h>
>
> +static void quirk_astbmc_vga(struct phb *phb __unused,
> +                            struct pci_device *pd)
> +{
> +       struct dt_node *np = pd->dn;
> +       uint32_t revision, mcr_configuration, mcr_scu_mpll, mcr_scu_strap;
> +
> +       /*
> +        * These accesses will only work if the BMC address 0x1E6E2180 is set
> +        * to 0x7B, which is its default state on current systems.  In future,
> +        * for security purposes it is proposed to configure this register to
> +        * disallow accesses from the host, and provide the properties that
> +        * the Linux ast VGA driver used through the device tree instead.
> +        * Here we set those properties so we can test how things would work
> +        * if the window into BMC memory was closed.
> +        *
> +        * If both the petitboot kernel and the host kernel have an ast driver
> +        * that reads properties from the device tree, setting 0x1E6E2180 to
> +        * 0x79 will disable the backdoor into BMC memory and the only way the
> +        * ast driver can operate is using the device tree properties.
> +        */
> +
> +       revision = ast_ahb_readl(SCU_REVISION_ID);
> +       mcr_configuration = ast_ahb_readl(MCR_CONFIGURATION);
> +       mcr_scu_mpll = ast_ahb_readl(MCR_SCU_MPLL);
> +       mcr_scu_strap = ast_ahb_readl(MCR_SCU_STRAP);
> +       dt_add_property_cells(np, "aspeed,scu-revision-id", revision);
> +       dt_add_property_cells(np, "aspeed,mcr-configuration", mcr_configuration);
> +       dt_add_property_cells(np, "aspeed,mcr-scu-mpll", mcr_scu_mpll);
> +       dt_add_property_cells(np, "aspeed,mcr-scu-strap", mcr_scu_strap);
> +       dt_add_property(np, "aspeed,must-init-vga", NULL, 0);

The kernel doesn't use that one. I assume it will be used in the future?

The change looks good.

Acked-by: Joel Stanley <joel at jms.id.au>

Cheers,

Joel