hw/nvram: Avoid unnecessary Xilinx eFuse backstore write

[PATCH] hw/nvram: Avoid unnecessary Xilinx eFuse backstore write

Posted by Tong Ho 2 years, 9 months ago

Add a check in the bit-set operation to write the backstore
only if the affected bit is 0 before.

With this in place, there will be no need for callers to
do the checking in order to avoid unnecessary writes.

Signed-off-by: Tong Ho <tong.ho@amd.com>
---
 hw/nvram/xlnx-efuse.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/nvram/xlnx-efuse.c b/hw/nvram/xlnx-efuse.c
index fdfffaab99..655c40b8d1 100644
--- a/hw/nvram/xlnx-efuse.c
+++ b/hw/nvram/xlnx-efuse.c
@@ -143,6 +143,8 @@ static bool efuse_ro_bits_find(XlnxEFuse *s, uint32_t k)
 
 bool xlnx_efuse_set_bit(XlnxEFuse *s, unsigned int bit)
 {
+    uint32_t set, *row;
+
     if (efuse_ro_bits_find(s, bit)) {
         g_autofree char *path = object_get_canonical_path(OBJECT(s));
 
@@ -152,8 +154,13 @@ bool xlnx_efuse_set_bit(XlnxEFuse *s, unsigned int bit)
         return false;
     }
 
-    s->fuse32[bit / 32] |= 1 << (bit % 32);
-    efuse_bdrv_sync(s, bit);
+    /* Avoid back-end write unless there is a real update */
+    row = &s->fuse32[bit / 32];
+    set = 1 << (bit % 32);
+    if (!(set & *row)) {
+        *row |= set;
+        efuse_bdrv_sync(s, bit);
+    }
     return true;
 }
 
-- 
2.25.1

Re: [PATCH] hw/nvram: Avoid unnecessary Xilinx eFuse backstore write

Posted by Peter Maydell 2 years, 6 months ago

On Wed, 26 Apr 2023 at 22:16, Tong Ho <tong.ho@amd.com> wrote:
>
> Add a check in the bit-set operation to write the backstore
> only if the affected bit is 0 before.
>
> With this in place, there will be no need for callers to
> do the checking in order to avoid unnecessary writes.
>
> Signed-off-by: Tong Ho <tong.ho@amd.com>
> ---



Applied to target-arm.next, thanks.

-- PMM

Re: [PATCH] hw/nvram: Avoid unnecessary Xilinx eFuse backstore write

Posted by Philippe Mathieu-Daudé 2 years, 9 months ago

On 26/4/23 23:16, Tong Ho wrote:
> Add a check in the bit-set operation to write the backstore
> only if the affected bit is 0 before.
> 
> With this in place, there will be no need for callers to
> do the checking in order to avoid unnecessary writes.
> 
> Signed-off-by: Tong Ho <tong.ho@amd.com>
> ---
>   hw/nvram/xlnx-efuse.c | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH] hw/nvram: Avoid unnecessary Xilinx eFuse backstore write

Posted by Francisco Iglesias 2 years, 9 months ago

On [2023 Apr 26] Wed 14:16:07, Tong Ho wrote:
> Add a check in the bit-set operation to write the backstore
> only if the affected bit is 0 before.
> 
> With this in place, there will be no need for callers to
> do the checking in order to avoid unnecessary writes.
> 
> Signed-off-by: Tong Ho <tong.ho@amd.com>

Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com>

> ---
>  hw/nvram/xlnx-efuse.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/nvram/xlnx-efuse.c b/hw/nvram/xlnx-efuse.c
> index fdfffaab99..655c40b8d1 100644
> --- a/hw/nvram/xlnx-efuse.c
> +++ b/hw/nvram/xlnx-efuse.c
> @@ -143,6 +143,8 @@ static bool efuse_ro_bits_find(XlnxEFuse *s, uint32_t k)
>  
>  bool xlnx_efuse_set_bit(XlnxEFuse *s, unsigned int bit)
>  {
> +    uint32_t set, *row;
> +
>      if (efuse_ro_bits_find(s, bit)) {
>          g_autofree char *path = object_get_canonical_path(OBJECT(s));
>  
> @@ -152,8 +154,13 @@ bool xlnx_efuse_set_bit(XlnxEFuse *s, unsigned int bit)
>          return false;
>      }
>  
> -    s->fuse32[bit / 32] |= 1 << (bit % 32);
> -    efuse_bdrv_sync(s, bit);
> +    /* Avoid back-end write unless there is a real update */
> +    row = &s->fuse32[bit / 32];
> +    set = 1 << (bit % 32);
> +    if (!(set & *row)) {
> +        *row |= set;
> +        efuse_bdrv_sync(s, bit);
> +    }
>      return true;
>  }
>  
> -- 
> 2.25.1
>

Re: [PATCH] hw/nvram: Avoid unnecessary Xilinx eFuse backstore write

Posted by Alistair Francis 2 years, 9 months ago

On Thu, Apr 27, 2023 at 10:31 AM Tong Ho <tong.ho@amd.com> wrote:
>
> Add a check in the bit-set operation to write the backstore
> only if the affected bit is 0 before.
>
> With this in place, there will be no need for callers to
> do the checking in order to avoid unnecessary writes.
>
> Signed-off-by: Tong Ho <tong.ho@amd.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/nvram/xlnx-efuse.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/hw/nvram/xlnx-efuse.c b/hw/nvram/xlnx-efuse.c
> index fdfffaab99..655c40b8d1 100644
> --- a/hw/nvram/xlnx-efuse.c
> +++ b/hw/nvram/xlnx-efuse.c
> @@ -143,6 +143,8 @@ static bool efuse_ro_bits_find(XlnxEFuse *s, uint32_t k)
>
>  bool xlnx_efuse_set_bit(XlnxEFuse *s, unsigned int bit)
>  {
> +    uint32_t set, *row;
> +
>      if (efuse_ro_bits_find(s, bit)) {
>          g_autofree char *path = object_get_canonical_path(OBJECT(s));
>
> @@ -152,8 +154,13 @@ bool xlnx_efuse_set_bit(XlnxEFuse *s, unsigned int bit)
>          return false;
>      }
>
> -    s->fuse32[bit / 32] |= 1 << (bit % 32);
> -    efuse_bdrv_sync(s, bit);
> +    /* Avoid back-end write unless there is a real update */
> +    row = &s->fuse32[bit / 32];
> +    set = 1 << (bit % 32);
> +    if (!(set & *row)) {
> +        *row |= set;
> +        efuse_bdrv_sync(s, bit);
> +    }
>      return true;
>  }
>
> --
> 2.25.1
>
>