lkml.org 
[lkml]   [2017]   [Sep]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC Part2 PATCH v3 11/26] KVM: X86: Extend struct kvm_arch to include SEV information
On Mon, Jul 24, 2017 at 03:02:48PM -0500, Brijesh Singh wrote:
> The patch adds a new member (sev_info) in 'struct kvm_arch', and

Never say "This patch" in a commit message of a patch. It is
tautologically useless.

> setter/getter functions for the sev_info field.

Also, I can see what the patch does from the hunk below. What is more
important to explain in the commit message is *why* you're doing what
you're doing.

>
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
> arch/x86/include/asm/kvm_host.h | 9 +++++++++
> arch/x86/kvm/svm.c | 45 +++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 54 insertions(+)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 4295f82..150177e 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -742,6 +742,13 @@ enum kvm_irqchip_mode {
> KVM_IRQCHIP_SPLIT, /* created with KVM_CAP_SPLIT_IRQCHIP */
> };
>
> +struct kvm_sev_info {
> + bool active; /* SEV enabled guest */
> + unsigned int handle; /* firmware handle */
> + unsigned int asid; /* asid for this guest */
> + int sev_fd; /* SEV device fd */
> +};
> +
> struct kvm_arch {
> unsigned int n_used_mmu_pages;
> unsigned int n_requested_mmu_pages;
> @@ -829,6 +836,8 @@ struct kvm_arch {
>
> bool x2apic_format;
> bool x2apic_broadcast_quirk_disabled;
> +
> + struct kvm_sev_info sev_info;
> };
>
> struct kvm_vm_stat {
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 256c9df..2a5a03a 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -322,6 +322,51 @@ enum {
> /* Secure Encrypted Virtualization */
> static unsigned int max_sev_asid;
>
> +static inline struct kvm_sev_info *to_sev_info(struct kvm *kvm)
> +{
> + return &kvm->arch.sev_info;
> +}
> +
> +static inline void sev_set_active(struct kvm *kvm)
> +{
> + to_sev_info(kvm)->active = true;
> +}

Is this the accepted way to do this in KVM land or can you simply access
all members directly:

kvm->arch.sev_info.<member>

Because I see stuff like that:

static void kvm_gen_update_masterclock(struct kvm *kvm)
{
...

struct kvm_arch *ka = &kvm->arch;

spin_lock(&ka->pvclock_gtod_sync_lock);

and

struct kvm_lapic *apic = svm->vcpu.arch.apic;

...

kvm_lapic_reg_write(apic, APIC_ICR2, icrh);

so why do you need the accessors?

--
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--

\
 
 \ /
  Last update: 2017-09-13 15:38    [W:0.397 / U:0.936 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site