linux-kernel - Re: [PATCH RFC v3 16/22] ata: libata-scsi: Allocate sdev early in port probe

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 7 Nov 2022 10:09:55 +0000
From:   John Garry <john.g.garry@...cle.com>
To:     Hannes Reinecke <hare@...e.de>,
        Damien Le Moal <damien.lemoal@...nsource.wdc.com>,
        John Garry <john.garry@...wei.com>, axboe@...nel.dk,
        jejb@...ux.ibm.com, martin.petersen@...cle.com,
        jinpu.wang@...ud.ionos.com, bvanassche@....org, hch@....de,
        ming.lei@...hat.com, niklas.cassel@....com
Cc:     linux-block@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-ide@...r.kernel.org, linux-scsi@...r.kernel.org,
        linuxarm@...wei.com
Subject: Re: [PATCH RFC v3 16/22] ata: libata-scsi: Allocate sdev early in
 port probe


>>>>>    @@ -4289,26 +4294,16 @@ void ata_scsi_scan_host(struct ata_port
>>>>> *ap, int sync)
>>>>>     repeat:

I've been trying to follow this thread, below, but got a bit lost ....

>>>>>        ata_for_each_link(link, ap, EDGE) {
>>>>>            ata_for_each_dev(dev, link, ENABLED) {
>>>>> -            struct scsi_device *sdev;
>>>>> +            struct Scsi_Host *shost = ap->scsi_host;
>>>>>                int channel = 0, id = 0;
>>>>>    -            if (dev->sdev)
>>>>> -                continue;
>>>>> -
>>>>>                if (ata_is_host_link(link))
>>>>>                    id = dev->devno;
>>>>>                else
>>>>>                    channel = link->pmp;
>>>>>    -            sdev = __scsi_add_device(ap->scsi_host, channel, 
>>>>> id, 0,
>>>>> -                         NULL);
>>>>> -            if (!IS_ERR(sdev)) {
>>>>> -                dev->sdev = sdev;
>>>>> -                ata_scsi_assign_ofnode(dev, ap);
>>>>
>>>> Is there something equivalent to what this function does inside
>>>> scsi_scan_target() ? I had a quick look but did not see anything...
>>>>

So are we discussing below whether we can have fixed channel, id, lun 
per ATA sdev per shost? If so, I don't think it would work as libsas 
uses dynamic target ids per host.

>>> Typically, the SCSI layer has two ways of scanning.
>>> One it the old-style serial scanning (originating in the old SCSI
>>> parallel model):
>>> The scanning code will blindly scan _all_ devices up to max_luns, and
>>> attach every device for which the scanning code returns 'OK'.
>>> The other one is to issue REPORT_LUNS and scan all LUNs returned there.
>>>
>>> Mapped to libata we would need to figure out a stable SCSI enumeration,
>>> given that we have PMP and slave devices to content with.
>>> To my knowledge we have ATA ports, each can have either a 'master' and
>>> 'slave' device, _or_ it be a PMP port when it can support up to 16
>>> devices, right?
>>
>> yes
>>
>>> Point being, master/slave and PMP are exclusive, right?
>>
>> Never heard of pmp with ide cable :)
>>
> See?
> 
>>> So we can make the master as LUN 0, and the slave as LUN 1.
>>
>> Yes, but isn't that a little wrong ? That would assume that the ata port
>> is the device and the ata devices the luns of that device. But beside
>> the "link busy" stuff that needs to be dealt with, master and slave are
>> independent devices, unlike LUNs. No ?
>> Well; technically, yes.
> 
> But we already enumerate the ata ports (which is typically done by the 
> hardware/PCI layer etc), and if we were try to model slave devices as 
> independent ports we would either have to insert a numbering (awkward) 
> or add a number at the en (even more awkward).
> 
> And the one key takeaway from the 'multiple actuators' discussion is 
> that LUNs _are_ independent (cf all the hoops they had to jump through 
> to define a command spanning several LUNs ...)(which, incidentally, we 
> could leverage here, too ...), and the target port really only serves as 
> an enumeration thingie to address the LUNs.
> 
> So we _could_ map the master device on LUN 0 and the slave device on LUN 
> 1 with no loss of functionality, _but_ enable a consistent SCSI enumeration

Thanks,
John