* [PATCH] ppc/pnv: Add a HIOMAP erase command
@ 2020-08-20 7:36 Cédric Le Goater
2020-08-20 10:12 ` David Gibson
2020-08-20 16:16 ` Corey Minyard
0 siblings, 2 replies; 4+ messages in thread
From: Cédric Le Goater @ 2020-08-20 7:36 UTC (permalink / raw)
To: David Gibson
Cc: Andrew Jeffery, qemu-devel, Klaus Heinrich Kiwi, qemu-ppc,
Cédric Le Goater, Joel Stanley
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] ppc/pnv: Add a HIOMAP erase command
2020-08-20 7:36 [PATCH] ppc/pnv: Add a HIOMAP erase command Cédric Le Goater
@ 2020-08-20 10:12 ` David Gibson
2020-08-20 16:16 ` Corey Minyard
1 sibling, 0 replies; 4+ messages in thread
From: David Gibson @ 2020-08-20 10:12 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Andrew Jeffery, Klaus Heinrich Kiwi, qemu-ppc, Joel Stanley,
qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2611 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ppc/pnv: Add a HIOMAP erase command
2020-08-20 7:36 [PATCH] ppc/pnv: Add a HIOMAP erase command Cédric Le Goater
2020-08-20 10:12 ` David Gibson
@ 2020-08-20 16:16 ` Corey Minyard
2020-08-20 16:45 ` Cédric Le Goater
1 sibling, 1 reply; 4+ messages in thread
From: Corey Minyard @ 2020-08-20 16:16 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Andrew Jeffery, qemu-devel, Klaus Heinrich Kiwi, qemu-ppc,
Joel Stanley, David Gibson
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
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ppc/pnv: Add a HIOMAP erase command
2020-08-20 16:16 ` Corey Minyard
@ 2020-08-20 16:45 ` Cédric Le Goater
0 siblings, 0 replies; 4+ messages in thread
From: Cédric Le Goater @ 2020-08-20 16:45 UTC (permalink / raw)
To: minyard
Cc: Andrew Jeffery, qemu-devel, Klaus Heinrich Kiwi, qemu-ppc,
Joel Stanley, David Gibson
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
>>
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-08-20 16:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-08-20 7:36 [PATCH] ppc/pnv: Add a HIOMAP erase command Cédric Le Goater
2020-08-20 10:12 ` David Gibson
2020-08-20 16:16 ` Corey Minyard
2020-08-20 16:45 ` Cédric Le Goater
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).