| Message ID | 20191021131215.3693-3-clg@kaod.org |
|---|---|
| State | New |
| Headers | show |
| Series | ppc/pnv: Add PNOR support | expand |
On Mon, Oct 21, 2019 at 03:12:12PM +0200, Cédric Le Goater wrote: > The routine ipmi_register_oem_netfn() lets external modules register > command handlers for OEM functions. Required for the PowerNV machine. Comments inline. > > Cc: Corey Minyard <cminyard@mvista.com> > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > include/hw/ipmi/ipmi.h | 36 ++++++++++++++++++++++++++++++++++++ > hw/ipmi/ipmi_bmc_sim.c | 41 ++++++----------------------------------- > 2 files changed, 42 insertions(+), 35 deletions(-) > > diff --git a/include/hw/ipmi/ipmi.h b/include/hw/ipmi/ipmi.h > index 6f2413b39b4a..cb7203b06767 100644 > --- a/include/hw/ipmi/ipmi.h > +++ b/include/hw/ipmi/ipmi.h > @@ -265,4 +265,40 @@ int ipmi_bmc_sdr_find(IPMIBmc *b, uint16_t recid, > const struct ipmi_sdr_compact **sdr, uint16_t *nextrec); > void ipmi_bmc_gen_event(IPMIBmc *b, uint8_t *evt, bool log); > > +typedef struct IPMIBmcSim IPMIBmcSim; This type isn't very useful outside of the simulator, but changes for that can come as they are needed. I don't see an easy way to avoid putting it here. > + > +typedef struct RspBuffer { > + uint8_t buffer[MAX_IPMI_MSG_SIZE]; > + unsigned int len; > +} RspBuffer; > + > +static inline void rsp_buffer_set_error(RspBuffer *rsp, uint8_t byte) > +{ > + rsp->buffer[2] = byte; > +} > + > +/* Add a byte to the response. */ > +static inline void rsp_buffer_push(RspBuffer *rsp, uint8_t byte) > +{ > + if (rsp->len >= sizeof(rsp->buffer)) { > + rsp_buffer_set_error(rsp, IPMI_CC_REQUEST_DATA_TRUNCATED); > + return; > + } > + rsp->buffer[rsp->len++] = byte; > +} > + > +typedef struct IPMICmdHandler { > + void (*cmd_handler)(IPMIBmcSim *s, > + uint8_t *cmd, unsigned int cmd_len, > + RspBuffer *rsp); > + unsigned int cmd_len_min; > +} IPMICmdHandler; > + > +typedef struct IPMINetfn { > + unsigned int cmd_nums; > + const IPMICmdHandler *cmd_handlers; > +} IPMINetfn; > + > +int ipmi_register_oem_netfn(IPMIBmc *b, const IPMINetfn *netfnd); > + > #endif > diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c > index 71e56f3b13d1..770aace55b08 100644 > --- a/hw/ipmi/ipmi_bmc_sim.c > +++ b/hw/ipmi/ipmi_bmc_sim.c > @@ -98,6 +98,7 @@ > #define IPMI_CMD_GET_SEL_TIME 0x48 > #define IPMI_CMD_SET_SEL_TIME 0x49 > > +#define IPMI_NETFN_OEM 0x3a > > /* Same as a timespec struct. */ > struct ipmi_time { > @@ -167,23 +168,8 @@ typedef struct IPMISensor { > #define MAX_SENSORS 20 > #define IPMI_WATCHDOG_SENSOR 0 > > -typedef struct IPMIBmcSim IPMIBmcSim; > -typedef struct RspBuffer RspBuffer; > - > #define MAX_NETFNS 64 > > -typedef struct IPMICmdHandler { > - void (*cmd_handler)(IPMIBmcSim *s, > - uint8_t *cmd, unsigned int cmd_len, > - RspBuffer *rsp); > - unsigned int cmd_len_min; > -} IPMICmdHandler; > - > -typedef struct IPMINetfn { > - unsigned int cmd_nums; > - const IPMICmdHandler *cmd_handlers; > -} IPMINetfn; > - > typedef struct IPMIRcvBufEntry { > QTAILQ_ENTRY(IPMIRcvBufEntry) entry; > uint8_t len; > @@ -279,28 +265,8 @@ struct IPMIBmcSim { > #define IPMI_BMC_WATCHDOG_ACTION_POWER_DOWN 2 > #define IPMI_BMC_WATCHDOG_ACTION_POWER_CYCLE 3 > > -struct RspBuffer { > - uint8_t buffer[MAX_IPMI_MSG_SIZE]; > - unsigned int len; > -}; > - > #define RSP_BUFFER_INITIALIZER { } > > -static inline void rsp_buffer_set_error(RspBuffer *rsp, uint8_t byte) > -{ > - rsp->buffer[2] = byte; > -} > - > -/* Add a byte to the response. */ > -static inline void rsp_buffer_push(RspBuffer *rsp, uint8_t byte) > -{ > - if (rsp->len >= sizeof(rsp->buffer)) { > - rsp_buffer_set_error(rsp, IPMI_CC_REQUEST_DATA_TRUNCATED); > - return; > - } > - rsp->buffer[rsp->len++] = byte; > -} > - > static inline void rsp_buffer_pushmore(RspBuffer *rsp, uint8_t *bytes, > unsigned int n) > { > @@ -640,6 +606,11 @@ static int ipmi_register_netfn(IPMIBmcSim *s, unsigned int netfn, > return 0; > } > > +int ipmi_register_oem_netfn(IPMIBmc *b, const IPMINetfn *netfnd) > +{ > + return ipmi_register_netfn(IPMI_BMC_SIMULATOR(b), IPMI_NETFN_OEM, netfnd); > +} I think I would prefer just exposing ipmi_register_netfn() and maybe rename it ipmi_sim_register_netfn() or something like that. There may be other netfns needed in the future. But with that change, this looks good to me: Reviewed-by: Corey Minyard <cminyard@mvista.com> > + > static const IPMICmdHandler *ipmi_get_handler(IPMIBmcSim *ibs, > unsigned int netfn, > unsigned int cmd) > -- > 2.21.0 >
On Mon, Oct 21, 2019 at 09:30:17AM -0500, Corey Minyard wrote: > On Mon, Oct 21, 2019 at 03:12:12PM +0200, Cédric Le Goater wrote: > > The routine ipmi_register_oem_netfn() lets external modules register > > command handlers for OEM functions. Required for the PowerNV machine. > > Comments inline. > > > > > Cc: Corey Minyard <cminyard@mvista.com> > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > > --- > > include/hw/ipmi/ipmi.h | 36 ++++++++++++++++++++++++++++++++++++ > > hw/ipmi/ipmi_bmc_sim.c | 41 ++++++----------------------------------- > > 2 files changed, 42 insertions(+), 35 deletions(-) > > > > diff --git a/include/hw/ipmi/ipmi.h b/include/hw/ipmi/ipmi.h > > index 6f2413b39b4a..cb7203b06767 100644 > > --- a/include/hw/ipmi/ipmi.h > > +++ b/include/hw/ipmi/ipmi.h > > @@ -265,4 +265,40 @@ int ipmi_bmc_sdr_find(IPMIBmc *b, uint16_t recid, > > const struct ipmi_sdr_compact **sdr, uint16_t *nextrec); > > void ipmi_bmc_gen_event(IPMIBmc *b, uint8_t *evt, bool log); > > > > +typedef struct IPMIBmcSim IPMIBmcSim; > > This type isn't very useful outside of the simulator, but changes for > that can come as they are needed. I don't see an easy way to avoid > putting it here. > > > + > > +typedef struct RspBuffer { > > + uint8_t buffer[MAX_IPMI_MSG_SIZE]; > > + unsigned int len; > > +} RspBuffer; > > + > > +static inline void rsp_buffer_set_error(RspBuffer *rsp, uint8_t byte) > > +{ > > + rsp->buffer[2] = byte; > > +} > > + > > +/* Add a byte to the response. */ > > +static inline void rsp_buffer_push(RspBuffer *rsp, uint8_t byte) > > +{ > > + if (rsp->len >= sizeof(rsp->buffer)) { > > + rsp_buffer_set_error(rsp, IPMI_CC_REQUEST_DATA_TRUNCATED); > > + return; > > + } > > + rsp->buffer[rsp->len++] = byte; > > +} > > + > > +typedef struct IPMICmdHandler { > > + void (*cmd_handler)(IPMIBmcSim *s, > > + uint8_t *cmd, unsigned int cmd_len, > > + RspBuffer *rsp); > > + unsigned int cmd_len_min; > > +} IPMICmdHandler; > > + > > +typedef struct IPMINetfn { > > + unsigned int cmd_nums; > > + const IPMICmdHandler *cmd_handlers; > > +} IPMINetfn; > > + > > +int ipmi_register_oem_netfn(IPMIBmc *b, const IPMINetfn *netfnd); > > + > > #endif > > diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c > > index 71e56f3b13d1..770aace55b08 100644 > > --- a/hw/ipmi/ipmi_bmc_sim.c > > +++ b/hw/ipmi/ipmi_bmc_sim.c > > @@ -98,6 +98,7 @@ > > #define IPMI_CMD_GET_SEL_TIME 0x48 > > #define IPMI_CMD_SET_SEL_TIME 0x49 > > > > +#define IPMI_NETFN_OEM 0x3a > > > > /* Same as a timespec struct. */ > > struct ipmi_time { > > @@ -167,23 +168,8 @@ typedef struct IPMISensor { > > #define MAX_SENSORS 20 > > #define IPMI_WATCHDOG_SENSOR 0 > > > > -typedef struct IPMIBmcSim IPMIBmcSim; > > -typedef struct RspBuffer RspBuffer; > > - > > #define MAX_NETFNS 64 > > > > -typedef struct IPMICmdHandler { > > - void (*cmd_handler)(IPMIBmcSim *s, > > - uint8_t *cmd, unsigned int cmd_len, > > - RspBuffer *rsp); > > - unsigned int cmd_len_min; > > -} IPMICmdHandler; > > - > > -typedef struct IPMINetfn { > > - unsigned int cmd_nums; > > - const IPMICmdHandler *cmd_handlers; > > -} IPMINetfn; > > - > > typedef struct IPMIRcvBufEntry { > > QTAILQ_ENTRY(IPMIRcvBufEntry) entry; > > uint8_t len; > > @@ -279,28 +265,8 @@ struct IPMIBmcSim { > > #define IPMI_BMC_WATCHDOG_ACTION_POWER_DOWN 2 > > #define IPMI_BMC_WATCHDOG_ACTION_POWER_CYCLE 3 > > > > -struct RspBuffer { > > - uint8_t buffer[MAX_IPMI_MSG_SIZE]; > > - unsigned int len; > > -}; > > - > > #define RSP_BUFFER_INITIALIZER { } > > > > -static inline void rsp_buffer_set_error(RspBuffer *rsp, uint8_t byte) > > -{ > > - rsp->buffer[2] = byte; > > -} > > - > > -/* Add a byte to the response. */ > > -static inline void rsp_buffer_push(RspBuffer *rsp, uint8_t byte) > > -{ > > - if (rsp->len >= sizeof(rsp->buffer)) { > > - rsp_buffer_set_error(rsp, IPMI_CC_REQUEST_DATA_TRUNCATED); > > - return; > > - } > > - rsp->buffer[rsp->len++] = byte; > > -} > > - > > static inline void rsp_buffer_pushmore(RspBuffer *rsp, uint8_t *bytes, > > unsigned int n) > > { > > @@ -640,6 +606,11 @@ static int ipmi_register_netfn(IPMIBmcSim *s, unsigned int netfn, > > return 0; > > } > > > > +int ipmi_register_oem_netfn(IPMIBmc *b, const IPMINetfn *netfnd) > > +{ > > + return ipmi_register_netfn(IPMI_BMC_SIMULATOR(b), IPMI_NETFN_OEM, netfnd); > > +} > > I think I would prefer just exposing ipmi_register_netfn() and maybe > rename it ipmi_sim_register_netfn() or something like that. There may > be other netfns needed in the future. > > But with that change, this looks good to me: > > Reviewed-by: Corey Minyard <cminyard@mvista.com> What's the plan for merging this, once it's ready? Is there an IPMI tree for it to be staged in? If not I could take it through the ppc tree, but I'd need some Acked-bys in that case. > > > + > > static const IPMICmdHandler *ipmi_get_handler(IPMIBmcSim *ibs, > > unsigned int netfn, > > unsigned int cmd) >
On Sun, Oct 27, 2019 at 06:47:39PM +0100, David Gibson wrote: > On Mon, Oct 21, 2019 at 09:30:17AM -0500, Corey Minyard wrote: > > On Mon, Oct 21, 2019 at 03:12:12PM +0200, Cédric Le Goater wrote: > > > The routine ipmi_register_oem_netfn() lets external modules register > > > command handlers for OEM functions. Required for the PowerNV machine. > > > > Comments inline. > > > > > > > > Cc: Corey Minyard <cminyard@mvista.com> > > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > > > --- > > > include/hw/ipmi/ipmi.h | 36 ++++++++++++++++++++++++++++++++++++ > > > hw/ipmi/ipmi_bmc_sim.c | 41 ++++++----------------------------------- > > > 2 files changed, 42 insertions(+), 35 deletions(-) > > > > > > diff --git a/include/hw/ipmi/ipmi.h b/include/hw/ipmi/ipmi.h > > > index 6f2413b39b4a..cb7203b06767 100644 > > > --- a/include/hw/ipmi/ipmi.h > > > +++ b/include/hw/ipmi/ipmi.h > > > @@ -265,4 +265,40 @@ int ipmi_bmc_sdr_find(IPMIBmc *b, uint16_t recid, > > > const struct ipmi_sdr_compact **sdr, uint16_t *nextrec); > > > void ipmi_bmc_gen_event(IPMIBmc *b, uint8_t *evt, bool log); > > > > > > +typedef struct IPMIBmcSim IPMIBmcSim; > > > > This type isn't very useful outside of the simulator, but changes for > > that can come as they are needed. I don't see an easy way to avoid > > putting it here. > > > > > + > > > +typedef struct RspBuffer { > > > + uint8_t buffer[MAX_IPMI_MSG_SIZE]; > > > + unsigned int len; > > > +} RspBuffer; > > > + > > > +static inline void rsp_buffer_set_error(RspBuffer *rsp, uint8_t byte) > > > +{ > > > + rsp->buffer[2] = byte; > > > +} > > > + > > > +/* Add a byte to the response. */ > > > +static inline void rsp_buffer_push(RspBuffer *rsp, uint8_t byte) > > > +{ > > > + if (rsp->len >= sizeof(rsp->buffer)) { > > > + rsp_buffer_set_error(rsp, IPMI_CC_REQUEST_DATA_TRUNCATED); > > > + return; > > > + } > > > + rsp->buffer[rsp->len++] = byte; > > > +} > > > + > > > +typedef struct IPMICmdHandler { > > > + void (*cmd_handler)(IPMIBmcSim *s, > > > + uint8_t *cmd, unsigned int cmd_len, > > > + RspBuffer *rsp); > > > + unsigned int cmd_len_min; > > > +} IPMICmdHandler; > > > + > > > +typedef struct IPMINetfn { > > > + unsigned int cmd_nums; > > > + const IPMICmdHandler *cmd_handlers; > > > +} IPMINetfn; > > > + > > > +int ipmi_register_oem_netfn(IPMIBmc *b, const IPMINetfn *netfnd); > > > + > > > #endif > > > diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c > > > index 71e56f3b13d1..770aace55b08 100644 > > > --- a/hw/ipmi/ipmi_bmc_sim.c > > > +++ b/hw/ipmi/ipmi_bmc_sim.c > > > @@ -98,6 +98,7 @@ > > > #define IPMI_CMD_GET_SEL_TIME 0x48 > > > #define IPMI_CMD_SET_SEL_TIME 0x49 > > > > > > +#define IPMI_NETFN_OEM 0x3a > > > > > > /* Same as a timespec struct. */ > > > struct ipmi_time { > > > @@ -167,23 +168,8 @@ typedef struct IPMISensor { > > > #define MAX_SENSORS 20 > > > #define IPMI_WATCHDOG_SENSOR 0 > > > > > > -typedef struct IPMIBmcSim IPMIBmcSim; > > > -typedef struct RspBuffer RspBuffer; > > > - > > > #define MAX_NETFNS 64 > > > > > > -typedef struct IPMICmdHandler { > > > - void (*cmd_handler)(IPMIBmcSim *s, > > > - uint8_t *cmd, unsigned int cmd_len, > > > - RspBuffer *rsp); > > > - unsigned int cmd_len_min; > > > -} IPMICmdHandler; > > > - > > > -typedef struct IPMINetfn { > > > - unsigned int cmd_nums; > > > - const IPMICmdHandler *cmd_handlers; > > > -} IPMINetfn; > > > - > > > typedef struct IPMIRcvBufEntry { > > > QTAILQ_ENTRY(IPMIRcvBufEntry) entry; > > > uint8_t len; > > > @@ -279,28 +265,8 @@ struct IPMIBmcSim { > > > #define IPMI_BMC_WATCHDOG_ACTION_POWER_DOWN 2 > > > #define IPMI_BMC_WATCHDOG_ACTION_POWER_CYCLE 3 > > > > > > -struct RspBuffer { > > > - uint8_t buffer[MAX_IPMI_MSG_SIZE]; > > > - unsigned int len; > > > -}; > > > - > > > #define RSP_BUFFER_INITIALIZER { } > > > > > > -static inline void rsp_buffer_set_error(RspBuffer *rsp, uint8_t byte) > > > -{ > > > - rsp->buffer[2] = byte; > > > -} > > > - > > > -/* Add a byte to the response. */ > > > -static inline void rsp_buffer_push(RspBuffer *rsp, uint8_t byte) > > > -{ > > > - if (rsp->len >= sizeof(rsp->buffer)) { > > > - rsp_buffer_set_error(rsp, IPMI_CC_REQUEST_DATA_TRUNCATED); > > > - return; > > > - } > > > - rsp->buffer[rsp->len++] = byte; > > > -} > > > - > > > static inline void rsp_buffer_pushmore(RspBuffer *rsp, uint8_t *bytes, > > > unsigned int n) > > > { > > > @@ -640,6 +606,11 @@ static int ipmi_register_netfn(IPMIBmcSim *s, unsigned int netfn, > > > return 0; > > > } > > > > > > +int ipmi_register_oem_netfn(IPMIBmc *b, const IPMINetfn *netfnd) > > > +{ > > > + return ipmi_register_netfn(IPMI_BMC_SIMULATOR(b), IPMI_NETFN_OEM, netfnd); > > > +} > > > > I think I would prefer just exposing ipmi_register_netfn() and maybe > > rename it ipmi_sim_register_netfn() or something like that. There may > > be other netfns needed in the future. > > > > But with that change, this looks good to me: > > > > Reviewed-by: Corey Minyard <cminyard@mvista.com> > > What's the plan for merging this, once it's ready? Is there an IPMI > tree for it to be staged in? If not I could take it through the ppc > tree, but I'd need some Acked-bys in that case. I have an IPMI tree for this. I was assuming it was going in to the PPC tree, but it's not big deal. -corey > > > > > > + > > > static const IPMICmdHandler *ipmi_get_handler(IPMIBmcSim *ibs, > > > unsigned int netfn, > > > unsigned int cmd) > > > > -- > David Gibson | I'll have my music baroque, and my code > david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ > | _way_ _around_! > http://www.ozlabs.org/~dgibson
On Sun, Oct 27, 2019 at 01:33:47PM -0500, Corey Minyard wrote: > On Sun, Oct 27, 2019 at 06:47:39PM +0100, David Gibson wrote: > > On Mon, Oct 21, 2019 at 09:30:17AM -0500, Corey Minyard wrote: > > > On Mon, Oct 21, 2019 at 03:12:12PM +0200, Cédric Le Goater wrote: > > > > The routine ipmi_register_oem_netfn() lets external modules register > > > > command handlers for OEM functions. Required for the PowerNV machine. > > > > > > Comments inline. > > > > > > > > > > > Cc: Corey Minyard <cminyard@mvista.com> > > > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > > > > --- > > > > include/hw/ipmi/ipmi.h | 36 ++++++++++++++++++++++++++++++++++++ > > > > hw/ipmi/ipmi_bmc_sim.c | 41 ++++++----------------------------------- > > > > 2 files changed, 42 insertions(+), 35 deletions(-) > > > > > > > > diff --git a/include/hw/ipmi/ipmi.h b/include/hw/ipmi/ipmi.h > > > > index 6f2413b39b4a..cb7203b06767 100644 > > > > --- a/include/hw/ipmi/ipmi.h > > > > +++ b/include/hw/ipmi/ipmi.h > > > > @@ -265,4 +265,40 @@ int ipmi_bmc_sdr_find(IPMIBmc *b, uint16_t recid, > > > > const struct ipmi_sdr_compact **sdr, uint16_t *nextrec); > > > > void ipmi_bmc_gen_event(IPMIBmc *b, uint8_t *evt, bool log); > > > > > > > > +typedef struct IPMIBmcSim IPMIBmcSim; > > > > > > This type isn't very useful outside of the simulator, but changes for > > > that can come as they are needed. I don't see an easy way to avoid > > > putting it here. > > > > > > > + > > > > +typedef struct RspBuffer { > > > > + uint8_t buffer[MAX_IPMI_MSG_SIZE]; > > > > + unsigned int len; > > > > +} RspBuffer; > > > > + > > > > +static inline void rsp_buffer_set_error(RspBuffer *rsp, uint8_t byte) > > > > +{ > > > > + rsp->buffer[2] = byte; > > > > +} > > > > + > > > > +/* Add a byte to the response. */ > > > > +static inline void rsp_buffer_push(RspBuffer *rsp, uint8_t byte) > > > > +{ > > > > + if (rsp->len >= sizeof(rsp->buffer)) { > > > > + rsp_buffer_set_error(rsp, IPMI_CC_REQUEST_DATA_TRUNCATED); > > > > + return; > > > > + } > > > > + rsp->buffer[rsp->len++] = byte; > > > > +} > > > > + > > > > +typedef struct IPMICmdHandler { > > > > + void (*cmd_handler)(IPMIBmcSim *s, > > > > + uint8_t *cmd, unsigned int cmd_len, > > > > + RspBuffer *rsp); > > > > + unsigned int cmd_len_min; > > > > +} IPMICmdHandler; > > > > + > > > > +typedef struct IPMINetfn { > > > > + unsigned int cmd_nums; > > > > + const IPMICmdHandler *cmd_handlers; > > > > +} IPMINetfn; > > > > + > > > > +int ipmi_register_oem_netfn(IPMIBmc *b, const IPMINetfn *netfnd); > > > > + > > > > #endif > > > > diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c > > > > index 71e56f3b13d1..770aace55b08 100644 > > > > --- a/hw/ipmi/ipmi_bmc_sim.c > > > > +++ b/hw/ipmi/ipmi_bmc_sim.c > > > > @@ -98,6 +98,7 @@ > > > > #define IPMI_CMD_GET_SEL_TIME 0x48 > > > > #define IPMI_CMD_SET_SEL_TIME 0x49 > > > > > > > > +#define IPMI_NETFN_OEM 0x3a > > > > > > > > /* Same as a timespec struct. */ > > > > struct ipmi_time { > > > > @@ -167,23 +168,8 @@ typedef struct IPMISensor { > > > > #define MAX_SENSORS 20 > > > > #define IPMI_WATCHDOG_SENSOR 0 > > > > > > > > -typedef struct IPMIBmcSim IPMIBmcSim; > > > > -typedef struct RspBuffer RspBuffer; > > > > - > > > > #define MAX_NETFNS 64 > > > > > > > > -typedef struct IPMICmdHandler { > > > > - void (*cmd_handler)(IPMIBmcSim *s, > > > > - uint8_t *cmd, unsigned int cmd_len, > > > > - RspBuffer *rsp); > > > > - unsigned int cmd_len_min; > > > > -} IPMICmdHandler; > > > > - > > > > -typedef struct IPMINetfn { > > > > - unsigned int cmd_nums; > > > > - const IPMICmdHandler *cmd_handlers; > > > > -} IPMINetfn; > > > > - > > > > typedef struct IPMIRcvBufEntry { > > > > QTAILQ_ENTRY(IPMIRcvBufEntry) entry; > > > > uint8_t len; > > > > @@ -279,28 +265,8 @@ struct IPMIBmcSim { > > > > #define IPMI_BMC_WATCHDOG_ACTION_POWER_DOWN 2 > > > > #define IPMI_BMC_WATCHDOG_ACTION_POWER_CYCLE 3 > > > > > > > > -struct RspBuffer { > > > > - uint8_t buffer[MAX_IPMI_MSG_SIZE]; > > > > - unsigned int len; > > > > -}; > > > > - > > > > #define RSP_BUFFER_INITIALIZER { } > > > > > > > > -static inline void rsp_buffer_set_error(RspBuffer *rsp, uint8_t byte) > > > > -{ > > > > - rsp->buffer[2] = byte; > > > > -} > > > > - > > > > -/* Add a byte to the response. */ > > > > -static inline void rsp_buffer_push(RspBuffer *rsp, uint8_t byte) > > > > -{ > > > > - if (rsp->len >= sizeof(rsp->buffer)) { > > > > - rsp_buffer_set_error(rsp, IPMI_CC_REQUEST_DATA_TRUNCATED); > > > > - return; > > > > - } > > > > - rsp->buffer[rsp->len++] = byte; > > > > -} > > > > - > > > > static inline void rsp_buffer_pushmore(RspBuffer *rsp, uint8_t *bytes, > > > > unsigned int n) > > > > { > > > > @@ -640,6 +606,11 @@ static int ipmi_register_netfn(IPMIBmcSim *s, unsigned int netfn, > > > > return 0; > > > > } > > > > > > > > +int ipmi_register_oem_netfn(IPMIBmc *b, const IPMINetfn *netfnd) > > > > +{ > > > > + return ipmi_register_netfn(IPMI_BMC_SIMULATOR(b), IPMI_NETFN_OEM, netfnd); > > > > +} > > > > > > I think I would prefer just exposing ipmi_register_netfn() and maybe > > > rename it ipmi_sim_register_netfn() or something like that. There may > > > be other netfns needed in the future. > > > > > > But with that change, this looks good to me: > > > > > > Reviewed-by: Corey Minyard <cminyard@mvista.com> > > > > What's the plan for merging this, once it's ready? Is there an IPMI > > tree for it to be staged in? If not I could take it through the ppc > > tree, but I'd need some Acked-bys in that case. > > I have an IPMI tree for this. I was assuming it was going in to the PPC > tree, but it's not big deal. I'd be more comfortable if the generic ipmi changes went through the ipmi tree. Note that I've moved the initial ppc specific patch from my ppc-for-4.2 tree to my ppc-for-4.3 tree, since it missed my previous pull request and it's not really post-freeze material. > > -corey > > > > > > > > > > + > > > > static const IPMICmdHandler *ipmi_get_handler(IPMIBmcSim *ibs, > > > > unsigned int netfn, > > > > unsigned int cmd) > > > > > > >
>>> What's the plan for merging this, once it's ready? Is there an IPMI >>> tree for it to be staged in? If not I could take it through the ppc >>> tree, but I'd need some Acked-bys in that case. >> >> I have an IPMI tree for this. I was assuming it was going in to the PPC >> tree, but it's not big deal. > > I'd be more comfortable if the generic ipmi changes went through the > ipmi tree. Here is the patch : http://patchwork.ozlabs.org/patch/1185187/ > Note that I've moved the initial ppc specific patch from > my ppc-for-4.2 tree to my ppc-for-4.3 tree, since it missed my > previous pull request and it's not really post-freeze material. OK. I was wondering where it had gone. Thanks, C.
On Thu, Nov 07, 2019 at 06:14:58PM +0100, Cédric Le Goater wrote: > >>> What's the plan for merging this, once it's ready? Is there an IPMI > >>> tree for it to be staged in? If not I could take it through the ppc > >>> tree, but I'd need some Acked-bys in that case. > >> > >> I have an IPMI tree for this. I was assuming it was going in to the PPC > >> tree, but it's not big deal. > > > > I'd be more comfortable if the generic ipmi changes went through the > > ipmi tree. > > Here is the patch : > > http://patchwork.ozlabs.org/patch/1185187/ Ok, I have this in my tree. I assume there is nothing like the linux-next tree for qemu, right? -corey > > > > Note that I've moved the initial ppc specific patch from > > my ppc-for-4.2 tree to my ppc-for-4.3 tree, since it missed my > > previous pull request and it's not really post-freeze material. > > OK. I was wondering where it had gone. > > Thanks, > > C.
On 07/11/2019 18:25, Corey Minyard wrote: > On Thu, Nov 07, 2019 at 06:14:58PM +0100, Cédric Le Goater wrote: >>>>> What's the plan for merging this, once it's ready? Is there an IPMI >>>>> tree for it to be staged in? If not I could take it through the ppc >>>>> tree, but I'd need some Acked-bys in that case. >>>> >>>> I have an IPMI tree for this. I was assuming it was going in to the PPC >>>> tree, but it's not big deal. >>> >>> I'd be more comfortable if the generic ipmi changes went through the >>> ipmi tree. >> >> Here is the patch : >> >> http://patchwork.ozlabs.org/patch/1185187/ > > Ok, I have this in my tree. > > I assume there is nothing like the linux-next tree for qemu, right? no. These IPMI OEM commands are handled by the OPAL firmware only. C.
On Mon, Oct 21, 2019 at 03:12:12PM +0200, Cédric Le Goater wrote: > The routine ipmi_register_oem_netfn() lets external modules register > command handlers for OEM functions. Required for the PowerNV machine. > > Cc: Corey Minyard <cminyard@mvista.com> > Signed-off-by: Cédric Le Goater <clg@kaod.org> David, Sorry, I wasn't thinking on this. I should have realized what you wanted. So, this is: Acked-by: Corey Minyard <cminyard@mvista.com> > --- > include/hw/ipmi/ipmi.h | 36 ++++++++++++++++++++++++++++++++++++ > hw/ipmi/ipmi_bmc_sim.c | 41 ++++++----------------------------------- > 2 files changed, 42 insertions(+), 35 deletions(-) > > diff --git a/include/hw/ipmi/ipmi.h b/include/hw/ipmi/ipmi.h > index 6f2413b39b4a..cb7203b06767 100644 > --- a/include/hw/ipmi/ipmi.h > +++ b/include/hw/ipmi/ipmi.h > @@ -265,4 +265,40 @@ int ipmi_bmc_sdr_find(IPMIBmc *b, uint16_t recid, > const struct ipmi_sdr_compact **sdr, uint16_t *nextrec); > void ipmi_bmc_gen_event(IPMIBmc *b, uint8_t *evt, bool log); > > +typedef struct IPMIBmcSim IPMIBmcSim; > + > +typedef struct RspBuffer { > + uint8_t buffer[MAX_IPMI_MSG_SIZE]; > + unsigned int len; > +} RspBuffer; > + > +static inline void rsp_buffer_set_error(RspBuffer *rsp, uint8_t byte) > +{ > + rsp->buffer[2] = byte; > +} > + > +/* Add a byte to the response. */ > +static inline void rsp_buffer_push(RspBuffer *rsp, uint8_t byte) > +{ > + if (rsp->len >= sizeof(rsp->buffer)) { > + rsp_buffer_set_error(rsp, IPMI_CC_REQUEST_DATA_TRUNCATED); > + return; > + } > + rsp->buffer[rsp->len++] = byte; > +} > + > +typedef struct IPMICmdHandler { > + void (*cmd_handler)(IPMIBmcSim *s, > + uint8_t *cmd, unsigned int cmd_len, > + RspBuffer *rsp); > + unsigned int cmd_len_min; > +} IPMICmdHandler; > + > +typedef struct IPMINetfn { > + unsigned int cmd_nums; > + const IPMICmdHandler *cmd_handlers; > +} IPMINetfn; > + > +int ipmi_register_oem_netfn(IPMIBmc *b, const IPMINetfn *netfnd); > + > #endif > diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c > index 71e56f3b13d1..770aace55b08 100644 > --- a/hw/ipmi/ipmi_bmc_sim.c > +++ b/hw/ipmi/ipmi_bmc_sim.c > @@ -98,6 +98,7 @@ > #define IPMI_CMD_GET_SEL_TIME 0x48 > #define IPMI_CMD_SET_SEL_TIME 0x49 > > +#define IPMI_NETFN_OEM 0x3a > > /* Same as a timespec struct. */ > struct ipmi_time { > @@ -167,23 +168,8 @@ typedef struct IPMISensor { > #define MAX_SENSORS 20 > #define IPMI_WATCHDOG_SENSOR 0 > > -typedef struct IPMIBmcSim IPMIBmcSim; > -typedef struct RspBuffer RspBuffer; > - > #define MAX_NETFNS 64 > > -typedef struct IPMICmdHandler { > - void (*cmd_handler)(IPMIBmcSim *s, > - uint8_t *cmd, unsigned int cmd_len, > - RspBuffer *rsp); > - unsigned int cmd_len_min; > -} IPMICmdHandler; > - > -typedef struct IPMINetfn { > - unsigned int cmd_nums; > - const IPMICmdHandler *cmd_handlers; > -} IPMINetfn; > - > typedef struct IPMIRcvBufEntry { > QTAILQ_ENTRY(IPMIRcvBufEntry) entry; > uint8_t len; > @@ -279,28 +265,8 @@ struct IPMIBmcSim { > #define IPMI_BMC_WATCHDOG_ACTION_POWER_DOWN 2 > #define IPMI_BMC_WATCHDOG_ACTION_POWER_CYCLE 3 > > -struct RspBuffer { > - uint8_t buffer[MAX_IPMI_MSG_SIZE]; > - unsigned int len; > -}; > - > #define RSP_BUFFER_INITIALIZER { } > > -static inline void rsp_buffer_set_error(RspBuffer *rsp, uint8_t byte) > -{ > - rsp->buffer[2] = byte; > -} > - > -/* Add a byte to the response. */ > -static inline void rsp_buffer_push(RspBuffer *rsp, uint8_t byte) > -{ > - if (rsp->len >= sizeof(rsp->buffer)) { > - rsp_buffer_set_error(rsp, IPMI_CC_REQUEST_DATA_TRUNCATED); > - return; > - } > - rsp->buffer[rsp->len++] = byte; > -} > - > static inline void rsp_buffer_pushmore(RspBuffer *rsp, uint8_t *bytes, > unsigned int n) > { > @@ -640,6 +606,11 @@ static int ipmi_register_netfn(IPMIBmcSim *s, unsigned int netfn, > return 0; > } > > +int ipmi_register_oem_netfn(IPMIBmc *b, const IPMINetfn *netfnd) > +{ > + return ipmi_register_netfn(IPMI_BMC_SIMULATOR(b), IPMI_NETFN_OEM, netfnd); > +} > + > static const IPMICmdHandler *ipmi_get_handler(IPMIBmcSim *ibs, > unsigned int netfn, > unsigned int cmd) > -- > 2.21.0 >
On Thu, Nov 21, 2019 at 08:22:50PM -0600, Corey Minyard wrote: > On Mon, Oct 21, 2019 at 03:12:12PM +0200, Cédric Le Goater wrote: > > The routine ipmi_register_oem_netfn() lets external modules register > > command handlers for OEM functions. Required for the PowerNV machine. > > > > Cc: Corey Minyard <cminyard@mvista.com> > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > > David, > > Sorry, I wasn't thinking on this. I should have realized what you > wanted. So, this is: > > Acked-by: Corey Minyard <cminyard@mvista.com> No worries, and thanks. I've put these patches into my ppc-for-5.0 tree with this ack. > > > --- > > include/hw/ipmi/ipmi.h | 36 ++++++++++++++++++++++++++++++++++++ > > hw/ipmi/ipmi_bmc_sim.c | 41 ++++++----------------------------------- > > 2 files changed, 42 insertions(+), 35 deletions(-) > > > > diff --git a/include/hw/ipmi/ipmi.h b/include/hw/ipmi/ipmi.h > > index 6f2413b39b4a..cb7203b06767 100644 > > --- a/include/hw/ipmi/ipmi.h > > +++ b/include/hw/ipmi/ipmi.h > > @@ -265,4 +265,40 @@ int ipmi_bmc_sdr_find(IPMIBmc *b, uint16_t recid, > > const struct ipmi_sdr_compact **sdr, uint16_t *nextrec); > > void ipmi_bmc_gen_event(IPMIBmc *b, uint8_t *evt, bool log); > > > > +typedef struct IPMIBmcSim IPMIBmcSim; > > + > > +typedef struct RspBuffer { > > + uint8_t buffer[MAX_IPMI_MSG_SIZE]; > > + unsigned int len; > > +} RspBuffer; > > + > > +static inline void rsp_buffer_set_error(RspBuffer *rsp, uint8_t byte) > > +{ > > + rsp->buffer[2] = byte; > > +} > > + > > +/* Add a byte to the response. */ > > +static inline void rsp_buffer_push(RspBuffer *rsp, uint8_t byte) > > +{ > > + if (rsp->len >= sizeof(rsp->buffer)) { > > + rsp_buffer_set_error(rsp, IPMI_CC_REQUEST_DATA_TRUNCATED); > > + return; > > + } > > + rsp->buffer[rsp->len++] = byte; > > +} > > + > > +typedef struct IPMICmdHandler { > > + void (*cmd_handler)(IPMIBmcSim *s, > > + uint8_t *cmd, unsigned int cmd_len, > > + RspBuffer *rsp); > > + unsigned int cmd_len_min; > > +} IPMICmdHandler; > > + > > +typedef struct IPMINetfn { > > + unsigned int cmd_nums; > > + const IPMICmdHandler *cmd_handlers; > > +} IPMINetfn; > > + > > +int ipmi_register_oem_netfn(IPMIBmc *b, const IPMINetfn *netfnd); > > + > > #endif > > diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c > > index 71e56f3b13d1..770aace55b08 100644 > > --- a/hw/ipmi/ipmi_bmc_sim.c > > +++ b/hw/ipmi/ipmi_bmc_sim.c > > @@ -98,6 +98,7 @@ > > #define IPMI_CMD_GET_SEL_TIME 0x48 > > #define IPMI_CMD_SET_SEL_TIME 0x49 > > > > +#define IPMI_NETFN_OEM 0x3a > > > > /* Same as a timespec struct. */ > > struct ipmi_time { > > @@ -167,23 +168,8 @@ typedef struct IPMISensor { > > #define MAX_SENSORS 20 > > #define IPMI_WATCHDOG_SENSOR 0 > > > > -typedef struct IPMIBmcSim IPMIBmcSim; > > -typedef struct RspBuffer RspBuffer; > > - > > #define MAX_NETFNS 64 > > > > -typedef struct IPMICmdHandler { > > - void (*cmd_handler)(IPMIBmcSim *s, > > - uint8_t *cmd, unsigned int cmd_len, > > - RspBuffer *rsp); > > - unsigned int cmd_len_min; > > -} IPMICmdHandler; > > - > > -typedef struct IPMINetfn { > > - unsigned int cmd_nums; > > - const IPMICmdHandler *cmd_handlers; > > -} IPMINetfn; > > - > > typedef struct IPMIRcvBufEntry { > > QTAILQ_ENTRY(IPMIRcvBufEntry) entry; > > uint8_t len; > > @@ -279,28 +265,8 @@ struct IPMIBmcSim { > > #define IPMI_BMC_WATCHDOG_ACTION_POWER_DOWN 2 > > #define IPMI_BMC_WATCHDOG_ACTION_POWER_CYCLE 3 > > > > -struct RspBuffer { > > - uint8_t buffer[MAX_IPMI_MSG_SIZE]; > > - unsigned int len; > > -}; > > - > > #define RSP_BUFFER_INITIALIZER { } > > > > -static inline void rsp_buffer_set_error(RspBuffer *rsp, uint8_t byte) > > -{ > > - rsp->buffer[2] = byte; > > -} > > - > > -/* Add a byte to the response. */ > > -static inline void rsp_buffer_push(RspBuffer *rsp, uint8_t byte) > > -{ > > - if (rsp->len >= sizeof(rsp->buffer)) { > > - rsp_buffer_set_error(rsp, IPMI_CC_REQUEST_DATA_TRUNCATED); > > - return; > > - } > > - rsp->buffer[rsp->len++] = byte; > > -} > > - > > static inline void rsp_buffer_pushmore(RspBuffer *rsp, uint8_t *bytes, > > unsigned int n) > > { > > @@ -640,6 +606,11 @@ static int ipmi_register_netfn(IPMIBmcSim *s, unsigned int netfn, > > return 0; > > } > > > > +int ipmi_register_oem_netfn(IPMIBmc *b, const IPMINetfn *netfnd) > > +{ > > + return ipmi_register_netfn(IPMI_BMC_SIMULATOR(b), IPMI_NETFN_OEM, netfnd); > > +} > > + > > static const IPMICmdHandler *ipmi_get_handler(IPMIBmcSim *ibs, > > unsigned int netfn, > > unsigned int cmd) >
diff --git a/include/hw/ipmi/ipmi.h b/include/hw/ipmi/ipmi.h index 6f2413b39b4a..cb7203b06767 100644 --- a/include/hw/ipmi/ipmi.h +++ b/include/hw/ipmi/ipmi.h @@ -265,4 +265,40 @@ int ipmi_bmc_sdr_find(IPMIBmc *b, uint16_t recid, const struct ipmi_sdr_compact **sdr, uint16_t *nextrec); void ipmi_bmc_gen_event(IPMIBmc *b, uint8_t *evt, bool log); +typedef struct IPMIBmcSim IPMIBmcSim; + +typedef struct RspBuffer { + uint8_t buffer[MAX_IPMI_MSG_SIZE]; + unsigned int len; +} RspBuffer; + +static inline void rsp_buffer_set_error(RspBuffer *rsp, uint8_t byte) +{ + rsp->buffer[2] = byte; +} + +/* Add a byte to the response. */ +static inline void rsp_buffer_push(RspBuffer *rsp, uint8_t byte) +{ + if (rsp->len >= sizeof(rsp->buffer)) { + rsp_buffer_set_error(rsp, IPMI_CC_REQUEST_DATA_TRUNCATED); + return; + } + rsp->buffer[rsp->len++] = byte; +} + +typedef struct IPMICmdHandler { + void (*cmd_handler)(IPMIBmcSim *s, + uint8_t *cmd, unsigned int cmd_len, + RspBuffer *rsp); + unsigned int cmd_len_min; +} IPMICmdHandler; + +typedef struct IPMINetfn { + unsigned int cmd_nums; + const IPMICmdHandler *cmd_handlers; +} IPMINetfn; + +int ipmi_register_oem_netfn(IPMIBmc *b, const IPMINetfn *netfnd); + #endif diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c index 71e56f3b13d1..770aace55b08 100644 --- a/hw/ipmi/ipmi_bmc_sim.c +++ b/hw/ipmi/ipmi_bmc_sim.c @@ -98,6 +98,7 @@ #define IPMI_CMD_GET_SEL_TIME 0x48 #define IPMI_CMD_SET_SEL_TIME 0x49 +#define IPMI_NETFN_OEM 0x3a /* Same as a timespec struct. */ struct ipmi_time { @@ -167,23 +168,8 @@ typedef struct IPMISensor { #define MAX_SENSORS 20 #define IPMI_WATCHDOG_SENSOR 0 -typedef struct IPMIBmcSim IPMIBmcSim; -typedef struct RspBuffer RspBuffer; - #define MAX_NETFNS 64 -typedef struct IPMICmdHandler { - void (*cmd_handler)(IPMIBmcSim *s, - uint8_t *cmd, unsigned int cmd_len, - RspBuffer *rsp); - unsigned int cmd_len_min; -} IPMICmdHandler; - -typedef struct IPMINetfn { - unsigned int cmd_nums; - const IPMICmdHandler *cmd_handlers; -} IPMINetfn; - typedef struct IPMIRcvBufEntry { QTAILQ_ENTRY(IPMIRcvBufEntry) entry; uint8_t len; @@ -279,28 +265,8 @@ struct IPMIBmcSim { #define IPMI_BMC_WATCHDOG_ACTION_POWER_DOWN 2 #define IPMI_BMC_WATCHDOG_ACTION_POWER_CYCLE 3 -struct RspBuffer { - uint8_t buffer[MAX_IPMI_MSG_SIZE]; - unsigned int len; -}; - #define RSP_BUFFER_INITIALIZER { } -static inline void rsp_buffer_set_error(RspBuffer *rsp, uint8_t byte) -{ - rsp->buffer[2] = byte; -} - -/* Add a byte to the response. */ -static inline void rsp_buffer_push(RspBuffer *rsp, uint8_t byte) -{ - if (rsp->len >= sizeof(rsp->buffer)) { - rsp_buffer_set_error(rsp, IPMI_CC_REQUEST_DATA_TRUNCATED); - return; - } - rsp->buffer[rsp->len++] = byte; -} - static inline void rsp_buffer_pushmore(RspBuffer *rsp, uint8_t *bytes, unsigned int n) { @@ -640,6 +606,11 @@ static int ipmi_register_netfn(IPMIBmcSim *s, unsigned int netfn, return 0; } +int ipmi_register_oem_netfn(IPMIBmc *b, const IPMINetfn *netfnd) +{ + return ipmi_register_netfn(IPMI_BMC_SIMULATOR(b), IPMI_NETFN_OEM, netfnd); +} + static const IPMICmdHandler *ipmi_get_handler(IPMIBmcSim *ibs, unsigned int netfn, unsigned int cmd)
The routine ipmi_register_oem_netfn() lets external modules register command handlers for OEM functions. Required for the PowerNV machine. Cc: Corey Minyard <cminyard@mvista.com> Signed-off-by: Cédric Le Goater <clg@kaod.org> --- include/hw/ipmi/ipmi.h | 36 ++++++++++++++++++++++++++++++++++++ hw/ipmi/ipmi_bmc_sim.c | 41 ++++++----------------------------------- 2 files changed, 42 insertions(+), 35 deletions(-)