[PATCH V3] ahci: Marvell 88SE9215 controllers prefer DMA for ATAPI

[PATCH V3] ahci: Marvell 88SE9215 controllers prefer DMA for ATAPI

[Huacai Chen]

We use CD/DVD drives under Marvell 88SE9215 SATA controller on many
Loongson-based machines. We found its PIO doesn't work well, and on the
opposite its DMA seems work very well. We don't know the detail of the
88SE9215 SATA controller, but we have tested different CD/DVD drives
and they all have problems under 88SE9215 (but they all work well under
an Intel SATA controller). So we can define a new dedicated AHCI board
id named board_ahci_yes_fbs_atapi_dma for 88SE9215, and for this id we
set the AHCI_HFLAG_ATAPI_DMA_QUIRK and ATA_QUIRK_ATAPI_MOD16_DMA flags
on the SATA controller to prefer ATAPI DMA.

Reported-by: Yuli Wang <wangyuli@uniontech.com>
Tested-by: Jie Fan <fanjie@uniontech.com>
Tested-by: Erpeng Xu <xuerpeng@uniontech.com>
Tested-by: Yuli Wang <wangyuli@uniontech.com>
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
---
 drivers/ata/ahci.c    | 11 ++++++++++-
 drivers/ata/ahci.h    |  1 +
 drivers/ata/libahci.c |  4 ++++
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 52ae8f9a7dd6..f3a6bfe098cd 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -63,6 +63,7 @@ enum board_ids {
 	board_ahci_pcs_quirk_no_devslp,
 	board_ahci_pcs_quirk_no_sntf,
 	board_ahci_yes_fbs,
+	board_ahci_yes_fbs_atapi_dma,
 
 	/* board IDs for specific chipsets in alphabetical order */
 	board_ahci_al,
@@ -188,6 +189,14 @@ static const struct ata_port_info ahci_port_info[] = {
 		.udma_mask	= ATA_UDMA6,
 		.port_ops	= &ahci_ops,
 	},
+	[board_ahci_yes_fbs_atapi_dma] = {
+		AHCI_HFLAGS	(AHCI_HFLAG_YES_FBS |
+				 AHCI_HFLAG_ATAPI_DMA_QUIRK),
+		.flags		= AHCI_FLAG_COMMON,
+		.pio_mask	= ATA_PIO4,
+		.udma_mask	= ATA_UDMA6,
+		.port_ops	= &ahci_ops,
+	},
 	/* by chipsets */
 	[board_ahci_al] = {
 		AHCI_HFLAGS	(AHCI_HFLAG_NO_PMP | AHCI_HFLAG_NO_MSI),
@@ -590,7 +599,7 @@ static const struct pci_device_id ahci_pci_tbl[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_MARVELL_EXT, 0x91a3),
 	  .driver_data = board_ahci_yes_fbs },
 	{ PCI_DEVICE(PCI_VENDOR_ID_MARVELL_EXT, 0x9215),
-	  .driver_data = board_ahci_yes_fbs },
+	  .driver_data = board_ahci_yes_fbs_atapi_dma },
 	{ PCI_DEVICE(PCI_VENDOR_ID_MARVELL_EXT, 0x9230),
 	  .driver_data = board_ahci_yes_fbs },
 	{ PCI_DEVICE(PCI_VENDOR_ID_MARVELL_EXT, 0x9235),
diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index c842e2de6ef9..2c10c8f440d1 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -246,6 +246,7 @@ enum {
 	AHCI_HFLAG_NO_SXS		= BIT(26), /* SXS not supported */
 	AHCI_HFLAG_43BIT_ONLY		= BIT(27), /* 43bit DMA addr limit */
 	AHCI_HFLAG_INTEL_PCS_QUIRK	= BIT(28), /* apply Intel PCS quirk */
+	AHCI_HFLAG_ATAPI_DMA_QUIRK	= BIT(29), /* force ATAPI to use DMA */
 
 	/* ap->flags bits */
 
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index e7ace4b10f15..22afa4ff860d 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -1322,6 +1322,10 @@ static void ahci_dev_config(struct ata_device *dev)
 {
 	struct ahci_host_priv *hpriv = dev->link->ap->host->private_data;
 
+	if ((dev->class == ATA_DEV_ATAPI) &&
+	    (hpriv->flags & AHCI_HFLAG_ATAPI_DMA_QUIRK))
+		dev->quirks |= ATA_QUIRK_ATAPI_MOD16_DMA;
+
 	if (hpriv->flags & AHCI_HFLAG_SECT255) {
 		dev->max_sectors = 255;
 		ata_dev_info(dev,
-- 
2.47.1

Re: [PATCH V3] ahci: Marvell 88SE9215 controllers prefer DMA for ATAPI

[Niklas Cassel]

On Wed, Mar 12, 2025 at 09:46:54PM +0800, Huacai Chen wrote:
> We use CD/DVD drives under Marvell 88SE9215 SATA controller on many
> Loongson-based machines. We found its PIO doesn't work well, and on the
> opposite its DMA seems work very well. We don't know the detail of the
> 88SE9215 SATA controller, but we have tested different CD/DVD drives
> and they all have problems under 88SE9215 (but they all work well under
> an Intel SATA controller). So we can define a new dedicated AHCI board
> id named board_ahci_yes_fbs_atapi_dma for 88SE9215, and for this id we
> set the AHCI_HFLAG_ATAPI_DMA_QUIRK and ATA_QUIRK_ATAPI_MOD16_DMA flags
> on the SATA controller to prefer ATAPI DMA.

This is a wall of text.

Could you please use paragraphs? (with an empty line between paragraphs).

(There can be multiple sentences in one paragraph.)

This is a good example:

commit 6bdbb73dc8d99fbb77f5db79dbb6f108708090b4
Author: Bibo Mao <maobibo@loongson.cn>
Date:   Sat Mar 8 13:52:04 2025 +0800

    LoongArch: KVM: Fix GPA size issue about VM
    
    Physical address space is 48 bit on Loongson-3A5000 physical machine,
    however it is 47 bit for VM on Loongson-3A5000 system. Size of physical
    address space of VM is the same with the size of virtual user space (a
    half) of physical machine.
    
    Variable cpu_vabits represents user address space, kernel address space
    is not included (user space and kernel space are both a half of total).
    Here cpu_vabits, rather than cpu_vabits - 1, is to represent the size of
    guest physical address space.
    
    Also there is strict checking about page fault GPA address, inject error
    if it is larger than maximum GPA address of VM.
    
    Cc: stable@vger.kernel.org
    Signed-off-by: Bibo Mao <maobibo@loongson.cn>
    Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>



Otherwise, this looks good to me.


Daniel, could you please test on your system as well?



Kind regards,
Niklas

Re: [PATCH V3] ahci: Marvell 88SE9215 controllers prefer DMA for ATAPI

[Daniel Kral]

On 3/12/25 16:47, Niklas Cassel wrote:
> Daniel, could you please test on your system as well?

Unfortunately, I don't have the device available here onsite, but I 
forwarded the patch to the previous tester and hopefully he can make 
some time to give this another spin!

Best regards,
     Daniel

Re: [PATCH V3] ahci: Marvell 88SE9215 controllers prefer DMA for ATAPI

[Niklas Cassel]

Hello Huacai,

On Wed, Mar 12, 2025 at 04:47:55PM +0100, Niklas Cassel wrote:
> On Wed, Mar 12, 2025 at 09:46:54PM +0800, Huacai Chen wrote:
> > We use CD/DVD drives under Marvell 88SE9215 SATA controller on many
> > Loongson-based machines. We found its PIO doesn't work well, and on the
> > opposite its DMA seems work very well. We don't know the detail of the
> > 88SE9215 SATA controller, but we have tested different CD/DVD drives
> > and they all have problems under 88SE9215 (but they all work well under
> > an Intel SATA controller). So we can define a new dedicated AHCI board
> > id named board_ahci_yes_fbs_atapi_dma for 88SE9215, and for this id we
> > set the AHCI_HFLAG_ATAPI_DMA_QUIRK and ATA_QUIRK_ATAPI_MOD16_DMA flags
> > on the SATA controller to prefer ATAPI DMA.
> 
> This is a wall of text.
> 
> Could you please use paragraphs? (with an empty line between paragraphs).
> 
> (There can be multiple sentences in one paragraph.)
> 
> This is a good example:
> 
> commit 6bdbb73dc8d99fbb77f5db79dbb6f108708090b4
> Author: Bibo Mao <maobibo@loongson.cn>
> Date:   Sat Mar 8 13:52:04 2025 +0800
> 
>     LoongArch: KVM: Fix GPA size issue about VM
>     
>     Physical address space is 48 bit on Loongson-3A5000 physical machine,
>     however it is 47 bit for VM on Loongson-3A5000 system. Size of physical
>     address space of VM is the same with the size of virtual user space (a
>     half) of physical machine.
>     
>     Variable cpu_vabits represents user address space, kernel address space
>     is not included (user space and kernel space are both a half of total).
>     Here cpu_vabits, rather than cpu_vabits - 1, is to represent the size of
>     guest physical address space.
>     
>     Also there is strict checking about page fault GPA address, inject error
>     if it is larger than maximum GPA address of VM.
>     
>     Cc: stable@vger.kernel.org
>     Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>     Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> 
> 
> 
> Otherwise, this looks good to me.


If you want this patch to be queued up for 6.15, please send a new version
this week, because after this week, it will instead be queued up for 6.16.


Kind regards,
Niklas

Re: [PATCH V3] ahci: Marvell 88SE9215 controllers prefer DMA for ATAPI

[Huacai Chen]

On Tue, Mar 18, 2025 at 4:02 PM Niklas Cassel <cassel@kernel.org> wrote:
>
> Hello Huacai,
>
> On Wed, Mar 12, 2025 at 04:47:55PM +0100, Niklas Cassel wrote:
> > On Wed, Mar 12, 2025 at 09:46:54PM +0800, Huacai Chen wrote:
> > > We use CD/DVD drives under Marvell 88SE9215 SATA controller on many
> > > Loongson-based machines. We found its PIO doesn't work well, and on the
> > > opposite its DMA seems work very well. We don't know the detail of the
> > > 88SE9215 SATA controller, but we have tested different CD/DVD drives
> > > and they all have problems under 88SE9215 (but they all work well under
> > > an Intel SATA controller). So we can define a new dedicated AHCI board
> > > id named board_ahci_yes_fbs_atapi_dma for 88SE9215, and for this id we
> > > set the AHCI_HFLAG_ATAPI_DMA_QUIRK and ATA_QUIRK_ATAPI_MOD16_DMA flags
> > > on the SATA controller to prefer ATAPI DMA.
> >
> > This is a wall of text.
> >
> > Could you please use paragraphs? (with an empty line between paragraphs).
> >
> > (There can be multiple sentences in one paragraph.)
> >
> > This is a good example:
> >
> > commit 6bdbb73dc8d99fbb77f5db79dbb6f108708090b4
> > Author: Bibo Mao <maobibo@loongson.cn>
> > Date:   Sat Mar 8 13:52:04 2025 +0800
> >
> >     LoongArch: KVM: Fix GPA size issue about VM
> >
> >     Physical address space is 48 bit on Loongson-3A5000 physical machine,
> >     however it is 47 bit for VM on Loongson-3A5000 system. Size of physical
> >     address space of VM is the same with the size of virtual user space (a
> >     half) of physical machine.
> >
> >     Variable cpu_vabits represents user address space, kernel address space
> >     is not included (user space and kernel space are both a half of total).
> >     Here cpu_vabits, rather than cpu_vabits - 1, is to represent the size of
> >     guest physical address space.
> >
> >     Also there is strict checking about page fault GPA address, inject error
> >     if it is larger than maximum GPA address of VM.
> >
> >     Cc: stable@vger.kernel.org
> >     Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >     Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> >
> >
> >
> > Otherwise, this looks good to me.
>
>
> If you want this patch to be queued up for 6.15, please send a new version
> this week, because after this week, it will instead be queued up for 6.16.
OK, I will send V4 today, sorry for the delay.

Huacai
>
>
> Kind regards,
> Niklas