Xen-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: "Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>, Wei Liu <wl@xen.org>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Tim Deegan <tim@xen.org>, Ian Jackson <ian.jackson@eu.citrix.com>,
	Julien Grall <julien.grall@arm.com>,
	Jan Beulich <jbeulich@suse.com>,
Subject: Re: [Xen-devel] [PATCH v2 2/2] xen/efi: optionally call SetVirtualAddressMap()
Date: Sat, 12 Oct 2019 18:13:28 +0100
Message-ID: <251d8594-0d4f-fd4b-20c3-4f5edf3dc23f@citrix.com> (raw)
In-Reply-To: <20191012170040.GB28966@mail-itl>

On 12/10/2019 18:00, Marek Marczykowski-Górecki wrote:
>>> For this reason, make it an optional feature, depending on
>>> !KEXEC.
>> This presupposes (at Xen's build time) that a kexec'd kernel is going to
>> want/need to use runtime services.  I'm not convinced this is
>> universally true,
> In fact, as it turned out in the discussion, right now it definitely
> can't, as it doesn't get runtime services table (or efi system table or
> any other info required for this).

On, in which case definitely drop the dependency.  Kexec works fine (and
is tested thoroughly by XenServer) with current Xen when EFI booted, and
the call to SetVirtualAddressMap() won't make any difference in the
kexec environment.

Whomever does the plumbing for EFI details in the future can also pass
the virtual map (like Linux already does) and everything will continue
to work fine.

>>> And to not inflate number of supported configurations, hide it
>>> behind EXPERT.
>> "number of supported configurations" isn't a relevant argument.  We will
>> have as few or as many as are appropriate to present to user, given a
>> baseline competency of "able to at read and comprehend the descriptions
>> given".
>> A valid reason for putting this behind EXPERT is because it is an
>> interim bit of duct tape, trying to work around other breakages in Xen,
> Rather in UEFI...

I suppose, yes.

>>> +    for ( i = 0; i < efi_memmap_size; i += efi_mdesc_size )
>>> +    {
>>> +        EFI_MEMORY_DESCRIPTOR *desc = efi_memmap + i;
>>> +
>>> +        if ( desc->Attribute & EFI_MEMORY_RUNTIME )
>>> +            desc->VirtualStart = desc->PhysicalStart;
>>> +        else
>>> +            desc->VirtualStart = INVALID_VIRTUAL_ADDRESS;
>>> +    }
>>> +    status = efi_rs->SetVirtualAddressMap(efi_memmap_size, efi_mdesc_size,
>>> +                                          mdesc_ver, efi_memmap);
>>> +    if ( status != EFI_SUCCESS )
>>> +    {
>>> +        printk(XENLOG_ERR "EFI: SetVirtualAddressMap() failed (%#lx), disabling runtime services\n",
>>> +               status);
>> ... here.  printk() isn't set up, and won't go anywhere useful.
> I can't. It's after ExitBootServices(). Isn't it going to land in
> console ring, to be printed later?

Urgh.  Yes - you're right.  It will sit in the console buffer and appear
at the top of `xl dmesg`.

Leave it as-is.


Xen-devel mailing list

      parent reply index

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-12 14:36 [Xen-devel] [PATCH v2 0/2] Optionally call EFI SetVirtualAddressMap() Marek Marczykowski-Górecki
2019-10-12 14:36 ` [Xen-devel] [PATCH v2 1/2] efi: remove old SetVirtualAddressMap() arrangement Marek Marczykowski-Górecki
2019-10-12 16:33   ` Andrew Cooper
2019-10-12 14:36 ` [Xen-devel] [PATCH v2 2/2] xen/efi: optionally call SetVirtualAddressMap() Marek Marczykowski-Górecki
2019-10-12 16:29   ` Andrew Cooper
2019-10-12 17:00     ` Marek Marczykowski-Górecki
2019-10-12 17:09       ` Marek Marczykowski-Górecki
2019-10-12 17:13       ` Andrew Cooper [this message]

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=251d8594-0d4f-fd4b-20c3-4f5edf3dc23f@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=jbeulich@suse.com \
    --cc=julien.grall@arm.com \
    --cc=konrad.wilk@oracle.com \
    --cc=marmarek@invisiblethingslab.com \
    --cc=sstabellini@kernel.org \
    --cc=tim@xen.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \


* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Xen-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/xen-devel/0 xen-devel/git/0.git
	git clone --mirror https://lore.kernel.org/xen-devel/1 xen-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 xen-devel xen-devel/ https://lore.kernel.org/xen-devel \
		xen-devel@lists.xenproject.org xen-devel@lists.xen.org xen-devel@archiver.kernel.org
	public-inbox-index xen-devel

Example config snippet for mirrors

Newsgroup available over NNTP:

AGPL code for this site: git clone https://public-inbox.org/ public-inbox