Re: [Qemu-devel] [PATCH v2] megasas: static SAS addresses

[Andreas Färber]

Am 01.08.2012 12:46, schrieb Hannes Reinecke:
> This patch introduces a new property 'sas_address' which
> allows the user to specify the SAS address for the HBA.
> The default address is following the NAA locally assigned
> identifier format with the locally assigned address
> 0x525400 as used eg for the MAC addresses.
> The lower bytes are set to the pci address which
> will ensure uniqueness for the local machine.
> 
> The port addresses are now calculated based on the magic
> number 0x1221 (which is found in real hardware, too) plus
> the device number.
> 
> Signed-off-by: Hannes Reinecke <address@hidden>
> Cc: Paolo Bonzini <address@hidden>
> Cc: Andreas Faerber <address@hidden>
> Cc: Anthony Liguori <address@hidden>
> Cc: Alexander Graf <address@hidden>

Regarding the recent _ -> - conversion, should the new property be
sas-address instead, with sas_address being handled in the option code?

The functional change looks good.

Some minor nits inline...

> 
> ---
>  hw/megasas.c |   65 ++++++++++++++++++++++++++++++++++++++++++---------------
>  hw/mfi.h     |    1 +
>  2 files changed, 49 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/megasas.c b/hw/megasas.c
> index 9a0eab1..862e454 100644
> --- a/hw/megasas.c
> +++ b/hw/megasas.c
> @@ -38,6 +38,9 @@
>  #define MEGASAS_MAX_SECTORS 0xFFFF      /* No real limit */
>  #define MEGASAS_MAX_ARRAYS 128
>  
> +#define NAA_LOCALLY_ASSIGNED_ID 0x3ULL
> +#define IEEE_COMPANY_LOCALLY_ASSIGNED 0x525400
> +
>  #define MEGASAS_FLAG_USE_JBOD      0
>  #define MEGASAS_MASK_USE_JBOD      (1 << MEGASAS_FLAG_USE_JBOD)
>  #define MEGASAS_FLAG_USE_MSIX      1
> @@ -89,6 +92,8 @@ typedef struct MegasasState {
>      int shutdown_event;
>      int boot_event;
>  
> +    uint64_t sas_addr;
> +
>      uint64_t reply_queue_pa;
>      void *reply_queue;
>      int reply_queue_len;
> @@ -372,14 +377,16 @@ static uint64_t megasas_fw_time(void)
>      return bcd_time;
>  }
>  
> -static uint64_t megasas_gen_sas_addr(uint64_t id)
> +/*
> + * Default disk sata address
> + * 0x1221 is them magic number as

the? their?

> + * present in real hardware,
> + * so use it here, too.
> + */
> +static uint64_t megasas_get_sata_addr(uint16_t id)
>  {
> -    uint64_t addr;
> -
> -    addr = 0x5001a4aULL << 36;
> -    addr |= id & 0xfffffffff;
> -
> -    return addr;
> +    uint64_t addr = (0x1221ULL << 48);
> +    return addr & (id << 24);
>  }
>  
>  /*
> @@ -652,10 +659,7 @@ static int megasas_ctrl_get_info(MegasasState *s, 
> MegasasCmd *cmd)
>      size_t dcmd_size = sizeof(info);
>      BusChild *kid;
>      int num_ld_disks = 0;
> -
> -    QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
> -        num_ld_disks++;
> -    }
> +    uint16_t sdev_id;
>  
>      memset(&info, 0x0, cmd->iov_size);
>      if (cmd->iov_size < dcmd_size) {
> @@ -669,10 +673,29 @@ static int megasas_ctrl_get_info(MegasasState *s, 
> MegasasCmd *cmd)
>      info.pci.subvendor = cpu_to_le16(PCI_VENDOR_ID_LSI_LOGIC);
>      info.pci.subdevice = cpu_to_le16(0x1013);
>  
> -    info.host.type = MFI_INFO_HOST_PCIX;
> +    /*
> +     * For some reason the firmware supports
> +     * only up to 8 device ports.
> +     * Despite supporting a far larger number
> +     * of devices for the physical devices.
> +     * So just display the first 8 devices
> +     * in the device port list, independent
> +     * on how many logical devices are actually

independent of

Andreas

> +     * present.
> +     */
> +    info.host.type = MFI_INFO_HOST_PCIE;
>      info.device.type = MFI_INFO_DEV_SAS3G;
> -    info.device.port_count = 2;
> -    info.device.port_addr[0] = 
> cpu_to_le64(megasas_gen_sas_addr((uint64_t)s));
> +    info.device.port_count = 8;
> +    QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
> +        SCSIDevice *sdev = DO_UPCAST(SCSIDevice, qdev, kid->child);
> +
> +        if (num_ld_disks < 8) {
> +            sdev_id = ((sdev->id & 0xFF) >> 8) | (sdev->lun & 0xFF);
> +            info.device.port_addr[num_ld_disks] =
> +                cpu_to_le64(megasas_get_sata_addr(sdev_id));
> +        }
> +        num_ld_disks++;
> +    }
>  
>      memcpy(info.product_name, "MegaRAID SAS 8708EM2", 20);
>      snprintf(info.serial_number, 32, "QEMU%08lx",
> @@ -761,7 +784,7 @@ static int megasas_mfc_get_defaults(MegasasState *s, 
> MegasasCmd *cmd)
>          return MFI_STAT_INVALID_PARAMETER;
>      }
>  
> -    info.sas_addr = cpu_to_le64(megasas_gen_sas_addr((uint64_t)s));
> +    info.sas_addr = cpu_to_le64(s->sas_addr);
>      info.stripe_size = 3;
>      info.flush_time = 4;
>      info.background_rate = 30;
> @@ -891,7 +914,7 @@ static int megasas_dcmd_pd_get_list(MegasasState *s, 
> MegasasCmd *cmd)
>          info.addr[num_pd_disks].scsi_dev_type = sdev->type;
>          info.addr[num_pd_disks].connect_port_bitmap = 0x1;
>          info.addr[num_pd_disks].sas_addr[0] =
> -            cpu_to_le64(megasas_gen_sas_addr((uint64_t)sdev));
> +            cpu_to_le64(megasas_get_sata_addr(sdev_id));
>          num_pd_disks++;
>          offset += sizeof(struct mfi_pd_address);
>      }
> @@ -994,7 +1017,7 @@ static int megasas_pd_get_info_submit(SCSIDevice *sdev, 
> int lun,
>      info->slot_number = (sdev->id & 0xFF);
>      info->path_info.count = 1;
>      info->path_info.sas_addr[0] =
> -        cpu_to_le64(megasas_gen_sas_addr((uint64_t)sdev));
> +        cpu_to_le64(megasas_get_sata_addr(sdev_id));
>      info->connected_port_bitmap = 0x1;
>      info->device_speed = 1;
>      info->link_speed = 1;
> @@ -2103,6 +2126,13 @@ static int megasas_scsi_init(PCIDevice *dev)
>          msix_vector_use(&s->dev, 0);
>      }
>  
> +    if (!s->sas_addr) {
> +        s->sas_addr = ((NAA_LOCALLY_ASSIGNED_ID << 24) |
> +                       IEEE_COMPANY_LOCALLY_ASSIGNED) << 36;
> +        s->sas_addr |= (pci_bus_num(dev->bus) << 16);
> +        s->sas_addr |= (PCI_SLOT(dev->devfn) << 8);
> +        s->sas_addr |= PCI_FUNC(dev->devfn);
> +    }
>      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) {
> @@ -2137,6 +2167,7 @@ static Property megasas_properties[] = {
>                         MEGASAS_DEFAULT_SGE),
>      DEFINE_PROP_UINT32("max_cmds", MegasasState, fw_cmds,
>                         MEGASAS_DEFAULT_FRAMES),
> +    DEFINE_PROP_HEX64("sas_address", MegasasState, sas_addr, 0),
>  #ifdef USE_MSIX
>      DEFINE_PROP_BIT("use_msix", MegasasState, flags,
>                      MEGASAS_FLAG_USE_MSIX, false),
> diff --git a/hw/mfi.h b/hw/mfi.h
> index 3045d4e..436b690 100644
> --- a/hw/mfi.h
> +++ b/hw/mfi.h
> @@ -656,6 +656,7 @@ struct mfi_info_device {
>  #define MFI_INFO_DEV_SAS3G      0x02
>  #define MFI_INFO_DEV_SATA1      0x04
>  #define MFI_INFO_DEV_SATA3G     0x08
> +#define MFI_INFO_DEV_PCIE       0x10
>      uint8_t reserved[6];
>      uint8_t port_count;
>      uint64_t port_addr[8];
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg