[PATCH] Call SetVirtualAddressMap() by default

[PATCH] Call SetVirtualAddressMap() by default

[Demi Marie Obenour]

To quote Andrew Cooper:

> I know we've had this argument before, but not calling
> SetVirtualAddressMap() isn't a viable option.  It's a prerequisite to
> function on literally millions of devices

Qubes OS has been shipping EFI_SET_VIRTUAL_ADDRESS_MAP for years, and I
believe OpenXT and EVE ship it as well. Mark EFI_SET_VIRTUAL_ADDRESS_MAP
as SUPPORTED and enable it by default.

Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
---
What is necessary for this patch to be accepted?

 xen/common/Kconfig | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index f1ea3199c8ebdc1e9d0968b9de007f4df44bac79..5b8edb6d81a1296969f968702c5e7f2c775ceba9 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -215,14 +215,15 @@ config KEXEC
 	  If unsure, say Y.
 
 config EFI_SET_VIRTUAL_ADDRESS_MAP
-    bool "EFI: call SetVirtualAddressMap()" if EXPERT
+    bool "EFI: call SetVirtualAddressMap()"
+    default y
     ---help---
       Call EFI SetVirtualAddressMap() runtime service to setup memory map for
       further runtime services. According to UEFI spec, it isn't strictly
       necessary, but many UEFI implementations misbehave when this call is
       missing.
 
-      If unsure, say N.
+      If unsure, say Y.
 
 config XENOPROF
 	def_bool y
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab

Re: [PATCH] Call SetVirtualAddressMap() by default

[Jan Beulich]

On 22.02.2023 20:14, Demi Marie Obenour wrote:
> To quote Andrew Cooper:
> 
>> I know we've had this argument before, but not calling
>> SetVirtualAddressMap() isn't a viable option.  It's a prerequisite to
>> function on literally millions of devices
> 
> Qubes OS has been shipping EFI_SET_VIRTUAL_ADDRESS_MAP for years, and I
> believe OpenXT and EVE ship it as well. Mark EFI_SET_VIRTUAL_ADDRESS_MAP
> as SUPPORTED and enable it by default.

This is insufficient justification. The main reason why we don't call
it is because it can be called only once. Any entity taking over later
(e.g. via kexec) can't do anything anymore about the virtual address
associations once set. Hence what's needed to justify a change like
this is an explanation of why this restriction is not really an issue
to anyone in any case.

Jan

Re: [PATCH] Call SetVirtualAddressMap() by default

[Marek Marczykowski-Górecki]

[-- Attachment #1: Type: text/plain, Size: 1634 bytes --]

On Thu, Feb 23, 2023 at 11:16:28AM +0100, Jan Beulich wrote:
> On 22.02.2023 20:14, Demi Marie Obenour wrote:
> > To quote Andrew Cooper:
> > 
> >> I know we've had this argument before, but not calling
> >> SetVirtualAddressMap() isn't a viable option.  It's a prerequisite to
> >> function on literally millions of devices
> > 
> > Qubes OS has been shipping EFI_SET_VIRTUAL_ADDRESS_MAP for years, and I
> > believe OpenXT and EVE ship it as well. Mark EFI_SET_VIRTUAL_ADDRESS_MAP
> > as SUPPORTED and enable it by default.
> 
> This is insufficient justification. The main reason why we don't call
> it is because it can be called only once. Any entity taking over later
> (e.g. via kexec) can't do anything anymore about the virtual address
> associations once set. Hence what's needed to justify a change like
> this is an explanation of why this restriction is not really an issue
> to anyone in any case.

AFAIR from the discussion about the original patch, kexec under Xen does
not preserve runtime services working anyway, so this limitation is more
about some possible kexec implementation in the future, not actually
breaking something right now. And since Linux calls
SetVirtualAddressMap() _and_ supports kexec just fine, it's definitely
possible to design this future kexec extension to work after
SetVirtualAddressMap() too.

Relevant parts of that older discussion:
- https://lore.kernel.org/all/272a9354-bcb4-50a4-a251-6a453221d6e3@citrix.com/T/#u
- https://lore.kernel.org/all/20191009235725.GT8065@mail-itl/T/#u

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] Call SetVirtualAddressMap() by default

[Jan Beulich]

On 23.02.2023 14:08, Marek Marczykowski-Górecki wrote:
> On Thu, Feb 23, 2023 at 11:16:28AM +0100, Jan Beulich wrote:
>> On 22.02.2023 20:14, Demi Marie Obenour wrote:
>>> To quote Andrew Cooper:
>>>
>>>> I know we've had this argument before, but not calling
>>>> SetVirtualAddressMap() isn't a viable option.  It's a prerequisite to
>>>> function on literally millions of devices
>>>
>>> Qubes OS has been shipping EFI_SET_VIRTUAL_ADDRESS_MAP for years, and I
>>> believe OpenXT and EVE ship it as well. Mark EFI_SET_VIRTUAL_ADDRESS_MAP
>>> as SUPPORTED and enable it by default.
>>
>> This is insufficient justification. The main reason why we don't call
>> it is because it can be called only once. Any entity taking over later
>> (e.g. via kexec) can't do anything anymore about the virtual address
>> associations once set. Hence what's needed to justify a change like
>> this is an explanation of why this restriction is not really an issue
>> to anyone in any case.
> 
> AFAIR from the discussion about the original patch, kexec under Xen does
> not preserve runtime services working anyway, so this limitation is more
> about some possible kexec implementation in the future, not actually
> breaking something right now. And since Linux calls
> SetVirtualAddressMap() _and_ supports kexec just fine, it's definitely
> possible to design this future kexec extension to work after
> SetVirtualAddressMap() too.
> 
> Relevant parts of that older discussion:
> - https://lore.kernel.org/all/272a9354-bcb4-50a4-a251-6a453221d6e3@citrix.com/T/#u
> - https://lore.kernel.org/all/20191009235725.GT8065@mail-itl/T/#u

Well, there are various statements there without further reference. I'm
having a hard time seeing how a full-fledged Linux could do well without
runtime services, or without being able to set the virtual address map
to its liking. If they can, then a question would be why they need to
set the virtual address map in the first place (yes, there is this
supposed "firmware bugs" argument, which unfortunately I lack any proof
of; at the very least I'm unaware of bug reports against Xen boiling
down to us not making this call). Plus maybe they can merely because old
and new OS are similar enough in their (virtual) memory layout? IOW
kexec-ing to Linux for crash dumping purposes is just one (important)
example of the functionality that needs retaining. Once we get better
PVH Dom0 support, maybe other Dom0 OSes surface with entirely different
needs. As said back then - part of the reason why in the original
implementation I've avoided making this call is because of the fear of
closing possible roads we may need to take in the future.

Jan

Re: [PATCH] Call SetVirtualAddressMap() by default

[Marek Marczykowski-Górecki]

[-- Attachment #1: Type: text/plain, Size: 4406 bytes --]

On Thu, Feb 23, 2023 at 02:21:11PM +0100, Jan Beulich wrote:
> On 23.02.2023 14:08, Marek Marczykowski-Górecki wrote:
> > On Thu, Feb 23, 2023 at 11:16:28AM +0100, Jan Beulich wrote:
> >> On 22.02.2023 20:14, Demi Marie Obenour wrote:
> >>> To quote Andrew Cooper:
> >>>
> >>>> I know we've had this argument before, but not calling
> >>>> SetVirtualAddressMap() isn't a viable option.  It's a prerequisite to
> >>>> function on literally millions of devices
> >>>
> >>> Qubes OS has been shipping EFI_SET_VIRTUAL_ADDRESS_MAP for years, and I
> >>> believe OpenXT and EVE ship it as well. Mark EFI_SET_VIRTUAL_ADDRESS_MAP
> >>> as SUPPORTED and enable it by default.
> >>
> >> This is insufficient justification. The main reason why we don't call
> >> it is because it can be called only once. Any entity taking over later
> >> (e.g. via kexec) can't do anything anymore about the virtual address
> >> associations once set. Hence what's needed to justify a change like
> >> this is an explanation of why this restriction is not really an issue
> >> to anyone in any case.
> > 
> > AFAIR from the discussion about the original patch, kexec under Xen does
> > not preserve runtime services working anyway, so this limitation is more
> > about some possible kexec implementation in the future, not actually
> > breaking something right now. And since Linux calls
> > SetVirtualAddressMap() _and_ supports kexec just fine, it's definitely
> > possible to design this future kexec extension to work after
> > SetVirtualAddressMap() too.
> > 
> > Relevant parts of that older discussion:
> > - https://lore.kernel.org/all/272a9354-bcb4-50a4-a251-6a453221d6e3@citrix.com/T/#u
> > - https://lore.kernel.org/all/20191009235725.GT8065@mail-itl/T/#u
> 
> Well, there are various statements there without further reference. I'm
> having a hard time seeing how a full-fledged Linux could do well without
> runtime services, or without being able to set the virtual address map
> to its liking. If they can, then a question would be why they need to
> set the virtual address map in the first place (yes, there is this
> supposed "firmware bugs" argument, which unfortunately I lack any proof
> of; at the very least I'm unaware of bug reports against Xen boiling
> down to us not making this call). 

The second link points at a thread of one of such bug reports. The first
link points at thread, where initial message contains list of systems
affected. If that is not enough, I can point you also dig up (at least)
tens of Qubes users reporting similar issues in the past. I'm sure
Andrew could do the same about XenServer (which is why "mapbs"
workaround exists).

> Plus maybe they can merely because old
> and new OS are similar enough in their (virtual) memory layout? IOW
> kexec-ing to Linux for crash dumping purposes is just one (important)
> example of the functionality that needs retaining. 

It works just fine with Xen calling SetVirtualAddressMap().
SetVirtualAddressMap() is relevant only for using runtime services, and
you don't need them for crash dumps. In fact, runtime services are not
accessible to post-kexec Linux anyway, so this call doesn't change
anything.
Additionally, given most stuff works
just fine with efi=no-rs proves it isn't severe limitation, if it really
would need to be there - but as Andrew noted, given Linux example, it
doesn't really need to be the case - it may simply require a bit more
thinking when adding runtime services capability past kexec.

> Once we get better
> PVH Dom0 support, maybe other Dom0 OSes surface with entirely different
> needs.

I find this claim rather weird. Runtime services are a thing that Xen
needs to call, not some domain. And Xen has control over its memory
layout.

_If_ PVH dom0 would really turn out to be incompatible with
SetVirtualAddressMap() call by Xen (which I highly doubt), then some
alternative for that case can be made. But that's only speculation.

> As said back then - part of the reason why in the original
> implementation I've avoided making this call is because of the fear of
> closing possible roads we may need to take in the future.

Yet, not calling SetVirtualAddressMap() leads to actual issues _right
now_, not in some hypothetical undefined future.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] Call SetVirtualAddressMap() by default

[Jan Beulich]

On 23.02.2023 14:56, Marek Marczykowski-Górecki wrote:
> On Thu, Feb 23, 2023 at 02:21:11PM +0100, Jan Beulich wrote:
>> On 23.02.2023 14:08, Marek Marczykowski-Górecki wrote:
>>> On Thu, Feb 23, 2023 at 11:16:28AM +0100, Jan Beulich wrote:
>>>> On 22.02.2023 20:14, Demi Marie Obenour wrote:
>>>>> To quote Andrew Cooper:
>>>>>
>>>>>> I know we've had this argument before, but not calling
>>>>>> SetVirtualAddressMap() isn't a viable option.  It's a prerequisite to
>>>>>> function on literally millions of devices
>>>>>
>>>>> Qubes OS has been shipping EFI_SET_VIRTUAL_ADDRESS_MAP for years, and I
>>>>> believe OpenXT and EVE ship it as well. Mark EFI_SET_VIRTUAL_ADDRESS_MAP
>>>>> as SUPPORTED and enable it by default.
>>>>
>>>> This is insufficient justification. The main reason why we don't call
>>>> it is because it can be called only once. Any entity taking over later
>>>> (e.g. via kexec) can't do anything anymore about the virtual address
>>>> associations once set. Hence what's needed to justify a change like
>>>> this is an explanation of why this restriction is not really an issue
>>>> to anyone in any case.
>>>
>>> AFAIR from the discussion about the original patch, kexec under Xen does
>>> not preserve runtime services working anyway, so this limitation is more
>>> about some possible kexec implementation in the future, not actually
>>> breaking something right now. And since Linux calls
>>> SetVirtualAddressMap() _and_ supports kexec just fine, it's definitely
>>> possible to design this future kexec extension to work after
>>> SetVirtualAddressMap() too.
>>>
>>> Relevant parts of that older discussion:
>>> - https://lore.kernel.org/all/272a9354-bcb4-50a4-a251-6a453221d6e3@citrix.com/T/#u
>>> - https://lore.kernel.org/all/20191009235725.GT8065@mail-itl/T/#u
>>
>> Well, there are various statements there without further reference. I'm
>> having a hard time seeing how a full-fledged Linux could do well without
>> runtime services, or without being able to set the virtual address map
>> to its liking. If they can, then a question would be why they need to
>> set the virtual address map in the first place (yes, there is this
>> supposed "firmware bugs" argument, which unfortunately I lack any proof
>> of; at the very least I'm unaware of bug reports against Xen boiling
>> down to us not making this call). 
> 
> The second link points at a thread of one of such bug reports.

Hmm, yes, digging through the about two dozen mails, I can see there is a
connection to (not) calling SetVirtualAddressMap() there.

>> Plus maybe they can merely because old
>> and new OS are similar enough in their (virtual) memory layout? IOW
>> kexec-ing to Linux for crash dumping purposes is just one (important)
>> example of the functionality that needs retaining. 
> 
> It works just fine with Xen calling SetVirtualAddressMap().
> SetVirtualAddressMap() is relevant only for using runtime services, and
> you don't need them for crash dumps. In fact, runtime services are not
> accessible to post-kexec Linux anyway, so this call doesn't change
> anything.
> Additionally, given most stuff works
> just fine with efi=no-rs proves it isn't severe limitation, if it really
> would need to be there - but as Andrew noted, given Linux example, it
> doesn't really need to be the case - it may simply require a bit more
> thinking when adding runtime services capability past kexec.

All of what you say here is what I had meant to cover by adding the
"(important)", which initially I didn't have. 

>> Once we get better
>> PVH Dom0 support, maybe other Dom0 OSes surface with entirely different
>> needs.
> 
> I find this claim rather weird. Runtime services are a thing that Xen
> needs to call, not some domain. And Xen has control over its memory
> layout.
> 
> _If_ PVH dom0 would really turn out to be incompatible with
> SetVirtualAddressMap() call by Xen (which I highly doubt), then some
> alternative for that case can be made. But that's only speculation.

The remark wasn't about Dom0 itself wanting runtime services access:
Dom0 isn't going to be provided such, at least not to the physical EFI's.
If such was needed for PVH Dom0, we'd need to wire it to virtual firmware
hooks (which in turn may or may not be viewed as against some of the
ideas of PVH).

The remark was instead meant to point out that such an alternative OS
may want to invoke another (native) instance of itself for e.g. crash
dumping purposes.

>> As said back then - part of the reason why in the original
>> implementation I've avoided making this call is because of the fear of
>> closing possible roads we may need to take in the future.
> 
> Yet, not calling SetVirtualAddressMap() leads to actual issues _right
> now_, not in some hypothetical undefined future.

That's the way you, Andrew, and others like to put it. My view at this
is that it's not the lack of the call, but the improper implementation
of firmware which leads to an apparent need for this call. Like for all
other firmware bug workarounds, I'm happy to accept any proposals for
workarounds, as long as such workarounds either don't impact spec-
compliant systems, or as long as they're off by default. In the case
here this would mean that I'm likely to approve a change which converts
the EFI_SET_VIRTUAL_ADDRESS_MAP Kconfig to a command line option (as
long as it looks technically correct, of course), improving flexibility
from the present build-time-only control. (Qubes and other distros
would then be free to change the default of that option from "false" to
"true", and that would be a pretty easy to maintain local change.)

However, part of the reason why originally this wasn't (even meant to
be) controllable via Kconfig (or any pre-Kconfig means) was that I
don't think the handling of the ->VirtualStart fields is correct for
this case in efi_init_memory(): The final layout simply cannot be done
there (after SetVirtualAddressMap() was already called). Since I don't
think laying out virtual space can be moved ahead, this would mean
moving the call to SetVirtualAddressMap() past this layout phase. Yet
then we may again end up calling runtime services functions (time
ones in particular, but note that I didn't check the order of things,
so "may" was deliberate) ahead of calling SetVirtualAddressMap().

Plus, once you look at the layout code, you'll find a number of cases
which simply end in an error message.

To confess - I was quite happy back at the time that I could use the
kexec concern as a justification for not needing to solve these issues
right away.

These issues would, however, need sorting (by code changes or verbal
justification) no matter whether the Kconfig default was to change
(or even just the EXPERT dependency to go away), or whether the
conversion to a command line option was the way to go.

Jan

Re: [PATCH] Call SetVirtualAddressMap() by default

[George Dunlap]

[-- Attachment #1: Type: text/plain, Size: 7299 bytes --]

On Fri, Feb 24, 2023 at 8:20 AM Jan Beulich <jbeulich@suse.com> wrote:

> On 23.02.2023 14:56, Marek Marczykowski-Górecki wrote:
> > On Thu, Feb 23, 2023 at 02:21:11PM +0100, Jan Beulich wrote:
> >> On 23.02.2023 14:08, Marek Marczykowski-Górecki wrote:
> >>> On Thu, Feb 23, 2023 at 11:16:28AM +0100, Jan Beulich wrote:
> >>>> On 22.02.2023 20:14, Demi Marie Obenour wrote:
> >>>>> To quote Andrew Cooper:
> >>>>>
> >>>>>> I know we've had this argument before, but not calling
> >>>>>> SetVirtualAddressMap() isn't a viable option.  It's a prerequisite
> to
> >>>>>> function on literally millions of devices
> >>>>>
> >>>>> Qubes OS has been shipping EFI_SET_VIRTUAL_ADDRESS_MAP for years,
> and I
> >>>>> believe OpenXT and EVE ship it as well. Mark
> EFI_SET_VIRTUAL_ADDRESS_MAP
> >>>>> as SUPPORTED and enable it by default.
> >>>>
> >>>> This is insufficient justification. The main reason why we don't call
> >>>> it is because it can be called only once. Any entity taking over later
> >>>> (e.g. via kexec) can't do anything anymore about the virtual address
> >>>> associations once set. Hence what's needed to justify a change like
> >>>> this is an explanation of why this restriction is not really an issue
> >>>> to anyone in any case.
> >>>
> >>> AFAIR from the discussion about the original patch, kexec under Xen
> does
> >>> not preserve runtime services working anyway, so this limitation is
> more
> >>> about some possible kexec implementation in the future, not actually
> >>> breaking something right now. And since Linux calls
> >>> SetVirtualAddressMap() _and_ supports kexec just fine, it's definitely
> >>> possible to design this future kexec extension to work after
> >>> SetVirtualAddressMap() too.
> >>>
> >>> Relevant parts of that older discussion:
> >>> -
> https://lore.kernel.org/all/272a9354-bcb4-50a4-a251-6a453221d6e3@citrix.com/T/#u
> >>> - https://lore.kernel.org/all/20191009235725.GT8065@mail-itl/T/#u
> >>
> >> Well, there are various statements there without further reference. I'm
> >> having a hard time seeing how a full-fledged Linux could do well without
> >> runtime services, or without being able to set the virtual address map
> >> to its liking. If they can, then a question would be why they need to
> >> set the virtual address map in the first place (yes, there is this
> >> supposed "firmware bugs" argument, which unfortunately I lack any proof
> >> of; at the very least I'm unaware of bug reports against Xen boiling
> >> down to us not making this call).
> >
> > The second link points at a thread of one of such bug reports.
>
> Hmm, yes, digging through the about two dozen mails, I can see there is a
> connection to (not) calling SetVirtualAddressMap() there.
>
> >> Plus maybe they can merely because old
> >> and new OS are similar enough in their (virtual) memory layout? IOW
> >> kexec-ing to Linux for crash dumping purposes is just one (important)
> >> example of the functionality that needs retaining.
> >
> > It works just fine with Xen calling SetVirtualAddressMap().
> > SetVirtualAddressMap() is relevant only for using runtime services, and
> > you don't need them for crash dumps. In fact, runtime services are not
> > accessible to post-kexec Linux anyway, so this call doesn't change
> > anything.
> > Additionally, given most stuff works
> > just fine with efi=no-rs proves it isn't severe limitation, if it really
> > would need to be there - but as Andrew noted, given Linux example, it
> > doesn't really need to be the case - it may simply require a bit more
> > thinking when adding runtime services capability past kexec.
>
> All of what you say here is what I had meant to cover by adding the
> "(important)", which initially I didn't have.
>
> >> Once we get better
> >> PVH Dom0 support, maybe other Dom0 OSes surface with entirely different
> >> needs.
> >
> > I find this claim rather weird. Runtime services are a thing that Xen
> > needs to call, not some domain. And Xen has control over its memory
> > layout.
> >
> > _If_ PVH dom0 would really turn out to be incompatible with
> > SetVirtualAddressMap() call by Xen (which I highly doubt), then some
> > alternative for that case can be made. But that's only speculation.
>
> The remark wasn't about Dom0 itself wanting runtime services access:
> Dom0 isn't going to be provided such, at least not to the physical EFI's.
> If such was needed for PVH Dom0, we'd need to wire it to virtual firmware
> hooks (which in turn may or may not be viewed as against some of the
> ideas of PVH).
>
> The remark was instead meant to point out that such an alternative OS
> may want to invoke another (native) instance of itself for e.g. crash
> dumping purposes.
>
> >> As said back then - part of the reason why in the original
> >> implementation I've avoided making this call is because of the fear of
> >> closing possible roads we may need to take in the future.
> >
> > Yet, not calling SetVirtualAddressMap() leads to actual issues _right
> > now_, not in some hypothetical undefined future.
>
> That's the way you, Andrew, and others like to put it. My view at this
> is that it's not the lack of the call, but the improper implementation
> of firmware which leads to an apparent need for this call. Like for all
> other firmware bug workarounds, I'm happy to accept any proposals for
> workarounds, as long as such workarounds either don't impact spec-
> compliant systems, or as long as they're off by default.


But it sounds (reading though this thread) like it doesn't impact any
spec-compliant systems -- that is, not any *known* ones, but only
hypothetical dom0's which are neither Linux (including kexec) nor NetBSD
nor FreeBSD.

If we were Microsoft, we could afford to say "we don't support this
hardware", and that would be enough to get the manufacturers to change
their tune; but we're not.  Making it difficult for our users will not
fundamentally make vendors write better code.

Particularly as my guess is that it's probably mainly a matter of testing:
They only do testing on Windows (or maybe Linux if they're particularly
keen), both of which seem to call SetVirtualAddressMap(); and so bits of
the code accidentally come to rely on it being called.  Sure, in a perfect
world, developers would read the spec, automatically follow it, and test on
all possible hardware; but given how software actually works, it seems
inevitable that we're going to have a never-ending stream of bugs because
we're behaving differently.

So literally the only benefit of your policy is to accommodate hypothetical
operating systems, who may need the functionality for unknown reasons.  And
the cost is to have vanilla Xen not work on loads of real systems.  I don't
think this is the right decision; and it seems like the sort of general
higher-level principle that it would make sense to have a project-wide vote
on if discussion failed to reach consensus.

(Obviously if there are other technical issues, those would need to be
addressed first.)

Supposing such a hypothetical operating system appears, is there any reason
we can't figure out how to provide it what it needs at that time?

 -George

[-- Attachment #2: Type: text/html, Size: 8901 bytes --]

Re: [PATCH] Call SetVirtualAddressMap() by default

[Jan Beulich]

On 10.03.2023 17:05, George Dunlap wrote:
> On Fri, Feb 24, 2023 at 8:20 AM Jan Beulich <jbeulich@suse.com> wrote:
>> On 23.02.2023 14:56, Marek Marczykowski-Górecki wrote:
>>> Yet, not calling SetVirtualAddressMap() leads to actual issues _right
>>> now_, not in some hypothetical undefined future.
>>
>> That's the way you, Andrew, and others like to put it. My view at this
>> is that it's not the lack of the call, but the improper implementation
>> of firmware which leads to an apparent need for this call. Like for all
>> other firmware bug workarounds, I'm happy to accept any proposals for
>> workarounds, as long as such workarounds either don't impact spec-
>> compliant systems, or as long as they're off by default.
> 
> 
> But it sounds (reading though this thread) like it doesn't impact any
> spec-compliant systems -- that is, not any *known* ones, but only
> hypothetical dom0's which are neither Linux (including kexec) nor NetBSD
> nor FreeBSD.

I can't say anything about the BSDs. Originally, when our EFI support was
first implemented, kexec would have been affected from all I know. The
way it works was changed meanwhile.

As to "known" affected systems: Elsewhere on this thread I pointed out
that I was happy that, at the time, I had an excuse not to worry about
getting our use of SetVirtualAddressMap() correct. Properly establishing
virtual addresses before making that call is still an open issue, for
which I do not yet see a good solution. Marek saying "It works just fine"
can only mean the limited set of systems that were tested. With more of
physical address space populated, said problem would arise. Can I point
at a particular system? No. But I do know that systems with "odd" memory
maps exist (that's what e.g. our PDX abstraction was introduced for),
and I anticipate problems there.

An intermediate approach might be to call SetVirtualAddressMap() on
systems where we can establish a complete virtual address map early
enough. (Saying that without looking at the code, so this may not be
viable in practice.) But I'm unsure this would improve the overall
situation: We'd then test one thing on typical systems and would end up
even less certain whether Xen also works on atypical ones.

> If we were Microsoft, we could afford to say "we don't support this
> hardware", and that would be enough to get the manufacturers to change
> their tune; but we're not.  Making it difficult for our users will not
> fundamentally make vendors write better code.

We can certainly agree on the goal of not making it more difficult than
necessary for users. Provided reasonable command line option documentation,
it shouldn't be overly difficult to establish which workarounds need
enabling for a given platform. For known problematic ones we could even
enable workarounds by default (just like we do in various cases when EFI
does not come into play; most of that was inherited from Linux and hence
is rather dated now, though).

> Particularly as my guess is that it's probably mainly a matter of testing:
> They only do testing on Windows (or maybe Linux if they're particularly
> keen), both of which seem to call SetVirtualAddressMap(); and so bits of
> the code accidentally come to rely on it being called.  Sure, in a perfect
> world, developers would read the spec, automatically follow it, and test on
> all possible hardware; but given how software actually works, it seems
> inevitable that we're going to have a never-ending stream of bugs because
> we're behaving differently.
> 
> So literally the only benefit of your policy is to accommodate hypothetical
> operating systems, who may need the functionality for unknown reasons.  And
> the cost is to have vanilla Xen not work on loads of real systems.  I don't
> think this is the right decision; and it seems like the sort of general
> higher-level principle that it would make sense to have a project-wide vote
> on if discussion failed to reach consensus.

Certainly. I'm pretty sure I would end up in a minority in such an event.
Yet if we started to enable random firmware workarounds by default which
can have an impact on spec-conforming systems, I guess I would have to
seriously consider to step down as the maintainer of our EFI code. But
maybe that's what's wanted by others anyway ...

> (Obviously if there are other technical issues, those would need to be
> addressed first.)
> 
> Supposing such a hypothetical operating system appears, is there any reason
> we can't figure out how to provide it what it needs at that time?

Well - about everything can be done in software. The question is how much
of the necessary road we've ended up closing by going the "account for
quirks by default" route, and hence how complicated things would end up
being.

Jan