On Wed, 2019-03-27 at 16:32 +0100, Ard Biesheuvel wrote:
> On Tue, 29 Jan 2019 at 14:55, Ryszard Knop <
> ryszard.k...@linux.intel.com> wrote:
> > +Team
> > 
> 
> As it turns out, this driver is still broken for non-1:1 mapped DMA.
> 
> In particular, I am hitting a crash on
> 
>       E1000MemCopy (
>         (UINT8 *) (UINTN) CpbReceive->BufferAddr,
>         (UINT8 *) (UINTN) ReceiveDescriptor->buffer_addr,
>         TempLen
>       );
> 
> (around line 676 in e1000.c), which uses the DMA address
> 'ReceiveDescriptor->buffer_addr' in a memory copy operation performed
> by the CPU. This causes a crash on systems where the DMA address is
> not also a valid CPU address.

Huh, this is new... I don't have access to any system behaving this
way, so I can't test this, but E1000.c -> E1000TxRxConfigure links
RxDesc->buffer_addr to the physical addresses, that descriptor is used
by the hardware to DMA data where needed, and we try to copy from that
same physical address later, while we should copy from unmapped
addresses instead.

This probably should be solved by having a separate array/something
with CurRxInd -> unmapped addresses, but I'll have to talk with my team
to solve this in a sensible way.

In the meantime, maybe you know if there's a way to simulate this
situation under QEMU or something?


> > On Tue, 2019-01-29 at 14:13 +0100, Ryszard Knop wrote:
> > > Hi Ard,
> > > 
> > > I've finally got some time to review and merge all of this. A bit
> > > problematic thing is that we internally have a separate tree that
> > > we
> > > need to merge those commits into, then generate the open source
> > > tree
> > > and related commits from that. This will result in somewhat
> > > broken
> > > history, so sorry about that in advance - we're still figuring
> > > out
> > > the
> > > proper way to handle multiple source trees on our end without
> > > messing
> > > it up. I'll push these changes to edk2-staging once we've got it
> > > all
> > > ready.
> > > 
> > > On Tue, 2018-11-06 at 18:58 +0100, ard.biesheuvela wrote:
> > > > This series fixes the GigUndiDxe in the edk2-staging/Intel_UNDI
> > > > branch
> > > > at github.com/tianocore so it can be built with GCC on Linux
> > > > for
> > > > ARM
> > > > and AARCH64 (as well as X64)
> > > > 
> > > > Ard Biesheuvel (19):
> > > >   IntelOpenSourceUndiPkg.dsc: add AARCH64 and ARM to supported
> > > >     architectures
> > > >   IntelUndiPkg: remove EOF markers
> > > >   IntelUndiPkg/GigUndiDxe: consistently use lowercase for e1000
> > > > in
> > > >     filenames
> > > >   IntelUndiPkg/GigUndiDxe: consistently use forward slashes as
> > > > path
> > > >     separators
> > > >   IntelUndiPkg/GigUndiDxe: move BRAND_STRUCT declaration after
> > > > type
> > > >     definition
> > > >   IntelUndiPkg/GigUndiDxe: use intermediate UINTN casts for
> > > > pointers
> > > >   IntelUndiPkg/GigUndiDxe: create GCC alternatives for MSFT
> > > > build
> > > >     options
> > > >   IntelUndiPkg/GigUndiDxe: add missing VOID** cast
> > > >   IntelUndiPkg/GigUndiDxe: add missing UINT8* cast
> > > >   IntelUndiPkg/GigUndiDxe: add missing braces to GUID literals
> > > >   IntelUndiPkg/GigUndiDxe: fix incorrect use of CPP token
> > > > pasting
> > > >   IntelUndiPkg/GigUndiDxe: cast E1000MemCopy () args to correct
> > > > pointer
> > > >     type
> > > >   IntelUndiPkg/GigUndiDxe: don't take address of cast
> > > > expression
> > > >   IntelUndiPkg/GigUndiDxe: redefine UNREFERENCED_nPARAMETER
> > > > macros
> > > > for
> > > >     GCC
> > > >   IntelUndiPkg/GigUndiDxe: remove forward declaration of non-
> > > > existent
> > > >     function
> > > >   IntelUndiPkg/GigUndiDxe: fix incorrect indentation
> > > >   IntelUndiPkg/GigUndiDxe: move MSFT warning overrides to INF
> > > > file
> > > >   IntelUndiPkg/GigUndiDxe: add missing EFIAPI modifiers
> > > >   IntelUndiPkg/GigUndiDxe: remove or reorganize unused
> > > > variables
> > > > 
> > > >  IntelUndiPkg/GigUndiDxe/AdapterInformation.c  |  6 ++-
> > > >  IntelUndiPkg/GigUndiDxe/AdapterInformation.h  |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/Brand.c               |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/ComponentName.c       |  5 ++-
> > > >  IntelUndiPkg/GigUndiDxe/ComponentName.h       |  2 +-
> > > >  IntelUndiPkg/GigUndiDxe/Decode.c              |  5 +--
> > > >  IntelUndiPkg/GigUndiDxe/Decode.h              |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/DeviceSupport.c       |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/DeviceSupport.h       |  9 ++---
> > > >  IntelUndiPkg/GigUndiDxe/Dma.c                 | 11 +++---
> > > >  IntelUndiPkg/GigUndiDxe/Dma.h                 |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/DriverConfiguration.c |  6 ++-
> > > >  IntelUndiPkg/GigUndiDxe/DriverConfiguration.h |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/DriverDiagnostics.c   | 12 +++---
> > > >  IntelUndiPkg/GigUndiDxe/DriverDiagnostics.h   |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/DriverHealth.c        |  5 ++-
> > > >  IntelUndiPkg/GigUndiDxe/EepromConfig.c        |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/EepromConfig.h        |  3 +-
> > > >  IntelUndiPkg/GigUndiDxe/GigUndiDxe.inf        | 39 +++++++++
> > > > ------
> > > > ----
> > > >  IntelUndiPkg/GigUndiDxe/Hii.c                 | 11 +++---
> > > >  IntelUndiPkg/GigUndiDxe/Hii.h                 |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/HiiInternalLib.c      |  3 --
> > > >  IntelUndiPkg/GigUndiDxe/HiiInternalLib.h      |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/Init.c                | 11 +++---
> > > >  IntelUndiPkg/GigUndiDxe/Init.h                |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/Inventory.vfr         |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/NVDataStruc.h         |  7 ++--
> > > >  IntelUndiPkg/GigUndiDxe/StartStop.c           |  5 ++-
> > > >  IntelUndiPkg/GigUndiDxe/StartStop.h           |  7 ++--
> > > >  IntelUndiPkg/GigUndiDxe/Version.h             |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/{E1000.c => e1000.c}  | 37 ++++++++---
> > > > ----
> > > > --
> > > > -
> > > >  IntelUndiPkg/GigUndiDxe/{E1000.h => e1000.h}  |  5 +--
> > > >  IntelUndiPkg/GigUndiDxe/e1000_80003es2lan.c   |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/e1000_80003es2lan.h   |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/e1000_82571.c         |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/e1000_82571.h         |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/e1000_82575.c         |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/e1000_82575.h         |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/e1000_api.c           |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/e1000_api.h           |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/e1000_defines.h       | 10 ++++-
> > > >  IntelUndiPkg/GigUndiDxe/e1000_hw.h            |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/e1000_i210.c          |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/e1000_i210.h          |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/e1000_ich8lan.c       | 18 ++++-----
> > > >  IntelUndiPkg/GigUndiDxe/e1000_ich8lan.h       |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/e1000_mac.c           |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/e1000_mac.h           |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/e1000_manage.c        |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/e1000_manage.h        |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/e1000_nvm.c           |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/e1000_nvm.h           |  1 -
> > > >  .../{E1000_osdep.c => e1000_osdep.c}          |  3 +-
> > > >  .../{E1000_osdep.h => e1000_osdep.h}          |  8 +---
> > > >  IntelUndiPkg/GigUndiDxe/e1000_phy.c           |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/e1000_phy.h           |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/e1000_regs.h          |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/wol/wol.c             |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/wol/wol.h             |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/wol/wol_1G.c          |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/wol/wolfamily.c       |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/wol/wolimpl.c         |  1 -
> > > >  IntelUndiPkg/GigUndiDxe/wol/wolimpl.h         |  3 +-
> > > >  IntelUndiPkg/GigUndiDxe/wol/wolinfo.c         |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/AdapterInformation.c |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/AdapterInformation.h |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/Brand.c              |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/ComponentName.c      |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/ComponentName.h      |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/Decode.c             |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/Decode.h             |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/DeviceSupport.c      |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/DeviceSupport.h      |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/Dma.c                |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/Dma.h                |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/DriverDiagnostics.c  |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/DriverDiagnostics.h  |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/DriverHealth.c       |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/DriverHealthCommon.h |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/EepromConfig.c       |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/EepromConfig.h       |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/Hii.c                |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/Hii.h                |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/HiiInternalLib.c     |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/HiiInternalLib.h     |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/I40e.c               |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/I40e.h               |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/I40eUndiDxe.inf      |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/I40e_osdep.h         |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/Init.c               |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/Init.h               |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/Inventory.vfr        |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/NVDataStruc.h        |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/StartStop.c          |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/StartStop.h          |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/Version.h            |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/i40e_adminq.c        |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/i40e_adminq.h        |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/i40e_adminq_cmd.h    |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/i40e_alloc.h         |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/i40e_common.c        |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/i40e_devids.h        |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/i40e_diag.c          |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/i40e_diag.h          |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/i40e_hmc.c           |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/i40e_hmc.h           |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/i40e_lan_hmc.c       |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/i40e_lan_hmc.h       |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/i40e_nvm.c           |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/i40e_prototype.h     |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/i40e_register.h      |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/i40e_status.h        |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/i40e_type.h          |  1 -
> > > >  IntelUndiPkg/I40eUndiDxe/i40e_virtchnl.h      |  1 -
> > > >  IntelUndiPkg/IntelOpenSourceUndiPkg.dsc       | 13 ++++++-
> > > >  IntelUndiPkg/XGigUndiDxe/AdapterInformation.c |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/AdapterInformation.h |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/Brand.c              |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/ComponentName.c      |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/ComponentName.h      |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/Decode.c             |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/Decode.h             |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/DeviceSupport.c      |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/DeviceSupport.h      |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/Dma.c                |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/Dma.h                |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/DriverDiagnostics.c  |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/DriverDiagnostics.h  |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/DriverHealth.c       |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/DriverHealthCommon.h |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/EepromConfig.c       |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/EepromConfig.h       |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/Hii.c                |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/Hii.h                |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/HiiInternalLib.c     |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/HiiInternalLib.h     |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/Init.c               |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/Init.h               |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/Inventory.vfr        |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/NVDataStruc.h        |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/StartStop.c          |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/StartStop.h          |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/Version.h            |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf      |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/Xgbe.c               |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/Xgbe.h               |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/ixgbe_82598.c        |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/ixgbe_82598.h        |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/ixgbe_82599.c        |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/ixgbe_82599.h        |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/ixgbe_api.c          |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/ixgbe_api.h          |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/ixgbe_common.c       |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/ixgbe_common.h       |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/ixgbe_dcb.h          |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/ixgbe_dcb_82599.h    |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h        |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/ixgbe_phy.c          |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/ixgbe_phy.h          |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/ixgbe_type.h         |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/ixgbe_x540.c         |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/ixgbe_x540.h         |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/ixgbe_x550.c         |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/ixgbe_x550.h         |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/wol/wol.c            |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/wol/wol.h            |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/wol/wol_10G.c        |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/wol/wolfamily.c      |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/wol/wolimpl.c        |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/wol/wolimpl.h        |  1 -
> > > >  IntelUndiPkg/XGigUndiDxe/wol/wolinfo.c        |  1 -
> > > >  171 files changed, 120 insertions(+), 270 deletions(-)
> > > >  rename IntelUndiPkg/GigUndiDxe/{E1000.c => e1000.c} (99%)
> > > >  rename IntelUndiPkg/GigUndiDxe/{E1000.h => e1000.h} (99%)
> > > >  rename IntelUndiPkg/GigUndiDxe/{E1000_osdep.c =>
> > > > e1000_osdep.c}
> > > > (99%)
> > > >  rename IntelUndiPkg/GigUndiDxe/{E1000_osdep.h =>
> > > > e1000_osdep.h}
> > > > (99%)
> > > > 
> > > 
> > > _______________________________________________
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > > https://lists.01.org/mailman/listinfo/edk2-devel

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to