On Tue, Apr 11, 2023 at 04:38:48PM +0900, Damien Le Moal wrote: > On 4/11/23 16:23, Christoph Hellwig wrote: > > On Tue, Apr 11, 2023 at 04:09:34PM +0900, Damien Le Moal wrote: > >> But yes, I guess we could just unconditionally enable CDL for ATA on device scan > >> to be on par with scsi, which has CDL always enabled. > > > > I'd prefer that. With a module option to not enable it just to be > > safe. > > Then it may be better to move the cdl_enable attribute store definition for ATA > devices to libata. That would be less messy that way. Let me see if that can be > done cleanly. I don't think that the SCSI mode select can just be replaced with simple SET FEATURES in libata. If we move the SET FEATURES call to a function in libata, and then use a function pointer in the scsi_host_template, and let only libata set this function pointer (similar to e.g. how the queue_depth sysfs attribute works), then the code will no longer work for SATA devices connected to a SAS HBA. The current code simply checks if VPD page89 (the ATA Information VPD page - which is defined in the SCSI to ATA Translation (SAT) standard) exists. This page (and thus the sdev->vpd_pg89 pointer) will only exist if either: 1) An ATA device is connected to a SATA controller. This page will then be implemented by libata. 2) An ATA device is connected to a SAS HBA. The SAS HB will then provide this page. (The page will not exist for SCSI devices connected to the same SAS HBA.) For case 1) with the current code, scsi.c will call scsi_mode_select() which will be translated by libata before being sent down to the device. For case 2) with the current code, scsi.c will send down a SCSI mode select to the SAS HBA, and the SAS HBA will be responsible for translating the command before sending it to the device. So I actually think that the current way to check if vpd page89 exists is probably the "cleanest" solution that I can think of. If you have a better suggestion that will work for both case 1) and case 2), I will be happy to change the code accordingly. Kind regards, Niklas
