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

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Wed, May 5, 2021 at 4:00 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> 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.

No, there is no need to bump the vm event interface version here. They
are folding the _pad into the cpsr and the structure is always zero
initialized. There is never "garbage" in the _pad field. As such there
is no change to the structure layout or anyone using it with a header
compiled on an older version. I asked them not to bump the version for
this change.

Thanks,
Tamas