From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.120]) by mx.groups.io with SMTP id smtpd.web11.9020.1587477272958042123 for ; Tue, 21 Apr 2020 06:54:33 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=KbSNZvJQ; spf=pass (domain: redhat.com, ip: 207.211.31.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1587477272; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=TmZVFgt/AKYuEpmHNMBfHZP0ybu53oa+A9Wo0NG89qg=; b=KbSNZvJQn/loE2vke6qd674Ydj8MjJJ9GDK41O+xKG1olefqjuoHpg2A8hFQXsHlyiHpmE k0vI5BFxJFVSRhV9R591OJdHMTgTMYZSbnYtODLDh529J6SF0gLVG9cX4QEBvIH1tT+bH+ x/NgIA4IwmZmIgZguuahKNuVVfj5bAg= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-359-vSue1UMSPL2OI_BP3NQ8-A-1; Tue, 21 Apr 2020 09:54:30 -0400 X-MC-Unique: vSue1UMSPL2OI_BP3NQ8-A-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 49824DBA8; Tue, 21 Apr 2020 13:54:28 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-115-52.ams2.redhat.com [10.36.115.52]) by smtp.corp.redhat.com (Postfix) with ESMTP id 08237A18A1; Tue, 21 Apr 2020 13:54:26 +0000 (UTC) Subject: Re: [PATCH 2/2] OvmfPkg/OvmfXen: Introduce XenDebugLibIoPort To: Anthony PERARD , devel@edk2.groups.io Cc: Ard Biesheuvel , Jordan Justen , Julien Grall References: <20200420162918.2312514-1-anthony.perard@citrix.com> <20200420162918.2312514-3-anthony.perard@citrix.com> From: "Laszlo Ersek" Message-ID: Date: Tue, 21 Apr 2020 15:54:26 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20200420162918.2312514-3-anthony.perard@citrix.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 04/20/20 18:29, Anthony PERARD wrote: > Introduce XenDebugLibIoPort which is enabled with > DEBUG_ON_HYPERVISOR_CONSOLE which send the debug output to Xen's > console. > > It's a copy PlatformDebugLibIoPort which always write to the IO port. > It works with both Xen HVM guest and Xen PVH guest whereas the default > PlatformDebugLibIoPort works only in HVM when QEMU is present. > > Signed-off-by: Anthony PERARD > --- > > Notes: > Do you think it would be better to introduce a PCD to > PlatformDebugLibIoPort where we can disable the check > for the debug port and simply always write to it? > > OvmfPkg/OvmfXen.dsc | 13 ++++++++++++ > .../PlatformDebugLibIoPort.inf | 6 +++--- > .../DebugLibDetect.h | 20 ------------------- > .../DebugLib.c | 16 --------------- > .../DebugLibDetect.c | 20 ++----------------- > 5 files changed, 18 insertions(+), 57 deletions(-) > copy OvmfPkg/Library/{PlatformDebugLibIoPort => XenDebugLibIoPort}/PlatformDebugLibIoPort.inf (76%) > copy OvmfPkg/Library/{PlatformDebugLibIoPort => XenDebugLibIoPort}/DebugLibDetect.h (57%) > copy OvmfPkg/Library/{PlatformDebugLibIoPort => XenDebugLibIoPort}/DebugLib.c (94%) > copy OvmfPkg/Library/{PlatformDebugLibIoPort => XenDebugLibIoPort}/DebugLibDetect.c (61%) I think this approach is a good first approximation, but I think we can refine it. Rather than introducing a new directory, we should operate within "OvmfPkg/Library/PlatformDebugLibIoPort". (1) Please write a patch that replaces all instances of the string "QEMU" with "hypervisor" in the following files: - DebugLib.c - DebugLibDetect.c - DebugLibDetect.h - DebugLibDetectRom.c (2) Please write a patch for the following: - move the PlatformDebugLibIoPortDetect() function definition to a separate file called "DebugIoPortQemu.c", - add the new C file to both existent INF files: - PlatformDebugLibIoPort.inf - PlatformRomDebugLibIoPort.inf (keep the source file lists alphabetically sorted) - from "DebugLibDetect.h", move the BOCHS_DEBUG_PORT_MAGIC macro definition to "DebugIoPortQemu.c". I think this patch can be called: OvmfPkg/PlatformDebugLibIoPort: factor out debug port detection After this patch, we well have two independent dimensions, for any particular INF file: - in PlatformDebugLibIoPortFound(), the library instance may or may not cache the result of debug port checking - in PlatformDebugLibIoPortDetect(), the library instance may abstract the debug port detection method (3) Please write another patch: - introduce "DebugIoPortNocheck.c", - implement PlatformDebugLibIoPortDetect() as "return TRUE", - introduce a new INF file "PlatformRomDebugLibIoPortNocheck.inf", for linking together: - DebugIoPortNocheck.c - DebugLib.c - DebugLibDetectRom.c This library instance will not cache the result of the detection, and the detection will return constant TRUE. (4) Then, this library instance can be put to use in OvmfXen.dsc (together with setting the proper PCD value). The library instance can be used in all firmware phases / module types. Thanks, Laszlo