diff mbox series

[v2,14/15] scsi: ufs: ufs-exynos: multi-host configuration for exynosauto

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

Commit Message

Chanho Park July 14, 2021, 7:11 a.m. UTC
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(+)

Comments

Bean Huo July 26, 2021, 9:08 a.m. UTC | #1
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
Chanho Park July 26, 2021, 10:40 a.m. UTC | #2
> > 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
Bean Huo July 26, 2021, 9:58 p.m. UTC | #3
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
Bean Huo July 26, 2021, 10:06 p.m. UTC | #4
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;
> 
> +}
Chanho Park July 27, 2021, 9:05 a.m. UTC | #5
> > +       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
Chanho Park July 27, 2021, 10:14 a.m. UTC | #6
> > 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
Bean Huo July 27, 2021, 7:36 p.m. UTC | #7
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
Chanho Park July 28, 2021, 1:39 a.m. UTC | #8
> > > 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
Bean Huo July 29, 2021, 9:10 p.m. UTC | #9
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 mbox series

Patch

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 = {