public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Laszlo Ersek" <lersek@redhat.com>
To: devel@edk2.groups.io, sumana.venur@intel.com
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/Bus/Pci/PciBusDxe: Increase the width of the data read during oprom shadow
Date: Thu, 7 Jan 2021 14:31:50 +0100	[thread overview]
Message-ID: <0b1f805e-6dad-219d-7989-43be64ffa376@redhat.com> (raw)
In-Reply-To: <20210107062959.25388-1-sumana.venur@intel.com>

On 01/07/21 07:29, Sumana Venur wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2989
> 
> Long times spent on shadowing oprom from graphics card to system memory. We are currently using 8 bit read cycles.
> This needs to be wider, at least 32bit (even 64bit) reads to reduce the time for oprom shadow
> 
> Signed-off-by: Sumana Venur <sumana.venur@intel.com>
> ---
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
> index c994ed5fe3..fe6c7db417 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
> @@ -521,7 +521,7 @@ LoadOpRomImage (
>      //
>      PciDevice->PciRootBridgeIo->Mem.Read (
>                                        PciDevice->PciRootBridgeIo,
> -                                      EfiPciWidthUint8,
> +                                      EfiPciWidthUint64,
>                                        RomBar,
>                                        (UINT32) RomImageSize,
>                                        Image
> 

This has three bugs:

(1) From the UEFI specification v2.8B,
EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL.Mem.Read():

    The caller is responsible for satisfying any alignment and memory
    width restrictions that a PCI Root Bridge on a platform might
    require. For example on some platforms, width requests of
    EfiPciWidthUint64 do not work.

I don't think the patch ensures that EfiPciWidthUint64 is safe on the
particular platform.

(2) RomImageSize (the Count parameter) is expressed in bytes, currently.
If you scale up the access width, you have to scale down the access count.

(3) If you use an access size that is larger than 1 byte, you have to
handle the tail separately. For example, if the access width is
EfiPciWidthUint32, and the ROM image size is (4*k+3) bytes, then the
tail needs to be handled separately.

Laszlo


  reply	other threads:[~2021-01-07 13:32 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-07  6:29 [PATCH] MdeModulePkg/Bus/Pci/PciBusDxe: Increase the width of the data read during oprom shadow Sumana Venur
2021-01-07 13:31 ` Laszlo Ersek [this message]
2021-01-08  2:37   ` [edk2-devel] " Sumana Venur
  -- strict thread matches above, loose matches on Subject: below --
2021-01-08  2:32 Sumana Venur
2021-01-08  3:31 ` [edk2-devel] " Michael D Kinney
2021-01-08  4:05 Sumana Venur
2021-01-08  8:16 ` [edk2-devel] " Laszlo Ersek
2021-01-10  7:52   ` Ni, Ray

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=0b1f805e-6dad-219d-7989-43be64ffa376@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