Re: [PATCH V4 2/15] KVM/MMU: Add tlb flush with range helper function
On 14/10/2018 10:16, Thomas Gleixner wrote:
>>> +static inline bool kvm_available_flush_tlb_with_range(void)
>>> +{
>>> + return kvm_x86_ops->tlb_remote_flush_with_range;
>>> +}
>> Seems that kvm_available_flush_tlb_with_range() is not used in this patch…
> What's wrong with that?
>
> It provides the implementation and later patches make use of it. It's a
> sensible way to split patches into small, self contained entities.
That's true, on the other hand I have indeed a concerns with this patch:
this series is not bisectable at all, because all the new code is dead
until the very last patch. Uses of the new feature should come _after_
the implementation.
I don't have any big problem with what Liran pointed out (and I can live
with the unused static functions that would warn with -Wunused, too),
but the above should be fixed in v5, basically by moving patches 12-15
at the beginning of the series.
Paolo
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V4 2/15] KVM/MMU: Add tlb flush with range helper function
On Sun, Oct 14, 2018 at 09:21:23PM +0800, Tianyu Lan wrote: > Sorry to confuse your. I get from CCers from get_maintainer.pl script. Unfortunately you seem to have made a mistake. My email address is 'li...@armlinux.org.uk' not 'li...@armlinux.org'. There is no 'li...@armlinux.org' in MAINTAINERS, yet your emails appear to be copied to this address. Consequently, if it was your intention to copy me with the entire series, that didn't happen. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V4 2/15] KVM/MMU: Add tlb flush with range helper function
Hi Russell:
Thanks for your review.
On Sun, Oct 14, 2018 at 5:36 PM Russell King - ARM Linux
wrote:
>
> On Sun, Oct 14, 2018 at 10:27:34AM +0100, Russell King - ARM Linux wrote:
> > On Sun, Oct 14, 2018 at 10:16:56AM +0200, Thomas Gleixner wrote:
> > > On Sun, 14 Oct 2018, Liran Alon wrote:
> > > > > On 13 Oct 2018, at 17:53, lantianyu1...@gmail.com wrote:
> > > > >
> > > > > From: Lan Tianyu
> > > > >
> > > > > This patch is to add wrapper functions for tlb_remote_flush_with_range
> > > > > callback.
> > > > >
> > > > > Signed-off-by: Lan Tianyu
> > > > > ---
> > > > > Change sicne V3:
> > > > > Remove code of updating "tlbs_dirty"
> > > > > Change since V2:
> > > > > Fix comment in the kvm_flush_remote_tlbs_with_range()
> > > > > ---
> > > > > arch/x86/kvm/mmu.c | 40
> > > > > 1 file changed, 40 insertions(+)
> > > > >
> > > > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> > > > > index c73d9f650de7..ff656d85903a 100644
> > > > > --- a/arch/x86/kvm/mmu.c
> > > > > +++ b/arch/x86/kvm/mmu.c
> > > > > @@ -264,6 +264,46 @@ static void mmu_spte_set(u64 *sptep, u64 spte);
> > > > > static union kvm_mmu_page_role
> > > > > kvm_mmu_calc_root_page_role(struct kvm_vcpu *vcpu);
> > > > >
> > > > > +
> > > > > +static inline bool kvm_available_flush_tlb_with_range(void)
> > > > > +{
> > > > > + return kvm_x86_ops->tlb_remote_flush_with_range;
> > > > > +}
> > > >
> > > > Seems that kvm_available_flush_tlb_with_range() is not used in this
> > > > patch…
> > >
> > > What's wrong with that?
> > >
> > > It provides the implementation and later patches make use of it. It's a
> > > sensible way to split patches into small, self contained entities.
> >
> > From what I can see of the patches that follow _this_ patch in the
> > series, this function remains unused. So, not only is it not used
> > in this patch, it's not used in this series.
>
> Note - I seem to have only received patches 1 through 4, so this is
> based on the patches I've received.
>
Sorry to confuse your. I get from CCers from get_maintainer.pl script.
The next patch "[PATCH V4 3/15]KVM: Replace old tlb flush function with
new one to flush a specified range" calls new function.
https://lkml.org/lkml/2018/10/13/254
The patch "[PATCH V4 4/15] KVM: Make kvm_set_spte_hva() return int"
changes under ARM directory. Please have a look. Thanks.
--
Best regards
Tianyu Lan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V4 2/15] KVM/MMU: Add tlb flush with range helper function
Hi Liran & Thomas:
Thanks for your review.
On Sun, Oct 14, 2018 at 5:20 PM Liran Alon wrote:
>
>
>
> > On 14 Oct 2018, at 11:16, Thomas Gleixner wrote:
> >
> > On Sun, 14 Oct 2018, Liran Alon wrote:
> >>> On 13 Oct 2018, at 17:53, lantianyu1...@gmail.com wrote:
> >>>
> >>> +
> >>> +static inline bool kvm_available_flush_tlb_with_range(void)
> >>> +{
> >>> + return kvm_x86_ops->tlb_remote_flush_with_range;
> >>> +}
> >>
> >> Seems that kvm_available_flush_tlb_with_range() is not used in this patch…
> >
> > What's wrong with that?
> >
> > It provides the implementation and later patches make use of it. It's a
> > sensible way to split patches into small, self contained entities.
> >
> > Thanks,
> >
> > tglx
> >
>
> I guess it’s a matter of taste, but I prefer to not add dead-code for patches
> in order for each commit to compile nicely without warnings of declared and
> unused functions.
> I would prefer to just add this utility function on the patch that actually
> use it.
>
> -Liran
>
Normally, I also prefer to put the function definition into the patch
which use it.
But the following patch "KVM: Replace old tlb flush function with new
one to flush a specified range"
and other patches which use new functions will change a lot of places.
It's not friendly for review and
so I split them into pieces.
--
Best regards
Tianyu Lan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V4 2/15] KVM/MMU: Add tlb flush with range helper function
On Sun, Oct 14, 2018 at 10:16:56AM +0200, Thomas Gleixner wrote:
> On Sun, 14 Oct 2018, Liran Alon wrote:
> > > On 13 Oct 2018, at 17:53, lantianyu1...@gmail.com wrote:
> > >
> > > From: Lan Tianyu
> > >
> > > This patch is to add wrapper functions for tlb_remote_flush_with_range
> > > callback.
> > >
> > > Signed-off-by: Lan Tianyu
> > > ---
> > > Change sicne V3:
> > > Remove code of updating "tlbs_dirty"
> > > Change since V2:
> > > Fix comment in the kvm_flush_remote_tlbs_with_range()
> > > ---
> > > arch/x86/kvm/mmu.c | 40
> > > 1 file changed, 40 insertions(+)
> > >
> > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> > > index c73d9f650de7..ff656d85903a 100644
> > > --- a/arch/x86/kvm/mmu.c
> > > +++ b/arch/x86/kvm/mmu.c
> > > @@ -264,6 +264,46 @@ static void mmu_spte_set(u64 *sptep, u64 spte);
> > > static union kvm_mmu_page_role
> > > kvm_mmu_calc_root_page_role(struct kvm_vcpu *vcpu);
> > >
> > > +
> > > +static inline bool kvm_available_flush_tlb_with_range(void)
> > > +{
> > > + return kvm_x86_ops->tlb_remote_flush_with_range;
> > > +}
> >
> > Seems that kvm_available_flush_tlb_with_range() is not used in this patch…
>
> What's wrong with that?
>
> It provides the implementation and later patches make use of it. It's a
> sensible way to split patches into small, self contained entities.
From what I can see of the patches that follow _this_ patch in the
series, this function remains unused. So, not only is it not used
in this patch, it's not used in this series.
I think the real question that needs asking is - what is the plan
for this function, and when will it be used?
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V4 2/15] KVM/MMU: Add tlb flush with range helper function
On Sun, Oct 14, 2018 at 10:27:34AM +0100, Russell King - ARM Linux wrote:
> On Sun, Oct 14, 2018 at 10:16:56AM +0200, Thomas Gleixner wrote:
> > On Sun, 14 Oct 2018, Liran Alon wrote:
> > > > On 13 Oct 2018, at 17:53, lantianyu1...@gmail.com wrote:
> > > >
> > > > From: Lan Tianyu
> > > >
> > > > This patch is to add wrapper functions for tlb_remote_flush_with_range
> > > > callback.
> > > >
> > > > Signed-off-by: Lan Tianyu
> > > > ---
> > > > Change sicne V3:
> > > > Remove code of updating "tlbs_dirty"
> > > > Change since V2:
> > > > Fix comment in the kvm_flush_remote_tlbs_with_range()
> > > > ---
> > > > arch/x86/kvm/mmu.c | 40
> > > > 1 file changed, 40 insertions(+)
> > > >
> > > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> > > > index c73d9f650de7..ff656d85903a 100644
> > > > --- a/arch/x86/kvm/mmu.c
> > > > +++ b/arch/x86/kvm/mmu.c
> > > > @@ -264,6 +264,46 @@ static void mmu_spte_set(u64 *sptep, u64 spte);
> > > > static union kvm_mmu_page_role
> > > > kvm_mmu_calc_root_page_role(struct kvm_vcpu *vcpu);
> > > >
> > > > +
> > > > +static inline bool kvm_available_flush_tlb_with_range(void)
> > > > +{
> > > > + return kvm_x86_ops->tlb_remote_flush_with_range;
> > > > +}
> > >
> > > Seems that kvm_available_flush_tlb_with_range() is not used in this patch…
> >
> > What's wrong with that?
> >
> > It provides the implementation and later patches make use of it. It's a
> > sensible way to split patches into small, self contained entities.
>
> From what I can see of the patches that follow _this_ patch in the
> series, this function remains unused. So, not only is it not used
> in this patch, it's not used in this series.
Note - I seem to have only received patches 1 through 4, so this is
based on the patches I've received.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V4 2/15] KVM/MMU: Add tlb flush with range helper function
> On 14 Oct 2018, at 11:16, Thomas Gleixner wrote:
>
> On Sun, 14 Oct 2018, Liran Alon wrote:
>>> On 13 Oct 2018, at 17:53, lantianyu1...@gmail.com wrote:
>>>
>>> +
>>> +static inline bool kvm_available_flush_tlb_with_range(void)
>>> +{
>>> + return kvm_x86_ops->tlb_remote_flush_with_range;
>>> +}
>>
>> Seems that kvm_available_flush_tlb_with_range() is not used in this patch…
>
> What's wrong with that?
>
> It provides the implementation and later patches make use of it. It's a
> sensible way to split patches into small, self contained entities.
>
> Thanks,
>
> tglx
>
I guess it’s a matter of taste, but I prefer to not add dead-code for patches
in order for each commit to compile nicely without warnings of declared and
unused functions.
I would prefer to just add this utility function on the patch that actually use
it.
-Liran
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V4 2/15] KVM/MMU: Add tlb flush with range helper function
On Sun, 14 Oct 2018, Liran Alon wrote:
> > On 13 Oct 2018, at 17:53, lantianyu1...@gmail.com wrote:
> >
> > From: Lan Tianyu
> >
> > This patch is to add wrapper functions for tlb_remote_flush_with_range
> > callback.
> >
> > Signed-off-by: Lan Tianyu
> > ---
> > Change sicne V3:
> > Remove code of updating "tlbs_dirty"
> > Change since V2:
> > Fix comment in the kvm_flush_remote_tlbs_with_range()
> > ---
> > arch/x86/kvm/mmu.c | 40
> > 1 file changed, 40 insertions(+)
> >
> > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> > index c73d9f650de7..ff656d85903a 100644
> > --- a/arch/x86/kvm/mmu.c
> > +++ b/arch/x86/kvm/mmu.c
> > @@ -264,6 +264,46 @@ static void mmu_spte_set(u64 *sptep, u64 spte);
> > static union kvm_mmu_page_role
> > kvm_mmu_calc_root_page_role(struct kvm_vcpu *vcpu);
> >
> > +
> > +static inline bool kvm_available_flush_tlb_with_range(void)
> > +{
> > + return kvm_x86_ops->tlb_remote_flush_with_range;
> > +}
>
> Seems that kvm_available_flush_tlb_with_range() is not used in this patch…
What's wrong with that?
It provides the implementation and later patches make use of it. It's a
sensible way to split patches into small, self contained entities.
Thanks,
tglx
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V4 2/15] KVM/MMU: Add tlb flush with range helper function
> On 13 Oct 2018, at 17:53, lantianyu1...@gmail.com wrote:
>
> From: Lan Tianyu
>
> This patch is to add wrapper functions for tlb_remote_flush_with_range
> callback.
>
> Signed-off-by: Lan Tianyu
> ---
> Change sicne V3:
> Remove code of updating "tlbs_dirty"
> Change since V2:
> Fix comment in the kvm_flush_remote_tlbs_with_range()
> ---
> arch/x86/kvm/mmu.c | 40
> 1 file changed, 40 insertions(+)
>
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index c73d9f650de7..ff656d85903a 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -264,6 +264,46 @@ static void mmu_spte_set(u64 *sptep, u64 spte);
> static union kvm_mmu_page_role
> kvm_mmu_calc_root_page_role(struct kvm_vcpu *vcpu);
>
> +
> +static inline bool kvm_available_flush_tlb_with_range(void)
> +{
> + return kvm_x86_ops->tlb_remote_flush_with_range;
> +}
Seems that kvm_available_flush_tlb_with_range() is not used in this patch…
> +
> +static void kvm_flush_remote_tlbs_with_range(struct kvm *kvm,
> + struct kvm_tlb_range *range)
> +{
> + int ret = -ENOTSUPP;
> +
> + if (range && kvm_x86_ops->tlb_remote_flush_with_range)
> + ret = kvm_x86_ops->tlb_remote_flush_with_range(kvm, range);
> +
> + if (ret)
> + kvm_flush_remote_tlbs(kvm);
> +}
> +
> +static void kvm_flush_remote_tlbs_with_list(struct kvm *kvm,
> + struct list_head *flush_list)
> +{
> + struct kvm_tlb_range range;
> +
> + range.flush_list = flush_list;
> +
> + kvm_flush_remote_tlbs_with_range(kvm, );
> +}
> +
> +static void kvm_flush_remote_tlbs_with_address(struct kvm *kvm,
> + u64 start_gfn, u64 pages)
> +{
> + struct kvm_tlb_range range;
> +
> + range.start_gfn = start_gfn;
> + range.pages = pages;
> + range.flush_list = NULL;
> +
> + kvm_flush_remote_tlbs_with_range(kvm, );
> +}
> +
> void kvm_mmu_set_mmio_spte_mask(u64 mmio_mask, u64 mmio_value)
> {
> BUG_ON((mmio_mask & mmio_value) != mmio_value);
> --
> 2.14.4
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH V4 2/15] KVM/MMU: Add tlb flush with range helper function
From: Lan Tianyu
This patch is to add wrapper functions for tlb_remote_flush_with_range
callback.
Signed-off-by: Lan Tianyu
---
Change sicne V3:
Remove code of updating "tlbs_dirty"
Change since V2:
Fix comment in the kvm_flush_remote_tlbs_with_range()
---
arch/x86/kvm/mmu.c | 40
1 file changed, 40 insertions(+)
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index c73d9f650de7..ff656d85903a 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -264,6 +264,46 @@ static void mmu_spte_set(u64 *sptep, u64 spte);
static union kvm_mmu_page_role
kvm_mmu_calc_root_page_role(struct kvm_vcpu *vcpu);
+
+static inline bool kvm_available_flush_tlb_with_range(void)
+{
+ return kvm_x86_ops->tlb_remote_flush_with_range;
+}
+
+static void kvm_flush_remote_tlbs_with_range(struct kvm *kvm,
+ struct kvm_tlb_range *range)
+{
+ int ret = -ENOTSUPP;
+
+ if (range && kvm_x86_ops->tlb_remote_flush_with_range)
+ ret = kvm_x86_ops->tlb_remote_flush_with_range(kvm, range);
+
+ if (ret)
+ kvm_flush_remote_tlbs(kvm);
+}
+
+static void kvm_flush_remote_tlbs_with_list(struct kvm *kvm,
+ struct list_head *flush_list)
+{
+ struct kvm_tlb_range range;
+
+ range.flush_list = flush_list;
+
+ kvm_flush_remote_tlbs_with_range(kvm, );
+}
+
+static void kvm_flush_remote_tlbs_with_address(struct kvm *kvm,
+ u64 start_gfn, u64 pages)
+{
+ struct kvm_tlb_range range;
+
+ range.start_gfn = start_gfn;
+ range.pages = pages;
+ range.flush_list = NULL;
+
+ kvm_flush_remote_tlbs_with_range(kvm, );
+}
+
void kvm_mmu_set_mmio_spte_mask(u64 mmio_mask, u64 mmio_value)
{
BUG_ON((mmio_mask & mmio_value) != mmio_value);
--
2.14.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
