| Message ID | 1455020010-17532-7-git-send-email-clg@fr.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 02/09/2016 02:13 PM, Cédric Le Goater wrote: > This patch provides a simple FRU support for the BMC simulator. FRUs > are loaded from a file which name is specified in the object > properties, each entry having a fixed size, also specified in the > properties. If the file is unknown or not accessible for some reason, > a unique entry of 1024 bytes is created as a default. Just enough to > start some simulation. > > Signed-off-by: Cédric Le Goater <clg@fr.ibm.com> > --- > hw/ipmi/ipmi_bmc_sim.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 140 insertions(+) > > diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c > index 69318eb6b556..b0754893fc08 100644 > --- a/hw/ipmi/ipmi_bmc_sim.c > +++ b/hw/ipmi/ipmi_bmc_sim.c > @@ -80,6 +80,9 @@ > #define IPMI_CMD_ENTER_SDR_REP_UPD_MODE 0x2A > #define IPMI_CMD_EXIT_SDR_REP_UPD_MODE 0x2B > #define IPMI_CMD_RUN_INIT_AGENT 0x2C > +#define IPMI_CMD_GET_FRU_AREA_INFO 0x10 > +#define IPMI_CMD_READ_FRU_DATA 0x11 > +#define IPMI_CMD_WRITE_FRU_DATA 0x12 > #define IPMI_CMD_GET_SEL_INFO 0x40 > #define IPMI_CMD_GET_SEL_ALLOC_INFO 0x41 > #define IPMI_CMD_RESERVE_SEL 0x42 > @@ -122,6 +125,13 @@ typedef struct IPMISdr { > uint8_t overflow; > } IPMISdr; > > +typedef struct IPMIFru { > + char *filename; > + unsigned int nentries; > + uint16_t size; > + uint8_t *data; > +} IPMIFru; > + > typedef struct IPMISensor { > uint8_t status; > uint8_t reading; > @@ -208,6 +218,7 @@ struct IPMIBmcSim { > > IPMISel sel; > IPMISdr sdr; > + IPMIFru fru; > IPMISensor sensors[MAX_SENSORS]; > char *sdr_filename; > > @@ -1314,6 +1325,103 @@ static void get_sel_info(IPMIBmcSim *ibs, > IPMI_ADD_RSP_DATA((ibs->sel.overflow << 7) | 0x02); > } > > +static void get_fru_area_info(IPMIBmcSim *ibs, > + uint8_t *cmd, unsigned int cmd_len, > + uint8_t *rsp, unsigned int *rsp_len, > + unsigned int max_rsp_len) > +{ > + uint8_t fruid; > + uint16_t fru_entry_size; > + > + IPMI_CHECK_CMD_LEN(3); Hi, This is little awkward for me. The cmd_len and rsp parameters of the macro are implied. Am I the only one this bothers? > + > + fruid = cmd[2]; > + > + if (fruid >= ibs->fru.nentries) { > + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; > + return; > + } > + > + fru_entry_size = ibs->fru.size; > + > + IPMI_ADD_RSP_DATA(fru_entry_size & 0xff); > + IPMI_ADD_RSP_DATA(fru_entry_size >> 8 & 0xff); > + IPMI_ADD_RSP_DATA(0x0); Same here. By the way, do you have some spec for the above or is an ad-hoc encoding of the fields? If yes, you could add a reference for the spec.(This is also for the other functions in this patch) Thanks, Marcel > +} > + > +#define min(x, y) ((x) < (y) ? (x) : (y)) > +#define max(x, y) ((x) > (y) ? (x) : (y)) > + > +static void read_fru_data(IPMIBmcSim *ibs, > + uint8_t *cmd, unsigned int cmd_len, > + uint8_t *rsp, unsigned int *rsp_len, > + unsigned int max_rsp_len) > +{ > + uint8_t fruid; > + uint16_t offset; > + int i; > + uint8_t *fru_entry; > + unsigned int count; > + > + IPMI_CHECK_CMD_LEN(5); > + > + fruid = cmd[2]; > + offset = (cmd[3] | cmd[4] << 8); > + > + if (fruid >= ibs->fru.nentries) { > + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; > + return; > + } > + > + if (offset >= ibs->fru.size - 1) { > + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; > + return; > + } > + > + fru_entry = &ibs->fru.data[fruid * ibs->fru.size]; > + > + count = min(cmd[5], ibs->fru.size - offset); > + > + IPMI_ADD_RSP_DATA(count & 0xff); > + for (i = 0; i < count; i++) { > + IPMI_ADD_RSP_DATA(fru_entry[offset + i]); > + } > +} > + > +static void write_fru_data(IPMIBmcSim *ibs, > + uint8_t *cmd, unsigned int cmd_len, > + uint8_t *rsp, unsigned int *rsp_len, > + unsigned int max_rsp_len) > +{ > + uint8_t fruid; > + uint16_t offset; > + uint8_t *fru_entry; > + unsigned int count; > + > + IPMI_CHECK_CMD_LEN(5); > + > + fruid = cmd[2]; > + offset = (cmd[3] | cmd[4] << 8); > + > + if (fruid >= ibs->fru.nentries) { > + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; > + return; > + } > + > + if (offset >= ibs->fru.size - 1) { > + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; > + return; > + } > + > + fru_entry = &ibs->fru.data[fruid * ibs->fru.size]; > + > + count = min(cmd_len - 5, ibs->fru.size - offset); > + > + memcpy(fru_entry + offset, cmd + 5, count); > + > + IPMI_ADD_RSP_DATA(count & 0xff); > +} > + > static void reserve_sel(IPMIBmcSim *ibs, > uint8_t *cmd, unsigned int cmd_len, > uint8_t *rsp, unsigned int *rsp_len, > @@ -1667,6 +1775,9 @@ static const IPMINetfn app_netfn = { > }; > > static const IPMICmdHandler storage_cmds[] = { > + [IPMI_CMD_GET_FRU_AREA_INFO] = get_fru_area_info, > + [IPMI_CMD_READ_FRU_DATA] = read_fru_data, > + [IPMI_CMD_WRITE_FRU_DATA] = write_fru_data, > [IPMI_CMD_GET_SDR_REP_INFO] = get_sdr_rep_info, > [IPMI_CMD_RESERVE_SDR_REP] = reserve_sdr_rep, > [IPMI_CMD_GET_SDR] = get_sdr, > @@ -1766,6 +1877,31 @@ static const VMStateDescription vmstate_ipmi_sim = { > } > }; > > +static void ipmi_fru_init(IPMIFru *fru) > +{ > + int fsize; > + int size = 0; > + > + fsize = get_image_size(fru->filename); > + if (fsize > 0) { > + size = QEMU_ALIGN_UP(fsize, fru->size); > + fru->data = g_malloc0(size); > + if (load_image_size(fru->filename, fru->data, fsize) != fsize) { > + error_report("Could not load file '%s'", fru->filename); > + g_free(fru->data); > + fru->data = NULL; > + } > + } > + > + if (!fru->data) { > + /* give one default FRU */ > + size = fru->size; > + fru->data = g_malloc0(size); > + } > + > + fru->nentries = size / fru->size; > +} > + > static void ipmi_sim_realize(DeviceState *dev, Error **errp) 652776 > { > IPMIBmc *b = IPMI_BMC(dev); > @@ -1788,6 +1924,8 @@ static void ipmi_sim_realize(DeviceState *dev, Error **errp) > > ipmi_sdr_init(ibs); > > + ipmi_fru_init(&ibs->fru); > + > ibs->acpi_power_state[0] = 0; > ibs->acpi_power_state[1] = 0; > > @@ -1806,6 +1944,8 @@ static void ipmi_sim_realize(DeviceState *dev, Error **errp) > } > > static Property ipmi_sim_properties[] = { > + DEFINE_PROP_UINT16("frusize", IPMIBmcSim, fru.size, 1024), > + DEFINE_PROP_STRING("frufile", IPMIBmcSim, fru.filename), > DEFINE_PROP_STRING("sdr", IPMIBmcSim, sdr_filename), > DEFINE_PROP_END_OF_LIST(), > }; >
On 02/14/2016 10:25 AM, Marcel Apfelbaum wrote: > On 02/09/2016 02:13 PM, Cédric Le Goater wrote: >> This patch provides a simple FRU support for the BMC simulator. FRUs >> are loaded from a file which name is specified in the object >> properties, each entry having a fixed size, also specified in the >> properties. If the file is unknown or not accessible for some reason, >> a unique entry of 1024 bytes is created as a default. Just enough to >> start some simulation. >> >> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com> >> --- >> hw/ipmi/ipmi_bmc_sim.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 140 insertions(+) >> >> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c >> index 69318eb6b556..b0754893fc08 100644 >> --- a/hw/ipmi/ipmi_bmc_sim.c >> +++ b/hw/ipmi/ipmi_bmc_sim.c >> @@ -80,6 +80,9 @@ >> #define IPMI_CMD_ENTER_SDR_REP_UPD_MODE 0x2A >> #define IPMI_CMD_EXIT_SDR_REP_UPD_MODE 0x2B >> #define IPMI_CMD_RUN_INIT_AGENT 0x2C >> +#define IPMI_CMD_GET_FRU_AREA_INFO 0x10 >> +#define IPMI_CMD_READ_FRU_DATA 0x11 >> +#define IPMI_CMD_WRITE_FRU_DATA 0x12 >> #define IPMI_CMD_GET_SEL_INFO 0x40 >> #define IPMI_CMD_GET_SEL_ALLOC_INFO 0x41 >> #define IPMI_CMD_RESERVE_SEL 0x42 >> @@ -122,6 +125,13 @@ typedef struct IPMISdr { >> uint8_t overflow; >> } IPMISdr; >> >> +typedef struct IPMIFru { >> + char *filename; >> + unsigned int nentries; >> + uint16_t size; >> + uint8_t *data; >> +} IPMIFru; >> + >> typedef struct IPMISensor { >> uint8_t status; >> uint8_t reading; >> @@ -208,6 +218,7 @@ struct IPMIBmcSim { >> >> IPMISel sel; >> IPMISdr sdr; >> + IPMIFru fru; >> IPMISensor sensors[MAX_SENSORS]; >> char *sdr_filename; >> >> @@ -1314,6 +1325,103 @@ static void get_sel_info(IPMIBmcSim *ibs, >> IPMI_ADD_RSP_DATA((ibs->sel.overflow << 7) | 0x02); >> } >> >> +static void get_fru_area_info(IPMIBmcSim *ibs, >> + uint8_t *cmd, unsigned int cmd_len, >> + uint8_t *rsp, unsigned int *rsp_len, >> + unsigned int max_rsp_len) >> +{ >> + uint8_t fruid; >> + uint16_t fru_entry_size; >> + >> + IPMI_CHECK_CMD_LEN(3); > > Hi, > > This is little awkward for me. The cmd_len and rsp > parameters of the macro are implied. hmm, I am not sure what you mean. Are you concerned by that fact we could overflow rsp and cmd ? I guess we could probably move the IPMI_CHECK_CMD_LEN in the caller of these commands and use an array of expected argument lengths. For rsp, IPMI_ADD_RSP_DATA needs to be used to check that we are not exceeding max_rsp_len > Am I the only one this bothers? > >> + >> + fruid = cmd[2]; >> + >> + if (fruid >= ibs->fru.nentries) { >> + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; >> + return; >> + } >> + >> + fru_entry_size = ibs->fru.size; >> + >> + IPMI_ADD_RSP_DATA(fru_entry_size & 0xff); >> + IPMI_ADD_RSP_DATA(fru_entry_size >> 8 & 0xff); >> + IPMI_ADD_RSP_DATA(0x0); > > Same here. By the way, do you have some spec for the above or > is an ad-hoc encoding of the fields? If yes, you could > add a reference for the spec.(This is also for the other functions in this patch) Yes I will add the reference. Thanks, C. > Thanks, > Marcel > >> +} >> + >> +#define min(x, y) ((x) < (y) ? (x) : (y)) >> +#define max(x, y) ((x) > (y) ? (x) : (y)) >> + >> +static void read_fru_data(IPMIBmcSim *ibs, >> + uint8_t *cmd, unsigned int cmd_len, >> + uint8_t *rsp, unsigned int *rsp_len, >> + unsigned int max_rsp_len) >> +{ >> + uint8_t fruid; >> + uint16_t offset; >> + int i; >> + uint8_t *fru_entry; >> + unsigned int count; >> + >> + IPMI_CHECK_CMD_LEN(5); >> + >> + fruid = cmd[2]; >> + offset = (cmd[3] | cmd[4] << 8); >> + >> + if (fruid >= ibs->fru.nentries) { >> + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; >> + return; >> + } >> + >> + if (offset >= ibs->fru.size - 1) { >> + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; >> + return; >> + } >> + >> + fru_entry = &ibs->fru.data[fruid * ibs->fru.size]; >> + >> + count = min(cmd[5], ibs->fru.size - offset); >> + >> + IPMI_ADD_RSP_DATA(count & 0xff); >> + for (i = 0; i < count; i++) { >> + IPMI_ADD_RSP_DATA(fru_entry[offset + i]); >> + } >> +} >> + >> +static void write_fru_data(IPMIBmcSim *ibs, >> + uint8_t *cmd, unsigned int cmd_len, >> + uint8_t *rsp, unsigned int *rsp_len, >> + unsigned int max_rsp_len) >> +{ >> + uint8_t fruid; >> + uint16_t offset; >> + uint8_t *fru_entry; >> + unsigned int count; >> + >> + IPMI_CHECK_CMD_LEN(5); >> + >> + fruid = cmd[2]; >> + offset = (cmd[3] | cmd[4] << 8); >> + >> + if (fruid >= ibs->fru.nentries) { >> + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; >> + return; >> + } >> + >> + if (offset >= ibs->fru.size - 1) { >> + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; >> + return; >> + } >> + >> + fru_entry = &ibs->fru.data[fruid * ibs->fru.size]; >> + >> + count = min(cmd_len - 5, ibs->fru.size - offset); >> + >> + memcpy(fru_entry + offset, cmd + 5, count); >> + >> + IPMI_ADD_RSP_DATA(count & 0xff); >> +} >> + >> static void reserve_sel(IPMIBmcSim *ibs, >> uint8_t *cmd, unsigned int cmd_len, >> uint8_t *rsp, unsigned int *rsp_len, >> @@ -1667,6 +1775,9 @@ static const IPMINetfn app_netfn = { >> }; >> >> static const IPMICmdHandler storage_cmds[] = { >> + [IPMI_CMD_GET_FRU_AREA_INFO] = get_fru_area_info, >> + [IPMI_CMD_READ_FRU_DATA] = read_fru_data, >> + [IPMI_CMD_WRITE_FRU_DATA] = write_fru_data, >> [IPMI_CMD_GET_SDR_REP_INFO] = get_sdr_rep_info, >> [IPMI_CMD_RESERVE_SDR_REP] = reserve_sdr_rep, >> [IPMI_CMD_GET_SDR] = get_sdr, >> @@ -1766,6 +1877,31 @@ static const VMStateDescription vmstate_ipmi_sim = { >> } >> }; >> >> +static void ipmi_fru_init(IPMIFru *fru) >> +{ >> + int fsize; >> + int size = 0; >> + >> + fsize = get_image_size(fru->filename); >> + if (fsize > 0) { >> + size = QEMU_ALIGN_UP(fsize, fru->size); >> + fru->data = g_malloc0(size); >> + if (load_image_size(fru->filename, fru->data, fsize) != fsize) { >> + error_report("Could not load file '%s'", fru->filename); >> + g_free(fru->data); >> + fru->data = NULL; >> + } >> + } >> + >> + if (!fru->data) { >> + /* give one default FRU */ >> + size = fru->size; >> + fru->data = g_malloc0(size); >> + } >> + >> + fru->nentries = size / fru->size; >> +} >> + >> static void ipmi_sim_realize(DeviceState *dev, Error **errp) > 652776 > >> { >> IPMIBmc *b = IPMI_BMC(dev); >> @@ -1788,6 +1924,8 @@ static void ipmi_sim_realize(DeviceState *dev, Error **errp) >> >> ipmi_sdr_init(ibs); >> >> + ipmi_fru_init(&ibs->fru); >> + >> ibs->acpi_power_state[0] = 0; >> ibs->acpi_power_state[1] = 0; >> >> @@ -1806,6 +1944,8 @@ static void ipmi_sim_realize(DeviceState *dev, Error **errp) >> } >> >> static Property ipmi_sim_properties[] = { >> + DEFINE_PROP_UINT16("frusize", IPMIBmcSim, fru.size, 1024), >> + DEFINE_PROP_STRING("frufile", IPMIBmcSim, fru.filename), >> DEFINE_PROP_STRING("sdr", IPMIBmcSim, sdr_filename), >> DEFINE_PROP_END_OF_LIST(), >> }; >> >
On 02/15/2016 07:17 PM, Cédric Le Goater wrote: > On 02/14/2016 10:25 AM, Marcel Apfelbaum wrote: >> On 02/09/2016 02:13 PM, Cédric Le Goater wrote: >>> This patch provides a simple FRU support for the BMC simulator. FRUs >>> are loaded from a file which name is specified in the object >>> properties, each entry having a fixed size, also specified in the >>> properties. If the file is unknown or not accessible for some reason, >>> a unique entry of 1024 bytes is created as a default. Just enough to >>> start some simulation. >>> >>> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com> >>> --- >>> hw/ipmi/ipmi_bmc_sim.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 140 insertions(+) >>> >>> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c >>> index 69318eb6b556..b0754893fc08 100644 >>> --- a/hw/ipmi/ipmi_bmc_sim.c >>> +++ b/hw/ipmi/ipmi_bmc_sim.c >>> @@ -80,6 +80,9 @@ >>> #define IPMI_CMD_ENTER_SDR_REP_UPD_MODE 0x2A >>> #define IPMI_CMD_EXIT_SDR_REP_UPD_MODE 0x2B >>> #define IPMI_CMD_RUN_INIT_AGENT 0x2C >>> +#define IPMI_CMD_GET_FRU_AREA_INFO 0x10 >>> +#define IPMI_CMD_READ_FRU_DATA 0x11 >>> +#define IPMI_CMD_WRITE_FRU_DATA 0x12 >>> #define IPMI_CMD_GET_SEL_INFO 0x40 >>> #define IPMI_CMD_GET_SEL_ALLOC_INFO 0x41 >>> #define IPMI_CMD_RESERVE_SEL 0x42 >>> @@ -122,6 +125,13 @@ typedef struct IPMISdr { >>> uint8_t overflow; >>> } IPMISdr; >>> >>> +typedef struct IPMIFru { >>> + char *filename; >>> + unsigned int nentries; >>> + uint16_t size; >>> + uint8_t *data; >>> +} IPMIFru; >>> + >>> typedef struct IPMISensor { >>> uint8_t status; >>> uint8_t reading; >>> @@ -208,6 +218,7 @@ struct IPMIBmcSim { >>> >>> IPMISel sel; >>> IPMISdr sdr; >>> + IPMIFru fru; >>> IPMISensor sensors[MAX_SENSORS]; >>> char *sdr_filename; >>> >>> @@ -1314,6 +1325,103 @@ static void get_sel_info(IPMIBmcSim *ibs, >>> IPMI_ADD_RSP_DATA((ibs->sel.overflow << 7) | 0x02); >>> } >>> >>> +static void get_fru_area_info(IPMIBmcSim *ibs, >>> + uint8_t *cmd, unsigned int cmd_len, >>> + uint8_t *rsp, unsigned int *rsp_len, >>> + unsigned int max_rsp_len) >>> +{ >>> + uint8_t fruid; >>> + uint16_t fru_entry_size; >>> + >>> + IPMI_CHECK_CMD_LEN(3); >> >> Hi, >> >> This is little awkward for me. The cmd_len and rsp >> parameters of the macro are implied. > > hmm, I am not sure what you mean. Are you concerned by that fact > we could overflow rsp and cmd ? Not really, no. Something more simple: IPMI_CHECK_CMD_LEN(3) should be actually IPMI_CHECK_CMD_LEN(3, cmd_len, rsp). What bothers me is that both cmd_len and rsp are implied by the macro. In other words, we don't know what parameters IPMI_CHECK_CMD_LEN really has. "3" is for sure not enough, so we need to guess or look a the macro definition to see what it uses. But again, maybe is only me. Thanks, Marcel > > I guess we could probably move the IPMI_CHECK_CMD_LEN in the caller > of these commands and use an array of expected argument lengths. > For rsp, IPMI_ADD_RSP_DATA needs to be used to check that we are not > exceeding max_rsp_len > >> Am I the only one this bothers? >> >>> + >>> + fruid = cmd[2]; >>> + >>> + if (fruid >= ibs->fru.nentries) { >>> + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; >>> + return; >>> + } >>> + >>> + fru_entry_size = ibs->fru.size; >>> + >>> + IPMI_ADD_RSP_DATA(fru_entry_size & 0xff); >>> + IPMI_ADD_RSP_DATA(fru_entry_size >> 8 & 0xff); >>> + IPMI_ADD_RSP_DATA(0x0); >> >> Same here. By the way, do you have some spec for the above or >> is an ad-hoc encoding of the fields? If yes, you could >> add a reference for the spec.(This is also for the other functions in this patch) > > Yes I will add the reference. > > Thanks, > > C. > > >> Thanks, >> Marcel >> >>> +} >>> + >>> +#define min(x, y) ((x) < (y) ? (x) : (y)) >>> +#define max(x, y) ((x) > (y) ? (x) : (y)) >>> + >>> +static void read_fru_data(IPMIBmcSim *ibs, >>> + uint8_t *cmd, unsigned int cmd_len, >>> + uint8_t *rsp, unsigned int *rsp_len, >>> + unsigned int max_rsp_len) >>> +{ >>> + uint8_t fruid; >>> + uint16_t offset; >>> + int i; >>> + uint8_t *fru_entry; >>> + unsigned int count; >>> + >>> + IPMI_CHECK_CMD_LEN(5); >>> + >>> + fruid = cmd[2]; >>> + offset = (cmd[3] | cmd[4] << 8); >>> + >>> + if (fruid >= ibs->fru.nentries) { >>> + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; >>> + return; >>> + } >>> + >>> + if (offset >= ibs->fru.size - 1) { >>> + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; >>> + return; >>> + } >>> + >>> + fru_entry = &ibs->fru.data[fruid * ibs->fru.size]; >>> + >>> + count = min(cmd[5], ibs->fru.size - offset); >>> + >>> + IPMI_ADD_RSP_DATA(count & 0xff); >>> + for (i = 0; i < count; i++) { >>> + IPMI_ADD_RSP_DATA(fru_entry[offset + i]); >>> + } >>> +} >>> + >>> +static void write_fru_data(IPMIBmcSim *ibs, >>> + uint8_t *cmd, unsigned int cmd_len, >>> + uint8_t *rsp, unsigned int *rsp_len, >>> + unsigned int max_rsp_len) >>> +{ >>> + uint8_t fruid; >>> + uint16_t offset; >>> + uint8_t *fru_entry; >>> + unsigned int count; >>> + >>> + IPMI_CHECK_CMD_LEN(5); >>> + >>> + fruid = cmd[2]; >>> + offset = (cmd[3] | cmd[4] << 8); >>> + >>> + if (fruid >= ibs->fru.nentries) { >>> + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; >>> + return; >>> + } >>> + >>> + if (offset >= ibs->fru.size - 1) { >>> + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; >>> + return; >>> + } >>> + >>> + fru_entry = &ibs->fru.data[fruid * ibs->fru.size]; >>> + >>> + count = min(cmd_len - 5, ibs->fru.size - offset); >>> + >>> + memcpy(fru_entry + offset, cmd + 5, count); >>> + >>> + IPMI_ADD_RSP_DATA(count & 0xff); >>> +} >>> + >>> static void reserve_sel(IPMIBmcSim *ibs, >>> uint8_t *cmd, unsigned int cmd_len, >>> uint8_t *rsp, unsigned int *rsp_len, >>> @@ -1667,6 +1775,9 @@ static const IPMINetfn app_netfn = { >>> }; >>> >>> static const IPMICmdHandler storage_cmds[] = { >>> + [IPMI_CMD_GET_FRU_AREA_INFO] = get_fru_area_info, >>> + [IPMI_CMD_READ_FRU_DATA] = read_fru_data, >>> + [IPMI_CMD_WRITE_FRU_DATA] = write_fru_data, >>> [IPMI_CMD_GET_SDR_REP_INFO] = get_sdr_rep_info, >>> [IPMI_CMD_RESERVE_SDR_REP] = reserve_sdr_rep, >>> [IPMI_CMD_GET_SDR] = get_sdr, >>> @@ -1766,6 +1877,31 @@ static const VMStateDescription vmstate_ipmi_sim = { >>> } >>> }; >>> >>> +static void ipmi_fru_init(IPMIFru *fru) >>> +{ >>> + int fsize; >>> + int size = 0; >>> + >>> + fsize = get_image_size(fru->filename); >>> + if (fsize > 0) { >>> + size = QEMU_ALIGN_UP(fsize, fru->size); >>> + fru->data = g_malloc0(size); >>> + if (load_image_size(fru->filename, fru->data, fsize) != fsize) { >>> + error_report("Could not load file '%s'", fru->filename); >>> + g_free(fru->data); >>> + fru->data = NULL; >>> + } >>> + } >>> + >>> + if (!fru->data) { >>> + /* give one default FRU */ >>> + size = fru->size; >>> + fru->data = g_malloc0(size); >>> + } >>> + >>> + fru->nentries = size / fru->size; >>> +} >>> + >>> static void ipmi_sim_realize(DeviceState *dev, Error **errp) >> >> >>> { >>> IPMIBmc *b = IPMI_BMC(dev); >>> @@ -1788,6 +1924,8 @@ static void ipmi_sim_realize(DeviceState *dev, Error **errp) >>> >>> ipmi_sdr_init(ibs); >>> >>> + ipmi_fru_init(&ibs->fru); >>> + >>> ibs->acpi_power_state[0] = 0; >>> ibs->acpi_power_state[1] = 0; >>> >>> @@ -1806,6 +1944,8 @@ static void ipmi_sim_realize(DeviceState *dev, Error **errp) >>> } >>> >>> static Property ipmi_sim_properties[] = { >>> + DEFINE_PROP_UINT16("frusize", IPMIBmcSim, fru.size, 1024), >>> + DEFINE_PROP_STRING("frufile", IPMIBmcSim, fru.filename), >>> DEFINE_PROP_STRING("sdr", IPMIBmcSim, sdr_filename), >>> DEFINE_PROP_END_OF_LIST(), >>> }; >>> >> >
On 02/15/2016 12:40 PM, Marcel Apfelbaum wrote: > On 02/15/2016 07:17 PM, Cédric Le Goater wrote: >> On 02/14/2016 10:25 AM, Marcel Apfelbaum wrote: >>> On 02/09/2016 02:13 PM, Cédric Le Goater wrote: >>>> This patch provides a simple FRU support for the BMC simulator. FRUs >>>> are loaded from a file which name is specified in the object >>>> properties, each entry having a fixed size, also specified in the >>>> properties. If the file is unknown or not accessible for some reason, >>>> a unique entry of 1024 bytes is created as a default. Just enough to >>>> start some simulation. >>>> >>>> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com> >>>> --- >>>> hw/ipmi/ipmi_bmc_sim.c | 140 >>>> +++++++++++++++++++++++++++++++++++++++++++++++++ >>>> 1 file changed, 140 insertions(+) >>>> >>>> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c >>>> index 69318eb6b556..b0754893fc08 100644 >>>> --- a/hw/ipmi/ipmi_bmc_sim.c >>>> +++ b/hw/ipmi/ipmi_bmc_sim.c >>>> @@ -80,6 +80,9 @@ >>>> #define IPMI_CMD_ENTER_SDR_REP_UPD_MODE 0x2A >>>> #define IPMI_CMD_EXIT_SDR_REP_UPD_MODE 0x2B >>>> #define IPMI_CMD_RUN_INIT_AGENT 0x2C >>>> +#define IPMI_CMD_GET_FRU_AREA_INFO 0x10 >>>> +#define IPMI_CMD_READ_FRU_DATA 0x11 >>>> +#define IPMI_CMD_WRITE_FRU_DATA 0x12 >>>> #define IPMI_CMD_GET_SEL_INFO 0x40 >>>> #define IPMI_CMD_GET_SEL_ALLOC_INFO 0x41 >>>> #define IPMI_CMD_RESERVE_SEL 0x42 >>>> @@ -122,6 +125,13 @@ typedef struct IPMISdr { >>>> uint8_t overflow; >>>> } IPMISdr; >>>> >>>> +typedef struct IPMIFru { >>>> + char *filename; >>>> + unsigned int nentries; >>>> + uint16_t size; >>>> + uint8_t *data; >>>> +} IPMIFru; >>>> + >>>> typedef struct IPMISensor { >>>> uint8_t status; >>>> uint8_t reading; >>>> @@ -208,6 +218,7 @@ struct IPMIBmcSim { >>>> >>>> IPMISel sel; >>>> IPMISdr sdr; >>>> + IPMIFru fru; >>>> IPMISensor sensors[MAX_SENSORS]; >>>> char *sdr_filename; >>>> >>>> @@ -1314,6 +1325,103 @@ static void get_sel_info(IPMIBmcSim *ibs, >>>> IPMI_ADD_RSP_DATA((ibs->sel.overflow << 7) | 0x02); >>>> } >>>> >>>> +static void get_fru_area_info(IPMIBmcSim *ibs, >>>> + uint8_t *cmd, unsigned int cmd_len, >>>> + uint8_t *rsp, unsigned int *rsp_len, >>>> + unsigned int max_rsp_len) >>>> +{ >>>> + uint8_t fruid; >>>> + uint16_t fru_entry_size; >>>> + >>>> + IPMI_CHECK_CMD_LEN(3); >>> >>> Hi, >>> >>> This is little awkward for me. The cmd_len and rsp >>> parameters of the macro are implied. >> >> hmm, I am not sure what you mean. Are you concerned by that fact >> we could overflow rsp and cmd ? > > Not really, no. Something more simple: > > IPMI_CHECK_CMD_LEN(3) should be actually IPMI_CHECK_CMD_LEN(3, > cmd_len, rsp). > What bothers me is that both cmd_len and rsp are implied by the macro. I would tend to agree. The hidden stuff in these macros was really a bad idea in hindsight. -corey > > In other words, we don't know what parameters IPMI_CHECK_CMD_LEN > really has. > "3" is for sure not enough, so we need to guess or look a the macro > definition > to see what it uses. > > But again, maybe is only me. > > Thanks, > Marcel > > >> >> I guess we could probably move the IPMI_CHECK_CMD_LEN in the caller >> of these commands and use an array of expected argument lengths. >> For rsp, IPMI_ADD_RSP_DATA needs to be used to check that we are not >> exceeding max_rsp_len >> >>> Am I the only one this bothers? >>> >>>> + >>>> + fruid = cmd[2]; >>>> + >>>> + if (fruid >= ibs->fru.nentries) { >>>> + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; >>>> + return; >>>> + } >>>> + >>>> + fru_entry_size = ibs->fru.size; >>>> + >>>> + IPMI_ADD_RSP_DATA(fru_entry_size & 0xff); >>>> + IPMI_ADD_RSP_DATA(fru_entry_size >> 8 & 0xff); >>>> + IPMI_ADD_RSP_DATA(0x0); >>> >>> Same here. By the way, do you have some spec for the above or >>> is an ad-hoc encoding of the fields? If yes, you could >>> add a reference for the spec.(This is also for the other functions >>> in this patch) >> >> Yes I will add the reference. >> >> Thanks, >> >> C. >> >> >>> Thanks, >>> Marcel >>> >>>> +} >>>> + >>>> +#define min(x, y) ((x) < (y) ? (x) : (y)) >>>> +#define max(x, y) ((x) > (y) ? (x) : (y)) >>>> + >>>> +static void read_fru_data(IPMIBmcSim *ibs, >>>> + uint8_t *cmd, unsigned int cmd_len, >>>> + uint8_t *rsp, unsigned int *rsp_len, >>>> + unsigned int max_rsp_len) >>>> +{ >>>> + uint8_t fruid; >>>> + uint16_t offset; >>>> + int i; >>>> + uint8_t *fru_entry; >>>> + unsigned int count; >>>> + >>>> + IPMI_CHECK_CMD_LEN(5); >>>> + >>>> + fruid = cmd[2]; >>>> + offset = (cmd[3] | cmd[4] << 8); >>>> + >>>> + if (fruid >= ibs->fru.nentries) { >>>> + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; >>>> + return; >>>> + } >>>> + >>>> + if (offset >= ibs->fru.size - 1) { >>>> + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; >>>> + return; >>>> + } >>>> + >>>> + fru_entry = &ibs->fru.data[fruid * ibs->fru.size]; >>>> + >>>> + count = min(cmd[5], ibs->fru.size - offset); >>>> + >>>> + IPMI_ADD_RSP_DATA(count & 0xff); >>>> + for (i = 0; i < count; i++) { >>>> + IPMI_ADD_RSP_DATA(fru_entry[offset + i]); >>>> + } >>>> +} >>>> + >>>> +static void write_fru_data(IPMIBmcSim *ibs, >>>> + uint8_t *cmd, unsigned int cmd_len, >>>> + uint8_t *rsp, unsigned int *rsp_len, >>>> + unsigned int max_rsp_len) >>>> +{ >>>> + uint8_t fruid; >>>> + uint16_t offset; >>>> + uint8_t *fru_entry; >>>> + unsigned int count; >>>> + >>>> + IPMI_CHECK_CMD_LEN(5); >>>> + >>>> + fruid = cmd[2]; >>>> + offset = (cmd[3] | cmd[4] << 8); >>>> + >>>> + if (fruid >= ibs->fru.nentries) { >>>> + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; >>>> + return; >>>> + } >>>> + >>>> + if (offset >= ibs->fru.size - 1) { >>>> + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; >>>> + return; >>>> + } >>>> + >>>> + fru_entry = &ibs->fru.data[fruid * ibs->fru.size]; >>>> + >>>> + count = min(cmd_len - 5, ibs->fru.size - offset); >>>> + >>>> + memcpy(fru_entry + offset, cmd + 5, count); >>>> + >>>> + IPMI_ADD_RSP_DATA(count & 0xff); >>>> +} >>>> + >>>> static void reserve_sel(IPMIBmcSim *ibs, >>>> uint8_t *cmd, unsigned int cmd_len, >>>> uint8_t *rsp, unsigned int *rsp_len, >>>> @@ -1667,6 +1775,9 @@ static const IPMINetfn app_netfn = { >>>> }; >>>> >>>> static const IPMICmdHandler storage_cmds[] = { >>>> + [IPMI_CMD_GET_FRU_AREA_INFO] = get_fru_area_info, >>>> + [IPMI_CMD_READ_FRU_DATA] = read_fru_data, >>>> + [IPMI_CMD_WRITE_FRU_DATA] = write_fru_data, >>>> [IPMI_CMD_GET_SDR_REP_INFO] = get_sdr_rep_info, >>>> [IPMI_CMD_RESERVE_SDR_REP] = reserve_sdr_rep, >>>> [IPMI_CMD_GET_SDR] = get_sdr, >>>> @@ -1766,6 +1877,31 @@ static const VMStateDescription >>>> vmstate_ipmi_sim = { >>>> } >>>> }; >>>> >>>> +static void ipmi_fru_init(IPMIFru *fru) >>>> +{ >>>> + int fsize; >>>> + int size = 0; >>>> + >>>> + fsize = get_image_size(fru->filename); >>>> + if (fsize > 0) { >>>> + size = QEMU_ALIGN_UP(fsize, fru->size); >>>> + fru->data = g_malloc0(size); >>>> + if (load_image_size(fru->filename, fru->data, fsize) != >>>> fsize) { >>>> + error_report("Could not load file '%s'", fru->filename); >>>> + g_free(fru->data); >>>> + fru->data = NULL; >>>> + } >>>> + } >>>> + >>>> + if (!fru->data) { >>>> + /* give one default FRU */ >>>> + size = fru->size; >>>> + fru->data = g_malloc0(size); >>>> + } >>>> + >>>> + fru->nentries = size / fru->size; >>>> +} >>>> + >>>> static void ipmi_sim_realize(DeviceState *dev, Error **errp) >>> >>> >>>> { >>>> IPMIBmc *b = IPMI_BMC(dev); >>>> @@ -1788,6 +1924,8 @@ static void ipmi_sim_realize(DeviceState >>>> *dev, Error **errp) >>>> >>>> ipmi_sdr_init(ibs); >>>> >>>> + ipmi_fru_init(&ibs->fru); >>>> + >>>> ibs->acpi_power_state[0] = 0; >>>> ibs->acpi_power_state[1] = 0; >>>> >>>> @@ -1806,6 +1944,8 @@ static void ipmi_sim_realize(DeviceState >>>> *dev, Error **errp) >>>> } >>>> >>>> static Property ipmi_sim_properties[] = { >>>> + DEFINE_PROP_UINT16("frusize", IPMIBmcSim, fru.size, 1024), >>>> + DEFINE_PROP_STRING("frufile", IPMIBmcSim, fru.filename), >>>> DEFINE_PROP_STRING("sdr", IPMIBmcSim, sdr_filename), >>>> DEFINE_PROP_END_OF_LIST(), >>>> }; >>>> >>> >> >
On 02/16/2016 04:38 AM, Corey Minyard wrote: > On 02/15/2016 12:40 PM, Marcel Apfelbaum wrote: >> On 02/15/2016 07:17 PM, Cédric Le Goater wrote: >>> On 02/14/2016 10:25 AM, Marcel Apfelbaum wrote: >>>> On 02/09/2016 02:13 PM, Cédric Le Goater wrote: >>>>> This patch provides a simple FRU support for the BMC simulator. FRUs >>>>> are loaded from a file which name is specified in the object >>>>> properties, each entry having a fixed size, also specified in the >>>>> properties. If the file is unknown or not accessible for some reason, >>>>> a unique entry of 1024 bytes is created as a default. Just enough to >>>>> start some simulation. >>>>> >>>>> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com> >>>>> --- >>>>> hw/ipmi/ipmi_bmc_sim.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++++ >>>>> 1 file changed, 140 insertions(+) >>>>> >>>>> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c >>>>> index 69318eb6b556..b0754893fc08 100644 >>>>> --- a/hw/ipmi/ipmi_bmc_sim.c >>>>> +++ b/hw/ipmi/ipmi_bmc_sim.c >>>>> @@ -80,6 +80,9 @@ >>>>> #define IPMI_CMD_ENTER_SDR_REP_UPD_MODE 0x2A >>>>> #define IPMI_CMD_EXIT_SDR_REP_UPD_MODE 0x2B >>>>> #define IPMI_CMD_RUN_INIT_AGENT 0x2C >>>>> +#define IPMI_CMD_GET_FRU_AREA_INFO 0x10 >>>>> +#define IPMI_CMD_READ_FRU_DATA 0x11 >>>>> +#define IPMI_CMD_WRITE_FRU_DATA 0x12 >>>>> #define IPMI_CMD_GET_SEL_INFO 0x40 >>>>> #define IPMI_CMD_GET_SEL_ALLOC_INFO 0x41 >>>>> #define IPMI_CMD_RESERVE_SEL 0x42 >>>>> @@ -122,6 +125,13 @@ typedef struct IPMISdr { >>>>> uint8_t overflow; >>>>> } IPMISdr; >>>>> >>>>> +typedef struct IPMIFru { >>>>> + char *filename; >>>>> + unsigned int nentries; >>>>> + uint16_t size; >>>>> + uint8_t *data; >>>>> +} IPMIFru; >>>>> + >>>>> typedef struct IPMISensor { >>>>> uint8_t status; >>>>> uint8_t reading; >>>>> @@ -208,6 +218,7 @@ struct IPMIBmcSim { >>>>> >>>>> IPMISel sel; >>>>> IPMISdr sdr; >>>>> + IPMIFru fru; >>>>> IPMISensor sensors[MAX_SENSORS]; >>>>> char *sdr_filename; >>>>> >>>>> @@ -1314,6 +1325,103 @@ static void get_sel_info(IPMIBmcSim *ibs, >>>>> IPMI_ADD_RSP_DATA((ibs->sel.overflow << 7) | 0x02); >>>>> } >>>>> >>>>> +static void get_fru_area_info(IPMIBmcSim *ibs, >>>>> + uint8_t *cmd, unsigned int cmd_len, >>>>> + uint8_t *rsp, unsigned int *rsp_len, >>>>> + unsigned int max_rsp_len) >>>>> +{ >>>>> + uint8_t fruid; >>>>> + uint16_t fru_entry_size; >>>>> + >>>>> + IPMI_CHECK_CMD_LEN(3); >>>> >>>> Hi, >>>> >>>> This is little awkward for me. The cmd_len and rsp >>>> parameters of the macro are implied. >>> >>> hmm, I am not sure what you mean. Are you concerned by that fact >>> we could overflow rsp and cmd ? >> >> Not really, no. Something more simple: >> >> IPMI_CHECK_CMD_LEN(3) should be actually IPMI_CHECK_CMD_LEN(3, cmd_len, rsp). >> What bothers me is that both cmd_len and rsp are implied by the macro. > > I would tend to agree. The hidden stuff in these macros was really a bad idea in > hindsight. IPMI_CHECK_CMD_LEN() could be replaced by an array of constants. I like the way IPMI_ADD_RSP_DATA() pushes bytes in the response buffer, it makes the code quite readable from the spec perspective. Maybe we could replace rsp and rsp_len with a simple stack-like struct and use one helper to push response bytes. It should not be too complex to cleanup. I will add a proposal in the next round. Thanks, C. > -corey > >> >> In other words, we don't know what parameters IPMI_CHECK_CMD_LEN really has. >> "3" is for sure not enough, so we need to guess or look a the macro definition >> to see what it uses. >> >> But again, maybe is only me. >> >> Thanks, >> Marcel >> >> >>> >>> I guess we could probably move the IPMI_CHECK_CMD_LEN in the caller >>> of these commands and use an array of expected argument lengths. >>> For rsp, IPMI_ADD_RSP_DATA needs to be used to check that we are not >>> exceeding max_rsp_len >>> >>>> Am I the only one this bothers? >>>> >>>>> + >>>>> + fruid = cmd[2]; >>>>> + >>>>> + if (fruid >= ibs->fru.nentries) { >>>>> + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; >>>>> + return; >>>>> + } >>>>> + >>>>> + fru_entry_size = ibs->fru.size; >>>>> + >>>>> + IPMI_ADD_RSP_DATA(fru_entry_size & 0xff); >>>>> + IPMI_ADD_RSP_DATA(fru_entry_size >> 8 & 0xff); >>>>> + IPMI_ADD_RSP_DATA(0x0); >>>> >>>> Same here. By the way, do you have some spec for the above or >>>> is an ad-hoc encoding of the fields? If yes, you could >>>> add a reference for the spec.(This is also for the other functions in this patch) >>> >>> Yes I will add the reference. >>> >>> Thanks, >>> >>> C. >>> >>> >>>> Thanks, >>>> Marcel >>>> >>>>> +} >>>>> + >>>>> +#define min(x, y) ((x) < (y) ? (x) : (y)) >>>>> +#define max(x, y) ((x) > (y) ? (x) : (y)) >>>>> + >>>>> +static void read_fru_data(IPMIBmcSim *ibs, >>>>> + uint8_t *cmd, unsigned int cmd_len, >>>>> + uint8_t *rsp, unsigned int *rsp_len, >>>>> + unsigned int max_rsp_len) >>>>> +{ >>>>> + uint8_t fruid; >>>>> + uint16_t offset; >>>>> + int i; >>>>> + uint8_t *fru_entry; >>>>> + unsigned int count; >>>>> + >>>>> + IPMI_CHECK_CMD_LEN(5); >>>>> + >>>>> + fruid = cmd[2]; >>>>> + offset = (cmd[3] | cmd[4] << 8); >>>>> + >>>>> + if (fruid >= ibs->fru.nentries) { >>>>> + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; >>>>> + return; >>>>> + } >>>>> + >>>>> + if (offset >= ibs->fru.size - 1) { >>>>> + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; >>>>> + return; >>>>> + } >>>>> + >>>>> + fru_entry = &ibs->fru.data[fruid * ibs->fru.size]; >>>>> + >>>>> + count = min(cmd[5], ibs->fru.size - offset); >>>>> + >>>>> + IPMI_ADD_RSP_DATA(count & 0xff); >>>>> + for (i = 0; i < count; i++) { >>>>> + IPMI_ADD_RSP_DATA(fru_entry[offset + i]); >>>>> + } >>>>> +} >>>>> + >>>>> +static void write_fru_data(IPMIBmcSim *ibs, >>>>> + uint8_t *cmd, unsigned int cmd_len, >>>>> + uint8_t *rsp, unsigned int *rsp_len, >>>>> + unsigned int max_rsp_len) >>>>> +{ >>>>> + uint8_t fruid; >>>>> + uint16_t offset; >>>>> + uint8_t *fru_entry; >>>>> + unsigned int count; >>>>> + >>>>> + IPMI_CHECK_CMD_LEN(5); >>>>> + >>>>> + fruid = cmd[2]; >>>>> + offset = (cmd[3] | cmd[4] << 8); >>>>> + >>>>> + if (fruid >= ibs->fru.nentries) { >>>>> + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; >>>>> + return; >>>>> + } >>>>> + >>>>> + if (offset >= ibs->fru.size - 1) { >>>>> + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; >>>>> + return; >>>>> + } >>>>> + >>>>> + fru_entry = &ibs->fru.data[fruid * ibs->fru.size]; >>>>> + >>>>> + count = min(cmd_len - 5, ibs->fru.size - offset); >>>>> + >>>>> + memcpy(fru_entry + offset, cmd + 5, count); >>>>> + >>>>> + IPMI_ADD_RSP_DATA(count & 0xff); >>>>> +} >>>>> + >>>>> static void reserve_sel(IPMIBmcSim *ibs, >>>>> uint8_t *cmd, unsigned int cmd_len, >>>>> uint8_t *rsp, unsigned int *rsp_len, >>>>> @@ -1667,6 +1775,9 @@ static const IPMINetfn app_netfn = { >>>>> }; >>>>> >>>>> static const IPMICmdHandler storage_cmds[] = { >>>>> + [IPMI_CMD_GET_FRU_AREA_INFO] = get_fru_area_info, >>>>> + [IPMI_CMD_READ_FRU_DATA] = read_fru_data, >>>>> + [IPMI_CMD_WRITE_FRU_DATA] = write_fru_data, >>>>> [IPMI_CMD_GET_SDR_REP_INFO] = get_sdr_rep_info, >>>>> [IPMI_CMD_RESERVE_SDR_REP] = reserve_sdr_rep, >>>>> [IPMI_CMD_GET_SDR] = get_sdr, >>>>> @@ -1766,6 +1877,31 @@ static const VMStateDescription vmstate_ipmi_sim = { >>>>> } >>>>> }; >>>>> >>>>> +static void ipmi_fru_init(IPMIFru *fru) >>>>> +{ >>>>> + int fsize; >>>>> + int size = 0; >>>>> + >>>>> + fsize = get_image_size(fru->filename); >>>>> + if (fsize > 0) { >>>>> + size = QEMU_ALIGN_UP(fsize, fru->size); >>>>> + fru->data = g_malloc0(size); >>>>> + if (load_image_size(fru->filename, fru->data, fsize) != fsize) { >>>>> + error_report("Could not load file '%s'", fru->filename); >>>>> + g_free(fru->data); >>>>> + fru->data = NULL; >>>>> + } >>>>> + } >>>>> + >>>>> + if (!fru->data) { >>>>> + /* give one default FRU */ >>>>> + size = fru->size; >>>>> + fru->data = g_malloc0(size); >>>>> + } >>>>> + >>>>> + fru->nentries = size / fru->size; >>>>> +} >>>>> + >>>>> static void ipmi_sim_realize(DeviceState *dev, Error **errp) >>>> >>>> >>>>> { >>>>> IPMIBmc *b = IPMI_BMC(dev); >>>>> @@ -1788,6 +1924,8 @@ static void ipmi_sim_realize(DeviceState *dev, Error **errp) >>>>> >>>>> ipmi_sdr_init(ibs); >>>>> >>>>> + ipmi_fru_init(&ibs->fru); >>>>> + >>>>> ibs->acpi_power_state[0] = 0; >>>>> ibs->acpi_power_state[1] = 0; >>>>> >>>>> @@ -1806,6 +1944,8 @@ static void ipmi_sim_realize(DeviceState *dev, Error **errp) >>>>> } >>>>> >>>>> static Property ipmi_sim_properties[] = { >>>>> + DEFINE_PROP_UINT16("frusize", IPMIBmcSim, fru.size, 1024), >>>>> + DEFINE_PROP_STRING("frufile", IPMIBmcSim, fru.filename), >>>>> DEFINE_PROP_STRING("sdr", IPMIBmcSim, sdr_filename), >>>>> DEFINE_PROP_END_OF_LIST(), >>>>> }; >>>>> >>>> >>> >> >
diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c index 69318eb6b556..b0754893fc08 100644 --- a/hw/ipmi/ipmi_bmc_sim.c +++ b/hw/ipmi/ipmi_bmc_sim.c @@ -80,6 +80,9 @@ #define IPMI_CMD_ENTER_SDR_REP_UPD_MODE 0x2A #define IPMI_CMD_EXIT_SDR_REP_UPD_MODE 0x2B #define IPMI_CMD_RUN_INIT_AGENT 0x2C +#define IPMI_CMD_GET_FRU_AREA_INFO 0x10 +#define IPMI_CMD_READ_FRU_DATA 0x11 +#define IPMI_CMD_WRITE_FRU_DATA 0x12 #define IPMI_CMD_GET_SEL_INFO 0x40 #define IPMI_CMD_GET_SEL_ALLOC_INFO 0x41 #define IPMI_CMD_RESERVE_SEL 0x42 @@ -122,6 +125,13 @@ typedef struct IPMISdr { uint8_t overflow; } IPMISdr; +typedef struct IPMIFru { + char *filename; + unsigned int nentries; + uint16_t size; + uint8_t *data; +} IPMIFru; + typedef struct IPMISensor { uint8_t status; uint8_t reading; @@ -208,6 +218,7 @@ struct IPMIBmcSim { IPMISel sel; IPMISdr sdr; + IPMIFru fru; IPMISensor sensors[MAX_SENSORS]; char *sdr_filename; @@ -1314,6 +1325,103 @@ static void get_sel_info(IPMIBmcSim *ibs, IPMI_ADD_RSP_DATA((ibs->sel.overflow << 7) | 0x02); } +static void get_fru_area_info(IPMIBmcSim *ibs, + uint8_t *cmd, unsigned int cmd_len, + uint8_t *rsp, unsigned int *rsp_len, + unsigned int max_rsp_len) +{ + uint8_t fruid; + uint16_t fru_entry_size; + + IPMI_CHECK_CMD_LEN(3); + + fruid = cmd[2]; + + if (fruid >= ibs->fru.nentries) { + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; + return; + } + + fru_entry_size = ibs->fru.size; + + IPMI_ADD_RSP_DATA(fru_entry_size & 0xff); + IPMI_ADD_RSP_DATA(fru_entry_size >> 8 & 0xff); + IPMI_ADD_RSP_DATA(0x0); +} + +#define min(x, y) ((x) < (y) ? (x) : (y)) +#define max(x, y) ((x) > (y) ? (x) : (y)) + +static void read_fru_data(IPMIBmcSim *ibs, + uint8_t *cmd, unsigned int cmd_len, + uint8_t *rsp, unsigned int *rsp_len, + unsigned int max_rsp_len) +{ + uint8_t fruid; + uint16_t offset; + int i; + uint8_t *fru_entry; + unsigned int count; + + IPMI_CHECK_CMD_LEN(5); + + fruid = cmd[2]; + offset = (cmd[3] | cmd[4] << 8); + + if (fruid >= ibs->fru.nentries) { + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; + return; + } + + if (offset >= ibs->fru.size - 1) { + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; + return; + } + + fru_entry = &ibs->fru.data[fruid * ibs->fru.size]; + + count = min(cmd[5], ibs->fru.size - offset); + + IPMI_ADD_RSP_DATA(count & 0xff); + for (i = 0; i < count; i++) { + IPMI_ADD_RSP_DATA(fru_entry[offset + i]); + } +} + +static void write_fru_data(IPMIBmcSim *ibs, + uint8_t *cmd, unsigned int cmd_len, + uint8_t *rsp, unsigned int *rsp_len, + unsigned int max_rsp_len) +{ + uint8_t fruid; + uint16_t offset; + uint8_t *fru_entry; + unsigned int count; + + IPMI_CHECK_CMD_LEN(5); + + fruid = cmd[2]; + offset = (cmd[3] | cmd[4] << 8); + + if (fruid >= ibs->fru.nentries) { + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; + return; + } + + if (offset >= ibs->fru.size - 1) { + rsp[2] = IPMI_CC_INVALID_DATA_FIELD; + return; + } + + fru_entry = &ibs->fru.data[fruid * ibs->fru.size]; + + count = min(cmd_len - 5, ibs->fru.size - offset); + + memcpy(fru_entry + offset, cmd + 5, count); + + IPMI_ADD_RSP_DATA(count & 0xff); +} + static void reserve_sel(IPMIBmcSim *ibs, uint8_t *cmd, unsigned int cmd_len, uint8_t *rsp, unsigned int *rsp_len, @@ -1667,6 +1775,9 @@ static const IPMINetfn app_netfn = { }; static const IPMICmdHandler storage_cmds[] = { + [IPMI_CMD_GET_FRU_AREA_INFO] = get_fru_area_info, + [IPMI_CMD_READ_FRU_DATA] = read_fru_data, + [IPMI_CMD_WRITE_FRU_DATA] = write_fru_data, [IPMI_CMD_GET_SDR_REP_INFO] = get_sdr_rep_info, [IPMI_CMD_RESERVE_SDR_REP] = reserve_sdr_rep, [IPMI_CMD_GET_SDR] = get_sdr, @@ -1766,6 +1877,31 @@ static const VMStateDescription vmstate_ipmi_sim = { } }; +static void ipmi_fru_init(IPMIFru *fru) +{ + int fsize; + int size = 0; + + fsize = get_image_size(fru->filename); + if (fsize > 0) { + size = QEMU_ALIGN_UP(fsize, fru->size); + fru->data = g_malloc0(size); + if (load_image_size(fru->filename, fru->data, fsize) != fsize) { + error_report("Could not load file '%s'", fru->filename); + g_free(fru->data); + fru->data = NULL; + } + } + + if (!fru->data) { + /* give one default FRU */ + size = fru->size; + fru->data = g_malloc0(size); + } + + fru->nentries = size / fru->size; +} + static void ipmi_sim_realize(DeviceState *dev, Error **errp) { IPMIBmc *b = IPMI_BMC(dev); @@ -1788,6 +1924,8 @@ static void ipmi_sim_realize(DeviceState *dev, Error **errp) ipmi_sdr_init(ibs); + ipmi_fru_init(&ibs->fru); + ibs->acpi_power_state[0] = 0; ibs->acpi_power_state[1] = 0; @@ -1806,6 +1944,8 @@ static void ipmi_sim_realize(DeviceState *dev, Error **errp) } static Property ipmi_sim_properties[] = { + DEFINE_PROP_UINT16("frusize", IPMIBmcSim, fru.size, 1024), + DEFINE_PROP_STRING("frufile", IPMIBmcSim, fru.filename), DEFINE_PROP_STRING("sdr", IPMIBmcSim, sdr_filename), DEFINE_PROP_END_OF_LIST(), };
This patch provides a simple FRU support for the BMC simulator. FRUs are loaded from a file which name is specified in the object properties, each entry having a fixed size, also specified in the properties. If the file is unknown or not accessible for some reason, a unique entry of 1024 bytes is created as a default. Just enough to start some simulation. Signed-off-by: Cédric Le Goater <clg@fr.ibm.com> --- hw/ipmi/ipmi_bmc_sim.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 140 insertions(+)