1. Patchew
  2. QEMU
  3. View series

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

Cédric Le Goater posted 1 patch 3 years, 8 months ago
Test docker-quick@centos7 passed
Test docker-mingw@fedora passed
Test checkpatch passed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200820073650.2315095-1-clg@kaod.org
There is a newer version of this series
hw/ppc/pnv_bmc.c | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
[PATCH] ppc/pnv: Add a HIOMAP erase command
Posted by Cédric Le Goater 3 years, 8 months ago 
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.

Reported-by: Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/pnv_bmc.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/pnv_bmc.c b/hw/ppc/pnv_bmc.c
index 2e1a03daa45a..0fb082fcb8ee 100644
--- a/hw/ppc/pnv_bmc.c
+++ b/hw/ppc/pnv_bmc.c
@@ -140,6 +140,29 @@ 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;
+}
+
+#define IPMI_ERR_UNSPECIFIED            0xff
+
+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 +178,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_ERR_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
Re: [PATCH] ppc/pnv: Add a HIOMAP erase command
Posted by David Gibson 3 years, 8 months ago 
On Thu, Aug 20, 2020 at 09:36:50AM +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.
> 
> Reported-by: Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Applied to ppc-for-5.2.

> ---
>  hw/ppc/pnv_bmc.c | 31 ++++++++++++++++++++++++++++++-
>  1 file changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/pnv_bmc.c b/hw/ppc/pnv_bmc.c
> index 2e1a03daa45a..0fb082fcb8ee 100644
> --- a/hw/ppc/pnv_bmc.c
> +++ b/hw/ppc/pnv_bmc.c
> @@ -140,6 +140,29 @@ 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;
> +}
> +
> +#define IPMI_ERR_UNSPECIFIED            0xff
> +
> +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 +178,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_ERR_UNSPECIFIED);
> +        }
> +        break;
> +
>      case HIOMAP_C_GET_INFO:
>          rsp_buffer_push(rsp, 2);  /* Version 2 */
>          rsp_buffer_push(rsp, BLOCK_SHIFT); /* block size */

-- 
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
Re: [PATCH] ppc/pnv: Add a HIOMAP erase command
Posted by Corey Minyard 3 years, 8 months ago 
On Thu, Aug 20, 2020 at 09:36:50AM +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.
> 
> Reported-by: Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  hw/ppc/pnv_bmc.c | 31 ++++++++++++++++++++++++++++++-
>  1 file changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/pnv_bmc.c b/hw/ppc/pnv_bmc.c
> index 2e1a03daa45a..0fb082fcb8ee 100644
> --- a/hw/ppc/pnv_bmc.c
> +++ b/hw/ppc/pnv_bmc.c
> @@ -140,6 +140,29 @@ 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;
> +}
> +
> +#define IPMI_ERR_UNSPECIFIED            0xff

Wouldn't it be better for this to be in include/hw/ipmi/ipmi.h and
be named IPMI_CC_UNSPECIFIED to match the other completion codes?

-corey

> +
> +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 +178,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_ERR_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
> 
>
Re: [PATCH] ppc/pnv: Add a HIOMAP erase command
Posted by Cédric Le Goater 3 years, 8 months ago 
On 8/20/20 6:16 PM, Corey Minyard wrote:
> On Thu, Aug 20, 2020 at 09:36:50AM +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.
>>
>> Reported-by: Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  hw/ppc/pnv_bmc.c | 31 ++++++++++++++++++++++++++++++-
>>  1 file changed, 30 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/ppc/pnv_bmc.c b/hw/ppc/pnv_bmc.c
>> index 2e1a03daa45a..0fb082fcb8ee 100644
>> --- a/hw/ppc/pnv_bmc.c
>> +++ b/hw/ppc/pnv_bmc.c
>> @@ -140,6 +140,29 @@ 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;
>> +}
>> +
>> +#define IPMI_ERR_UNSPECIFIED            0xff
> 
> Wouldn't it be better for this to be in include/hw/ipmi/ipmi.h and
> be named IPMI_CC_UNSPECIFIED to match the other completion codes?

Yes. This is cleaner. Sending a v2 for a replacement. 

Thanks,

C. 

> -corey
> 
>> +
>> +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 +178,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_ERR_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
>>
>>

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.