Re: [PATCH v6 21/27] KVM: arm/arm64: Add hook for arch-specific KVM initialisation
Hi Dave,
On 19/03/2019 17:52, Dave Martin wrote:
> This patch adds a kvm_arm_init_arch_resources() hook to perform
> subarch-specific initialisation when starting up KVM.
>
> This will be used in a subsequent patch for global SVE-related
> setup on arm64.
>
> No functional change.
>
> Signed-off-by: Dave Martin
> ---
> arch/arm/include/asm/kvm_host.h | 2 ++
> arch/arm64/include/asm/kvm_host.h | 2 ++
> virt/kvm/arm/arm.c| 4
> 3 files changed, 8 insertions(+)
>
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 770d732..a49ee01 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -53,6 +53,8 @@
>
> DECLARE_STATIC_KEY_FALSE(userspace_irqchip_in_use);
>
> +static inline int kvm_arm_init_arch_resources(void) { return 0; }
> +
> u32 *kvm_vcpu_reg(struct kvm_vcpu *vcpu, u8 reg_num, u32 mode);
> int __attribute_const__ kvm_target_cpu(void);
> int kvm_reset_vcpu(struct kvm_vcpu *vcpu);
> diff --git a/arch/arm64/include/asm/kvm_host.h
> b/arch/arm64/include/asm/kvm_host.h
> index 205438a..3e89509 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -58,6 +58,8 @@
>
> DECLARE_STATIC_KEY_FALSE(userspace_irqchip_in_use);
>
> +static inline int kvm_arm_init_arch_resources(void) { return 0; }
> +
> int __attribute_const__ kvm_target_cpu(void);
> int kvm_reset_vcpu(struct kvm_vcpu *vcpu);
> int kvm_arch_vm_ioctl_check_extension(struct kvm *kvm, long ext);
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index 99c3738..c69e137 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -1664,6 +1664,10 @@ int kvm_arch_init(void *opaque)
> if (err)
> return err;
>
> + err = kvm_arm_init_arch_resources();
> + if (err)
> + return err;
> +
Nit: Does this need to be the very first thing we do for arch
initialization?
In the same function I see a call to init_common_resources(), so I
would've pictured kvm_arm_init_arch_resources() being called close to it
(either right before or right after).
Otherwise:
Reviewed-by: Julien Thierry
Cheers,
--
Julien Thierry
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v6 21/27] KVM: arm/arm64: Add hook for arch-specific KVM initialisation
On Wed, Mar 27, 2019 at 10:07:17AM +, Julien Thierry wrote:
> Hi Dave,
>
> On 19/03/2019 17:52, Dave Martin wrote:
> > This patch adds a kvm_arm_init_arch_resources() hook to perform
> > subarch-specific initialisation when starting up KVM.
> >
> > This will be used in a subsequent patch for global SVE-related
> > setup on arm64.
> >
> > No functional change.
> >
> > Signed-off-by: Dave Martin
> > ---
> > arch/arm/include/asm/kvm_host.h | 2 ++
> > arch/arm64/include/asm/kvm_host.h | 2 ++
> > virt/kvm/arm/arm.c| 4
> > 3 files changed, 8 insertions(+)
> >
> > diff --git a/arch/arm/include/asm/kvm_host.h
> > b/arch/arm/include/asm/kvm_host.h
> > index 770d732..a49ee01 100644
> > --- a/arch/arm/include/asm/kvm_host.h
> > +++ b/arch/arm/include/asm/kvm_host.h
> > @@ -53,6 +53,8 @@
> >
> > DECLARE_STATIC_KEY_FALSE(userspace_irqchip_in_use);
> >
> > +static inline int kvm_arm_init_arch_resources(void) { return 0; }
> > +
> > u32 *kvm_vcpu_reg(struct kvm_vcpu *vcpu, u8 reg_num, u32 mode);
> > int __attribute_const__ kvm_target_cpu(void);
> > int kvm_reset_vcpu(struct kvm_vcpu *vcpu);
> > diff --git a/arch/arm64/include/asm/kvm_host.h
> > b/arch/arm64/include/asm/kvm_host.h
> > index 205438a..3e89509 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -58,6 +58,8 @@
> >
> > DECLARE_STATIC_KEY_FALSE(userspace_irqchip_in_use);
> >
> > +static inline int kvm_arm_init_arch_resources(void) { return 0; }
> > +
> > int __attribute_const__ kvm_target_cpu(void);
> > int kvm_reset_vcpu(struct kvm_vcpu *vcpu);
> > int kvm_arch_vm_ioctl_check_extension(struct kvm *kvm, long ext);
> > diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> > index 99c3738..c69e137 100644
> > --- a/virt/kvm/arm/arm.c
> > +++ b/virt/kvm/arm/arm.c
> > @@ -1664,6 +1664,10 @@ int kvm_arch_init(void *opaque)
> > if (err)
> > return err;
> >
> > + err = kvm_arm_init_arch_resources();
> > + if (err)
> > + return err;
> > +
>
> Nit: Does this need to be the very first thing we do for arch
> initialization?
>
> In the same function I see a call to init_common_resources(), so I
> would've pictured kvm_arm_init_arch_resources() being called close to it
> (either right before or right after).
With git format-patch -U4 (so, one extra line of context):
@@ -1663,8 +1663,12 @@ int kvm_arch_init(void *opaque)
err = init_common_resources();
if (err)
return err;
+ err = kvm_arm_init_arch_resources();
+ if (err)
+ return err;
+
Does that answer your concern?
I'm guessing we might at some point find we have to move this call if we
add more code into kvm_arm_init_arch_resources(), but for now there is
no dependency between the SVE init stuff and anything else here. So
the it doesn't matter exactly when we call it today, so long as it is
called before anyone can start creating vcpus.
> Otherwise:
>
> Reviewed-by: Julien Thierry
I'll wait for your response before applying this.
Thanks
---Dave
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v6 21/27] KVM: arm/arm64: Add hook for arch-specific KVM initialisation
On 27/03/2019 10:41, Dave Martin wrote:
> On Wed, Mar 27, 2019 at 10:07:17AM +, Julien Thierry wrote:
>> Hi Dave,
>>
>> On 19/03/2019 17:52, Dave Martin wrote:
>>> This patch adds a kvm_arm_init_arch_resources() hook to perform
>>> subarch-specific initialisation when starting up KVM.
>>>
>>> This will be used in a subsequent patch for global SVE-related
>>> setup on arm64.
>>>
>>> No functional change.
>>>
>>> Signed-off-by: Dave Martin
>>> ---
>>> arch/arm/include/asm/kvm_host.h | 2 ++
>>> arch/arm64/include/asm/kvm_host.h | 2 ++
>>> virt/kvm/arm/arm.c| 4
>>> 3 files changed, 8 insertions(+)
>>>
>>> diff --git a/arch/arm/include/asm/kvm_host.h
>>> b/arch/arm/include/asm/kvm_host.h
>>> index 770d732..a49ee01 100644
>>> --- a/arch/arm/include/asm/kvm_host.h
>>> +++ b/arch/arm/include/asm/kvm_host.h
>>> @@ -53,6 +53,8 @@
>>>
>>> DECLARE_STATIC_KEY_FALSE(userspace_irqchip_in_use);
>>>
>>> +static inline int kvm_arm_init_arch_resources(void) { return 0; }
>>> +
>>> u32 *kvm_vcpu_reg(struct kvm_vcpu *vcpu, u8 reg_num, u32 mode);
>>> int __attribute_const__ kvm_target_cpu(void);
>>> int kvm_reset_vcpu(struct kvm_vcpu *vcpu);
>>> diff --git a/arch/arm64/include/asm/kvm_host.h
>>> b/arch/arm64/include/asm/kvm_host.h
>>> index 205438a..3e89509 100644
>>> --- a/arch/arm64/include/asm/kvm_host.h
>>> +++ b/arch/arm64/include/asm/kvm_host.h
>>> @@ -58,6 +58,8 @@
>>>
>>> DECLARE_STATIC_KEY_FALSE(userspace_irqchip_in_use);
>>>
>>> +static inline int kvm_arm_init_arch_resources(void) { return 0; }
>>> +
>>> int __attribute_const__ kvm_target_cpu(void);
>>> int kvm_reset_vcpu(struct kvm_vcpu *vcpu);
>>> int kvm_arch_vm_ioctl_check_extension(struct kvm *kvm, long ext);
>>> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
>>> index 99c3738..c69e137 100644
>>> --- a/virt/kvm/arm/arm.c
>>> +++ b/virt/kvm/arm/arm.c
>>> @@ -1664,6 +1664,10 @@ int kvm_arch_init(void *opaque)
>>> if (err)
>>> return err;
>>>
>>> + err = kvm_arm_init_arch_resources();
>>> + if (err)
>>> + return err;
>>> +
>>
>> Nit: Does this need to be the very first thing we do for arch
>> initialization?
>>
>> In the same function I see a call to init_common_resources(), so I
>> would've pictured kvm_arm_init_arch_resources() being called close to it
>> (either right before or right after).
>
> With git format-patch -U4 (so, one extra line of context):
>
> @@ -1663,8 +1663,12 @@ int kvm_arch_init(void *opaque)
> err = init_common_resources();
> if (err)
> return err;
>
> + err = kvm_arm_init_arch_resources();
> + if (err)
> + return err;
> +
>
> Does that answer your concern?
>
Ah yes, sorry for the noise! Disregard my comment :) .
Thanks,
Julien
> I'm guessing we might at some point find we have to move this call if we
> add more code into kvm_arm_init_arch_resources(), but for now there is
> no dependency between the SVE init stuff and anything else here. So
> the it doesn't matter exactly when we call it today, so long as it is
> called before anyone can start creating vcpus.
>
>> Otherwise:
>>
>> Reviewed-by: Julien Thierry
>
> I'll wait for your response before applying this.
>
> Thanks
> ---Dave
>
--
Julien Thierry
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
