On 05/11/2015 00:06, Brian King wrote: > On 11/04/2015 07:02 AM, Hannes Reinecke wrote: >> On 11/04/2015 12:46 PM, Laurent Vivier wrote: >>> >>> >>> On 04/11/2015 12:16, Hannes Reinecke wrote: >>>> On 11/04/2015 11:20 AM, Laurent Vivier wrote: >>>>> QEMU allows until 32 LUNs. >>>>> >>>>> Signed-off-by: Laurent Vivier <lvivier@xxxxxxxxxx> >>>>> --- >>>>> drivers/scsi/ibmvscsi/ibmvscsi.c | 5 ++++- >>>>> 1 file changed, 4 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c >>>>> index 04de287..4480d3e 100644 >>>>> --- a/drivers/scsi/ibmvscsi/ibmvscsi.c >>>>> +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c >>>>> @@ -84,6 +84,7 @@ >>>>> */ >>>>> static int max_id = 64; >>>>> static int max_channel = 3; >>>>> +static int max_lun = 8; >>>>> static int init_timeout = 300; >>>>> static int login_timeout = 60; >>>>> static int info_timeout = 30; >>>>> @@ -117,6 +118,8 @@ module_param_named(fast_fail, fast_fail, int, S_IRUGO | S_IWUSR); >>>>> MODULE_PARM_DESC(fast_fail, "Enable fast fail. [Default=1]"); >>>>> module_param_named(client_reserve, client_reserve, int, S_IRUGO ); >>>>> MODULE_PARM_DESC(client_reserve, "Attempt client managed reserve/release"); >>>>> +module_param(max_lun, int, S_IRUGO); >>>>> +MODULE_PARM_DESC(max_lun, "Maximum LUN value [Default=8]"); >>>>> >>>>> static void ibmvscsi_handle_crq(struct viosrp_crq *crq, >>>>> struct ibmvscsi_host_data *hostdata); >>>>> @@ -2289,7 +2292,7 @@ static int ibmvscsi_probe(struct vio_dev *vdev, const struct vio_device_id *id) >>>>> goto init_pool_failed; >>>>> } >>>>> >>>>> - host->max_lun = 8; >>>>> + host->max_lun = max_lun; >>>>> host->max_id = max_id; >>>>> host->max_channel = max_channel; >>>>> host->max_cmd_len = 16; >>>>> >>>> Please, don't do this. >>>> >>>> 'max_lun' should only be set if the HBA / transport has some hard >>>> limitations on the number of bytes it can use. >>>> Otherwise the scanning algorithm in scsi_scan.c should do the >>>> correct thing, independent on the 'max_lun' setting. >>> >>> So you are saying we can remove the line ? >>> >> Ho-hum. In principle you could, as ibmvscsi is using SRP internally, >> which does support 64 bit LUNs. >> >> However, due to some weird design decisions there is >> the function 'lun_from_dev()', which mangles the incoming LUN number >> into something ... else. >> Which leaves only 4 bits free for the actual LUN number, requiring >> you to use max_lun = 16. >> >> Personally I would just do away with that function and use the >> incoming LUN numbers as is. > > I pulled out my copy of SAM, what lun_from_dev is doing is translating > the incoming bus / id / LUN to a LUN in the logical unit addressing format, > as defined in 4.6.9 of SAM-4. > > -------------------------------------- > |Bit | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0 | > -------------------------------------- > | n | (10b) | Target | > -------------------------------------- > | n+1| Bus | LUN | > -------------------------------------- > > So this means, in the current implementation, we have 6 bits for target (max=63), > 3 bits for bus (max=7), and 5 bits for LUN (max=31). > > It might not be a bad idea to enforce these limits on the module parameters. Otherwise > we'll have a mess when we run through lun_from_dev... > > As far as eliminating lun_from_dev and just passing the LUN through, I don't think we > can do that. The LUN list returned by Report LUNs to a VSCSI disk assumes we are > doing the translation above. So what I understand is this patch is OK but I have to check the given value is less than 32 (the same max value as for QEMU) ? Laurent -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
