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:4.106 / U:0.000 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site