[Skiboot] [PATCH v2 8/8] Add Swift platform
Alexey Kardashevskiy
aik at ozlabs.ru
Wed Jul 10 12:26:59 AEST 2019
On 09/07/2019 07:07, Reza Arbab wrote:
> Signed-off-by: Reza Arbab <arbab at linux.ibm.com>
No commit log at all?
What is that "swift"? How many/which model/DD cpus, npus, gpus, any
mention of any spec, anything here or in the swift.c header comment will do.
> ---
> platforms/astbmc/Makefile.inc | 2 +-
> platforms/astbmc/swift.c | 143 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 144 insertions(+), 1 deletion(-)
> create mode 100644 platforms/astbmc/swift.c
>
> diff --git a/platforms/astbmc/Makefile.inc b/platforms/astbmc/Makefile.inc
> index 5130275debce..65a8b46eae8e 100644
> --- a/platforms/astbmc/Makefile.inc
> +++ b/platforms/astbmc/Makefile.inc
> @@ -6,7 +6,7 @@ ASTBMC_OBJS = pnor.o common.o slots.o \
> garrison.o barreleye.o \
> witherspoon.o zaius.o romulus.o p9dsu.o \
> vesnin.o nicole.o \
> - talos.o
> + talos.o swift.o
>
> ASTBMC = $(PLATDIR)/astbmc/built-in.a
> $(ASTBMC): $(ASTBMC_OBJS:%=$(PLATDIR)/astbmc/%)
> diff --git a/platforms/astbmc/swift.c b/platforms/astbmc/swift.c
> new file mode 100644
> index 000000000000..78aa4e33cc76
> --- /dev/null
> +++ b/platforms/astbmc/swift.c
> @@ -0,0 +1,143 @@
> +/* Copyright 2019 IBM Corp.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> + * implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include <skiboot.h>
> +#include <ipmi.h>
> +#include <npu3.h>
> +#include "astbmc.h"
> +
> +static void swift_npu3_device_detect(struct npu3 *npu)
> +{
> + struct npu3_dev *dev;
> + uint32_t node, gpu_index;
> + char slot[6];
> +
> + node = P9_GCID2NODEID(npu->chip_id);
> +
> + switch (npu->index) {
> + case 0:
> + gpu_index = node * 2 + 1;
> + break;
> + case 2:
> + gpu_index = node * 2;
> + break;
> + default:
> + return;
> + }
> +
> + snprintf(slot, sizeof(slot), "GPU%d", gpu_index);
> +
> + npu3_for_each_dev(dev, npu) {
> + dev->type = NPU3_DEV_TYPE_NVLINK;
> + dt_add_property_string(dev->dn, "ibm,slot-label", slot);
> + dt_add_property_u64(dev->dn, "ibm,link-speed", 25000000000ull);
> + dt_add_property_cells(dev->dn, "nvidia,link-speed", 9);
I have always been curious - what are these "25000000000ull" and "9" and
where do they come from and can we have them defined? Thanks,
> + }
> +}
> +
> +#define SWIFT_POSSIBLE_GPUS 4
> +
> +#define DN(g) devs[g]->nvlink.gpu->dn
This one does not seem very useful, can be open coded.
> +#define G(g) (devs[g] ? devs[g]->nvlink.gpu->dn->phandle : 0)
> +#define N(g) (devs[g] ? devs[g]->npu->nvlink.phb.dt_node->phandle : 0)
> +
> +#define add_peers_prop(g, p...) \
> + if (devs[g]) \
> + dt_add_property_cells(DN(g), "ibm,nvlink-peers", ##p)
> +
> +/* Add GPU interconnect properties to the dt */
> +static void swift_npu3_fixup(void)
> +{
> + struct npu3 *npu;
> + struct npu3_dev *dev;
> + struct npu3_dev *devs[SWIFT_POSSIBLE_GPUS] = {};
> + uint32_t index;
> +
> + if (npu3_chip_possible_gpus() != 2) {
> + prlog(PR_ERR, "NPU: Unknown link topology detected\n");
> + return;
> + }
> +
> + /* Collect the first link we find for each GPU */
> + npu3_for_each_nvlink_npu(npu) {
> + npu3_for_each_nvlink_dev(dev, npu) {
> + index = npu3_dev_gpu_index(dev);
> + if (index == -1 || index >= ARRAY_SIZE(devs))
> + continue;
> +
> + if (dev->nvlink.gpu && !devs[index])
> + devs[index] = dev;
> + }
> + }
> +
> + add_peers_prop(0, G(3), G(3),
> + G(2), G(2), G(2),
> + G(1), G(1), G(1),
> + N(0), N(0), N(0), N(0));
2, 3, 3, 4 items per line and the next one 1, 3, 3, 1, 4 - what is the
point in such grouping? There must be something about the code
readability but I just do not see it :) Also, this looks like some GPUs
only have 2 links between them and some have 3, is that correct?
> +
> + add_peers_prop(1, G(2),
> + G(3), G(3), G(3),
> + G(0), G(0), G(0),
> + G(2),
> + N(1), N(1), N(1), N(1));
> +
> + add_peers_prop(2, G(1),
> + G(3), G(3), G(3),
> + G(0), G(0), G(0),
> + G(1),
> + N(2), N(2), N(2), N(2));
> +
> + add_peers_prop(3, G(2), G(2), G(2),
> + G(1), G(1), G(1),
> + G(0), G(0),
> + N(3), N(3), N(3), N(3));
> +}
> +
> +static void swift_exit(void)
> +{
> + swift_npu3_fixup();
> + astbmc_exit();
> +}
> +
> +static bool swift_probe(void)
> +{
> + if (!dt_node_is_compatible(dt_root, "ibm,swift"))
> + return false;
> +
> + /* Lot of common early inits here */
> + astbmc_early_init();
> +
> + /* Setup UART for use by OPAL (Linux hvc) */
> + uart_set_console_policy(UART_CONSOLE_OPAL);
> +
> + return true;
> +}
> +
> +DECLARE_PLATFORM(swift) = {
> + .bmc = &bmc_plat_ast2500_openbmc,
> + .cec_power_down = astbmc_ipmi_power_down,
> + .cec_reboot = astbmc_ipmi_reboot,
> + .elog_commit = ipmi_elog_commit,
> + .exit = swift_exit,
> + .init = astbmc_init,
> + .name = "Swift",
> + .npu3_device_detect = swift_npu3_device_detect,
> + .pci_get_slot_info = dt_slot_get_slot_info,
> + .probe = swift_probe,
> + .resource_loaded = flash_resource_loaded,
> + .start_preload_resource = flash_start_preload_resource,
> + .terminate = ipmi_terminate,
> +};
>
--
Alexey
More information about the Skiboot
mailing list