Re: [PATCH v3 10/10] arm64: Change type of hsr, cpsr, spsr_el1 to uint64_t

[Jan Beulich <jbeulich@xxxxxxxx>]

On 05.05.2021 09:43, Michal Orzel wrote:
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -267,10 +267,10 @@ struct vcpu_guest_core_regs
>  
>      /* Return address and mode */
>      __DECL_REG(pc64,         pc32);             /* ELR_EL2 */
> -    uint32_t cpsr;                              /* SPSR_EL2 */
> +    uint64_t cpsr;                              /* SPSR_EL2 */
>  
>      union {
> -        uint32_t spsr_el1;       /* AArch64 */
> +        uint64_t spsr_el1;       /* AArch64 */
>          uint32_t spsr_svc;       /* AArch32 */
>      };

This change affects, besides domctl, also default_initialise_vcpu(),
which Arm's arch_initialise_vcpu() calls. I realize do_arm_vcpu_op()
only allows two unrelated VCPUOP_* to pass, but then I don't
understand why arch_initialise_vcpu() doesn't simply return e.g.
-EOPNOTSUPP. Hence I suspect I'm missing something.

> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -38,7 +38,7 @@
>  #include "hvm/save.h"
>  #include "memory.h"
>  
> -#define XEN_DOMCTL_INTERFACE_VERSION 0x00000013
> +#define XEN_DOMCTL_INTERFACE_VERSION 0x00000014

So this is to cover for the struct vcpu_guest_core_regs change.

> --- a/xen/include/public/vm_event.h
> +++ b/xen/include/public/vm_event.h
> @@ -266,8 +266,7 @@ struct vm_event_regs_arm {
>      uint64_t ttbr1;
>      uint64_t ttbcr;
>      uint64_t pc;
> -    uint32_t cpsr;
> -    uint32_t _pad;
> +    uint64_t cpsr;
>  };

Then I wonder why this isn't accompanied by a similar bump of
VM_EVENT_INTERFACE_VERSION. I don't see you drop any checking /
filling of the _pad field, so existing callers may pass garbage
there, and new callers need to be prevented from looking at the
upper half when running on an older hypervisor.

Jan