From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-x229.google.com (mail-it0-x229.google.com [IPv6:2607:f8b0:4001:c0b::229]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 84F4B81EDF for ; Tue, 15 Nov 2016 01:19:50 -0800 (PST) Received: by mail-it0-x229.google.com with SMTP id q124so180942099itd.1 for ; Tue, 15 Nov 2016 01:19:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=2NlBsv90KpIFGRa8vIkGDb4LDtRD88Ov82fZDF5im0k=; b=JFE6fiEbFXQkMt9ixMfQlrrtS+OjhNorOIvs13Qqx/3PjSKcrHn0Qr+UDoD0qiSbzr dAIufDFj3XlX/f/wdCazA7jP9vXzVMDVxjZ3q3U/pDBFz/U21DCbebciO4VZ4OUsZEGe bymqs8Hbj2NjoIBqvWjs8FFpn3k5HP4BRRm/o= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=2NlBsv90KpIFGRa8vIkGDb4LDtRD88Ov82fZDF5im0k=; b=IKF+zJgNhZRdKhfgKGXA80ZdlkdNqKYWHcA66mGejePnTROv5seA0e/2v0LvDrTxFB ExJC1hS47Gn9SmZ4vBLNodezfNUhKA4p6f95urj18NC+m5h1I3I91jrbyJamdgF4Pduz EC/uYzs0+FK6Wb3/mgTcLVLj3CWG6B67NuC6J1gmjmEbTwBX6pEKetRu3MR/ZI6OpDZO tvbs71WNtMW7zgCYKuyK5htbE7ONS0RLj0Kwdhk9ARKrufVC/uZXJCrLhoogjmBqW6Bd gW1JD+DfQC+q4VDm1F4+Awei7wRc+wYl+hD4xf9Eyw+FedrG+LBbg5Fw7MWvPMU2o5wS kuog== X-Gm-Message-State: ABUngvfVaWNDcH84tmxRVgz6QJ7nKN+IsjZC5Mho9wgVlT0rbpaoABZ6XXyuhCHaTzlH+qSiFCPjIbPNh2o9DkFk X-Received: by 10.36.66.142 with SMTP id i136mr1879262itb.63.1479201594452; Tue, 15 Nov 2016 01:19:54 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.59.147 with HTTP; Tue, 15 Nov 2016 01:19:54 -0800 (PST) In-Reply-To: <20161114151614.GR27644@bivouac.eciton.net> References: <1478955748-14819-1-git-send-email-ard.biesheuvel@linaro.org> <1478955748-14819-4-git-send-email-ard.biesheuvel@linaro.org> <20161114151614.GR27644@bivouac.eciton.net> From: Ard Biesheuvel Date: Tue, 15 Nov 2016 10:19:54 +0100 Message-ID: To: Leif Lindholm , Ryan Harkin , Jeremy Linton Cc: edk2-devel-01 Subject: Re: [PATCH v2 3/4] ArmPkg/ArmDmaLib: clean up abuse of device address X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 15 Nov 2016 09:19:50 -0000 Content-Type: text/plain; charset=UTF-8 On 14 November 2016 at 16:16, Leif Lindholm wrote: > On Sat, Nov 12, 2016 at 02:02:27PM +0100, Ard Biesheuvel wrote: >> In preparation of adding support to ArmDmalib for DMA bus masters whose >> view of memory is offset by a constant compared to the CPU's view, clean >> up some abuse of the device address. >> >> The device address is not defined in terms of the CPU's address space, >> and so it should not be used in CopyMem () or cache maintenance operations >> that require a valid mapping. This not only applies to the above use case, >> but also to the DebugUncachedMemoryAllocationLib that unmaps the >> primary, cached mapping of an allocation, and returns a host address >> which is an uncached alias offset by a constant. >> >> Since we should never access the device address from the CPU, there is >> no need to record it in the MAPINFO struct. Instead, record the buffer >> address in case of double buffering, since we do need to copy the contents >> (in case of a bus master write) and free the buffer (in all cases) when >> DmaUnmap() is called. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Ard Biesheuvel > > For the fix itself: > Reviewed-by: Leif Lindholm > > However, can we wait for a few Tested-by:s to ensure this fix does not > reveal any companion bugs? > Perhaps, yes. In case anyone is up to doing that, please find the branch here https://git.linaro.org/people/ard.biesheuvel/uefi-next.git/log/?h=armdmalib-offset However, given that the split CPU/bus master view is introduced in the next patch, the only use case where the device address differs from the host address is when using the DebugUncachedMemoryAllocationLib, which is currently broken AFAICT (it attempts to unmap the linear mapping of the allocation by setting the memory attributes to '0', which triggers an assert in the ArmPkg MMU code)