Re: [PATCH v2] ppc/pnv: Add a HIOMAP erase command

[Corey Minyard]

On Thu, Aug 20, 2020 at 06:46:38PM +0200, Cédric Le Goater wrote:
> The OPAL test suite runs a read-erase-write test on the PNOR :
> 
>   https://github.com/open-power/op-test/blob/master/testcases/OpTestPNOR.py
> 
> which revealed that the IPMI HIOMAP handlers didn't support
> HIOMAP_C_ERASE. Implement the sector erase command by writing 0xFF in
> the PNOR memory region.
> 
> Cc: Corey Minyard <cminyard@mvista.com>
> Reported-by: Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Thanks a bunch.

Acked-by: Corey Minyard <cminyard@mvista.com>

> ---
> 
>  Changes in v2:
> 
>  - Introduced IPMI_CC_UNSPECIFIED as suggested by Corey.
>  
>  include/hw/ipmi/ipmi.h |  1 +
>  hw/ppc/pnv_bmc.c       | 29 ++++++++++++++++++++++++++++-
>  2 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/ipmi/ipmi.h b/include/hw/ipmi/ipmi.h
> index 8a99d958bbc3..c1efdaa4cb42 100644
> --- a/include/hw/ipmi/ipmi.h
> +++ b/include/hw/ipmi/ipmi.h
> @@ -53,6 +53,7 @@ enum ipmi_op {
>  #define IPMI_CC_INVALID_DATA_FIELD                       0xcc
>  #define IPMI_CC_BMC_INIT_IN_PROGRESS                     0xd2
>  #define IPMI_CC_COMMAND_NOT_SUPPORTED                    0xd5
> +#define IPMI_CC_UNSPECIFIED                              0xff
>  
>  #define IPMI_NETFN_APP                0x06
>  #define IPMI_NETFN_OEM                0x3a
> diff --git a/hw/ppc/pnv_bmc.c b/hw/ppc/pnv_bmc.c
> index 2e1a03daa45a..67ebb16c4d5f 100644
> --- a/hw/ppc/pnv_bmc.c
> +++ b/hw/ppc/pnv_bmc.c
> @@ -140,6 +140,27 @@ static uint16_t bytes_to_blocks(uint32_t bytes)
>      return bytes >> BLOCK_SHIFT;
>  }
>  
> +static uint32_t blocks_to_bytes(uint16_t blocks)
> +{
> +    return blocks << BLOCK_SHIFT;
> +}
> +
> +static int hiomap_erase(PnvPnor *pnor, uint32_t offset, uint32_t size)
> +{
> +    MemTxResult result;
> +    int i;
> +
> +    for (i = 0; i < size / 4; i++) {
> +        result = memory_region_dispatch_write(&pnor->mmio, offset + i * 4,
> +                                              0xFFFFFFFF, MO_32,
> +                                              MEMTXATTRS_UNSPECIFIED);
> +        if (result != MEMTX_OK) {
> +            return -1;
> +        }
> +    }
> +    return 0;
> +}
> +
>  static void hiomap_cmd(IPMIBmcSim *ibs, uint8_t *cmd, unsigned int cmd_len,
>                         RspBuffer *rsp)
>  {
> @@ -155,10 +176,16 @@ static void hiomap_cmd(IPMIBmcSim *ibs, uint8_t *cmd, 
> unsigned int cmd_len,
>      switch (cmd[2]) {
>      case HIOMAP_C_MARK_DIRTY:
>      case HIOMAP_C_FLUSH:
> -    case HIOMAP_C_ERASE:
>      case HIOMAP_C_ACK:
>          break;
>  
> +    case HIOMAP_C_ERASE:
> +        if (hiomap_erase(pnor, blocks_to_bytes(cmd[5] << 8 | cmd[4]),
> +                        blocks_to_bytes(cmd[7] << 8 | cmd[6]))) {
> +            rsp_buffer_set_error(rsp, IPMI_CC_UNSPECIFIED);
> +        }
> +        break;
> +
>      case HIOMAP_C_GET_INFO:
>          rsp_buffer_push(rsp, 2);  /* Version 2 */
>          rsp_buffer_push(rsp, BLOCK_SHIFT); /* block size */
> -- 
> 2.25.4
> 
>