Re: [PATCH V2 2/2] efi/fb: Convert PCI bus address to resource if translated by the bridge

[okaya]

On 2018-06-25 04:20, Ard Biesheuvel wrote:
> On 22 June 2018 at 21:29, Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> 
> wrote:
> > On 22 June 2018 at 20:30, Sinan Kaya <okaya@xxxxxxxxxxxxxx> wrote:
> > > On 6/22/2018 2:01 PM, Ard Biesheuvel wrote:
> > > > > Yes, it is part of the PCI I/O protocol definition. FrameBufferBase 
> > > > > is
> > > > > described as
> > > > >
> > > > > """
> > > > > Base address of graphics linear frame buffer. Info contains
> > > > > information required to allow software to draw directly to the
> > > > > frame buffer without using Blt().Offset zero in
> > > > > FrameBufferBase represents the upper left pixel of the
> > > > > display.
> > > > > """
> > > > I just tried AMD Radeon and NVidia graphics cards on a system with
> > > > non-1:1 mapped MMIO windows, and in both cases, the GOP protocol
> > > > structure is populated correctly, i.e., using the CPU address not 
> > > > the
> > > > PCIe address.
> > > >
> > > > EDK2 only recently gained support for MMIO translation in the host
> > > > bridge driver, so I so wonder if this is a platform issue rather 
> > > > than
> > > > a driver issue. It may be worth a try to dump the results of
> > > > GetBarAttributes() of all PCI I/O protocol instances (either in UEFI
> > > > or in the stub), to double check that the correct values are 
> > > > returned.
> > >
> > > Thanks for checking out other platforms. I'll mark the issue as a 
> > > BIOS
> > > issue and bounce your feedback to the BIOS provider.
> >
> > I screwed up my testing, unfortunately. Both the public AMD GOP driver
> > I tried, and the Nvidia GT218 under x86 emulation break when using
> > MMIO translation. However, GraphicsOutputDxe in the EDK2 tree gets it
> > right, and uses PciIo->GetBarAttributes() to get the address of the
> > framebuffer region, which will return the CPU address not the PCI
> > address.
> >
> > > Let's hold onto this patch for the moment.
> >
> > Yes. I'd like to get this resolved as well, but if the drivers are
> > dereferencing BAR values as CPU addresses, this is unlikely to be the
> > only thing that is broken under outbound translation.
>
> Note that this was fixed fairly recently in EDK2, so BIOS vendors
> providing UEFI firmware for ARM platforms with outbound MMIO
> translation should probably incorporate this EDK2 patch
>
> commit dc080d3b61e570e7a3163fc24afa6f8388d0c0bf
> Author: Heyi Guo <heyi.guo@xxxxxxxxxx>
> Date:   Thu Feb 8 11:13:27 2018 +0800
>
>     MdeModulePkg/PciBus: return CPU address for GetBarAttributes
>
>     According to UEFI spec 2.7, PciIo->GetBarAttributes should return 
> host
>     address (CPU view ddress) rather than device address (PCI view
>     address), and
>     device address = host address + address translation offset,
>     so we subtract translation from device address before returning.
>
> Note that this is not the only MMIO translation related change made by
> Heyi Guo to the generic PCI host bridge and bus drivers, but given
> that those did not support MMIO translation at all, I take it your
> affected platforms will already have their own changes to accommodate
> this.

Platform has been doing mmio translation for quite a while. Because all 
accesses go through pci io protocol, the rest of the UEFI never needed 
to be aware of bar address or do direct access.

This is the first time I hear of direct access. Maybe, GOP is a special 
case.

I started copying your response to the bios vendor.

They are probably missing that patch. I will pass it along.