Re: [PATCH v3 6/9] x86/HVM: convert hvm_virtual_to_linear_addr() to be remote-capable

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Sep 21, 2021 at 09:19:37AM +0200, Jan Beulich wrote:
> While all present callers want to act on "current", stack dumping for
> HVM vCPU-s will require the function to be able to act on a remote vCPU.
> To avoid touching all present callers, convert the existing function to
> an inline wrapper around the extend new one.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> Alternatively the actual dumping patch could avoid using this more
> elaborate function and, ignoring access checks, simply add in the SS
> segment base itself (if needed in the first place).
> ---
> v3: New.
> 
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -2526,7 +2526,8 @@ int hvm_set_cr4(unsigned long value, boo
>      return X86EMUL_OKAY;
>  }
>  
> -bool_t hvm_virtual_to_linear_addr(
> +bool hvm_vcpu_virtual_to_linear(
> +    struct vcpu *v,
>      enum x86_segment seg,
>      const struct segment_register *reg,
>      unsigned long offset,
> @@ -2535,8 +2536,9 @@ bool_t hvm_virtual_to_linear_addr(
>      const struct segment_register *active_cs,
>      unsigned long *linear_addr)
>  {
> -    const struct vcpu *curr = current;
>      unsigned long addr = offset, last_byte;
> +    const struct cpu_user_regs *regs = v == current ? guest_cpu_user_regs()
> +                                                    : &v->arch.user_regs;
>      bool_t okay = 0;

Since you change the function return type to bool, you should also
change the type of the returned variable IMO. It's just a two line
change.

Also is it worth adding some check that the remote vCPU is paused? Or
else you might get inconsistent results by using data that's stale  by
the time Xen acts on it.

Thanks, Roger.