From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io0-x231.google.com (mail-io0-x231.google.com [IPv6:2607:f8b0:4001:c06::231]) (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 ECD2081F38 for ; Fri, 25 Nov 2016 07:21:22 -0800 (PST) Received: by mail-io0-x231.google.com with SMTP id x94so129592705ioi.3 for ; Fri, 25 Nov 2016 07:21:22 -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=Sh13MZpfK0X3sq1qZA2dpVZdkng1zISsYs4Br19JVgQ=; b=b7r0ve/hN2VDtEsEBCwljAaJ1WsiqE2X21hrbGb8oQe6dPzPyWX9jTyjIhin8v+Dr9 FDeprlD/Wlfe0uHPu6Sjc0te5mJpWLYl40jTWZx9Y28wJanYxhB64n3LFyfvzMQZFzqH o8JuBMI0VkzPJsUYnK8NhyHpUy+Oi80M3psDE= 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=Sh13MZpfK0X3sq1qZA2dpVZdkng1zISsYs4Br19JVgQ=; b=UySp5cFHWHy3DG9z8HdvWMWCE3DBd0PiJ8TuikcnGsu3DrWE0031GTdCPWiKxjZ714 bbqNadDLhIjpUHxxZKtKe6hz+s6Gb4o4z4Nrf8OgIiIGFyb3DR6bsYmyA/MiEEb2fN1C /fXkAvPQIzv2N7/XE9c3dGL+rGViIutqRtGyuhPBcAtDhBu0ii4jt5vRF5wTcjytgIAZ Vdta9a98mWHcWn1zDXWLh1X7gU82n7cmm1zTcHn7MiNBGU5j1mZvKw+/20MUQqWKsgpO TDtEEKJ0sjz/e6YIb82JE9YnN9ggZzOLjaAECXewpS5EErjiuGtimj8g+njrUSQ5mV6y x0vA== X-Gm-Message-State: AKaTC01LzQTl9Fy9zSUNmG1bAHsGlFjDqU9XYWeMwt9LqEjmd5ICxwi8m4mHkFvlAagaTv26bKusQPxTSIzhRBZM X-Received: by 10.36.73.207 with SMTP id e76mr7388455itd.63.1480087282091; Fri, 25 Nov 2016 07:21:22 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.59.147 with HTTP; Fri, 25 Nov 2016 07:21:21 -0800 (PST) In-Reply-To: <5DD889BC-CC56-4FFE-929E-C547515D0573@linaro.org> References: <1479315571-14953-1-git-send-email-ard.biesheuvel@linaro.org> <1479315571-14953-2-git-send-email-ard.biesheuvel@linaro.org> <20161116174848.GC27644@bivouac.eciton.net> <734D49CCEBEEF84792F5B80ED585239D58E7C6D4@SHSMSX104.ccr.corp.intel.com> <4E810E45-F1CC-429C-B3F4-FC6182F7D9B2@linaro.org> <734D49CCEBEEF84792F5B80ED585239D58E7D6F5@SHSMSX104.ccr.corp.intel.com> <734D49CCEBEEF84792F5B80ED585239D58E7FCF6@SHSMSX104.ccr.corp.intel.com> <734D49CCEBEEF84792F5B80ED585239D58E80120@SHSMSX104.ccr.corp.intel.com> <734D49CCEBEEF84792F5B80ED585239D58E804E1@SHSMSX104.ccr.corp.intel.com> <5DD889BC-CC56-4FFE-929E-C547515D0573@linaro.org> From: Ard Biesheuvel Date: Fri, 25 Nov 2016 15:21:21 +0000 Message-ID: To: "Ni, Ruiyu" Cc: "Kinney, Michael D" , "edk2-devel@lists.01.org" , "Gao, Liming" , "afish@apple.com" , Leif Lindholm Subject: Re: [PATCH v3 1/5] MdeModulePkg: introduce non-discoverable device protocol 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: Fri, 25 Nov 2016 15:21:23 -0000 Content-Type: text/plain; charset=UTF-8 On 18 November 2016 at 13:50, Ard Biesheuvel wrote: > > On 18 Nov 2016, at 14:39, Ni, Ruiyu wrote: > >>>>>>>>>> 1. Can you add "PCI" keyword into the protocol name? >>>>>>>>>> e.g.: EDKII_NON_DISCOVERABLE_PCI_DEVICE_PROTOCOL_GUID >>>>>>>>>> >>>>>>>>> >>>>>>>>> No. This protocol does not describe pci devices, and it is a peculiarity of the >>>>>>>>> edk2 driver stack that some non-pci devices can only be driven by pci drivers. >>>>>>>>> >>>>>>>>> in other words, pci is part of the /driver/ side, and it is perfectly possible for, >>>>>>>>> e.g., a non-discoverable ahci device to be driven by a different non-pci driver >>>>>>>>> in the future. >>>>>>>>> >>>>>>>> >>>>>>>> I see. So some types of devices are handled by the current >>>>>>>> NonDiscoveablePciDevice driver, and some other types of devices may be >>>>>>>> handled by a future NonDiscoverableXXXDevice driver. >>>>>>>> Now since the AHCI type is already handled by the NonDiscoverablePciDevice >>>>>>>> driver, when there is a new NonDiscoverableXXXDevice driver, how can the two >>>>>>>> know whether it should manage the AHCI type device or not? >>>>>>> >>>>>>> Good question. But how does the UEFI driver model deal with that? What happens if i have two drivers that both support >>>>> the >>>>>>> Ahci Pci class codes? >>>>>> PCI CFG header contains VendorID/DeviceID fields which can be used to distinguish >>>>>> them. >>>>>> >>>>> >>>>> No, that is not what I mean. >>>>> >>>>> Your question is how we should deal with multiple drivers that >>>>> support, for instance, the AHCI non-discoverable device type. My >>>>> answer is that this is not any different from a platform configuration >>>>> that has more than one PCI I/O based driver that supports the AHCI PCI >>>>> class codes. The UEFI driver model has priority rules and protocols to >>>>> decide which driver gets precedence. I don't see how it should be any >>>>> different here. >>>> >>>> I see they are different. Based on PciIo, the *HCI drivers can query >>>> additional information from PCI CFG header, instead of just using >>>> the PCI class code. >>>> >>>> But with the NonDiscoverableDevice protocol, there is no additional >>>> information can help the *HCI drivers decide which to manage. >>>> >>>> I don't see any practical negative point which prevents degrading >>>> NonDiscoverableDevice protocol to NonDiscoverable*Pci*Protocol. >>>> After all, as I said, all *HCI drivers are based on PciIo. >>>> >>> >>> Yes the *drivers* are based on PCI. But that does not make the >>> *devices* PCI devices. That is the whole problem we are trying to deal >>> with. So describing the non-PCI devices as PCI devices is incorrect >>> imo. The fact that we will use PCI drivers to drive non-PCI devices is >>> an implementation detail of EDK2, and is a property of the *driver* >>> side not the *device* side. So using PCI class codes etc to wire up >>> the correct driver should be local to the driver, and not pollute the >>> description of the device. >>> >>> For example, if we would ever split the AHCI driver into a AHCI part >>> and a PCI part (which I know is unlikely to occur), I would want the >>> non-PCI AHCI driver to be used with the same protocol. Perhaps that >>> means we need a protocol for each type of device rather than an enum? >>> In any case, putting PCI-specific metadata into the device description >>> makes the situation worse, because now both the *device* and the >>> *driver* side are forced to use PCI internals to describe devices that >>> have nothing to do with PCI >> >> If I understand correctly, you want the protocol producer can simply >> produce such protocol without the knowledge of PCI. I agree! >> But we do need to make the protocol definition stable enough. I do not >> like to see the enum type being extended in future to support more types >> of devices. >> 1. Can you use different GUIDs for different types of devices? > > Yes that seems like a reasonable approach, in the spirit of EDK2 :-) > >> 2. As I replied as comment #2 to patch 3/5, do you have better way to >> deal with the SDHCI Host controller driver access? > The best way is to revert to the previous solution. This means two SDHCI slots will be modeled as two separate non-discoverable devices, and will each receive a separate instance of the PCI I/O protocol, describing a SDHCI-PCI device with a single slot. But actually, I don't think that matters at all. The way the SDHCI driver is implemented is debatable anway: I still think it should be a bus driver, with each slot a separate device on this bus.