On 6/5/23 15:44, Philippe Mathieu-Daudé wrote:
ping?

On 28/3/23 23:01, Philippe Mathieu-Daudé wrote:
From: Philippe Mathieu-Daudé <[email protected]>

GCC9 is confused when building with CFLAG -O3:

   hw/scsi/megasas.c: In function ‘megasas_scsi_realize’:
   hw/scsi/megasas.c:2387:26: error: duplicated ‘if’ condition [-Werror=duplicated-cond]
    2387 |     } else if (s->fw_sge >= 128 - MFI_PASS_FRAME_SIZE) {
   hw/scsi/megasas.c:2385:19: note: previously used here
    2385 |     if (s->fw_sge >= MEGASAS_MAX_SGE - MFI_PASS_FRAME_SIZE) {
   cc1: all warnings being treated as errors

When this device was introduced in commit e8f943c3bcc, the author
cared about modularity, using a definition for the firmware limit.

However if the firmware limit isn't changed (MEGASAS_MAX_SGE = 128),
the code ends doing the same check twice.

Per the maintainer [*]:

The original code assumed that one could change MFI_PASS_FRAME_SIZE,
but it turned out not to be possible as it's being hardcoded in the
drivers themselves (even though the interface provides mechanisms to
query it). So we can remove the duplicate lines.

Add the 'MEGASAS_MIN_SGE' definition for the '64' magic value,
slightly rewrite the condition check to simplify a bit the logic
and remove the unnecessary / duplicated check.

[*] https://lore.kernel.org/qemu-devel/[email protected]/

Suggested-by: Paolo Bonzini <[email protected]>
Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
---
v1: https://lore.kernel.org/qemu-devel/[email protected]/
---
  hw/scsi/megasas.c | 16 ++++++++++------
  1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 9cbbb16121..32c70c9e99 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -42,6 +42,7 @@
  #define MEGASAS_MAX_FRAMES 2048         /* Firmware limit at 65535 */
  #define MEGASAS_DEFAULT_FRAMES 1000     /* Windows requires this */
  #define MEGASAS_GEN2_DEFAULT_FRAMES 1008     /* Windows requires this */
+#define MEGASAS_MIN_SGE 64
  #define MEGASAS_MAX_SGE 128             /* Firmware limit */
  #define MEGASAS_DEFAULT_SGE 80
  #define MEGASAS_MAX_SECTORS 0xFFFF      /* No real limit */
@@ -2356,6 +2357,7 @@ static void megasas_scsi_realize(PCIDevice *dev, Error **errp)
      MegasasState *s = MEGASAS(dev);
      MegasasBaseClass *b = MEGASAS_GET_CLASS(s);
      uint8_t *pci_conf;
+    uint32_t sge;
      int i, bar_type;
      Error *err = NULL;
      int ret;
@@ -2424,13 +2426,15 @@ static void megasas_scsi_realize(PCIDevice *dev, Error **errp)
      if (!s->hba_serial) {
          s->hba_serial = g_strdup(MEGASAS_HBA_SERIAL);
      }
-    if (s->fw_sge >= MEGASAS_MAX_SGE - MFI_PASS_FRAME_SIZE) {
-        s->fw_sge = MEGASAS_MAX_SGE - MFI_PASS_FRAME_SIZE;
-    } else if (s->fw_sge >= 128 - MFI_PASS_FRAME_SIZE) {
-        s->fw_sge = 128 - MFI_PASS_FRAME_SIZE;
-    } else {
-        s->fw_sge = 64 - MFI_PASS_FRAME_SIZE;
+
+    sge = s->fw_sge + MFI_PASS_FRAME_SIZE;
+    if (sge < MEGASAS_MIN_SGE) {
+        sge = MEGASAS_MIN_SGE;
+    } else if (sge >= MEGASAS_MAX_SGE) {
+        sge = MEGASAS_MAX_SGE;
      }
+    s->fw_sge = sge - MFI_PASS_FRAME_SIZE;
+
      if (s->fw_cmds > MEGASAS_MAX_FRAMES) {
          s->fw_cmds = MEGASAS_MAX_FRAMES;
      }

Reviewed-by: Hannes Reinecke <[email protected]>

Cheers,

Hannes
--
Dr. Hannes Reinecke                Kernel Storage Architect
[email protected]                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman


Reply via email to