Re: [PATCH v1 1/1] MdeModulePkg/NvmExpressDxe: Allow other NSIDs for Admin commands

[tyler.j.erickson@seagate.com]

[-- Attachment #1: Type: text/plain, Size: 3334 bytes --]

Hi Hao,

I tried making both the NamespaceID and NSID values the same when calling
the passthru function for these admin commands and it didn't work. I think
that is due to another place in the passthru code filtering the NamespaceId
input values on line 517-520.
The NamespaceId parameter is being checked to make sure it isn't greater
than the number of supported namespaces by the controller and it makes sure
it isn't set to (UINT32)-1 (All F's).
I think this is correct since the NamespaceId input is what is discovered
when calling the EFI_NVM_EXPRESS_PASS_THRU_GET_NEXT_NAMESPACE function.
This is what I used to get the available namespaces available in the system.

In my case I'm sending some commands in my application with the NSID set to
UINT32_MAX in order to request controller wide SMART/Health log data among
other things. There are some commands where setting the NSID to another
value may also be useful. For example, DST can use the NSID in the command
to change between testing only the controller (0), testing all namespaces
(UINT32_MAX), and a specific namespace.

The modification I made was done where the passthru command's NSID was
actually being checked, which seemed like the best place to add this
exception for admin commands.

-Tyler


On Tue, Sep 3, 2019 at 9:39 PM Wu, Hao A <hao.a.wu@intel.com> wrote:

> > -----Original Message-----
> > From: Wu, Hao A
> > Sent: Wednesday, September 04, 2019 11:39 AM
> > To: devel@edk2.groups.io; Tyler Erickson
> > Cc: Wu, Hao A; Ni, Ray
> > Subject: [PATCH v1 1/1] MdeModulePkg/NvmExpressDxe: Allow other NSIDs
> > for Admin commands
> >
> > Repost the mail to the list.
> >
> > Best Regards,
> > Hao Wu
> >
> > -----Original Message-----
> > From: Tyler Erickson [mailto:tyler.j.erickson@seagate.com]
> > Sent: Tuesday, September 03, 2019 9:55 PM
> > To: edk2-devel@lists.01.org
> > Cc: Wu, Hao A; Ni, Ray
> > Subject: [PATCH v1 1/1] MdeModulePkg/NvmExpressDxe: Allow other NSIDs
> > for Admin commands
> >
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Signed-off-by: Tyler Erickson <tyler.j.erickson@seagate.com>
> > ---
> >  MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> > b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> > index 8e721379466a..78a3c663ded4 100644
> > --- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> > +++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> > @@ -561,7 +561,7 @@ NvmExpressPassThru (
> >    Sq  = Private->SqBuffer[QueueId] + Private->SqTdbl[QueueId].Sqt;
> >    Cq  = Private->CqBuffer[QueueId] + Private->CqHdbl[QueueId].Cqh;
> >
> > -  if (Packet->NvmeCmd->Nsid != NamespaceId) {
> > +  if (Packet->QueueType != NVME_ADMIN_QUEUE && Packet->NvmeCmd-
> > >Nsid != NamespaceId) {
>
>
> Hello,
>
> Per my understanding to the codes, I think the:
>
> * Input parameter 'NamespaceId' for the PassThru() service
> * The 'Nsid' field of the EFI_NVM_EXPRESS_COMMAND
>
> are of the same meaning.
>
> Do you think setting these 2 values identical when calling the PassThru()
> service can resolve the issue you met?
>
> Best Regards,
> Hao Wu
>
>
> >      return EFI_INVALID_PARAMETER;
> >    }
> >
> > --
> > 2.17.1
>
>

[-- Attachment #2: Type: text/html, Size: 4704 bytes --]