From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c06::231; helo=mail-io0-x231.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org 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 060B3223C176D for ; Tue, 27 Feb 2018 08:04:26 -0800 (PST) Received: by mail-io0-x231.google.com with SMTP id 30so21729536iog.2 for ; Tue, 27 Feb 2018 08:10:33 -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=FPJLqJ7A5ni7Q2vUggRj92/BdA8U3zyJCDmxYsJk8ao=; b=AT2F3PbNrzId+txXz7cb/RHMe8cizQNh54gs/4z3BnQst6rAfcAd0DPaEqeVUZf3FC EUmxRuag+fwevd4Awq6WVXGvPMA+DgA/Op3BuU+ce8wtXAifYrdWR6GrlS3Edsn81cLW EO6vG3ENywMFmOOuJ1B5kmyk1QEZ7JY7KjoYI= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=FPJLqJ7A5ni7Q2vUggRj92/BdA8U3zyJCDmxYsJk8ao=; b=Sy+Fejk2mshMhN2Cjl0MlKgjyPsCyZDlAA4vXPz7HieXfVL7+pCeQPxEIa1MZCLuwl AkGrXzbDRy7ztBlUolUl3li4js71AFh0RpIrq3DM5O8PVSTU7MbEmiXgw+g28mDNbTyn XQa+vqpgVINOP59LL2cCTjj3eMTbAOmfwrt5cpm/W3n6v1RWk5JfTOVkmDfuWOTyNYS0 LH9G/aIP75feA1XPSjcvsSx4t6/svicDvGAO5jz8vst58AeyN7UgYns8bdvYianYQLh6 wNMJunwNtDndMTlbY7zqpMR1Z6GvvY/X2GYsMUZ1gpW3VGZAw/HfIAUhrEq1kebMUeMz ZglA== X-Gm-Message-State: APf1xPBVaQnI+zxaZuoHcLJg4XJbkaw3ta1WIG8UABLq6l7xY4biT80E 9H00fKgVD6Yovq+mluEatflUSRnCas/Szptef8MVhw== X-Google-Smtp-Source: AG47ELtxI/2amTkgoVIJFJMDyKq0hYzn4WwfkU/duKLs+EOSwmsGDdghERyb9XtxU3RtKgh3ONYgIOxLaooUeGpyvs4= X-Received: by 10.107.52.73 with SMTP id b70mr17108697ioa.60.1519747831916; Tue, 27 Feb 2018 08:10:31 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.138.209 with HTTP; Tue, 27 Feb 2018 08:10:31 -0800 (PST) In-Reply-To: <2e4facde-6d3f-ef42-f8ee-6446736f3617@redhat.com> References: <20180224142515.461-1-ard.biesheuvel@linaro.org> <20180227015036.GC2261@SZX1000114654> <27ef5753-9c6c-3b99-e732-084d9e444158@redhat.com> <2e4facde-6d3f-ef42-f8ee-6446736f3617@redhat.com> From: Ard Biesheuvel Date: Tue, 27 Feb 2018 16:10:31 +0000 Message-ID: To: Laszlo Ersek Cc: Guo Heyi , "edk2-devel@lists.01.org" , Leif Lindholm Subject: Re: [PATCH edk2-platforms 1/2] Platform, Silicon: use DxeRuntimeDebugLibSerialPort for runtime DXE drivers X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 27 Feb 2018 16:04:27 -0000 Content-Type: text/plain; charset="UTF-8" On 27 February 2018 at 16:09, Laszlo Ersek wrote: > On 02/27/18 15:21, Ard Biesheuvel wrote: >> On 27 February 2018 at 10:43, Laszlo Ersek wrote: >>> On 02/27/18 10:23, Ard Biesheuvel wrote: >>>> On 27 February 2018 at 01:50, Guo Heyi wrote: >>>>> Hi Ard, >>>>> >>>>> Sorry for the late of seeing this patch. I have one question: why don't we >>>>> implement a runtime serial port lib, which will switch UART base address in >>>>> virtual address map change? I think this will be useful when we want to debug >>>>> runtime driver in OS stage. And if we have a runtime version of SerialPortLib, >>>>> then we don't need a runtime version of DebugLib which just disable touching >>>>> serial port. >>>>> >>>> >>>> Well, only if the serial port is not exposed to the OS as well. The >>>> Linux PL011 driver is especially easy to confuse, and having both the >>>> firmware and the OS control it at the same time is likely to cause >>>> problems. >>>> >>>> However, I do agree that having the ability to assign a UART to DEBUG >>>> at runtime is useful, and so I do intend to create a runtime version >>>> of the PL011 library, in which case DxeRuntimeDebugLibSerialPort can >>>> be replaced with BaseDebugLibSerialPort for DXE_RUNTIME_DRIVER >>>> modules. >>>> >>> >>> Converting the PL011 base address from phys to virt can be done in the >>> library instance, yes (and then every runtime driver module linked >>> against this library instance will individually convert the address for >>> its own use). The messier aspect is getting the PL011 base address into >>> the UEFI memmap, marked as MMIO / RUNTIME, so that the OS assign a >>> virtual mapping to it in the first place. >>> >>> The flash drivers generally do this with AddMemorySpace / >>> SetMemorySpaceAttributes. >>> >>> (Side point: while I agree that those are good APIs to invoke, I think >>> they should also call AllocateMemorySpace right after; otherwise a >>> "stray" AllocateMemorySpace elsewhere could allocate a chunk out of the >>> middle of what the flash driver *thinks* it owns.) >>> >>> The question is where this pair (or triplet) of GCD APIs should be called: >>> >>> - In a platform DXE driver? Perhaps. >>> >>> - In the DebugLib instance constructor / destructor? That could result >>> in some ugly reference counting -- you might want to keep the PL011 area >>> registered in GCD as long as *at least one* such runtime driver is loaded. >>> >>> This is different from the flash driver because the flash driver is the >>> sole runtime DXE (or SMM) driver that accesses & owns the flash MMIO >>> range. With the PL011 register block, that's not the case; all runtime >>> drivers that produce debug messages own it co-operatively. >>> >> >> Given that this SerialPortLib will be of the DXE_RUNTIME_DRIVER type >> anyway, couldn't we base it on a generic RuntimeUart protocol that we >> can depex on in the library, and produce in a separate [singleton] >> runtime DXE driver, which takes care of the UART initialization as >> well as the GCD memory space handling? > > Yes, that should totally work. > >> In fact, we could modify DxeRuntimeDebugLibSerialPort to attach to >> this protocol if it exists at EndOfDxe, and either do nothing at >> runtime (as it does currently) or produce UART output via the protocol >> if it exists. > > This is a good idea IMO. What about the following: since you'd have to > register an event notification function for EndOfDxe, why not register a > protocol notify directly for the runtime UART protocol? > Even better.