public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: Sajeesh Kk <sajee.kk@gmail.com>
Cc: Rafael Machado <rafaelrodrigues.machado@gmail.com>,
	edk2-devel@lists.01.org
Subject: Re: BlockIo2 Protocol test tool
Date: Mon, 4 Feb 2019 17:54:20 +0100	[thread overview]
Message-ID: <cf496fa2-416a-480a-46f6-22698964df48@redhat.com> (raw)
In-Reply-To: <CA+1y_6W3Ew0B26HXCeCh4o2keRJvM93z=efSihdhf2-qDgkThw@mail.gmail.com>

On 02/02/19 08:05, Sajeesh Kk wrote:
> Hello Laszlo,
> 
> Thanks for the quick response. I have some doubts regarding
> "TransactionStatus" that should be signaled by the driver when an IO
> operation is completed. If my understanding is not correct please educate.
> 
> 1. As per UEFI spec, the error codes EFI_DEVICE_ERROR, EFI_NO_MEDIA,
> EFI_MEDIA_CHANGED or EFI_WRITE_PROTECTED should not be signaled when non
> Blocking IO is being used. so if an IO fails, the driver should not signal
> the event as per the spec. if this is the case,
>     the event should be signaled only when IO is completed successfully ?

My understanding is that there are three cases:

(1) The IO completes successfully. In this case, the completion
notification is asynchronous (i.e., the event is signaled)

(2) The IO fails, and the BlockIo2 driver determines the failure
asynchronously. In this case, the original call returns EFI_SUCCESS, and
the error notification occurs again via signaling the event.

(3) The IO fails, but the failure is determined immediately
(synchronously), as part of the original call. In this case, no
background operation is queued (so there is nothing to signal an event
for later), and the error is returned immediately.


IMO, the spec requires the driver to check for the EFI_NO_MEDIA and the
EFI_MEDIA_CHANGED conditions *at least* synchronously, that is, to
return them (if they occur) *at least* as case (3).

Regarding EFI_DEVICE_ERROR, I think the driver is free to check for that
only asynchronously, when the actual attempt is being made, in the
background, to work with the device. Therefore, EFI_DEVICE_ERROR may be
reported both as case (2) and as case (3). What's important is that,
*if* the driver reports EFI_DEVICE_ERROR synchronously (3), then the
signaling (case (2)) should not occur -- as it would be redundant.

In summary:
- we have three cases about status reporting,
- some error codes *must* be checked-for synchronously (at least),
- some other error codes may be identified both synchronously and
asynchronously; in either case, they should be reported accordingly.


> 2. If the driver does not have enough resources to complete an IO request,
> the driver should signal the event with same status (EFI_OUT_OF_RESOURCES)
> so that caller will retry the IO ? In such cases, what should be the status
> of  ReadEx/WriteEx callbacks itself that should returned to the caller ?

IMO, the same logic applies to EFI_OUT_OF_RESOURCES as to
EFI_DEVICE_ERROR. If the driver sees immediately that it has lacking
resources, it should return EFI_OUT_OF_RESOURCES immediately. (For
example, if it lacks resources for even *queueing* the request, this
would be the right thing to do.) If the driver realizes only during
background processing that it has run out of resources, then
EFI_OUT_OF_RESOURCES should be returned through the token, asynchronously.

Note that EFI_OUT_OF_RESOURCES is documented as "The request could not
be *completed* due to a lack of resources". This includes both cases
above. (I.e., couldn't be queued, or could be queued, but not
completed.) Therefore both cases (2) and (3) apply, dependent on what
happens.

Now, whether a BlockIo2 driver tries to allocate all *possibly*
necessary resources in advance, when queueing a request, is a
Quality-of-Implementation question, IMO. I think it's not required by
the spec, but it could benefits user experience, and debugging too.
Synchronous error reports save higher level drivers the burden of async
context restoration and recovery --> fewer bugs hit by the user. Also,
it helps with debugging: the developer will get a usable call stack when
the issue hits.

OTOH, if a driver tries too hard to synchronously allocate all possibly
necessary resources for a request, even if there's a good chance that
part of those resources will never be used, then it's a waste. It could
cause other drivers to run out of resources, or it could fail some
requests that would otherwise complete fine in practice.

So it's up to your device & driver whether you should report
EFI_OUT_OF_RESOURCES *only* synchronously. I think it's valid to report
it asynchronously too.

> In short, i am bit confused what should be the "Transaction Status" that
> should be signaled by the driver. Please comment.

Hopefully the above helps.

Thanks,
Laszlo

> On Mon, Jan 28, 2019 at 2:17 PM Laszlo Ersek <lersek@redhat.com> wrote:
> 
>> Hi,
>>
>> On 01/26/19 17:57, Sajeesh Kk wrote:
>>> Hello Rafeal,
>>>
>>> I believe The UEFI Sct test suite cannot be used for performance testing.
>>> Are there any tools which can be used from UEFI shell to measure disk IO
>>> perfomance using BlockIO2 Protocol ?
>>> Please let me know.
>>
>> from a cursory look, FatPkg/EnhancedFatDxe appears to implement the the
>> ReadEx/WriteEx methods of EFI_FILE_PROTOCOL
>> (EFI_FILE_PROTOCOL_REVISION2). I think the implementation uses the
>> DiskIo2 protocol.
>>
>> And "MdeModulePkg/Universal/Disk/DiskIoDxe" should automatically install
>> DiskIo2 on top of your BlockIo2.
>>
>> So, if you can write a simple UEFI application that uses the *Ex()
>> methods of EFI_FILE_PROTOCOL, that could be a start. (Nothing in edk2
>> seems to use ReadEx / WriteEx currently.)
>>
>> Thanks,
>> Laszlo
>>
> 



      reply	other threads:[~2019-02-04 16:54 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-26  3:24 BlockIo2 Protocol test tool Sajeesh Kk
2019-01-26 17:10 ` Rafael Machado
2019-01-26 16:57   ` Sajeesh Kk
2019-01-27 11:05     ` Rafael Machado
2019-01-28  8:47     ` Laszlo Ersek
2019-02-02  7:05       ` Sajeesh Kk
2019-02-04 16:54         ` Laszlo Ersek [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cf496fa2-416a-480a-46f6-22698964df48@redhat.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox