Re: [PATCH] xen/arm: XEN_DOMCTL_get_address_size hypercall support

[Milan Djokic <milan_djokic@xxxxxxxx>]

Hello Andrew,

On 10/2/25 12:10, Andrew Cooper wrote:
> On 01/10/2025 9:01 pm, Milan Djokic wrote:
> > Signed-off-by: Milan Djokic <milan_djokic@xxxxxxxx>
> >
> > ---
> > XEN_DOMCTL_get_address_size hypercall is not implemented for arm (only for x86)
> > It would be useful to have this hypercall supported for arm64, in order to get
> > current guest addressing mode and also to verify that 
> > XEN_DOMCTL_set_address_size
> > performs switch to target addressing mode (instead of relying on its returned 
> > error code only).
> Please don't copy this misfeature of x86 PV guests into ARM.
>
> Letting domains be of variable bitness after domain create leads to a
> whole lot of bugs, many security relevant.
>
> 32bit vs 64bit should be an input to domain_create(), not something that
> is edited after the domain has been constructed.
Yes, the idea behind this patch is not to introduce variable bitness, 
just to have the ability to get current addressing mode through 
hypercall. In our case, we have used it only in domain creation path 
(after domain_create(), to verify that target addressing mode is set).
Of course, whether this hypercall support would be useful in mainline is 
open for discussion.
> > diff --git a/xen/arch/arm/arm64/domctl.c b/xen/arch/arm/arm64/domctl.c
> > index 8720d126c9..f227309e06 100644
> > --- a/xen/arch/arm/arm64/domctl.c
> > +++ b/xen/arch/arm/arm64/domctl.c
> > @@ -33,6 +34,37 @@ static long switch_mode(struct domain *d, enum domain_type 
> > type)
> >       return 0;
> >   }
> >   
> > +static long get_address_size(struct domain *d, uint32_t *address_size)
> > +{
> > +    long rc = 0;
> > +    struct vcpu *v;
> > +    /* Check invalid arguments */
> > +    if ( d == NULL || address_size == NULL) {
> > +        rc = -EINVAL;
> > +    }
> > +    /* Domain structure type field and actual vcpu mode must be aligned */
> > +    if(rc == 0) {
> > +        for_each_vcpu(d, v) {
> > +            if(vcpu_get_mode(v) != d->arch.type) {
> > +                rc = -EFAULT;
> > +            }
> > +        }
> This is deeply suspicious.
>
> Under what circumstances can the vCPU setting be different from the
> domain setting?
It should never happen, this is more of a sanity check. Alternative 
would be to use only one of vCPU or domain settings, checking against 
both seemed more complete to me.
> ~Andrew
BR,
Milan