From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt0-x236.google.com (mail-qt0-x236.google.com [IPv6:2607:f8b0:400d:c0d::236]) (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 8719181E92 for ; Tue, 15 Nov 2016 03:34:42 -0800 (PST) Received: by mail-qt0-x236.google.com with SMTP id n6so67879994qtd.1 for ; Tue, 15 Nov 2016 03:34:47 -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=QmKl9kaDz25Oe8KPoT9bkkng8/AckkhXj3qiFRzFKec=; b=c3PrNw7cVUvSnAUeX5wB7tPWGxxSYcxmg5aFcVpEkGoRn3da/HRM4/sbxIOw9Fc+61 e5ranDmMpPNm+alT97xOEo2OYlg0D0+EG8p9wkIt2GTVOnq5PIk0KcYTmEpzVeQA2L/U D0YxXAyhBacfI861w0yWYYDY5/ESF8DZSb0IM= 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=QmKl9kaDz25Oe8KPoT9bkkng8/AckkhXj3qiFRzFKec=; b=RR4bfcCmnfKX1/OUWfvyXT3lNin05eTN6fWPt4OU1Vplr51rpsIFk8f7pfNtq4BZm+ +J9OTe7NLQJ70UyY/ySEPzBEStrzbdghNNwtw3oh3hJe8cG7T76UleWt7QxtC5sJ9IUy OnEYLaqO4lwQVFZzOhxopoMthZECJ8O0Brpxy/SLWPT7gw2rT4iEJm/Bjc4qbuydB9Z+ Kw7gcrU0xwviKz0m2yL1TpDLtx+YhgsV6E+pRhd2Fw8F0az0uNTYqkmzv4r65/hAANep SjQzSje5/tx1eTWQ4dBz+Qlak4o4Lyqdg2Vcaff3KftgiqklO/LmA1OVYoKt0ISzmypO 5qNg== X-Gm-Message-State: ABUngvfsv5EgeE2ZWWkjKZgw+LCK2IWFL4Yc9ejLIJwKQDUjCk+HdXtwIFnOp9VKAjKgQxSJOG9W7YQKy1KwxqfQ X-Received: by 10.25.23.80 with SMTP id n77mr148970lfi.72.1479209685926; Tue, 15 Nov 2016 03:34:45 -0800 (PST) MIME-Version: 1.0 Received: by 10.25.27.68 with HTTP; Tue, 15 Nov 2016 03:34:45 -0800 (PST) In-Reply-To: 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: Ryan Harkin Date: Tue, 15 Nov 2016 11:34:45 +0000 Message-ID: To: Ard Biesheuvel Cc: Leif Lindholm , Jeremy Linton , 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 11:34:42 -0000 Content-Type: text/plain; charset=UTF-8 On 15 November 2016 at 09:19, Ard Biesheuvel wrote: > 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 > I tested your branch on the usual victims (R0/1/2, FVP Foundation & AEMv8 and TC2) and they all work fine for me. Tested-by: Ryan Harkin > 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)