On 19/11/2019 01:52, David Gibson wrote: > On Mon, Nov 18, 2019 at 10:22:22AM +0100, Cédric Le Goater wrote: >> The BMC of the OpenPOWER systems monitors the machine state using >> sensors, controls the power and controls the access to the PNOR flash >> device containing the firmware image required to boot the host. >> >> QEMU models the power cycle process, access to the sensors and access >> to the PNOR device. But, for these features to be available, the QEMU >> PowerNV machine needs two extras devices on the command line, an IPMI >> BT device for communication and a BMC backend device: >> >> -device ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10 >> >> The BMC properties are then defined accordingly in the device tree and >> OPAL self adapts. If a BMC device and an IPMI BT device are not >> available, OPAL does not try to communicate with the BMC in any >> manner. This is not how real systems behave. >> >> To be closer to the default behavior, create an IPMI BMC simulator >> device and an IPMI BT device at machine initialization time. We loose >> the ability to define an external BMC device but there are benefits: >> >> - a better match with real systems, >> - a better test coverage of the OPAL code, >> - system powerdown and reset commands that work, >> - a QEMU device tree compliant with the specifications (*). >> >> (*) Still needs a MBOX device. >> >> Signed-off-by: Cédric Le Goater <c...@kaod.org> > > This doesn't apply to ppc-for-5.0 for me. I'm not sure which change > in there it's conflicting with, but there seems to be something.
Sorry I should have been more precise. This is because we need an IPMI patch to be merged first in Corey's tree : ipmi: Add support to customize OEM functions http://patchwork.ozlabs.org/patch/1185187/ and another one merged in the PPC tree: ppc/pnv: Add HIOMAP commands http://patchwork.ozlabs.org/patch/1185185/ David, if Corey agrees, I think it would be simpler if you took them all. Thanks, C. > >> --- >> include/hw/ppc/pnv.h | 2 +- >> hw/ppc/pnv.c | 33 ++++++++++++++++----------------- >> hw/ppc/pnv_bmc.c | 20 +++++++++++++++++--- >> 3 files changed, 34 insertions(+), 21 deletions(-) >> >> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h >> index 07c56c05ad30..90f1343ed07c 100644 >> --- a/include/hw/ppc/pnv.h >> +++ b/include/hw/ppc/pnv.h >> @@ -198,7 +198,7 @@ static inline bool pnv_is_power9(PnvMachineState *pnv) >> */ >> void pnv_dt_bmc_sensors(IPMIBmc *bmc, void *fdt); >> void pnv_bmc_powerdown(IPMIBmc *bmc); >> -int pnv_bmc_hiomap(IPMIBmc *bmc); >> +IPMIBmc *pnv_bmc_create(void); >> >> /* >> * POWER8 MMIO base addresses >> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c >> index c3ac0d6d5b4a..2117d879895c 100644 >> --- a/hw/ppc/pnv.c >> +++ b/hw/ppc/pnv.c >> @@ -551,27 +551,10 @@ static void pnv_powerdown_notify(Notifier *n, void >> *opaque) >> >> static void pnv_reset(MachineState *machine) >> { >> - PnvMachineState *pnv = PNV_MACHINE(machine); >> void *fdt; >> - Object *obj; >> >> qemu_devices_reset(); >> >> - /* >> - * OpenPOWER systems have a BMC, which can be defined on the >> - * command line with: >> - * >> - * -device ipmi-bmc-sim,id=bmc0 >> - * >> - * This is the internal simulator but it could also be an external >> - * BMC. >> - */ >> - obj = object_resolve_path_type("", "ipmi-bmc-sim", NULL); >> - if (obj) { >> - pnv->bmc = IPMI_BMC(obj); >> - pnv_bmc_hiomap(pnv->bmc); >> - } >> - >> fdt = pnv_dt_create(machine); >> >> /* Pack resulting tree */ >> @@ -629,6 +612,16 @@ static bool pnv_match_cpu(const char *default_type, >> const char *cpu_type) >> return ppc_default->pvr_match(ppc_default, ppc->pvr); >> } >> >> +static void ipmi_bt_init(ISABus *bus, IPMIBmc *bmc, uint32_t irq) >> +{ >> + Object *obj; >> + >> + obj = OBJECT(isa_create(bus, "isa-ipmi-bt")); >> + object_property_set_link(obj, OBJECT(bmc), "bmc", &error_fatal); >> + object_property_set_int(obj, irq, "irq", &error_fatal); >> + object_property_set_bool(obj, true, "realized", &error_fatal); >> +} >> + >> static void pnv_init(MachineState *machine) >> { >> PnvMachineState *pnv = PNV_MACHINE(machine); >> @@ -751,6 +744,9 @@ static void pnv_init(MachineState *machine) >> } >> g_free(chip_typename); >> >> + /* Create the machine BMC simulator */ >> + pnv->bmc = pnv_bmc_create(); >> + >> /* Instantiate ISA bus on chip 0 */ >> pnv->isa_bus = pnv_isa_create(pnv->chips[0], &error_fatal); >> >> @@ -760,6 +756,9 @@ static void pnv_init(MachineState *machine) >> /* Create an RTC ISA device too */ >> mc146818_rtc_init(pnv->isa_bus, 2000, NULL); >> >> + /* Create the IPMI BT device for communication with the BMC */ >> + ipmi_bt_init(pnv->isa_bus, pnv->bmc, 10); >> + >> /* >> * OpenPOWER systems use a IPMI SEL Event message to notify the >> * host to powerdown >> diff --git a/hw/ppc/pnv_bmc.c b/hw/ppc/pnv_bmc.c >> index aa5c89586c63..07fa1e1c7e45 100644 >> --- a/hw/ppc/pnv_bmc.c >> +++ b/hw/ppc/pnv_bmc.c >> @@ -17,6 +17,8 @@ >> */ >> >> #include "qemu/osdep.h" >> +#include "qemu-common.h" >> +#include "qapi/error.h" >> #include "target/ppc/cpu.h" >> #include "qemu/log.h" >> #include "hw/ipmi/ipmi.h" >> @@ -211,8 +213,20 @@ static const IPMINetfn hiomap_netfn = { >> .cmd_handlers = hiomap_cmds >> }; >> >> -int pnv_bmc_hiomap(IPMIBmc *bmc) >> +/* >> + * Instantiate the machine BMC. PowerNV uses the QEMU internal >> + * simulator but it could also be external. >> + */ >> +IPMIBmc *pnv_bmc_create(void) >> { >> - return ipmi_sim_register_netfn(IPMI_BMC_SIMULATOR(bmc), >> - IPMI_NETFN_OEM, &hiomap_netfn); >> + Object *obj; >> + >> + obj = object_new(TYPE_IPMI_BMC_SIMULATOR); >> + object_property_set_bool(obj, true, "realized", &error_fatal); >> + >> + /* Install the HIOMAP protocol handlers to access the PNOR */ >> + ipmi_sim_register_netfn(IPMI_BMC_SIMULATOR(obj), IPMI_NETFN_OEM, >> + &hiomap_netfn); >> + >> + return IPMI_BMC(obj); >> } >
