From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-x22c.google.com (mail-wm0-x22c.google.com [IPv6:2a00:1450:400c:c09::22c]) (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 A9BAD81D97 for ; Wed, 30 Nov 2016 06:06:59 -0800 (PST) Received: by mail-wm0-x22c.google.com with SMTP id t79so220041292wmt.0 for ; Wed, 30 Nov 2016 06:06:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=IdfXnzPhtT3QV5AwA9EDARqa3RYkLxHCzXt5ZYY2+60=; b=KU487ucke6HqyXtuty/jhEprBLr8S53lmHcbJ3w7cNbzI0Dba5FWhLxEF8g8S3JpY5 +4S9VO+0Ljy5p2AI69opfkZB1VDr7z34W7pbyAxzR3Cb3M8T1VNbht9Is4DA3h1VZb0h L1WkAd0p6zdNn4+Z6ZZrIcvPUzX1RC7mCzkUk= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=IdfXnzPhtT3QV5AwA9EDARqa3RYkLxHCzXt5ZYY2+60=; b=Dengt6DbPzkmk7JxGKDkI9opVosxwDAqfR9XkKn3nfbjk70RHpBj1Y9tjLluUaqqFB GY4IH/XuiIr6PMdbi3jJx1z4vJksSfDxGLlc96Dir06BTdZi5l/JXp+2uInJfJEojyNo Q+s5ZrtgyX4xRr3LWnQp+y93hTWyOLtzN1twklVQl/gk2tGCgpoC/TfnSg1VQzz23CFp xZ8g8ihnr4SfeQF2yVwNHv/DiGGqOQVNKGiPAEMZ03Z1+y7Kb0MUlcHgb7vVx239uqJt MJlBSHi1UIj95+R/hRK8TjWlOEVeDHAnJhYRxqavjY/GxY3Moa/dybzrxvTebIy/Whs+ uigA== X-Gm-Message-State: AKaTC01Tc1wxCEIkeEHc9y781Uj/c6pRsdV5stUYEP7I4vatqYOGZbTbYuOYWgozqq7Gb+2Z X-Received: by 10.28.60.5 with SMTP id j5mr30806984wma.119.1480514817799; Wed, 30 Nov 2016 06:06:57 -0800 (PST) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id g73sm8163853wme.16.2016.11.30.06.06.56 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 30 Nov 2016 06:06:57 -0800 (PST) Date: Wed, 30 Nov 2016 14:06:55 +0000 From: Leif Lindholm To: "Ni, Ruiyu" Cc: Ard Biesheuvel , "edk2-devel@lists.01.org" , "Gao, Liming" , "afish@apple.com" , "Kinney, Michael D" , "mw@semihalf.com" , "Tian, Feng" Message-ID: <20161130140655.GG27069@bivouac.eciton.net> References: <1480088538-21834-1-git-send-email-ard.biesheuvel@linaro.org> <1480088538-21834-4-git-send-email-ard.biesheuvel@linaro.org> <734D49CCEBEEF84792F5B80ED585239D58E8CFA1@SHSMSX104.ccr.corp.intel.com> MIME-Version: 1.0 In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D58E8CFA1@SHSMSX104.ccr.corp.intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [PATCH v4 3/5] MdeModulePkg: implement generic PCI I/O driver for non-discoverable devices 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: Wed, 30 Nov 2016 14:07:00 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi Ray, Ard is on holiday this week, and mostly managing to stay off the email... Anyway, I could really use this support in order to bring the Marvell Armada 70x0 support (worked on by the Semihalf guys) into OpenPlatformPkg, so I will pick this up while he is away - at least parts 1-3. 4/5 is required for 5/5, so I will not be looking to push the final one myself. Would you OK with pushing just 1-3 once we get this one resolved? On Mon, Nov 28, 2016 at 01:56:21AM +0000, Ni, Ruiyu wrote: > > diff --git > > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > > PciDeviceIo.c > > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > > PciDeviceIo.c > > new file mode 100644 > > index 000000000000..fd59267a8d66 > > --- /dev/null > > +++ > > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > > Pc > > +++ iDeviceIo.c > > @@ -0,0 +1,797 @@ ... > > +VOID > > +InitializePciIoProtocol ( > > + NON_DISCOVERABLE_PCI_DEVICE *Dev > > + ) > > +{ > > + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc; > > + INTN Idx; > > + > > + InitializeListHead (&Dev->UncachedAllocationList); The above line has crept in here, but belongs to 4/5 (causes compilation error). I will fix that and resend - is it OK if I resend this patch only? > > + // Iterate over the resources to populate the virtual BARs > > + // > > + Idx = Dev->BarOffset; > > + for (Desc = Dev->Device->Resources, Dev->BarCount = 0; > > + Desc->Desc != ACPI_END_TAG_DESCRIPTOR; > > + Desc = (VOID *)((UINT8 *)Desc + Desc->Len + 3)) { > > + > > + ASSERT (Desc->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR); > > + ASSERT (Desc->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM); > > + > > + if (Idx >= PCI_MAX_BARS || > > + (Idx == PCI_MAX_BARS - 1 && Desc->AddrSpaceGranularity == 64)) { > > + DEBUG ((DEBUG_ERROR, > > + "%a: resource count exceeds number of emulated BARs\n", > > + __FUNCTION__)); > > + ASSERT (FALSE); > > + break; > > + } > > + > > + Dev->ConfigSpace.Device.Bar[Idx] = (UINT32)Desc->AddrRangeMin; > > + Dev->BarCount++; > > + > > + if (Desc->AddrSpaceGranularity == 64) { > > + Dev->ConfigSpace.Device.Bar[Idx] |= 0x4; > > + Dev->ConfigSpace.Device.Bar[++Idx] = (UINT32)(Desc->AddrRangeMin >> 32); > > 1. You need to use RShiftU64 otherwise the above code will break the > build process in 32bit. I don't understand how that could cause breakage (and experiments with IA32 and ARM fail to reproduce it with GCC). - Bar[x] is an array of UINT32 - Desc->AddrRangeMin is UINT64 - The result of (Desc->AddrRangeMin >> 32) is explicitly cast to UINT32 (the type of Bar[x]). Is the problem related to the shift value being signed? Or does some other compiler refuse to cast a UINT64 to a UINT32? Regards, Leif > > + } > > + } > > +} > > diff --git > > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > > PciDeviceIo.h > > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > > PciDeviceIo.h > > new file mode 100644 > > index 000000000000..bc0a3d3258f9 > > --- /dev/null > > +++ > > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > > Pc > > +++ iDeviceIo.h > > @@ -0,0 +1,84 @@ > > +/** @file > > + > > + Copyright (C) 2016, Linaro Ltd. All rights reserved.
> > + > > + This program and the accompanying materials are licensed and made > > + available under the terms and conditions of the BSD License which > > + accompanies this distribution. The full text of the license may be > > + found at http://opensource.org/licenses/bsd-license.php > > + > > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" > > BASIS, > > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER > > EXPRESS OR IMPLIED. > > + > > +**/ > > + > > +#ifndef __NON_DISCOVERABLE_PCI_DEVICE_IO_H__ > > +#define __NON_DISCOVERABLE_PCI_DEVICE_IO_H__ > > + > > +#include > > +#include > > +#include #include > > + > > +#include > > + > > +#include > > + > > +#include > > +#include > > +#include > > + > > +#define NON_DISCOVERABLE_PCI_DEVICE_SIG SIGNATURE_32 ('P', 'P', 'I', > > +'D') > > + > > +#define NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(PciIoPointer) \ > > + CR (PciIoPointer, NON_DISCOVERABLE_PCI_DEVICE, PciIo, \ > > + NON_DISCOVERABLE_PCI_DEVICE_SIG) > > + > > +#define PCI_ID_VENDOR_UNKNOWN 0xffff > > +#define PCI_ID_DEVICE_DONTCARE 0x0000 > > + > > +#define PCI_MAX_BARS 6 > > + > > +typedef struct { > > + UINT32 Signature; > > + // > > + // The bound non-discoverable device protocol instance > > + // > > + NON_DISCOVERABLE_DEVICE *Device; > > + // > > + // The exposed PCI I/O protocol instance. > > + // > > + EFI_PCI_IO_PROTOCOL PciIo; > > + // > > + // The emulated PCI config space of the device. Only the minimally > > +required > > + // items are assigned. > > + // > > + PCI_TYPE00 ConfigSpace; > > + // > > + // The first virtual BAR to assign based on the resources described > > + // by the non-discoverable device. > > + // > > + UINT32 BarOffset; > > + // > > + // The number of virtual BARs we expose based on the number of > > + // resources > > + // > > + UINT32 BarCount; > > + // > > + // The PCI I/O attributes for this device > > + // > > + UINT64 Attributes; > > + // > > + // Whether this device has been enabled > > + // > > + BOOLEAN Enabled; > > +} NON_DISCOVERABLE_PCI_DEVICE; > > + > > +VOID > > +InitializePciIoProtocol ( > > + NON_DISCOVERABLE_PCI_DEVICE *Device > > + ); > > + > > +extern EFI_COMPONENT_NAME_PROTOCOL gComponentName; extern > > +EFI_COMPONENT_NAME2_PROTOCOL gComponentName2; > > + > > +#endif > > diff --git a/MdeModulePkg/MdeModulePkg.dsc > > b/MdeModulePkg/MdeModulePkg.dsc index 20e5e8c78be0..5dd30556cb3c > > 100644 > > --- a/MdeModulePkg/MdeModulePkg.dsc > > +++ b/MdeModulePkg/MdeModulePkg.dsc > > @@ -265,6 +265,7 @@ [Components] > > MdeModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf > > MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf > > MdeModulePkg/Bus/Isa/Ps2MouseDxe/Ps2MouseDxe.inf > > + > > + > > MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePci > > Dev > > + iceDxe.inf > > > > MdeModulePkg/Core/Dxe/DxeMain.inf { > > > > -- > > 2.7.4 >