From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from alexa-out-sd-01.qualcomm.com (alexa-out-sd-01.qualcomm.com [199.106.114.38]) by mx.groups.io with SMTP id smtpd.web12.10005.1645708748290449475 for ; Thu, 24 Feb 2022 05:19:08 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@quicinc.com header.s=qcdkim header.b=H4WtBd9t; spf=pass (domain: quicinc.com, ip: 199.106.114.38, mailfrom: quic_tpilar@quicinc.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; i=@quicinc.com; q=dns/txt; s=qcdkim; t=1645708748; x=1677244748; h=message-id:date:mime-version:subject:from:to:cc:reply-to: references:in-reply-to:content-transfer-encoding; bh=R9IKtceEE55f9Kxpqk+O7iTqKJOXMFe8XWHNB5ls89I=; b=H4WtBd9tslfmOGz4jaOKD7AuwheNyxM3kvrvreyG1TJuAHvaP7JYN9/S Gmawus7nuC2YkdGKx/3lPEeeVRsAgujq4sh0PjODaCPLnBpp2M4bPtV7r pAhS0av3qvRY8CaoiIzOz223YjuF1QrIBSpgvQZ0zBna3vakzBTCIYptL 4=; Received: from unknown (HELO ironmsg01-sd.qualcomm.com) ([10.53.140.141]) by alexa-out-sd-01.qualcomm.com with ESMTP; 24 Feb 2022 05:19:07 -0800 X-QCInternal: smtphost Received: from nasanex01c.na.qualcomm.com ([10.47.97.222]) by ironmsg01-sd.qualcomm.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Feb 2022 05:19:07 -0800 Received: from nalasex01a.na.qualcomm.com (10.47.209.196) by nasanex01c.na.qualcomm.com (10.47.97.222) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.986.15; Thu, 24 Feb 2022 05:19:07 -0800 Received: from [10.111.134.175] (10.80.80.8) by nalasex01a.na.qualcomm.com (10.47.209.196) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.986.15; Thu, 24 Feb 2022 05:19:05 -0800 Message-ID: Date: Thu, 24 Feb 2022 13:19:03 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.6.1 Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg: Correct high-memory use in NvmExpressDxe From: "Tomas Pilar (tpilar)" To: Ard Biesheuvel , edk2-devel-groups-io CC: Ray Ni , Ard Biesheuvel , Leif Lindholm Reply-To: , References: <20220224125753.2021633-1-quic_tpilar@quicinc.com> <1c5ee62c-251a-af20-e277-4534976f397b@quicinc.com> In-Reply-To: <1c5ee62c-251a-af20-e277-4534976f397b@quicinc.com> Return-Path: quic_tpilar@quicinc.com X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01a.na.qualcomm.com (10.52.223.231) To nalasex01a.na.qualcomm.com (10.47.209.196) Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit On 24/02/2022 13:14, Tomas Pilar (tpilar) wrote: > > > On 24/02/2022 13:13, Ard Biesheuvel wrote: >> On Thu, 24 Feb 2022 at 13:58, Tomas Pilar (tpilar) >> wrote: >>> Move the logic that sets EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE Pci >>> >>> attribute to DriverBindingStart() before the memory that backs the >>> >>> DMA engine is allocated. >>> >>> >>> >>> This ensures that the DMA-backing memory is not forcibly allocated >>> >>> below 4G in system address map. Otherwise the allocation fails on >>> >>> platforms that do not have any memory below the 4G mark and the drive >>> >>> initialisation fails. >>> >>> >>> >>> Cc: Ray Ni >>> >>> Cc: Ard Biesheuvel >>> >>> Cc: Leif Lindholm >>> >>> Signed-off-by: Tomas Pilar >> Ehm, nope, that is not exactly what I meant. >> >> The existing code stores the original PCI attributes in the controller >> private data, enables MMIO/IO decoding and bus mastering, and only >> then sets the dual address cycle attribute. >> >> All of that needs to move, so that the captured attributes are accurate. >> >> > Okay, I was wondering. My thought was that we probably want to > re-enable bus mastering > on reset so I kept that bit of code in the original location. > Also, doesn't this code:   if (!EFI_ERROR (Status)) {     Supports &= (UINT64)EFI_PCI_DEVICE_ENABLE;     Status    = PciIo->Attributes (                          PciIo,                          EfiPciIoAttributeOperationEnable,                          Supports,                          NULL                          );   } *strip* the PCI_DEVICE_ENABLE set of attributes rather than add them? I am somewhat confused about this.