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

["Vitaly Cheptsov" <cheptsov@ispras.ru>]

Hello Hao,

No, it is not possible to change that, since there is no firmware preference 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 array is installed and the fact that a RAID array is supported are separate matters and may not be distinguishable via IS_RAID. At least this is how Intel controllers work to date. A clear name for the PCD should not cause any confusion. If you think TreatRaidAsSata is not great, we could choose ForceRaidAsSingleDrive.

Best regards,
Vitaly

> On 14 Dec 2020, at 10:56, Wu, Hao A <hao.a.wu@intel.com> wrote:
> 
> 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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Vitaly
>> Cheptsov
>> Sent: Monday, December 14, 2020 3:34 PM
>> To: Wu, Hao A <hao.a.wu@intel.com>
>> Cc: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Albecki,
>> Mateusz <mateusz.albecki@intel.com>; Laszlo Ersek <lersek@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@intel.com> wrote:
>>> 
>>> 
>>>> 
>>>> -----Original Message-----
>>>> From: Vitaly Cheptsov <cheptsov@ispras.ru>
>>>> Sent: Friday, December 11, 2020 5:25 PM
>>>> To: devel@edk2.groups.io
>>>> Cc: Vitaly Cheptsov <cheptsov@ispras.ru>; Wang, Jian J
>>>> <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Albecki,
>>>> Mateusz <mateusz.albecki@intel.com>; Laszlo Ersek <lersek@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@intel.com>
>>>> Cc: Hao A Wu <hao.a.wu@intel.com>
>>>> Cc: Mateusz Albecki <mateusz.albecki@intel.com>
>>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>>> Signed-off-by: Vitaly Cheptsov <cheptsov@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)
>>> 
>> 
>> 
>> 
>> 
>