| Message ID | 20210714071131.101204-15-chanho61.park@samsung.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2,01/15] scsi: ufs: add quirk to handle broken UIC command | expand |
On Wed, 2021-07-14 at 16:11 +0900, Chanho Park wrote: > UFS controller of ExynosAuto v9 SoC supports multi-host interface for > I/O > > virtualization. In general, we're using para-virtualized driver to > > support a block device by several virtual machines. However, it > should > > be relayed by backend driver. Multi-host functionality extends the > host > > controller by providing register interfaces that can be used by each > > VM's ufs drivers respectively. By this, we can provide direct access > to > > the UFS device for multiple VMs. It's similar with SR-IOV of PCIe. > Hi Chanho Park, very inteseted in this patch. you mentined SR-IOV. In order to make your patch work on the UFS, does UFS device side need changes? or the common current 3.1 UFS can work with this new controller? > > > We divide this M-HCI as PH(Physical Host) and VHs(Virtual Host). The > PH > > supports all UFSHCI functions(all SAPs) same as conventional UFSHCI > but > > the VH only supports data transfer function. Thus, except UTP_CMD_SAP > and > > UTP_TMPSAP, the PH should handle all the physical features. > > > > This patch provides an initial implementation of PH part. M-HCI can > > support up to four interfaces but this patch initially supports only > 1 > > PH and 1 VH. For this, we uses TASK_TAG[7:5] field so TASK_TAG[4:0] > for > > 32 doorbel will be supported. After the PH is initiated, this will > send I don't understand here. how many doorbell registers you have now? and doesn VHs have a doorbell register also? and each doorbell register still supprts 32 tags? Kind regards Beab
> > UFS controller of ExynosAuto v9 SoC supports multi-host interface for > > I/O > > > > virtualization. In general, we're using para-virtualized driver to > > > > support a block device by several virtual machines. However, it should > > > > be relayed by backend driver. Multi-host functionality extends the > > host > > > > controller by providing register interfaces that can be used by each > > > > VM's ufs drivers respectively. By this, we can provide direct access > > to > > > > the UFS device for multiple VMs. It's similar with SR-IOV of PCIe. > > > > Hi Chanho Park, > very inteseted in this patch. you mentined SR-IOV. In order to make your > patch work on the UFS, does UFS device side need changes? > > or the common current 3.1 UFS can work with this new controller? Our UFS controller was built on top of JEDEC 2.1 spec with some custom implementations to support this feature. UFS device side, no changes will be necessary. > > > > > > > > > We divide this M-HCI as PH(Physical Host) and VHs(Virtual Host). The > > PH > > > > supports all UFSHCI functions(all SAPs) same as conventional UFSHCI > > but > > > > the VH only supports data transfer function. Thus, except UTP_CMD_SAP > > and > > > > UTP_TMPSAP, the PH should handle all the physical features. > > > > > > > > This patch provides an initial implementation of PH part. M-HCI can > > > > support up to four interfaces but this patch initially supports only > > 1 > > > > PH and 1 VH. For this, we uses TASK_TAG[7:5] field so TASK_TAG[4:0] > > for > > > > 32 doorbel will be supported. After the PH is initiated, this will > > send > > > I don't understand here. how many doorbell registers you have now? > and doesn VHs have a doorbell register also? and each doorbell register > still supprts 32 tags? A PH has its own doorbell register and each VHs also has it as well. The TASK_TAG[7:5] can be used to distinguish the origin of the request among VHs and remaining TASK_TAG[4:0] will be used for supporting 32 tags. Best Regards, Chanho Park
On Mon, 2021-07-26 at 19:40 +0900, Chanho Park wrote: > > I don't understand here. how many doorbell registers you have now? > > and doesn VHs have a doorbell register also? and each doorbell > > register > > still supprts 32 tags? > > > A PH has its own doorbell register and each VHs also has it as well. > > The TASK_TAG[7:5] can be used to distinguish the origin of the > request among VHs and remaining TASK_TAG[4:0] will be used for > supporting 32 tags. > > > > Best Regards, > > Chanho Park Thanks for your reply. so you split the "Task Tag" filed byte3 in the UPIU header to two parts, bit7~bit5 is for the VHs ID, and bit4~bit0 is for the task ID. but this is not defined in the Spec 2.1. correct? Kind regards, Bean
On Wed, 2021-07-14 at 16:11 +0900, Chanho Park wrote: > > > +static int exynosauto_ufs_post_hce_enable(struct exynos_ufs *ufs) > > +{ > > + struct ufs_hba *hba = ufs->hba; > > + > > + /* Enable Virtual Host #1 */ > > + ufshcd_rmwl(hba, MHCTRL_EN_VH_MASK, MHCTRL_EN_VH(1), MHCTRL); > > + /* Default VH Transfer permissions */ > > + hci_writel(ufs, 0x03FFE1FE, HCI_MH_ALLOWABLE_TRAN_OF_VH); > > + /* IID information is replaced in TASKTAG[7:5] instead of IID > in UCD */ > > + hci_writel(ufs, 0x1, HCI_MH_IID_IN_TASK_TAG); Here you meant the IID in the UPIU header will be set according to the Task_tag[7:5] value? and this will be done by your HW controller or SW driver? Kind regards, Bean > > + > > + return 0; > > +}
> > + hci_writel(ufs, 0x1, HCI_MH_IID_IN_TASK_TAG); > > Here you meant the IID in the UPIU header will be set according to the > Task_tag[7:5] value? and this will be done by your HW controller or SW > driver? > You're right. We can manually set the value of TASK_TAG[7:5] by S/W but when the bit is set, a VHID value will be automatically written by H/W controller. Best Regards, Chanho Park
> > A PH has its own doorbell register and each VHs also has it as well. > > > > The TASK_TAG[7:5] can be used to distinguish the origin of the > > request among VHs and remaining TASK_TAG[4:0] will be used for > > supporting 32 tags. > > > > > > > > Best Regards, > > > > Chanho Park > > Thanks for your reply. > > so you split the "Task Tag" filed byte3 in the UPIU header to two > parts, bit7~bit5 is for the VHs ID, and bit4~bit0 is for the task ID. > but this is not defined in the Spec 2.1. correct? > You're right. For PH, TASK_TAG[7:5] will be set to "0" but a VHID will be used in case of VH. Best Regards, Chanho Park
On Tue, 2021-07-27 at 19:14 +0900, Chanho Park wrote: > > > A PH has its own doorbell register and each VHs also has it as > > > well. > > > The TASK_TAG[7:5] can be used to distinguish the origin of the > > > request among VHs and remaining TASK_TAG[4:0] will be used for > > > supporting 32 tags. > > > Best Regards, > > > Chanho Park > > Thanks for your reply. > > so you split the "Task Tag" filed byte3 in the UPIU header to two > > parts, bit7~bit5 is for the VHs ID, and bit4~bit0 is for the task > > ID. > > but this is not defined in the Spec 2.1. correct? > > > You're right. > > For PH, TASK_TAG[7:5] will be set to "0" but a VHID will be used in > case of VH. > > > > Best Regards, > > Chanho Park Hi Chanho Park, Thansk for yoru reply. I didn't see your changes about task_tag and IID. Having a look at ufshcd_prepare_utp_scsi_cmd_upiu(), the task tag in the UPIU header is still only task tag. and IID is always 0x00. If you didn't add these changes, your patch is un-readable, and also the driver doesn't have a real usage case. Also, you mentioned there is no support/change needed from the UFS device side. But, IMO, if you changed the UPIU header, there are changes needed on the UFS device side in order to use your driver. Kind regards, Bean
> > > so you split the "Task Tag" filed byte3 in the UPIU header to two > > > parts, bit7~bit5 is for the VHs ID, and bit4~bit0 is for the task > > > ID. > > > but this is not defined in the Spec 2.1. correct? > > > > > > You're right. > > > > For PH, TASK_TAG[7:5] will be set to "0" but a VHID will be used in > > case of VH. > > > > > > > > Best Regards, > > > > Chanho Park > > Hi Chanho Park, > Thansk for yoru reply. > > I didn't see your changes about task_tag and IID. Having a look at > ufshcd_prepare_utp_scsi_cmd_upiu(), the task tag in the UPIU header is > still only task tag. and IID is always 0x00. > > If you didn't add these changes, your patch is un-readable, and also the > driver doesn't have a real usage case. I already replied regarding this in another mail thread and please refer below comments. The controller will handle the tag translation. https://lore.kernel.org/linux-scsi/002901d782c6$937ac0f0$ba7042d0$@samsung.com/ > Also, you mentioned there is no support/change needed from the UFS device > side. But, IMO, if you changed the UPIU header, there are changes needed > on the UFS device side in order to use your driver. Please see the figure[1]. Once IID_IN_TASKTAG bit is set, the function arbiter will translate the value of UPIU header before delivering it to the device. [1]: https://lore.kernel.org/linux-scsi/20210714071131.101204-1-chanho61.park@samsung.com/ Best Regards, Chanho Park
On Wed, 2021-07-28 at 10:39 +0900, Chanho Park wrote: > > Hi Chanho Park, > > Thansk for yoru reply. > > I didn't see your changes about task_tag and IID. Having a look at > > ufshcd_prepare_utp_scsi_cmd_upiu(), the task tag in the UPIU header > > is > > still only task tag. and IID is always 0x00. > > If you didn't add these changes, your patch is un-readable, and > > also the > > driver doesn't have a real usage case. > > > I already replied regarding this in another mail thread and please > refer below comments. The controller will handle the tag translation. > > https://lore.kernel.org/linux-scsi/002901d782c6$937ac0f0$ba7042d0$@samsung.com/ > > > > > Also, you mentioned there is no support/change needed from the UFS > > device > > side. But, IMO, if you changed the UPIU header, there are changes > > needed > > on the UFS device side in order to use your driver. > > > Please see the figure[1]. Once IID_IN_TASKTAG bit is set, the > function arbiter will translate the value of UPIU header before > delivering it to the device. > > > > [1]: > https://lore.kernel.org/linux-scsi/20210714071131.101204-1-chanho61.park@samsung.com/ > > > > Best Regards, > > Chanho Park Hi Chanho Park, thansk for your further reply. I didn't find out your cover-letter in your patchset before. IID will be automatically set by HW controller according to the VH_ID in the tasg_tag byte[7:5]. Before sending this RPMB to UFS device, doese your controller reset task_tag byte[7:5] to 0x0 or your controller will keep VH_ID? one more question, you mentioned para-virtualization, did you use Xen or KVM? Kind regards, Bean
diff --git a/drivers/scsi/ufs/ufs-exynos.c b/drivers/scsi/ufs/ufs-exynos.c index 200137e6ecce..5b293a4ac6ea 100644 --- a/drivers/scsi/ufs/ufs-exynos.c +++ b/drivers/scsi/ufs/ufs-exynos.c @@ -83,6 +83,21 @@ #define UFS_SHARABLE (UFS_WR_SHARABLE | UFS_RD_SHARABLE) #define UFS_SHAREABILITY_OFFSET 0x710 +/* Multi-host registers */ +#define MHCTRL 0xC4 +#define MHCTRL_EN_VH_MASK (0xE) +#define MHCTRL_EN_VH(vh) (vh << 1) +#define PH2VH_MBOX 0xD8 + +#define MH_MSG_MASK (0xFF) + +#define MH_MSG(id, msg) ((id << 8) | (msg & 0xFF)) +#define MH_MSG_PH_READY 0x1 +#define MH_MSG_VH_READY 0x2 + +#define HCI_MH_ALLOWABLE_TRAN_OF_VH 0x30C +#define HCI_MH_IID_IN_TASK_TAG 0X308 + enum { UNIPRO_L1_5 = 0,/* PHY Adapter */ UNIPRO_L2, /* Data Link */ @@ -173,6 +188,20 @@ static int exynosauto_ufs_drv_init(struct device *dev, struct exynos_ufs *ufs) return 0; } +static int exynosauto_ufs_post_hce_enable(struct exynos_ufs *ufs) +{ + struct ufs_hba *hba = ufs->hba; + + /* Enable Virtual Host #1 */ + ufshcd_rmwl(hba, MHCTRL_EN_VH_MASK, MHCTRL_EN_VH(1), MHCTRL); + /* Default VH Transfer permissions */ + hci_writel(ufs, 0x03FFE1FE, HCI_MH_ALLOWABLE_TRAN_OF_VH); + /* IID information is replaced in TASKTAG[7:5] instead of IID in UCD */ + hci_writel(ufs, 0x1, HCI_MH_IID_IN_TASK_TAG); + + return 0; +} + static int exynosauto_ufs_pre_link(struct exynos_ufs *ufs) { struct ufs_hba *hba = ufs->hba; @@ -231,6 +260,20 @@ static int exynosauto_ufs_pre_pwr_change(struct exynos_ufs *ufs, return 0; } +static int exynosauto_ufs_post_pwr_change(struct exynos_ufs *ufs, + struct ufs_pa_layer_attr *pwr) +{ + struct ufs_hba *hba = ufs->hba; + u32 enabled_vh; + + enabled_vh = ufshcd_readl(hba, MHCTRL) & MHCTRL_EN_VH_MASK; + + /* Send physical host ready message to virtual hosts */ + ufshcd_writel(hba, MH_MSG(enabled_vh, MH_MSG_PH_READY), PH2VH_MBOX); + + return 0; +} + static int exynos7_ufs_pre_link(struct exynos_ufs *ufs) { struct ufs_hba *hba = ufs->hba; @@ -1395,8 +1438,10 @@ static struct exynos_ufs_drv_data exynosauto_ufs_drvs = { EXYNOS_UFS_OPT_SKIP_CONFIG_PHY_ATTR | EXYNOS_UFS_OPT_BROKEN_RX_SEL_IDX, .drv_init = exynosauto_ufs_drv_init, + .post_hce_enable = exynosauto_ufs_post_hce_enable, .pre_link = exynosauto_ufs_pre_link, .pre_pwr_change = exynosauto_ufs_pre_pwr_change, + .post_pwr_change = exynosauto_ufs_post_pwr_change, }; static struct exynos_ufs_drv_data exynos_ufs_drvs = {
UFS controller of ExynosAuto v9 SoC supports multi-host interface for I/O virtualization. In general, we're using para-virtualized driver to support a block device by several virtual machines. However, it should be relayed by backend driver. Multi-host functionality extends the host controller by providing register interfaces that can be used by each VM's ufs drivers respectively. By this, we can provide direct access to the UFS device for multiple VMs. It's similar with SR-IOV of PCIe. We divide this M-HCI as PH(Physical Host) and VHs(Virtual Host). The PH supports all UFSHCI functions(all SAPs) same as conventional UFSHCI but the VH only supports data transfer function. Thus, except UTP_CMD_SAP and UTP_TMPSAP, the PH should handle all the physical features. This patch provides an initial implementation of PH part. M-HCI can support up to four interfaces but this patch initially supports only 1 PH and 1 VH. For this, we uses TASK_TAG[7:5] field so TASK_TAG[4:0] for 32 doorbel will be supported. After the PH is initiated, this will send a ready message to VHs through a mailbox register. The message handler is not fully implemented yet such as supporting reset / abort cases. Signed-off-by: Chanho Park <chanho61.park@samsung.com> --- drivers/scsi/ufs/ufs-exynos.c | 45 +++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+)