Re: [Xen-devel] struct vcpu_guest_core_reg stable ABI?

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 16/05/2019 08:58, Jan Beulich wrote:
>>>> On 15.05.19 at 22:12, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 15/05/2019 20:58, Julien Grall wrote:
>>> Hi all,
>>>
>>> It looks like the structures vcpu_guest_core_regs and
>>> vcpu_guest_context does not correctly reflect the AArch64 state. For
>>> instance, all Arm64 system registers (e.g sctlr, cpsr, spsr_el1)
>>> should be 64-bit wide not 32-bit wide.
>>>
>>> On ARMv8.0, some of the registers have only the low 32-bit defined,
>>> the rest is RES0. RES0 only means they are reserved for future use, it
>>> does not mean they can be ignored. Newer revision (such as
>>> ARMv8.0-SSBS) actually began to define bit in the top 32-bit.
>>>
>>> This means that the structures vcpu_guest_core_regs and
>>> vcpu_guest_context would not be able to store the top 32-bit and
>>> therefore misrepresenting the hardware.
>>>
>>> From my understanding, vcpu_guest_context is defined between the tools
>>> and Xen. So it would be possible to modify it without caring on
>>> backward compatibly.
>>>
>>> Howerver, struct vcpu_guest_core_reg seems to be outside of the
>>> #ifdef. So I assume it is part of the stable ABI. Am I correct?
>>>
>>> Do you have any suggestion how this could safely be extended?
>> Stuff like this should never have been in the public API to begin with. 
>> x86 has some nasty issues which I have yet to find a good-enough way to fix.
>>
>> For ARM, and future architectures, I'd use the fact that there are no
>> non-tools interfaces which use this structure to allow yourself the
>> wiggleroom to declare history a mistake, and fix it by making it tools-only.
> I'm unconvinced of the "declare history a mistake" part, but I agree
> with the suggestion of simply moving the structure declaration down
> into the guarded area. It simply was a mistake to not put it there in
> the first place.
>
> For x86 PV at least I don't really see how we could have gone
> without exposing this - we have to allow guests to specify at least
> some of a to-be-brought-up-vCPU's registers.  Anything else
> wouldn't really have been PV anymore.

All that is necessary for PV is %rip.  %rsp would also be convenient,
but literally nothing else is needed from the guests point of view.  The
ABI already has guests starting with a flat memory layout using the
Xen-provided selectors.  Everything else can be set up  by the entry code.

The real problem x86 has is guest_user_regs being in the public API. 
This absolutely shouldn't have happened, because it fixes the larger
chunk of the top-of-stack cpu_info block in the public API.

> For PVH this may have been avoidable.

PVH (for better or worse) followed suit with PV, because PV already
existed.  This was due to the expected lack of an LAPIC in guest
context, and see where that ended up.

Do *any* OSes actually use VCPUOP_initialise for PVH guests?  I can't
think of any offhand.

Furthermore, we now have an even more tangled mess of hypercalls where
DOMCTL_{get,set}vcpucontext and VCPUOP_initialise have an asymmetric API
when issues from the toolstack.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel