From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 44ED7C46467 for ; Wed, 11 Jan 2023 13:03:12 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 2A9B2854BF; Wed, 11 Jan 2023 14:03:09 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=sancloud.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id 44EDE854BF; Wed, 11 Jan 2023 14:03:06 +0100 (CET) Received: from APC01-PSA-obe.outbound.protection.outlook.com (mail-psaapc01on2042.outbound.protection.outlook.com [40.107.255.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 6F8EC8540D for ; Wed, 11 Jan 2023 14:03:01 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=sancloud.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=paul.barker@sancloud.com ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=c6lNRmHw+KHY5mNMwwHJABqcW4iICLhirnnq5i/1XMzSdDo9IySkSelUphQJD9jWFZSUsZC7dhU9DrWUN/rPszES4409GNHJJHv7BWbS3qWG6PYoV+Htymo7MM3N9BJqeAY2tSwpPU0A10WrkQUtCZXTvBhy1BS+DgN3CPl34796o4XcI212ez3BgYOpEFSTwLPAaO4pkB/O7Z7c9Is4saRm67gpxB0j+22FN00d2tyX7dQlSZZykAET0fxBgSWYw9U2XWsJtjbrpbpAfOBV6w2JTOEODTAKQNMFaVuMV5/de0uyWju+9RVxph4Ir7sGeNZniT7nfnTAeLI4gJMCPg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=W6kFqTd9i0mTzf6gmeplNYhCgNgq9AiEapI12EO6vf8=; b=FrJ4c8aJW8kfRDeSTQZ3mJ1MXpIWk9m3C4QvVrQ5k3jmsquGmiIjc6DthFHlScWMbqjqeipZo4ub+Hhc9vsoWE8FnZ0QfLrqSIftOwfBiY7ywMJvlRmDg1TmZmdPIBMQkQWiLwfG3Bl6hwzEg2RQeDSTT89bjsGVN4+EHlXEkqu5FdW5IQjTNoS0wcygZ//2TuBOeKusGqtcHo3iQJf+wyD8Ed2tA2LkA7znnGACSCr1zs5MSYtKUPHlujkQul9TEBw2aOIN+fz+HMnkgDNYfcyFmamCOQCdhvnr4F/zhHY8ei1Vx5DfBcY8WMOJhTBhOlBCIqUUwg0Jy8dwnKDGbQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=sancloud.com; dmarc=pass action=none header.from=sancloud.com; dkim=pass header.d=sancloud.com; arc=none Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=sancloud.com; Received: from SEYPR06MB5064.apcprd06.prod.outlook.com (2603:1096:101:55::13) by SEYPR06MB5866.apcprd06.prod.outlook.com (2603:1096:101:c6::7) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6002.11; Wed, 11 Jan 2023 13:02:53 +0000 Received: from SEYPR06MB5064.apcprd06.prod.outlook.com ([fe80::195a:2d6f:8000:fe5a]) by SEYPR06MB5064.apcprd06.prod.outlook.com ([fe80::195a:2d6f:8000:fe5a%3]) with mapi id 15.20.6002.011; Wed, 11 Jan 2023 13:02:53 +0000 Message-ID: <72148849-96bb-0de3-88a2-4bb9037d18cb@sancloud.com> Date: Wed, 11 Jan 2023 13:02:42 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.6.1 Subject: Re: [PATCH v5 1/3] efi_loader: Add SPI I/O protocol support To: Heinrich Schuchardt , Paul Barker Cc: u-boot@lists.denx.de, Simon Glass , Tom Rini , Jagan Teki , Ilias Apalodimas References: <20221123175006.4080122-1-paul.barker@sancloud.com> <20221123175006.4080122-2-paul.barker@sancloud.com> <0b4e6d32-a5f1-34c3-24d8-b500a538b8da@sancloud.com> <2cb806c7-a74b-14b1-053f-c7839c019604@gmx.de> From: Paul Barker In-Reply-To: <2cb806c7-a74b-14b1-053f-c7839c019604@gmx.de> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-ClientProxiedBy: LO6P265CA0011.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:339::17) To SEYPR06MB5064.apcprd06.prod.outlook.com (2603:1096:101:55::13) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: SEYPR06MB5064:EE_|SEYPR06MB5866:EE_ X-MS-Office365-Filtering-Correlation-Id: 8ba77d58-ad0a-4260-61c6-08daf3d42684 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 95GVPixmp/vVJ9zZv4nnYImCWXiUK3aSPoenKs3MzkjdmDcOihn22oDjwXKwuBNwoHytvuAvsZZSdU4Uv62K3pYgADduiHY85OGthjYt0kgmqyHPTXGBOicy8Vh6gxvPi+llT0pJ0OEk5VsZqQHT1IBxztnRV+96yDUOlf3aec5duCahx2u5LjNTKcW1jrJgNdYM63VMyezNW85h8U9Jf1R6E0bi9WVoqRj6hD3i4QdojL8HzF/e4F3FqCTZOZUIvvoknRwCbbmOM2b2pR2AaYuII4LZkN6MRwx7h0QfU0eXDyu01X1jmSkoi6wMioseG8V1OfruPTUQZujY4eZ1gT7VNqRwBAkdR2s0TNNJHfdFKCFm6MZfbjstWrB4fSGWXKF+/KmOr5bGvEJBgyRflZKZrVcKqy8wWjkygMyX/N0y9HApXNVSh960R+J2Pg/FFNMtWJeI+eVUZbOhzKsS6UbLjuMkJYE7A+TmuBFiM+vR6neSI9jt6wbeho1OY8xCTEIl2Utj0/EmNHlRF3QJZSHM8TmoTVR6sVb0u3qe2p9PmZqVDLL1UI7kkHbYE8kFgUGPzDcozptCdYSYC+mrnMcty5bpKoV2iAcU0ufSHq7V/dJDRg0+4aUbgTSLCDcHvMIQ4rLD5YNajCX1kKZeMcLE0MBMiABKxDh/SJIZ4L48l63U9Xkg5qlmFrWfzCiF+yYaJyNbiZSwn0F0LIYY3BHOyrBrTZw/DW6DMJv1bmbYuOQf4qvGsEB6EWlSqOop X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:SEYPR06MB5064.apcprd06.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230022)(346002)(136003)(396003)(39830400003)(366004)(376002)(451199015)(36756003)(31696002)(26005)(478600001)(54906003)(86362001)(316002)(786003)(110136005)(6486002)(966005)(44832011)(6512007)(5660300002)(8936002)(2906002)(66556008)(6666004)(66946007)(4326008)(41300700001)(41320700001)(66476007)(8676002)(38100700002)(6506007)(53546011)(83380400001)(2616005)(31686004)(186003)(43740500002)(45980500001); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?MkpZOG5ZY2R4dkk1YldIbFAzczVZR2tMV2NYSXZNY0tZeTNEYUNRNTVzNzZo?= =?utf-8?B?OFpBdDUxd2dKdUZ2M2JVSEJCL09rVFFianYvMnBLTG9UZE41NFdMdUpVZG9t?= =?utf-8?B?NC80WGtZRk1WbVkwdnpHdERkOU0wRVkySHVpQllyUmlTUTB6dE1kcFhVaDgz?= =?utf-8?B?UENSZHQxMVBCSHAvZHpTQm90YXVUWTcrUjhuMHNCNlNIeU85YUpkNUlBS0l1?= =?utf-8?B?QXlXNFVsdGU3Z1BaL1F4cnUzLzBndkY5cWtUajFZQ0ZyNFhicFpTMzUwajFJ?= =?utf-8?B?NzhTYVR5U3J4OWpDYU9CWXpQbEFKT1ZJeGo4V3F6UXpxdXZ6d3JIMjJYYzBB?= =?utf-8?B?akt6Q244ZWpLOUhWdUFyUkE1MVJXRUZKM0taTWZnQXprbzBGYXFxYWNsMmpt?= =?utf-8?B?cTUyeGNOblhFWnY3MGpZaEhlcnNKYnhwOFMrdVNkdFltOFBqWmhXb2RHQmY0?= =?utf-8?B?SnZySW12b0ZKYWJpSlFsaEM1cEh1Y1o1VlFBRjR1dWNNS1ZVSFNDVDBqV05E?= =?utf-8?B?UWdEWEgxdjMxVW1xVUp4R3pibVpMMEFNTzJoTDMyOUk5VCsvNnVpOEJGbHBS?= =?utf-8?B?T1RVL0hUOUFndTF1cHUyaktHZXRmeEowMGxBb25XV3d6UHBJSGhtRExxMWhD?= =?utf-8?B?M0l6cEk1VTk0L3I4MUNUbldZSWtkbS9pa1JoK1FZdzI5T1FTSjdUN1RHNTBU?= =?utf-8?B?L1J0TUpqV3VvTnpETFQvb3BXWjNIbnRqLy9CWUJ1dDdJQ3NJcjRWaU9MNnNG?= =?utf-8?B?bGFscWtOcmkvT1RXdjhXbHpqd01PZnUwdEw2NHJ1N25DRnRkQ2sxcWcwN2pq?= =?utf-8?B?bG00S3YxRjdKbVBpQkdUU0t6ZXhYZGd0TXhqL0xLZ3IwSnBCbVpucnFHZ1M0?= =?utf-8?B?QjM1MGlTMWNLTlJSbGd5Zm14MXc1ODF4TENvSC9DbU1aRm1KYXMwckNmNlZi?= =?utf-8?B?dVBEeGlHYU9MVjJITzNNeXUzQ0ZNUUp0b2hFTGpkc3ptc3pSZmFjZEhGcTNU?= =?utf-8?B?MG4zMGFoQ0hpcXRRZ1RwVW05RTU4RC9oZk5xajRSSVJSSU9LRDI1L0R1Kzhr?= =?utf-8?B?REdEZ3c4VHMzWVQyVzBsak1UTUNMbmttZzllVnpmZnd2ekR2Qlh3SHE4K3h3?= =?utf-8?B?b1FuYVdsQnV2Nk5jd05UU3RLaHV5aXdOSG9YTXBmVnkrMDBEU1dXOWRoRWpB?= =?utf-8?B?MWpvdytxOTREaEN3Yy83c2E1QWpZZXB6ZWFaT0lIVjRONUlORmU3YlNLVUxD?= =?utf-8?B?ZVRiNlZxQ0FYc3VqbjMwUTJEZEFIVVM3RWxFekcrdXBPR29wK2Jma0QwYjc1?= =?utf-8?B?cTVobnQwczk5N3hCZHRGU3JMZWw3aEo0anUwVVVhSklaZEt1U2lLTE1vcXNp?= =?utf-8?B?OHpKdXdjbGxGeis1bURhTmlod0pIbC84S3daajlwUDJaSkFmS1lZVGl2VXdR?= =?utf-8?B?Vm55bzRIblhEUFBwOGlrR2JaVFBiUE15K0tqMjZzVEpGV09SdEVnNXpiTFJw?= =?utf-8?B?NHpVK1A0cG5FVkFTYXVZVmNXQnMreVBITUV5TE5VVVEzL01OeW9yRmJ6amg0?= =?utf-8?B?OFIwenlGQnFBMFJIYUJURVR5aXl3ZDJHdHJMSEJEMFhyTlBLb1dPT2g1ZFRh?= =?utf-8?B?R0djdUIzMHBiVnNrRGdFT0lNNzlkbEh0NmxMQWNaalUzNVhPN0dhMGpxeGZC?= =?utf-8?B?UFI1UmNBVDF4eHZjM2RUUnlnUlBONUJGRURvY0V6MTIvZ0NIZnRmYzR2Zng5?= =?utf-8?B?dW9LbEtnSnl2VC9uVzF6TFgwdDA5TG9YWXJzNWY3Und6WkVheER0NjVaaHht?= =?utf-8?B?K1VBelJMM0xEYk5QQy8rcWVFZlpBb3U2UWd6TG5JcDVyU0xWd3gyKzl4Nmti?= =?utf-8?B?clhZQWxKN3YzdFN3WGpBOHRLcXd0cHdNSWdqRC9wWjV1QVFUUWNKemVVN2FX?= =?utf-8?B?bVQrRmRXU21JN1gxTlpBVzRqT29QZWtBZkFDUEdrRm9vTDY5SEFuY1VuNDNX?= =?utf-8?B?S1FrMXAvWHEybWNZdytVN1dIb3ZRR05PSGpBVlY4WEZWb0RYdk0yZGl3Yk1v?= =?utf-8?B?N3Myb00zSVdacDFaZHduTzJYdGR0SGtLY3FtQzQvZGVxSFRTaU1FU3Y5NGpY?= =?utf-8?B?emxabXlwei91amtHNndkcmpGbTZqbW95Z3I0NmkySXR4Y01SaW9OWkNTSjkz?= =?utf-8?B?d1E9PQ==?= X-OriginatorOrg: sancloud.com X-MS-Exchange-CrossTenant-Network-Message-Id: 8ba77d58-ad0a-4260-61c6-08daf3d42684 X-MS-Exchange-CrossTenant-AuthSource: SEYPR06MB5064.apcprd06.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Jan 2023 13:02:53.4922 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3e0f949f-6a74-4378-baf2-0abfca8d5e06 X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: K9AUP6quc/yoSfU9+iQcUdnCQ4iWz9+b8JIEu93OTcHnseVeIWKY3bfFhprRoNXhRg0RKwq7mE/mDRYOLFurEiKPNXFux2wbiMwXUrydYHw= X-MS-Exchange-Transport-CrossTenantHeadersStamped: SEYPR06MB5866 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean On 24/12/2022 14:09, Heinrich Schuchardt wrote: > On 12/24/22 13:25, Paul Barker wrote: >> On 13/12/2022 07:15, Ilias Apalodimas wrote: >>> Hi Paul, >>> >>> Apologies for the delayed reply. >>> >>> [...] >>> >>>> +static efi_status_t >>>> +export_spi_peripheral(struct efi_spi_bus *bus, struct udevice *dev) >>>> +{ >>>> +    efi_string_t name_utf16, vendor_utf16, part_number_utf16; >>>> +    struct efi_spi_peripheral_priv *priv; >>>> +    efi_status_t status; >>>> +    efi_handle_t handle = NULL; >>>> +    struct udevice *dev_bus = dev->parent; >>>> +    struct spi_slave *target; >>>> +    const char *name = dev_read_name(dev); >>>> +    const char *vendor = dev_read_string(dev, "u-boot,uefi-spi-vendor"); >>>> +    const char *part_number = dev_read_string(dev, >>>> +            "u-boot,uefi-spi-part-number"); >>>> +    efi_guid_t *guid = (efi_guid_t *)dev_read_u8_array_ptr(dev, >>>> +            "u-boot,uefi-spi-io-guid", 16); >>>> + >>>> +    if (device_get_uclass_id(dev) == UCLASS_SPI_EMUL) { >>>> +        debug("Skipping emulated SPI peripheral %s\n", name); >>>> +        goto fail_1; >>>> +    } >>>> + >>>> +    if (!vendor || !part_number || !guid) { >>>> +        debug("Skipping SPI peripheral %s\n", name); >>>> +        status = EFI_UNSUPPORTED; >>>> +        goto fail_1; >>>> +    } >>>> + >>>> +    if (!device_active(dev)) { >>>> +        int ret = device_probe(dev); >>>> +        if (ret) { >>>> +            debug("Skipping SPI peripheral %s, probe failed\n", >>>> +                  name); >>>> +            goto fail_1; >>>> +        } >>>> +    } >>>> + >>>> +    target = dev_get_parent_priv(dev); >>>> +    if (!target) { >>>> +        debug("Skipping uninitialized SPI peripheral %s\n", name); >>>> +        status = EFI_UNSUPPORTED; >>>> +        goto fail_1; >>>> +    } >>>> + >>>> +    debug("Registering SPI dev %d:%d, name %s\n", >>>> +          dev_bus->seq_, spi_chip_select(dev), name); >>>> + >>>> +    priv = calloc(1, sizeof(*priv)); >>>> +    if (!priv) { >>>> +        status = EFI_OUT_OF_RESOURCES; >>>> +        goto fail_1; >>>> +    } >>>> + >>>> +    vendor_utf16 = efi_convert_string(vendor); >>>> +    if (!vendor_utf16) { >>>> +        status = EFI_OUT_OF_RESOURCES; >>>> +        goto fail_2; >>>> +    } >>>> + >>>> +    part_number_utf16 = efi_convert_string(part_number); >>>> +    if (!part_number_utf16) { >>>> +        status = EFI_OUT_OF_RESOURCES; >>>> +        goto fail_3; >>>> +    } >>>> + >>>> +    name_utf16 = efi_convert_string(name); >>>> +    if (!name_utf16) { >>>> +        status = EFI_OUT_OF_RESOURCES; >>>> +        goto fail_4; >>>> +    } >>>> + >>>> +    priv->target = target; >>>> + >>>> +    efi_spi_init_part(&priv->part, target, vendor_utf16, part_number_utf16); >>>> + >>>> +    efi_spi_init_peripheral(&priv->peripheral, &priv->part, >>>> +                bus, target, guid, name_utf16); >>>> + >>>> +    efi_spi_append_peripheral(&priv->peripheral, bus); >>>> + >>>> +    efi_spi_init_io_protocol(&priv->io_protocol, &priv->peripheral, target); >>>> + >>>> +    status = efi_install_multiple_protocol_interfaces(&handle, guid, >>>> +                              &priv->io_protocol, >>>> +                              NULL); >>> >>> There's a protocols installed here as well as in >>> efi_spi_protocol_register().  But I don't see those being uninstalled >>> somewhere.  Shouldn't destroy_efi_spi_bus() call >>> efi_uninstall_multiple_protocol_interfaces() as well ? >> >> Yes, `destroy_efi_spi_bus()` and `destroy_efi_spi_peripheral()` >> should cleanup everything created by `export_spi_bus()` and >> `export_spi_peripheral()` respectively. >> >> I think we can just call `efi_delete_handle()` on the relevant handle in >> `destroy_efi_spi_peripheral()` as that will remove all protocols anyway >> and the call is simpler. I can make that change in v6 of the series. > > This patch does not correctly interface with the driver model. > > What we need is: > > * When a SPI flash device is probed successfully this causes the > installation of the protocols and thereby the creation of the handle. > * When trying to remove a SPI flash device this causes the > uninstallation of the protocols. > > For interfacing with the driver model you should use the events > EVT_DM_POST_PROBE and EVT_DM_PRE_REMOVE. Cf. efi_bl_init(). > > When the driver model tries to remove the SPI device you have to call > UninstallMultipleProtocolInterfaces(). > > efi_delete_handle() does not check if one of the protocols installed on > the handles has been opened (e.g. with BY_DRIVER) and therefore cannot > request those parts of the loaded code that still hold references to the > protocol interfaces to close the protocols. This may lead to crashes. > > If UninstallMultipleProtocolInterfaces() returns an error, the remove > event handler must return an error to avoid the removal of the device. Hi Heinrich, I appreciate your feedback, though I have to say I'm disappointed that this feedback wasn't provided on an earlier iteration of the patch series. I've got some other pressing priorities this week but will try to re-write the relevant functions next week and re-submit. Thanks, -- Paul Barker Principal Software Engineer SanCloud Ltd e: paul.barker@sancloud.com w: https://sancloud.com/