[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Call SetVirtualAddressMap() by default
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@xxxxxxxxxx/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
|
