| Message ID | 1447348987-2387-1-git-send-email-clg@fr.ibm.com |
|---|---|
| State | RFC |
| Headers | show |
On Thu, 2015-11-12 at 18:23 +0100, Cédric Le Goater wrote: > This adds the BT device used to communicate with the BMC. This is > sufficient to handle IPMI messaging, power_downs and reboots the > way openpower systems do it. > > This patch needs to run on Ben's tree adding the support for the > PowerNV platform in qemu, plus some patches of mine. You will find > the resulting tree here : > > https://github.com/legoater/qemu/tree/powernv > > It's basic, probably a little too hacky, but it does provide enough > of a framework to handle communication with the BMC. Is it something > we want to add to qemu ? Thanks ! There's actually a pretty complete IPMI stack with BT etc... being written by Corey Minyard. He submitted it a while back, it didn't go in but he told me he plans to resubmit reasonably soon: https://lists.gnu.org/archive/html/qemu-devel/2015-04/msg00641.html Cheers, Ben. > Comments welcome. > > Thanks, > > C. > > > Signed-off-by: Cédric Le Goater <clg@fr.ibm.com> > --- > hw/bt.c | 8 +++++++ > platforms/qemu/qemu.c | 53 > ++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 61 insertions(+) > > Index: skiboot.git/platforms/qemu/qemu.c > =================================================================== > --- skiboot.git.orig/platforms/qemu/qemu.c > +++ skiboot.git/platforms/qemu/qemu.c > @@ -21,6 +21,13 @@ > #include <console.h> > #include <opal.h> > #include <psi.h> > +#include <bt.h> > +#include <ipmi.h> > + > +/* BT config */ > +#define BT_IO_BASE 0xe4 > +#define BT_IO_COUNT 3 > +#define BT_LPC_IRQ 10 > > static void qemu_init(void) > { > @@ -32,6 +39,33 @@ static void qemu_init(void) > * chiptod_init() > */ > lpc_rtc_init(); > + bt_init(); > +} > + > +static void qemu_dt_fixup_bt(struct dt_node *lpc) > +{ > + struct dt_node *bt; > + char namebuf[32]; > + > + /* First check if the BT interface is already there */ > + dt_for_each_child(lpc, bt) { > + if (dt_node_is_compatible(bt, "bt")) > + return; > + } > + > + snprintf(namebuf, sizeof(namebuf), "ipmi-bt@i%x", > BT_IO_BASE); > + bt = dt_new(lpc, namebuf); > + > + dt_add_property_cells(bt, "reg", > + 1, /* IO space */ > + BT_IO_BASE, BT_IO_COUNT); > + dt_add_property_strings(bt, "compatible", "ipmi-bt"); > + > + /* Mark it as reserved to avoid Linux trying to claim it */ > + dt_add_property_strings(bt, "status", "reserved"); > + > + dt_add_property_cells(bt, "interrupts", BT_LPC_IRQ); > + dt_add_property_cells(bt, "interrupt-parent", lpc->phandle); > } > > static void qemu_dt_fixup_uart(struct dt_node *lpc) > @@ -113,6 +147,7 @@ static void qemu_dt_fixup(void) > > qemu_dt_fixup_rtc(primary_lpc); > qemu_dt_fixup_uart(primary_lpc); > + qemu_dt_fixup_bt(primary_lpc); > } > > static void qemu_ext_irq_serirq_cpld(unsigned int chip_id) > @@ -120,6 +155,21 @@ static void qemu_ext_irq_serirq_cpld(uns > lpc_all_interrupts(chip_id); > } > > +static int64_t qemu_ipmi_power_down(uint64_t request) > +{ > + if (request != IPMI_CHASSIS_PWR_DOWN) { > + prlog(PR_WARNING, "PLAT: unexpected shutdown request > %llx\n", > + request); > + } > + > + return ipmi_chassis_control(request); > +} > + > +static int64_t qemu_ipmi_reboot(void) > +{ > + return ipmi_chassis_control(IPMI_CHASSIS_HARD_RESET); > +} > + > static bool qemu_probe(void) > { > if (!dt_node_is_compatible(dt_root, "qemu,powernv")) > @@ -145,4 +195,7 @@ DECLARE_PLATFORM(qemu) = { > .probe = qemu_probe, > .init = qemu_init, > .external_irq = qemu_ext_irq_serirq_cpld, > + .cec_power_down = qemu_ipmi_power_down, > + .cec_reboot = qemu_ipmi_reboot, > + .terminate = ipmi_terminate, > };
On 11/12/2015 11:24 PM, Benjamin Herrenschmidt wrote: > On Thu, 2015-11-12 at 18:23 +0100, Cédric Le Goater wrote: >> This adds the BT device used to communicate with the BMC. This is >> sufficient to handle IPMI messaging, power_downs and reboots the >> way openpower systems do it. >> >> This patch needs to run on Ben's tree adding the support for the >> PowerNV platform in qemu, plus some patches of mine. You will find >> the resulting tree here : >> >> https://github.com/legoater/qemu/tree/powernv >> >> It's basic, probably a little too hacky, but it does provide enough >> of a framework to handle communication with the BMC. Is it something >> we want to add to qemu ? > > Thanks ! > > There's actually a pretty complete IPMI stack with BT etc... being > written by Corey Minyard. He submitted it a while back, it didn't go in > but he told me he plans to resubmit reasonably soon: > > https://lists.gnu.org/archive/html/qemu-devel/2015-04/msg00641.html That looks pretty mature. Impressive. So I did some cookery today. I merged his patches and yours in this tree : https://github.com/legoater/qemu/tree/powernv-ipmi Added the missing IPMI commands for pnv and the results looks promising. Just run a qemu with these devices : -device ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10 and skiboot/Linux will run fine. I did not port the SMS_ATN part. It should not be a big deal as all the framework is already in Corey's code. As for my little BT thingy, I guess it is born obsolete. But it is small and does the job. May be we can keep it around for testing purpose until Corey merges his patchset ? Dunno, you tell me. I can put some efforts on both tree, Corey's tree and mine. It shouldn't be a problem. Thanks, C.
On Fri, 2015-11-13 at 18:33 +0100, Cédric Le Goater wrote: > As for my little BT thingy, I guess it is born obsolete. But it is small > and does the job. May be we can keep it around for testing purpose until > Corey merges his patchset ? Dunno, you tell me. I can put some efforts > on both tree, Corey's tree and mine. It shouldn't be a problem. Right, I've been wondering. Maybe we can put your little thing in my tree without actually trying to merge it upstream... I don't know when exactly Corey is going to submit his series again, when he does, we should voice public support for it. Cheers, Ben.
On 11/13/2015 10:11 PM, Benjamin Herrenschmidt wrote: > On Fri, 2015-11-13 at 18:33 +0100, Cédric Le Goater wrote: >> As for my little BT thingy, I guess it is born obsolete. But it is small >> and does the job. May be we can keep it around for testing purpose until >> Corey merges his patchset ? Dunno, you tell me. I can put some efforts >> on both tree, Corey's tree and mine. It shouldn't be a problem. > > Right, I've been wondering. Maybe we can put your little thing in my > tree without actually trying to merge it upstream... > > I don't know when exactly Corey is going to submit his series again, > when he does, we should voice public support for it. which he did two days ago : http://lists.nongnu.org/archive/html/qemu-devel/2015-11/msg03153.html I missed that. Let's support it then. I will advertise the merge I did and ask a few questions on how we should populate the device tree. Thanks, C.
On Sat, 2015-11-14 at 17:23 +0100, Cédric Le Goater wrote: > which he did two days ago : > > http://lists.nongnu.org/archive/html/qemu-devel/2015-11/msg03153.html > > I missed that. Let's support it then. I will advertise the merge I did > and ask a few questions on how we should populate the device tree. We can probably populate it from OPAL. Unless we start to add optional DT hooks for ISADevice ... In that case, we could populate the UART too. Cheers, Ben.
On 11/15/2015 07:13 AM, Benjamin Herrenschmidt wrote: > On Sat, 2015-11-14 at 17:23 +0100, Cédric Le Goater wrote: >> which he did two days ago : >> >> http://lists.nongnu.org/archive/html/qemu-devel/2015-11/msg03153.html >> >> I missed that. Let's support it then. I will advertise the merge I did >> and ask a few questions on how we should populate the device tree. > > We can probably populate it from OPAL. Unless we start to add optional DT > hooks for ISADevice ... In that case, we could populate the UART too. I do not really know where to send patches (qemu-devel@). So, here is a try on top of your branch : https://github.com/legoater/qemu/commits/powernv-benh It looks for isa devices, static and dynamic like the ipmi-bt will be, and populates the device tree as skiboot would do it. The skiboot qemu platform needs a couple of fixes to survive, such as : @@ -53,6 +156,14 @@ static void qemu_dt_fixup_uart(struct dt #define UART_IO_COUNT 8 #define UART_LPC_IRQ 4 + /* First check if the BT interface is already there */ + dt_for_each_child(lpc, uart) { + if (dt_node_is_compatible(uart, "pnpPNP,501")) { + prlog(PR_WARNING, "QEMU: uart device already here\n"); + return; + } + } + snprintf(namebuf, sizeof(namebuf), "serial@i%x", UART_IO_BASE); uart = dt_new(lpc, namebuf); @@ -84,6 +195,14 @@ static void qemu_dt_fixup_rtc(struct dt_ struct dt_node *rtc; char namebuf[32]; + /* First check if the BT interface is already there */ + dt_for_each_child(lpc, rtc) { + if (dt_node_is_compatible(rtc, "pnpPNP,b00")) { + prlog(PR_WARNING, "QEMU: rtc device already here\n"); + return; + } + } + /* * Follows the structure expected by the kernel file * arch/powerpc/sysdev/rtc_cmos_setup.c The rest should be fine. Cheers, C.
Benjamin Herrenschmidt <benh@kernel.crashing.org> writes: > On Sat, 2015-11-14 at 17:23 +0100, Cédric Le Goater wrote: >> which he did two days ago : >> >> http://lists.nongnu.org/archive/html/qemu-devel/2015-11/msg03153.html >> >> I missed that. Let's support it then. I will advertise the merge I did >> and ask a few questions on how we should populate the device tree. > > We can probably populate it from OPAL. Unless we start to add optional DT > hooks for ISADevice ... In that case, we could populate the UART too. Always having BT in qemu is probably a good idea anyway, solves a bunch of little problems (e.g. shutdown/reboot)
On 01/08/2016 07:06 AM, Stewart Smith wrote: > Benjamin Herrenschmidt <benh@kernel.crashing.org> writes: >> On Sat, 2015-11-14 at 17:23 +0100, Cédric Le Goater wrote: >>> which he did two days ago : >>> >>> http://lists.nongnu.org/archive/html/qemu-devel/2015-11/msg03153.html >>> >>> I missed that. Let's support it then. I will advertise the merge I did >>> and ask a few questions on how we should populate the device tree. >> >> We can probably populate it from OPAL. Unless we start to add optional DT >> hooks for ISADevice ... In that case, we could populate the UART too. > > Always having BT in qemu is probably a good idea anyway, solves a bunch > of little problems (e.g. shutdown/reboot) Well, the current BT device in qemu can be configured on the command line, it also has a configurable irq number : -device ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10 So, it seems safer to let qemu populate the device tree depending on its qom-tree than allow skiboot to hardcode non existing devices. C.
On Mon, 2016-01-11 at 10:50 +0100, Cédric Le Goater wrote: > Well, the current BT device in qemu can be configured on the command > line, it also has a configurable irq number : > > -device ipmi-bmc-sim,id=bmc0 -device isa-ipmi- > bt,bmc=bmc0,irq=10 > > So, it seems safer to let qemu populate the device tree depending > on its qom-tree than allow skiboot to hardcode non existing devices. We could also have the powernv platform instanciate BT by default. Cheers, Ben.
On 01/11/2016 11:11 AM, Benjamin Herrenschmidt wrote: > On Mon, 2016-01-11 at 10:50 +0100, Cédric Le Goater wrote: >> Well, the current BT device in qemu can be configured on the command >> line, it also has a configurable irq number : >> >> -device ipmi-bmc-sim,id=bmc0 -device isa-ipmi- >> bt,bmc=bmc0,irq=10 >> >> So, it seems safer to let qemu populate the device tree depending >> on its qom-tree than allow skiboot to hardcode non existing devices. > > We could also have the powernv platform instanciate BT by default. yes. I gave it a try last year and add a hard time to dissociate the BT device from the backend, that can be the simulator or an external BMC. I will spend some more energy to figure out why the QOM layer was complaining. Do we have plans for a FSP simulator ? Anyhow a hardcoded BT should not be a major obstacle. Cheers, C.
Hi Cedric, > Do we have plans for a FSP simulator ? Anyhow a hardcoded BT should not > be a major obstacle. We're doing a little work to add support to qemu for the AST2400 platform, which is used in a few OpenPOWER implementations. On top of that, it's possible we could run something like OpenBMC. https://github.com/amboar/qemu/commits/ast2400 Once we're able to connect both sides of the BT interface, and then we'd have a nice way of testing... Cheers, Jeremy
Cédric Le Goater <clg@fr.ibm.com> writes: > On 01/11/2016 11:11 AM, Benjamin Herrenschmidt wrote: >> On Mon, 2016-01-11 at 10:50 +0100, Cédric Le Goater wrote: >>> Well, the current BT device in qemu can be configured on the command >>> line, it also has a configurable irq number : >>> >>> -device ipmi-bmc-sim,id=bmc0 -device isa-ipmi- >>> bt,bmc=bmc0,irq=10 >>> >>> So, it seems safer to let qemu populate the device tree depending >>> on its qom-tree than allow skiboot to hardcode non existing devices. >> >> We could also have the powernv platform instanciate BT by default. +1 > yes. I gave it a try last year and add a hard time to dissociate the BT > device from the backend, that can be the simulator or an external BMC. > I will spend some more energy to figure out why the QOM layer was > complaining. External BMC emulator (i.e. different QEMU process) is certainly something we'd want to do - and that'd help the openbmc folks. > > Do we have plans for a FSP simulator ? Anyhow a hardcoded BT should not > be a major obstacle. I certainly have no plans for an FSP simulator.... and I'm not sure it would be worth the effort, that protocol is kind of a bit crazy on our side, let alone what we'd have to implement to emulate FSP side of it... Is there a simple/sane way to have a device there by default but be able to be disabled if one is so inclined?
On 01/12/2016 04:52 AM, Stewart Smith wrote: > Cédric Le Goater <clg@fr.ibm.com> writes: > >> On 01/11/2016 11:11 AM, Benjamin Herrenschmidt wrote: >>> On Mon, 2016-01-11 at 10:50 +0100, Cédric Le Goater wrote: >>>> Well, the current BT device in qemu can be configured on the command >>>> line, it also has a configurable irq number : >>>> >>>> -device ipmi-bmc-sim,id=bmc0 -device isa-ipmi- >>>> bt,bmc=bmc0,irq=10 >>>> >>>> So, it seems safer to let qemu populate the device tree depending >>>> on its qom-tree than allow skiboot to hardcode non existing devices. >>> >>> We could also have the powernv platform instanciate BT by default. > > +1 Well, we will need to find a way to "lazy bind" the BMC object (simulator or external) and the IPMI interface object (isa-ipmi-bt in our case). Currently, these are tightly coupled and you can not define the first on the command line and the second in the MachineClass ->init ops. That will crash qemu. Using the ->reset ops to work around this issue is a solution but it will not be welcomed by maintainers. They consider this method hideous. C. >> yes. I gave it a try last year and add a hard time to dissociate the BT >> device from the backend, that can be the simulator or an external BMC. >> I will spend some more energy to figure out why the QOM layer was >> complaining. > > External BMC emulator (i.e. different QEMU process) is certainly > something we'd want to do - and that'd help the openbmc folks. > >> >> Do we have plans for a FSP simulator ? Anyhow a hardcoded BT should not >> be a major obstacle. > > I certainly have no plans for an FSP simulator.... and I'm not sure it > would be worth the effort, that protocol is kind of a bit crazy on our > side, let alone what we'd have to implement to emulate FSP side of it... > > Is there a simple/sane way to have a device there by default but be able > to be disabled if one is so inclined?
Index: skiboot.git/platforms/qemu/qemu.c =================================================================== --- skiboot.git.orig/platforms/qemu/qemu.c +++ skiboot.git/platforms/qemu/qemu.c @@ -21,6 +21,13 @@ #include <console.h> #include <opal.h> #include <psi.h> +#include <bt.h> +#include <ipmi.h> + +/* BT config */ +#define BT_IO_BASE 0xe4 +#define BT_IO_COUNT 3 +#define BT_LPC_IRQ 10 static void qemu_init(void) { @@ -32,6 +39,33 @@ static void qemu_init(void) * chiptod_init() */ lpc_rtc_init(); + bt_init(); +} + +static void qemu_dt_fixup_bt(struct dt_node *lpc) +{ + struct dt_node *bt; + char namebuf[32]; + + /* First check if the BT interface is already there */ + dt_for_each_child(lpc, bt) { + if (dt_node_is_compatible(bt, "bt")) + return; + } + + snprintf(namebuf, sizeof(namebuf), "ipmi-bt@i%x", BT_IO_BASE); + bt = dt_new(lpc, namebuf); + + dt_add_property_cells(bt, "reg", + 1, /* IO space */ + BT_IO_BASE, BT_IO_COUNT); + dt_add_property_strings(bt, "compatible", "ipmi-bt"); + + /* Mark it as reserved to avoid Linux trying to claim it */ + dt_add_property_strings(bt, "status", "reserved"); + + dt_add_property_cells(bt, "interrupts", BT_LPC_IRQ); + dt_add_property_cells(bt, "interrupt-parent", lpc->phandle); } static void qemu_dt_fixup_uart(struct dt_node *lpc) @@ -113,6 +147,7 @@ static void qemu_dt_fixup(void) qemu_dt_fixup_rtc(primary_lpc); qemu_dt_fixup_uart(primary_lpc); + qemu_dt_fixup_bt(primary_lpc); } static void qemu_ext_irq_serirq_cpld(unsigned int chip_id) @@ -120,6 +155,21 @@ static void qemu_ext_irq_serirq_cpld(uns lpc_all_interrupts(chip_id); } +static int64_t qemu_ipmi_power_down(uint64_t request) +{ + if (request != IPMI_CHASSIS_PWR_DOWN) { + prlog(PR_WARNING, "PLAT: unexpected shutdown request %llx\n", + request); + } + + return ipmi_chassis_control(request); +} + +static int64_t qemu_ipmi_reboot(void) +{ + return ipmi_chassis_control(IPMI_CHASSIS_HARD_RESET); +} + static bool qemu_probe(void) { if (!dt_node_is_compatible(dt_root, "qemu,powernv")) @@ -145,4 +195,7 @@ DECLARE_PLATFORM(qemu) = { .probe = qemu_probe, .init = qemu_init, .external_irq = qemu_ext_irq_serirq_cpld, + .cec_power_down = qemu_ipmi_power_down, + .cec_reboot = qemu_ipmi_reboot, + .terminate = ipmi_terminate, };
This adds the BT device used to communicate with the BMC. This is sufficient to handle IPMI messaging, power_downs and reboots the way openpower systems do it. This patch needs to run on Ben's tree adding the support for the PowerNV platform in qemu, plus some patches of mine. You will find the resulting tree here : https://github.com/legoater/qemu/tree/powernv It's basic, probably a little too hacky, but it does provide enough of a framework to handle communication with the BMC. Is it something we want to add to qemu ? Comments welcome. Thanks, C. Signed-off-by: Cédric Le Goater <clg@fr.ibm.com> --- hw/bt.c | 8 +++++++ platforms/qemu/qemu.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+)