From: Oliver Upton <oliver.upton@linux.dev>
To: kvm-riscv@lists.infradead.org
Subject: [PATCH] KVM: Wrap kvm_{gfn,hva}_range.pte in a per-action union
Date: Mon, 31 Jul 2023 17:37:37 +0000 [thread overview]
Message-ID: <ZMfxYR41K71UV/84@linux.dev> (raw)
In-Reply-To: <20230729004144.1054885-1-seanjc@google.com>
On Fri, Jul 28, 2023 at 05:41:44PM -0700, Sean Christopherson wrote:
> Wrap kvm_{gfn,hva}_range.pte in a union so that future notifier events can
> pass event specific information up and down the stack without needing to
> constantly expand and churn the APIs. Lockless aging of SPTEs will pass
> around a bitmap, and support for memory attributes will pass around the
> new attributes for the range.
>
> Add a "KVM_NO_ARG" placeholder to simplify handling events without an
> argument (creating a dummy union variable is midly annoying).
>
> Opportunstically drop explicit zero-initialization of the "pte" field, as
> omitting the field (now a union) has the same effect.
>
> Cc: Yu Zhao <yuzhao@google.com>
> Link: https://lore.kernel.org/all/CAOUHufagkd2Jk3_HrVoFFptRXM=hX2CV8f+M-dka-hJU4bP8kw at mail.gmail.com
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Looks good, and I don't think it'll conflict with anything on the arm64
side.
Reviewed-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>
> If this looks good, my thought is to squeeze it into 6.6 so that the MGLRU
> and guest_memfd() series can build on it. Or those series could just
> include it?
Eh, I'm not a huge fan of having two series independently reposting a
common base. It can be a bit annoying when the two authors have slightly
different interpretations on how to improve it...
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index dfbaafbe3a00..f84ef9399aee 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -526,7 +526,7 @@ typedef void (*on_unlock_fn_t)(struct kvm *kvm);
> struct kvm_hva_range {
> unsigned long start;
> unsigned long end;
> - pte_t pte;
> + union kvm_mmu_notifier_arg arg;
> hva_handler_t handler;
> on_lock_fn_t on_lock;
> on_unlock_fn_t on_unlock;
> @@ -547,6 +547,8 @@ static void kvm_null_fn(void)
> }
> #define IS_KVM_NULL_FN(fn) ((fn) == (void *)kvm_null_fn)
>
> +static const union kvm_mmu_notifier_arg KVM_NO_ARG;
> +
I'm guessing you were trying to keep this short, but it might be nice to
use MMU_NOTIFIER_ (or similar) as the prefix to make the scope
immediately obvious.
--
Thanks,
Oliver
WARNING: multiple messages have this Message-ID (diff)
From: Oliver Upton <oliver.upton@linux.dev>
To: Sean Christopherson <seanjc@google.com>
Cc: Marc Zyngier <maz@kernel.org>,
Huacai Chen <chenhuacai@kernel.org>,
Anup Patel <anup@brainfault.org>,
Paul Walmsley <paul.walmsley@sifive.com>,
Palmer Dabbelt <palmer@dabbelt.com>,
Albert Ou <aou@eecs.berkeley.edu>,
Paolo Bonzini <pbonzini@redhat.com>,
linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
linux-mips@vger.kernel.org, kvm@vger.kernel.org,
kvm-riscv@lists.infradead.org, linux-riscv@lists.infradead.org,
linux-kernel@vger.kernel.org, Yu Zhao <yuzhao@google.com>
Subject: Re: [PATCH] KVM: Wrap kvm_{gfn,hva}_range.pte in a per-action union
Date: Mon, 31 Jul 2023 17:37:37 +0000 [thread overview]
Message-ID: <ZMfxYR41K71UV/84@linux.dev> (raw)
In-Reply-To: <20230729004144.1054885-1-seanjc@google.com>
On Fri, Jul 28, 2023 at 05:41:44PM -0700, Sean Christopherson wrote:
> Wrap kvm_{gfn,hva}_range.pte in a union so that future notifier events can
> pass event specific information up and down the stack without needing to
> constantly expand and churn the APIs. Lockless aging of SPTEs will pass
> around a bitmap, and support for memory attributes will pass around the
> new attributes for the range.
>
> Add a "KVM_NO_ARG" placeholder to simplify handling events without an
> argument (creating a dummy union variable is midly annoying).
>
> Opportunstically drop explicit zero-initialization of the "pte" field, as
> omitting the field (now a union) has the same effect.
>
> Cc: Yu Zhao <yuzhao@google.com>
> Link: https://lore.kernel.org/all/CAOUHufagkd2Jk3_HrVoFFptRXM=hX2CV8f+M-dka-hJU4bP8kw@mail.gmail.com
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Looks good, and I don't think it'll conflict with anything on the arm64
side.
Reviewed-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>
> If this looks good, my thought is to squeeze it into 6.6 so that the MGLRU
> and guest_memfd() series can build on it. Or those series could just
> include it?
Eh, I'm not a huge fan of having two series independently reposting a
common base. It can be a bit annoying when the two authors have slightly
different interpretations on how to improve it...
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index dfbaafbe3a00..f84ef9399aee 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -526,7 +526,7 @@ typedef void (*on_unlock_fn_t)(struct kvm *kvm);
> struct kvm_hva_range {
> unsigned long start;
> unsigned long end;
> - pte_t pte;
> + union kvm_mmu_notifier_arg arg;
> hva_handler_t handler;
> on_lock_fn_t on_lock;
> on_unlock_fn_t on_unlock;
> @@ -547,6 +547,8 @@ static void kvm_null_fn(void)
> }
> #define IS_KVM_NULL_FN(fn) ((fn) == (void *)kvm_null_fn)
>
> +static const union kvm_mmu_notifier_arg KVM_NO_ARG;
> +
I'm guessing you were trying to keep this short, but it might be nice to
use MMU_NOTIFIER_ (or similar) as the prefix to make the scope
immediately obvious.
--
Thanks,
Oliver
WARNING: multiple messages have this Message-ID (diff)
From: Oliver Upton <oliver.upton@linux.dev>
To: Sean Christopherson <seanjc@google.com>
Cc: Marc Zyngier <maz@kernel.org>,
Huacai Chen <chenhuacai@kernel.org>,
Anup Patel <anup@brainfault.org>,
Paul Walmsley <paul.walmsley@sifive.com>,
Palmer Dabbelt <palmer@dabbelt.com>,
Albert Ou <aou@eecs.berkeley.edu>,
Paolo Bonzini <pbonzini@redhat.com>,
linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
linux-mips@vger.kernel.org, kvm@vger.kernel.org,
kvm-riscv@lists.infradead.org, linux-riscv@lists.infradead.org,
linux-kernel@vger.kernel.org, Yu Zhao <yuzhao@google.com>
Subject: Re: [PATCH] KVM: Wrap kvm_{gfn,hva}_range.pte in a per-action union
Date: Mon, 31 Jul 2023 17:37:37 +0000 [thread overview]
Message-ID: <ZMfxYR41K71UV/84@linux.dev> (raw)
In-Reply-To: <20230729004144.1054885-1-seanjc@google.com>
On Fri, Jul 28, 2023 at 05:41:44PM -0700, Sean Christopherson wrote:
> Wrap kvm_{gfn,hva}_range.pte in a union so that future notifier events can
> pass event specific information up and down the stack without needing to
> constantly expand and churn the APIs. Lockless aging of SPTEs will pass
> around a bitmap, and support for memory attributes will pass around the
> new attributes for the range.
>
> Add a "KVM_NO_ARG" placeholder to simplify handling events without an
> argument (creating a dummy union variable is midly annoying).
>
> Opportunstically drop explicit zero-initialization of the "pte" field, as
> omitting the field (now a union) has the same effect.
>
> Cc: Yu Zhao <yuzhao@google.com>
> Link: https://lore.kernel.org/all/CAOUHufagkd2Jk3_HrVoFFptRXM=hX2CV8f+M-dka-hJU4bP8kw@mail.gmail.com
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Looks good, and I don't think it'll conflict with anything on the arm64
side.
Reviewed-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>
> If this looks good, my thought is to squeeze it into 6.6 so that the MGLRU
> and guest_memfd() series can build on it. Or those series could just
> include it?
Eh, I'm not a huge fan of having two series independently reposting a
common base. It can be a bit annoying when the two authors have slightly
different interpretations on how to improve it...
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index dfbaafbe3a00..f84ef9399aee 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -526,7 +526,7 @@ typedef void (*on_unlock_fn_t)(struct kvm *kvm);
> struct kvm_hva_range {
> unsigned long start;
> unsigned long end;
> - pte_t pte;
> + union kvm_mmu_notifier_arg arg;
> hva_handler_t handler;
> on_lock_fn_t on_lock;
> on_unlock_fn_t on_unlock;
> @@ -547,6 +547,8 @@ static void kvm_null_fn(void)
> }
> #define IS_KVM_NULL_FN(fn) ((fn) == (void *)kvm_null_fn)
>
> +static const union kvm_mmu_notifier_arg KVM_NO_ARG;
> +
I'm guessing you were trying to keep this short, but it might be nice to
use MMU_NOTIFIER_ (or similar) as the prefix to make the scope
immediately obvious.
--
Thanks,
Oliver
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
WARNING: multiple messages have this Message-ID (diff)
From: Oliver Upton <oliver.upton@linux.dev>
To: Sean Christopherson <seanjc@google.com>
Cc: Marc Zyngier <maz@kernel.org>,
Huacai Chen <chenhuacai@kernel.org>,
Anup Patel <anup@brainfault.org>,
Paul Walmsley <paul.walmsley@sifive.com>,
Palmer Dabbelt <palmer@dabbelt.com>,
Albert Ou <aou@eecs.berkeley.edu>,
Paolo Bonzini <pbonzini@redhat.com>,
linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
linux-mips@vger.kernel.org, kvm@vger.kernel.org,
kvm-riscv@lists.infradead.org, linux-riscv@lists.infradead.org,
linux-kernel@vger.kernel.org, Yu Zhao <yuzhao@google.com>
Subject: Re: [PATCH] KVM: Wrap kvm_{gfn,hva}_range.pte in a per-action union
Date: Mon, 31 Jul 2023 17:37:37 +0000 [thread overview]
Message-ID: <ZMfxYR41K71UV/84@linux.dev> (raw)
In-Reply-To: <20230729004144.1054885-1-seanjc@google.com>
On Fri, Jul 28, 2023 at 05:41:44PM -0700, Sean Christopherson wrote:
> Wrap kvm_{gfn,hva}_range.pte in a union so that future notifier events can
> pass event specific information up and down the stack without needing to
> constantly expand and churn the APIs. Lockless aging of SPTEs will pass
> around a bitmap, and support for memory attributes will pass around the
> new attributes for the range.
>
> Add a "KVM_NO_ARG" placeholder to simplify handling events without an
> argument (creating a dummy union variable is midly annoying).
>
> Opportunstically drop explicit zero-initialization of the "pte" field, as
> omitting the field (now a union) has the same effect.
>
> Cc: Yu Zhao <yuzhao@google.com>
> Link: https://lore.kernel.org/all/CAOUHufagkd2Jk3_HrVoFFptRXM=hX2CV8f+M-dka-hJU4bP8kw@mail.gmail.com
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Looks good, and I don't think it'll conflict with anything on the arm64
side.
Reviewed-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>
> If this looks good, my thought is to squeeze it into 6.6 so that the MGLRU
> and guest_memfd() series can build on it. Or those series could just
> include it?
Eh, I'm not a huge fan of having two series independently reposting a
common base. It can be a bit annoying when the two authors have slightly
different interpretations on how to improve it...
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index dfbaafbe3a00..f84ef9399aee 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -526,7 +526,7 @@ typedef void (*on_unlock_fn_t)(struct kvm *kvm);
> struct kvm_hva_range {
> unsigned long start;
> unsigned long end;
> - pte_t pte;
> + union kvm_mmu_notifier_arg arg;
> hva_handler_t handler;
> on_lock_fn_t on_lock;
> on_unlock_fn_t on_unlock;
> @@ -547,6 +547,8 @@ static void kvm_null_fn(void)
> }
> #define IS_KVM_NULL_FN(fn) ((fn) == (void *)kvm_null_fn)
>
> +static const union kvm_mmu_notifier_arg KVM_NO_ARG;
> +
I'm guessing you were trying to keep this short, but it might be nice to
use MMU_NOTIFIER_ (or similar) as the prefix to make the scope
immediately obvious.
--
Thanks,
Oliver
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2023-07-31 17:37 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-29 0:41 [PATCH] KVM: Wrap kvm_{gfn,hva}_range.pte in a per-action union Sean Christopherson
2023-07-29 0:41 ` Sean Christopherson
2023-07-29 0:41 ` Sean Christopherson
2023-07-29 0:41 ` Sean Christopherson
2023-07-31 17:37 ` Oliver Upton [this message]
2023-07-31 17:37 ` Oliver Upton
2023-07-31 17:37 ` Oliver Upton
2023-07-31 17:37 ` Oliver Upton
2023-07-31 17:41 ` Sean Christopherson
2023-07-31 17:41 ` Sean Christopherson
2023-07-31 17:41 ` Sean Christopherson
2023-07-31 17:41 ` Sean Christopherson
2023-07-31 20:35 ` Yu Zhao
2023-07-31 20:35 ` Yu Zhao
2023-07-31 20:35 ` Yu Zhao
2023-07-31 20:35 ` Yu Zhao
2023-08-18 0:08 ` Sean Christopherson
2023-08-18 0:08 ` Sean Christopherson
2023-08-18 0:08 ` Sean Christopherson
2023-08-18 0:08 ` Sean Christopherson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZMfxYR41K71UV/84@linux.dev \
--to=oliver.upton@linux.dev \
--cc=kvm-riscv@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.