linux-kernel - Re: [RFC 13/14] KVM: x86/MMU: Wrap uses of kvm_handle_gfn

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CANgfPd-C774w5M0nvhS-H8vH79LbGjmyDOLau1QqNAncjhLGyg@mail.gmail.com>
Date:   Wed, 1 Feb 2023 14:30:39 -0800
From:   Ben Gardon <bgardon@...gle.com>
To:     Sean Christopherson <seanjc@...gle.com>
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        Paolo Bonzini <pbonzini@...hat.com>,
        Peter Xu <peterx@...hat.com>,
        David Matlack <dmatlack@...gle.com>,
        Vipin Sharma <vipinsh@...gle.com>,
        Nagareddy Reddy <nspreddy@...gle.com>
Subject: Re: [RFC 13/14] KVM: x86/MMU: Wrap uses of kvm_handle_gfn_range in mmu.c

On Wed, Feb 1, 2023 at 1:25 PM Sean Christopherson <seanjc@...gle.com> wrote:
>
> On Wed, Dec 21, 2022, Ben Gardon wrote:
> > @@ -978,9 +978,13 @@ static void slot_rmap_walk_next(struct slot_rmap_walk_iterator *iterator)
> >            slot_rmap_walk_okay(_iter_);                               \
> >            slot_rmap_walk_next(_iter_))
> >
> > -__always_inline bool kvm_handle_gfn_range(struct kvm *kvm,
> > -                                       struct kvm_gfn_range *range,
> > -                                       rmap_handler_t handler)
> > +typedef bool (*rmap_handler_t)(struct kvm *kvm, struct kvm_rmap_head *rmap_head,
> > +                            struct kvm_memory_slot *slot, gfn_t gfn,
> > +                            int level, pte_t pte);
> > +
> > +static __always_inline bool
> > +kvm_handle_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range,
>
> Don't split function returns/attributes from the function declaration.  I don't
> think the rule ended up getting officially documented and enforced, but Linus was
> unequivocal when it came up[*], and I happen to agree with him :-)
>
> Actually, since I'm guessing you got the idea from existing code, can you fold
> in the attached patches to purge the existing cases in mmu.c before those uglies
> get moved around?  Assuming you don't dislike the proposed rename, that is.
>
> [*] https://lore.kernel.org/mm-commits/CAHk-=wjS-Jg7sGMwUPpDsjv392nDOOs0CtUtVkp=S6Q7JzFJRw@mail.gmail.com

Sounds good to me. Added the attached patches to the start of the series.

I didn't love those weird splits in the function def. Happy to see
them cleaned up too.