From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.ispras.ru (mail.ispras.ru [83.149.199.84]) by mx.groups.io with SMTP id smtpd.web09.31974.1607934531018373338 for ; Mon, 14 Dec 2020 00:28:51 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: ispras.ru, ip: 83.149.199.84, mailfrom: cheptsov@ispras.ru) Received: from [10.75.227.233] (unknown [213.87.155.74]) by mail.ispras.ru (Postfix) with ESMTPSA id ED61740A3659; Mon, 14 Dec 2020 08:28:48 +0000 (UTC) From: "Vitaly Cheptsov" Mime-Version: 1.0 (1.0) Subject: Re: [edk2-devel] [PATCH 1/2] MdeModulePkg/SataControllerDxe: Add support for drives in RAID mode Date: Mon, 14 Dec 2020 11:28:48 +0300 Message-Id: References: Cc: devel@edk2.groups.io, "Wang, Jian J" , "Albecki, Mateusz" , Laszlo Ersek In-Reply-To: To: "Wu, Hao A" X-Mailer: iPhone Mail (18B92) Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hello Hao, No, it is not possible to change that, since there is no firmware preferen= ce for that. The firmware does not support UEFI, and we are running through= DuetPkg. I believe it is not quite a workaround as the fact that an actual RAID arr= ay is installed and the fact that a RAID array is supported are separate ma= tters and may not be distinguishable via IS_RAID. At least this is how Inte= l controllers work to date. A clear name for the PCD should not cause any c= onfusion. If you think TreatRaidAsSata is not great, we could choose ForceR= aidAsSingleDrive. Best regards, Vitaly > On 14 Dec 2020, at 10:56, Wu, Hao A wrote: >=20 > =EF=BB=BFHello Vitaly, >=20 > It sounds to me that the controller driver should properly reflect the m= ode > according to the actual configuration. Is it possible to update the beha= vior of > the controller driver? >=20 > In my opinion, it seems weird to add WA in these general drivers for pla= tform > driver issue. It might also cause confusion for users of AtaAtapiPassThr= u to > think it has RAID support. >=20 > Best Regards, > Hao Wu >=20 >> -----Original Message----- >> From: devel@edk2.groups.io On Behalf Of Vitaly >> Cheptsov >> Sent: Monday, December 14, 2020 3:34 PM >> To: Wu, Hao A >> Cc: devel@edk2.groups.io; Wang, Jian J ; Albecki= , >> Mateusz ; Laszlo Ersek >> Subject: Re: [edk2-devel] [PATCH 1/2] MdeModulePkg/SataControllerDxe: A= dd >> support for drives in RAID mode >>=20 >> Hello Hao, >>=20 >> This is for the case when the drives are not used as a RAID, but the co= ntroller is >> initialised in RAID mode. However, you are right that if a dedicated RA= ID driver is >> present, it is best to use it instead. To support both cases can we int= roduce an >> off-by-default PCD (e.g. TreatRaidAsSata) to workaround this? >>=20 >> Best regards, >> Vitaly >>=20 >>>> On 14 Dec 2020, at 09:22, Wu, Hao A wrote: >>>=20 >>>=20 >>>>=20 >>>> -----Original Message----- >>>> From: Vitaly Cheptsov >>>> Sent: Friday, December 11, 2020 5:25 PM >>>> To: devel@edk2.groups.io >>>> Cc: Vitaly Cheptsov ; Wang, Jian J >>>> ; Wu, Hao A ; Albecki, >>>> Mateusz ; Laszlo Ersek >>>> Subject: [PATCH 1/2] MdeModulePkg/SataControllerDxe: Add support for >>>> drives in RAID mode >>>>=20 >>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D3118 >>>>=20 >>>> This resolves the problem of using drivers connected to Intel G33 >>>> builtin SATA controller when run from DuetPkg when it can only be >>>> configured in RAID mode through the firmware settings. >>>=20 >>>=20 >>> Hello Vitaly, >>>=20 >>> If my understanding is correct, this driver (SataControllerDxe) and >>> the AtaAtapiPassThru driver are written for non-RAID case only. >>>=20 >>> Both drivers (especially AtaAtapiPassThru) do not distinguish >>> logic/physical SCSI channels, which I think only works for the >>> non-RAID case. I am not sure if this patch series will have an impact = to existing >> RAID drivers. >>>=20 >>> Best Regards, >>> Hao Wu >>>=20 >>>=20 >>>>=20 >>>> Cc: Jian J Wang >>>> Cc: Hao A Wu >>>> Cc: Mateusz Albecki >>>> Cc: Laszlo Ersek >>>> Signed-off-by: Vitaly Cheptsov >>>> --- >>>> MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c | 4 ++-- >>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>>=20 >>>> diff --git a/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c >>>> b/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c >>>> index ab06e2833c..301335c967 100644 >>>> --- a/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c >>>> +++ b/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c >>>> @@ -324,7 +324,7 @@ SataControllerSupported ( >>>> return EFI_UNSUPPORTED; >>>> } >>>>=20 >>>> - if (IS_PCI_IDE (&PciData) || IS_PCI_SATADPA (&PciData)) { >>>> + if (IS_PCI_IDE (&PciData) || IS_PCI_SATADPA (&PciData) || >>>> + IS_PCI_RAID >>>> + (&PciData)) { >>>> return EFI_SUCCESS; >>>> } >>>>=20 >>>> @@ -465,7 +465,7 @@ SataControllerStart ( >>>> if (IS_PCI_IDE (&PciData)) { >>>> Private->IdeInit.ChannelCount =3D IDE_MAX_CHANNEL; >>>> Private->DeviceCount =3D IDE_MAX_DEVICES; >>>> - } else if (IS_PCI_SATADPA (&PciData)) { >>>> + } else if (IS_PCI_SATADPA (&PciData) || IS_PCI_RAID (&PciData)) { >>>> // >>>> // Read Ports Implemented(PI) to calculate max port number (0 base= d). >>>> // >>>> -- >>>> 2.24.3 (Apple Git-128) >>>=20 >>=20 >>=20 >>=20 >>=20 >=20