[edk2-devel] [PATCH 1/2] MdeModulePkg/SataControllerDxe: Add support for drives in RAID mode

[Wu, Hao A]

Hello Vitaly,

It sounds to me that the controller driver should properly reflect the mode
according to the actual configuration. Is it possible to update the behavior of
the controller driver?

In my opinion, it seems weird to add WA in these general drivers for platform
driver issue. It might also cause confusion for users of AtaAtapiPassThru to
think it has RAID support.

Best Regards,
Hao Wu

> -----Original Message-----
> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Vitaly
> Cheptsov
> Sent: Monday, December 14, 2020 3:34 PM
> To: Wu, Hao A <hao.a.wu at intel.com>
> Cc: devel at edk2.groups.io; Wang, Jian J <jian.j.wang at intel.com>; Albecki,
> Mateusz <mateusz.albecki at intel.com>; Laszlo Ersek <lersek at redhat.com>
> Subject: Re: [edk2-devel] [PATCH 1/2] MdeModulePkg/SataControllerDxe: Add
> support for drives in RAID mode
> 
> Hello Hao,
> 
> This is for the case when the drives are not used as a RAID, but the controller is
> initialised in RAID mode. However, you are right that if a dedicated RAID driver is
> present, it is best to use it instead. To support both cases can we introduce an
> off-by-default PCD (e.g. TreatRaidAsSata) to workaround this?
> 
> Best regards,
> Vitaly
> 
> > On 14 Dec 2020, at 09:22, Wu, Hao A <hao.a.wu at intel.com> wrote:
> >
> > 
> >>
> >> -----Original Message-----
> >> From: Vitaly Cheptsov <cheptsov at ispras.ru>
> >> Sent: Friday, December 11, 2020 5:25 PM
> >> To: devel at edk2.groups.io
> >> Cc: Vitaly Cheptsov <cheptsov at ispras.ru>; Wang, Jian J
> >> <jian.j.wang at intel.com>; Wu, Hao A <hao.a.wu at intel.com>; Albecki,
> >> Mateusz <mateusz.albecki at intel.com>; Laszlo Ersek <lersek at redhat.com>
> >> Subject: [PATCH 1/2] MdeModulePkg/SataControllerDxe: Add support for
> >> drives in RAID mode
> >>
> >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3118
> >>
> >> 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.
> >
> >
> > Hello Vitaly,
> >
> > If my understanding is correct, this driver (SataControllerDxe) and
> > the AtaAtapiPassThru driver are written for non-RAID case only.
> >
> > 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.
> >
> > Best Regards,
> > Hao Wu
> >
> >
> >>
> >> Cc: Jian J Wang <jian.j.wang at intel.com>
> >> Cc: Hao A Wu <hao.a.wu at intel.com>
> >> Cc: Mateusz Albecki <mateusz.albecki at intel.com>
> >> Cc: Laszlo Ersek <lersek at redhat.com>
> >> Signed-off-by: Vitaly Cheptsov <cheptsov at ispras.ru>
> >> ---
> >> MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> 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;
> >>   }
> >>
> >> -  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;
> >>   }
> >>
> >> @@ -465,7 +465,7 @@ SataControllerStart (
> >>   if (IS_PCI_IDE (&PciData)) {
> >>     Private->IdeInit.ChannelCount = IDE_MAX_CHANNEL;
> >>     Private->DeviceCount          = 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 based).
> >>     //
> >> --
> >> 2.24.3 (Apple Git-128)
> >
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#68762): https://edk2.groups.io/g/devel/message/68762
Mute This Topic: https://groups.io/mt/78875596/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-