Re: [PULL 4/5] hw/ufs: Support for UFS logical unit

[Jeuk Kim <jeuk20.kim@gmail.com>]

On 2023-09-15 4:59 PM, Paolo Bonzini wrote:
> On 9/15/23 00:19, Jeuk Kim wrote:
>> First, ufs-lu has a feature called "unit descriptor". This feature 
>> shows the status of the ufs-lu
>>
>> and only works with UFS-specific "query request" commands, not SCSI 
>> commands.
>
> This looks like something that can be implemented in the UFS subsystem.
>
>> UFS also has something called a well-known lu. Unlike typical SCSI 
>> devices, where each lu is independent,
>> UFS can control other lu's through the well-known lu.
>
> This can also be implemented in UfsBus.
>
>> Finally, UFS-LU will have features that SCSI-HD does not have, such 
>> as the zone block command.
>
> These should be implemented in scsi-hd as well.
>
>> In addition to this, I wanted some scsi commands to behave 
>> differently from scsi-hd, for example,
>> the Inquiry command should read "QEMU UFS" instead of "QEMU HARDDISK",
>> and the mode_sense_page command should have a different result.
>
> Some of these don't have much justification, and others (such as the 
> control page) could be done in scsi-hd as well.
>
> We should look into cleaning this up and making ufs-lu share a lot 
> more code with scsi-hd; possibly even supporting -device scsi-hd with 
> UFS devices.  I am not going to ask you for a revert, but if this is 
> not done before 8.2 is out, I will ask you to disable it by default in 
> hw/ufs/Kconfig.
>
> In the future, please Cc the SCSI maintainers for UFS patches.
>
> Paolo
>
Dear Paolo

Hi. I've been looking into how ufs-lu can share code with scsi-hd.

I have verified that ufs-lu can use scsi-hd's code, and I would like to modify it to do so.

I've validated two possible fixes.
I'd like to hear your thoughts.

Option1.
As you mentioned, using ufsbus, which inherits from scsibus, removes the ufs-lu device type and use scsi-hd. (like -device ufs,id=ufs0 -device scsi-hd,bus=ufs0)
I've verified that this method is implementable, except for one problem.
Because we are using the scsi-hd type instead of the ufs-lu type, the ufs has to manage all the ufs-lu specific data (such as the unit descriptor).
However, since there is no ufs_lu_realize() function, we need a way to notify the ufs when a new scsi-hd device is added.
Would there be a way to let the ufs know that a new scsi-hd has been added at scsi_hd_realize() time?

Option 2.
Use qdev_new() & qdev_realize() to make ufs-lu have a virtual scsi bus and scsi-hd.
The ufs-lu can pass through SCSI commands to the virtual scsi-hd.
This is similar to the method used by the device "usb-storage".

With this method, I can keep the ufs-lu device type (ufs_lu_realize() makes it convenient to manage ufs-lu related data) and avoid duplicating code with scsi-hd.
So I prefer this approach, but the annotation for "usb-storage" is marked with a "Hack alert", so I'm not sure if this is the right way.
The code can be found in usb_msd_storage_realize() (hw/usb/dev-storage-classic.c:51).

I am wondering if you could give me some advice on this and your preferred direction for fixing it.

Thank you so much.

Jeuk