Re: [Xen-devel] [PATCH v6] x86/hvm: add vcpu parameter to guest memory copy function

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.02.17 at 18:35, <roger.pau@xxxxxxxxxx> wrote:
> Current __hvm_copy assumes that the destination memory belongs to the current
> vcpu, but this is not always the case since for PVHv2 Dom0 build hvm copy
> functions are used with current being the idle vcpu. Add a new vcpu parameter
> to hvm copy in order to solve that. Note that only hvm_copy_to_guest_phys is
> changed to take a vcpu parameter, because that's the only one at the moment
> that's required in order to build a PVHv2 Dom0.
> 
> While there, also assert that the passed vcpu belongs to a HVM guest.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
with two remarks:

> @@ -3143,12 +3144,12 @@ static enum hvm_copy_result __hvm_copy(
>           * - 32-bit WinXP (& older Windows) on AMD CPUs for LAPIC accesses,
>           * - newer Windows (like Server 2012) for HPET accesses.
>           */
> -        if ( !nestedhvm_vcpu_in_guestmode(curr)
> -             && is_hvm_vcpu(curr)
> +        if ( !nestedhvm_vcpu_in_guestmode(v)
> +             && is_hvm_vcpu(v) && v == current

Ideally cheap checks should go first, so if I end up committing this I
think I'll take the liberty of inverting the order of these three checks.

> @@ -3209,16 +3210,16 @@ static enum hvm_copy_result __hvm_copy(
>  }
>  
>  enum hvm_copy_result hvm_copy_to_guest_phys(
> -    paddr_t paddr, void *buf, int size)
> +    paddr_t paddr, void *buf, int size, struct vcpu *v)

Well, I would have wished for the new parameter to go first here,
but we can deal with that whenever we make all of its sibling
functions take a vcpu argument too.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel