All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: "Cédric Le Goater" <clg@fr.ibm.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>,
	Corey Minyard <cminyard@mvista.com>,
	qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v3 10/10] ipmi: provide support for FRUs
Date: Tue, 15 Mar 2016 16:42:23 +0200	[thread overview]
Message-ID: <20160315164206-mutt-send-email-mst@redhat.com> (raw)
In-Reply-To: <56E81DD1.1050608@fr.ibm.com>

On Tue, Mar 15, 2016 at 03:36:01PM +0100, Cédric Le Goater wrote:
> On 03/15/2016 03:30 PM, Michael S. Tsirkin wrote:
> > On Thu, Mar 10, 2016 at 03:04:03PM +0100, 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.
> >>
> >> These commands complies with the IPMI spec : "34. FRU Inventory Device
> >> Commands".
> >>
> >> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
> >> Acked-by: Corey Minyard <cminyard@mvista.com>
> > 
> > Since pull failed, I decided to hold this off until after release.
> > Loading new files needs some thought.
> 
> OK. See below for the build break.
> 
> >> ---
> >>
> >> Changes since v2:
> >>
> >>  - changed 'struct rsp_buffer' to 'RspBuffer'
> >>  
> >> Changes since v1:
> >>
> >>  - change property name to 'fruareasize' and 'frudatafile'
> >>  - add documentation of new properties
> >>  
> >>  hw/ipmi/ipmi_bmc_sim.c |  131 +++++++++++++++++++++++++++++++++++++++++++++++++
> >>  qemu-options.hx        |    8 ++
> >>  2 files changed, 137 insertions(+), 2 deletions(-)
> >>
> >> Index: qemu-powernv.git/hw/ipmi/ipmi_bmc_sim.c
> >> ===================================================================
> >> --- qemu-powernv.git.orig/hw/ipmi/ipmi_bmc_sim.c
> >> +++ qemu-powernv.git/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 areasize;
> >> +    uint8_t *data;
> >> +} IPMIFru;
> >> +
> >>  typedef struct IPMISensor {
> >>      uint8_t status;
> >>      uint8_t reading;
> >> @@ -213,6 +223,7 @@ struct IPMIBmcSim {
> >>  
> >>      IPMISel sel;
> >>      IPMISdr sdr;
> >> +    IPMIFru fru;
> >>      IPMISensor sensors[MAX_SENSORS];
> >>      char *sdr_filename;
> >>  
> >> @@ -1322,6 +1333,94 @@ static void get_sel_info(IPMIBmcSim *ibs
> >>      rsp_buffer_push(rsp, (ibs->sel.overflow << 7) | 0x02);
> >>  }
> >>  
> >> +static void get_fru_area_info(IPMIBmcSim *ibs,
> >> +                         uint8_t *cmd, unsigned int cmd_len,
> >> +                         RspBuffer *rsp)
> >> +{
> >> +    uint8_t fruid;
> >> +    uint16_t fru_entry_size;
> >> +
> >> +    fruid = cmd[2];
> >> +
> >> +    if (fruid >= ibs->fru.nentries) {
> >> +        rsp_buffer_set_error(rsp, IPMI_CC_INVALID_DATA_FIELD);
> >> +        return;
> >> +    }
> >> +
> >> +    fru_entry_size = ibs->fru.areasize;
> >> +
> >> +    rsp_buffer_push(rsp, fru_entry_size & 0xff);
> >> +    rsp_buffer_push(rsp, fru_entry_size >> 8 & 0xff);
> >> +    rsp_buffer_push(rsp, 0x0);
> >> +}
> >> +
> >> +#define min(x, y) ((x) < (y) ? (x) : (y))
> >> +#define max(x, y) ((x) > (y) ? (x) : (y))
> 
> 
> Do you want a resend to fix these ^ and use the MIN and MAX macros
> from osdep.h ?
> 
> C.

After the release pls.

> >> +static void read_fru_data(IPMIBmcSim *ibs,
> >> +                         uint8_t *cmd, unsigned int cmd_len,
> >> +                         RspBuffer *rsp)
> >> +{
> >> +    uint8_t fruid;
> >> +    uint16_t offset;
> >> +    int i;
> >> +    uint8_t *fru_entry;
> >> +    unsigned int count;
> >> +
> >> +    fruid = cmd[2];
> >> +    offset = (cmd[3] | cmd[4] << 8);
> >> +
> >> +    if (fruid >= ibs->fru.nentries) {
> >> +        rsp_buffer_set_error(rsp, IPMI_CC_INVALID_DATA_FIELD);
> >> +        return;
> >> +    }
> >> +
> >> +    if (offset >= ibs->fru.areasize - 1) {
> >> +        rsp_buffer_set_error(rsp, IPMI_CC_INVALID_DATA_FIELD);
> >> +        return;
> >> +    }
> >> +
> >> +    fru_entry = &ibs->fru.data[fruid * ibs->fru.areasize];
> >> +
> >> +    count = min(cmd[5], ibs->fru.areasize - offset);
> >> +
> >> +    rsp_buffer_push(rsp, count & 0xff);
> >> +    for (i = 0; i < count; i++) {
> >> +        rsp_buffer_push(rsp, fru_entry[offset + i]);
> >> +    }
> >> +}
> >> +
> >> +static void write_fru_data(IPMIBmcSim *ibs,
> >> +                         uint8_t *cmd, unsigned int cmd_len,
> >> +                         RspBuffer *rsp)
> >> +{
> >> +    uint8_t fruid;
> >> +    uint16_t offset;
> >> +    uint8_t *fru_entry;
> >> +    unsigned int count;
> >> +
> >> +    fruid = cmd[2];
> >> +    offset = (cmd[3] | cmd[4] << 8);
> >> +
> >> +    if (fruid >= ibs->fru.nentries) {
> >> +        rsp_buffer_set_error(rsp, IPMI_CC_INVALID_DATA_FIELD);
> >> +        return;
> >> +    }
> >> +
> >> +    if (offset >= ibs->fru.areasize - 1) {
> >> +        rsp_buffer_set_error(rsp, IPMI_CC_INVALID_DATA_FIELD);
> >> +        return;
> >> +    }
> >> +
> >> +    fru_entry = &ibs->fru.data[fruid * ibs->fru.areasize];
> >> +
> >> +    count = min(cmd_len - 5, ibs->fru.areasize - offset);
> >> +
> >> +    memcpy(fru_entry + offset, cmd + 5, count);
> >> +
> >> +    rsp_buffer_push(rsp, count & 0xff);
> >> +}
> >> +
> >>  static void reserve_sel(IPMIBmcSim *ibs,
> >>                          uint8_t *cmd, unsigned int cmd_len,
> >>                          RspBuffer *rsp)
> >> @@ -1658,6 +1757,9 @@ static const IPMINetfn app_netfn = {
> >>  };
> >>  
> >>  static const IPMICmdHandler storage_cmds[] = {
> >> +    [IPMI_CMD_GET_FRU_AREA_INFO] = { get_fru_area_info, 3 },
> >> +    [IPMI_CMD_READ_FRU_DATA] = { read_fru_data, 5 },
> >> +    [IPMI_CMD_WRITE_FRU_DATA] = { write_fru_data, 5 },
> >>      [IPMI_CMD_GET_SDR_REP_INFO] = { get_sdr_rep_info },
> >>      [IPMI_CMD_RESERVE_SDR_REP] = { reserve_sdr_rep },
> >>      [IPMI_CMD_GET_SDR] = { get_sdr, 8 },
> >> @@ -1760,6 +1862,31 @@ static const VMStateDescription vmstate_
> >>      }
> >>  };
> >>  
> >> +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->areasize);
> >> +        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->areasize;
> >> +        fru->data = g_malloc0(size);
> >> +    }
> >> +
> >> +    fru->nentries = size / fru->areasize;
> >> +}
> >> +
> >>  static void ipmi_sim_realize(DeviceState *dev, Error **errp)
> >>  {
> >>      IPMIBmc *b = IPMI_BMC(dev);
> >> @@ -1782,6 +1909,8 @@ static void ipmi_sim_realize(DeviceState
> >>  
> >>      ipmi_sdr_init(ibs);
> >>  
> >> +    ipmi_fru_init(&ibs->fru);
> >> +
> >>      ibs->acpi_power_state[0] = 0;
> >>      ibs->acpi_power_state[1] = 0;
> >>  
> >> @@ -1800,6 +1929,8 @@ static void ipmi_sim_realize(DeviceState
> >>  }
> >>  
> >>  static Property ipmi_sim_properties[] = {
> >> +    DEFINE_PROP_UINT16("fruareasize", IPMIBmcSim, fru.areasize, 1024),
> >> +    DEFINE_PROP_STRING("frudatafile", IPMIBmcSim, fru.filename),
> >>      DEFINE_PROP_STRING("sdrfile", IPMIBmcSim, sdr_filename),
> >>      DEFINE_PROP_END_OF_LIST(),
> >>  };
> >> Index: qemu-powernv.git/qemu-options.hx
> >> ===================================================================
> >> --- qemu-powernv.git.orig/qemu-options.hx
> >> +++ qemu-powernv.git/qemu-options.hx
> >> @@ -387,7 +387,7 @@ possible drivers and properties, use @co
> >>  @code{-device @var{driver},help}.
> >>  
> >>  Some drivers are:
> >> -@item -device ipmi-bmc-sim,id=@var{id}[,slave_addr=@var{val}][,sdrfile=@var{file}]
> >> +@item -device ipmi-bmc-sim,id=@var{id}[,slave_addr=@var{val}][,sdrfile=@var{file}][,furareasize=@var{val}][,furdatafile=@var{file}]
> >>  
> >>  Add an IPMI BMC.  This is a simulation of a hardware management
> >>  interface processor that normally sits on a system.  It provides
> >> @@ -405,7 +405,11 @@ The BMC to connect to, one of ipmi-bmc-s
> >>  @item slave_addr=@var{val}
> >>  Define slave address to use for the BMC.  The default is 0x20.
> >>  @item sdrfile=@var{file}
> >> -file containing raw Sensor Data Records (SDR) data.  The default is none.
> >> +file containing raw Sensor Data Records (SDR) data. The default is none.
> >> +@item fruareasize=@var{val}
> >> +size of a Field Replaceable Unit (FRU) area.  The default is 1024.
> >> +@item frudatafile=@var{file}
> >> +file containing raw Field Replaceable Unit (FRU) inventory data. The default is none.
> >>  @end table
> >>  
> >>  @item -device ipmi-bmc-extern,id=@var{id},chardev=@var{id}[,slave_addr=@var{val}]
> > 

  reply	other threads:[~2016-03-15 14:42 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-10 14:03 [Qemu-devel] [PATCH v3 00/10] ipmi: a couple of enhancements to the BMC simulator (round 2) Cédric Le Goater
2016-03-10 14:03 ` [Qemu-devel] [PATCH v3 01/10] ipmi: remove IPMI_CHECK_CMD_LEN() macro Cédric Le Goater
2016-03-10 14:03 ` [Qemu-devel] [PATCH v3 02/10] ipmi: replace IPMI_ADD_RSP_DATA() macro with inline helpers Cédric Le Goater
2016-03-10 14:03 ` [Qemu-devel] [PATCH v3 03/10] ipmi: remove IPMI_CHECK_RESERVATION() macro Cédric Le Goater
2016-03-10 14:03 ` [Qemu-devel] [PATCH v3 04/10] ipmi: add rsp_buffer_set_error() helper Cédric Le Goater
2016-03-10 14:03 ` [Qemu-devel] [PATCH v3 05/10] ipmi: add a realize function to the device class Cédric Le Goater
2016-03-10 14:03 ` [Qemu-devel] [PATCH v3 06/10] ipmi: use a function to initialize the SDR table Cédric Le Goater
2016-03-10 14:04 ` [Qemu-devel] [PATCH v3 07/10] ipmi: remove the need of an ending record in " Cédric Le Goater
2016-03-10 14:04 ` [Qemu-devel] [PATCH v3 08/10] ipmi: add some local variables in ipmi_sdr_init Cédric Le Goater
2016-03-10 14:04 ` [Qemu-devel] [PATCH v3 09/10] ipmi: use a file to load SDRs Cédric Le Goater
2016-03-15 14:30   ` Michael S. Tsirkin
2016-03-10 14:04 ` [Qemu-devel] [PATCH v3 10/10] ipmi: provide support for FRUs Cédric Le Goater
2016-03-15 14:30   ` Michael S. Tsirkin
2016-03-15 14:36     ` Cédric Le Goater
2016-03-15 14:42       ` Michael S. Tsirkin [this message]
2016-06-08  7:25     ` Cédric Le Goater

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160315164206-mutt-send-email-mst@redhat.com \
    --to=mst@redhat.com \
    --cc=clg@fr.ibm.com \
    --cc=cminyard@mvista.com \
    --cc=marcel@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.