Re: [PATCH] KVM: vmx: add mismatched size in vmcs_check32

2021-04-08 Thread Haiwei Li 
On Fri, Apr 9, 2021 at 12:05 AM Sean Christopherson  wrote:
>
> On Thu, Apr 08, 2021, lihaiwei.ker...@gmail.com wrote:
> > From: Haiwei Li 
> >
> > vmcs_check32 misses the check for 64-bit and 64-bit high.
>
> Can you clarify in the changelog that, while it is architecturally legal to
> access 64-bit and 64-bit high fields with a 32-bit read/write in 32-bit mode,
> KVM should never do partial accesses to VMCS fields.  And/or note that the
> 32-bit accesses are done in vmcs_{read,write}64() when necessary?  Hmm, maybe:
>
>   Add compile-time assertions in vmcs_check32() to disallow accesses to
>   64-bit and 64-bit high fields via vmcs_{read,write}32().  Upper level
>   KVM code should never do partial accesses to VMCS fields.  KVM handles
>   the split accesses automatically in vmcs_{read,write}64() when running
>   as a 32-bit kernel.

Good suggestion, thanks. I will send v2.

Re: [PATCH] KVM: VMX: Check the corresponding bits according to the intel sdm

2021-03-25 Thread Haiwei Li 
On Thu, Mar 25, 2021 at 11:49 PM Sean Christopherson  wrote:
>
> On Thu, Mar 25, 2021, Haiwei Li wrote:
> > On Tue, Mar 23, 2021 at 10:37 AM  wrote:
> > >
> > > From: Haiwei Li 
> > >
> > > According to IA-32 SDM Vol.3D "A.1 BASIC VMX INFORMATION", two inspections
> > > are missing.
> > > * Bit 31 is always 0. Earlier versions of this manual specified that the
> > > VMCS revision identifier was a 32-bit field in bits 31:0 of this MSR. For
> > > all processors produced prior to this change, bit 31 of this MSR was read
> > > as 0.
> > > * The values of bits 47:45 and bits 63:57 are reserved and are read as 0.
> > >
> > > Signed-off-by: Haiwei Li 
> > > ---
> > >  arch/x86/kvm/vmx/vmx.c | 14 ++
> > >  1 file changed, 14 insertions(+)
> > >
> > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > > index 32cf828..0d6d13c 100644
> > > --- a/arch/x86/kvm/vmx/vmx.c
> > > +++ b/arch/x86/kvm/vmx/vmx.c
> > > @@ -2577,6 +2577,20 @@ static __init int setup_vmcs_config(struct 
> > > vmcs_config *vmcs_conf,
> > >
> > > rdmsr(MSR_IA32_VMX_BASIC, vmx_msr_low, vmx_msr_high);
> > >
> > > +   /*
> > > +* IA-32 SDM Vol 3D: Bit 31 is always 0.
> > > +* For all earlier processors, bit 31 of this MSR was read as 0.
> > > +*/
> > > +   if (vmx_msr_low & (1u<<31))
> > > +   return -EIO;
> >
> > Drop this code as Jim said.
> >
> > > +
> > > +   /*
> > > +* IA-32 SDM Vol 3D: bits 47:45 and bits 63:57 are reserved and 
> > > are read
> > > +* as 0.
> > > +*/
> > > +   if (vmx_msr_high & 0xfe00e000)
> > > +   return -EIO;
> >
> > Is this ok? Can we pick up the part? :)
>
> No.  "Reserved and are read as 0" does not guarantee the bits will always be
> reserved.  There are very few bits used for feature enumeration in x86 that 
> are
> guaranteed to be '0' for all eternity.
>
> The whole point of reserving bits in registers is so that the CPU vendor, 
> Intel
> in this case, can introduce new features and enumerate them to software 
> without
> colliding with existing features or breaking software.  E.g. if Intel adds a 
> new
> feature and uses any of these bits to enumerate the feature, this check would
> prevent KVM from loading on CPUs that support the feature.

Got it, only explicit restrictions should be checked. Thanks.

--
Haiwei Li

Re: [PATCH] KVM: VMX: Check the corresponding bits according to the intel sdm

2021-03-25 Thread Haiwei Li 
On Tue, Mar 23, 2021 at 10:37 AM  wrote:
>
> From: Haiwei Li 
>
> According to IA-32 SDM Vol.3D "A.1 BASIC VMX INFORMATION", two inspections
> are missing.
> * Bit 31 is always 0. Earlier versions of this manual specified that the
> VMCS revision identifier was a 32-bit field in bits 31:0 of this MSR. For
> all processors produced prior to this change, bit 31 of this MSR was read
> as 0.
> * The values of bits 47:45 and bits 63:57 are reserved and are read as 0.
>
> Signed-off-by: Haiwei Li 
> ---
>  arch/x86/kvm/vmx/vmx.c | 14 ++
>  1 file changed, 14 insertions(+)
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 32cf828..0d6d13c 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -2577,6 +2577,20 @@ static __init int setup_vmcs_config(struct vmcs_config 
> *vmcs_conf,
>
> rdmsr(MSR_IA32_VMX_BASIC, vmx_msr_low, vmx_msr_high);
>
> +   /*
> +* IA-32 SDM Vol 3D: Bit 31 is always 0.
> +* For all earlier processors, bit 31 of this MSR was read as 0.
> +*/
> +   if (vmx_msr_low & (1u<<31))
> +   return -EIO;

Drop this code as Jim said.

> +
> +   /*
> +* IA-32 SDM Vol 3D: bits 47:45 and bits 63:57 are reserved and are 
> read
> +* as 0.
> +        */
> +   if (vmx_msr_high & 0xfe00e000)
> +   return -EIO;

Is this ok? Can we pick up the part? :)

--
Haiwei Li

Re: [PATCH] KVM: clean up the unused argument

2021-03-25 Thread Haiwei Li 
Kindly ping. :)

On Mon, Mar 15, 2021 at 12:40 PM Keqian Zhu  wrote:
>
>
> This looks OK. The use of vcpu argument is removed in commit d383b3146d80 
> (KVM: x86: Fix NULL dereference at kvm_msr_ignored_check())
>
> Reviewed-by: Keqian Zhu 
>
> On 2021/3/13 13:10, lihaiwei.ker...@gmail.com wrote:
> > From: Haiwei Li 
> >
> > kvm_msr_ignored_check function never uses vcpu argument. Clean up the
> > function and invokers.
> >
> > Signed-off-by: Haiwei Li 
> > ---
> >  arch/x86/kvm/x86.c | 9 -
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 012d5df..27e9ee8 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -271,8 +271,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
> >   * When called, it means the previous get/set msr reached an invalid msr.
> >   * Return true if we want to ignore/silent this failed msr access.
> >   */
> > -static bool kvm_msr_ignored_check(struct kvm_vcpu *vcpu, u32 msr,
> > -   u64 data, bool write)
> > +static bool kvm_msr_ignored_check(u32 msr, u64 data, bool write)
> >  {
> >   const char *op = write ? "wrmsr" : "rdmsr";
> >
> > @@ -1447,7 +1446,7 @@ static int do_get_msr_feature(struct kvm_vcpu *vcpu, 
> > unsigned index, u64 *data)
> >   if (r == KVM_MSR_RET_INVALID) {
> >   /* Unconditionally clear the output for simplicity */
> >   *data = 0;
> > - if (kvm_msr_ignored_check(vcpu, index, 0, false))
> > + if (kvm_msr_ignored_check(index, 0, false))
> >   r = 0;
> >   }
> >
> > @@ -1613,7 +1612,7 @@ static int kvm_set_msr_ignored_check(struct kvm_vcpu 
> > *vcpu,
> >   int ret = __kvm_set_msr(vcpu, index, data, host_initiated);
> >
> >   if (ret == KVM_MSR_RET_INVALID)
> > - if (kvm_msr_ignored_check(vcpu, index, data, true))
> > + if (kvm_msr_ignored_check(index, data, true))
> >   ret = 0;
> >
> >   return ret;
> > @@ -1651,7 +1650,7 @@ static int kvm_get_msr_ignored_check(struct kvm_vcpu 
> > *vcpu,
> >   if (ret == KVM_MSR_RET_INVALID) {
> >   /* Unconditionally clear *data for simplicity */
> >   *data = 0;
> > - if (kvm_msr_ignored_check(vcpu, index, 0, false))
> > + if (kvm_msr_ignored_check(index, 0, false))
> >   ret = 0;
> >   }
> >
> >

Re: [PATCH] KVM: x86/vPMU: Forbid writing to MSR_F15H_PERF MSRs when guest doesn't have X86_FEATURE_PERFCTR_CORE

2021-03-25 Thread Haiwei Li 
On Thu, Mar 25, 2021 at 4:10 PM Vitaly Kuznetsov  wrote:
>
> Haiwei Li  writes:
>
> > On Tue, Mar 23, 2021 at 4:48 PM Vitaly Kuznetsov  
> > wrote:
> >>
> >> MSR_F15H_PERF_CTL0-5, MSR_F15H_PERF_CTR0-5 MSRs are only available when
> >> X86_FEATURE_PERFCTR_CORE CPUID bit was exposed to the guest. KVM, however,
> >> allows these MSRs unconditionally because kvm_pmu_is_valid_msr() ->
> >> amd_msr_idx_to_pmc() check always passes and because kvm_pmu_set_msr() ->
> >> amd_pmu_set_msr() doesn't fail.
> >>
> >> In case of a counter (CTRn), no big harm is done as we only increase
> >> internal PMC's value but in case of an eventsel (CTLn), we go deep into
> >> perf internals with a non-existing counter.
> >>
> >> Note, kvm_get_msr_common() just returns '0' when these MSRs don't exist
> >> and this also seems to contradict architectural behavior which is #GP
> >> (I did check one old Opteron host) but changing this status quo is a bit
> >> scarier.
> >
> > When msr doesn't exist, kvm_get_msr_common() returns KVM_MSR_RET_INVALID
> > in `default:` and kvm_complete_insn_gp() will inject #GP to guest.
> >
>
> I'm looking at the following in kvm_get_msr_common():
>
> switch (msr_info->index) {
> ...
> case MSR_F15H_PERF_CTL0 ... MSR_F15H_PERF_CTR5:
> ...
> if (kvm_pmu_is_valid_msr(vcpu, msr_info->index))
> return kvm_pmu_get_msr(vcpu, msr_info);
> msr_info->data = 0;
> break;
> ...
> }
> return 0;
>
> so it's kind of 'always exists' or am I wrong?

I am sorry. You are right. You were talking about `MSR_F15H_PERF_CTL0
... MSR_F15H_PERF_CTR5`.
I thought you were talking about the registers not catched in
kvm_get_msr_common().

>
> > Also i have wrote a kvm-unit-test, tested both on amd EPYC and intel
> > CascadeLake. A #GP error was printed.
> > Just like:
> >
> > Unhandled exception 13 #GP at ip 00400420
> > error_code=  rflags=00010006  cs=0008
> > rax= rcx=0620 rdx=006164a0
> > rbx=9500
> > rbp=00517490 rsi=00616ae0 rdi=0001
> >  r8=0001  r9=03f8 r10=000d
> > r11=
> > r12= r13= r14=
> > r15=
> > cr0=8011 cr2= cr3=0040b000
> > cr4=0020
> > cr8=
> > STACK: @400420 400338
>
> Did this happen on read or write? The later is expected, the former is
> not. Could you maybe drop your code here, I'd like to see what's going
> on.

I did a bad test. The msr tested is not catched in
kvm_get_msr_common(). As said, I misunderstood
what you meant.

I have tested your patch and replied. If you have any to test, I'm
glad to help. :)

--
Haiwei Li

Re: [PATCH] KVM: x86/vPMU: Forbid writing to MSR_F15H_PERF MSRs when guest doesn't have X86_FEATURE_PERFCTR_CORE

2021-03-25 Thread Haiwei Li 
On Tue, Mar 23, 2021 at 4:48 PM Vitaly Kuznetsov  wrote:
>
> MSR_F15H_PERF_CTL0-5, MSR_F15H_PERF_CTR0-5 MSRs are only available when
> X86_FEATURE_PERFCTR_CORE CPUID bit was exposed to the guest. KVM, however,
> allows these MSRs unconditionally because kvm_pmu_is_valid_msr() ->
> amd_msr_idx_to_pmc() check always passes and because kvm_pmu_set_msr() ->
> amd_pmu_set_msr() doesn't fail.
>
> In case of a counter (CTRn), no big harm is done as we only increase
> internal PMC's value but in case of an eventsel (CTLn), we go deep into
> perf internals with a non-existing counter.
>
> Note, kvm_get_msr_common() just returns '0' when these MSRs don't exist
> and this also seems to contradict architectural behavior which is #GP
> (I did check one old Opteron host) but changing this status quo is a bit
> scarier.

When msr doesn't exist, kvm_get_msr_common() returns KVM_MSR_RET_INVALID
in `default:` and kvm_complete_insn_gp() will inject #GP to guest.

Also i have wrote a kvm-unit-test, tested both on amd EPYC and intel
CascadeLake. A #GP error was printed.
Just like:

Unhandled exception 13 #GP at ip 00400420
error_code=  rflags=00010006  cs=0008
rax= rcx=0620 rdx=006164a0
rbx=9500
rbp=00517490 rsi=00616ae0 rdi=0001
 r8=0001  r9=03f8 r10=000d
r11=
r12= r13= r14=
r15=
cr0=8011 cr2= cr3=0040b000
cr4=0020
cr8=
STACK: @400420 400338

Re: [PATCH] KVM: x86/vPMU: Forbid writing to MSR_F15H_PERF MSRs when guest doesn't have X86_FEATURE_PERFCTR_CORE

2021-03-24 Thread Haiwei Li 
On Tue, Mar 23, 2021 at 4:48 PM Vitaly Kuznetsov  wrote:
>
> MSR_F15H_PERF_CTL0-5, MSR_F15H_PERF_CTR0-5 MSRs are only available when
> X86_FEATURE_PERFCTR_CORE CPUID bit was exposed to the guest. KVM, however,
> allows these MSRs unconditionally because kvm_pmu_is_valid_msr() ->
> amd_msr_idx_to_pmc() check always passes and because kvm_pmu_set_msr() ->
> amd_pmu_set_msr() doesn't fail.

I have tested on AMD EPYC platform with perfctr_core(`cat
/proc/cpuinfo | grep perfctr_core`).
I started a vm without `perfctr-core`(-cpu host,-perfctr-core).

Before patch :

$ rdmsr 0xc0010200
0
$ wrmsr 0xc0010200 1
$ rdmsr 0xc0010200
1

After patch:

# rdmsr 0xc0010200
0
# wrmsr 0xc0010200 1
wrmsr: CPU 0 cannot set MSR 0xc0010200 to 0x0001
# rdmsr 0xc0010200
0

So,

Tested-by: Haiwei Li

Re: [PATCH] KVM: VMX: Check the corresponding bits according to the intel sdm

2021-03-22 Thread Haiwei Li 
On Tue, Mar 23, 2021 at 11:16 AM Jim Mattson  wrote:
>
> On Mon, Mar 22, 2021 at 7:37 PM  wrote:
> >
> > From: Haiwei Li 
> >
> > According to IA-32 SDM Vol.3D "A.1 BASIC VMX INFORMATION", two inspections
> > are missing.
> > * Bit 31 is always 0. Earlier versions of this manual specified that the
> > VMCS revision identifier was a 32-bit field in bits 31:0 of this MSR. For
> > all processors produced prior to this change, bit 31 of this MSR was read
> > as 0.
>
> For all *Intel* processors produced prior to this change, bit 31 of
> this MSR may have been 0. However, a conforming hypervisor may have
> selected a full 32-bit VMCS revision identifier with the high bit set
> for nested VMX. Furthermore, there are other vendors, such as VIA,
> which have implemented the VMX extensions, and they, too, may have
> selected a full 32-bit VMCS revision identifier with the high bit set.
> Intel should know better than to change the documentation after the
> horse is out of the barn.

Got it, thanks.

>
> What, exactly, is the value you are adding with this check?

I did this just to match the sdm.

--
Haiwei Li

Re: [PATCH] kvm: lapic: add module parameters for LAPIC_TIMER_ADVANCE_ADJUST_MAX/MIN

2021-03-12 Thread Haiwei Li 
On Sat, Mar 13, 2021 at 8:58 AM Sean Christopherson  wrote:
>
> On Wed, Mar 10, 2021, Haiwei Li wrote:
> > On Wed, Mar 10, 2021 at 7:42 AM Sean Christopherson  
> > wrote:
> > >
> > > On Wed, Mar 03, 2021, Haiwei Li wrote:
> > > > On 21/3/3 10:09, lihaiwei.ker...@gmail.com wrote:
> > > > > From: Haiwei Li 
> > > > >
> > > > > In my test environment, advance_expire_delta is frequently greater 
> > > > > than
> > > > > the fixed LAPIC_TIMER_ADVANCE_ADJUST_MAX. And this will hinder the
> > > > > adjustment.
> > > >
> > > > Supplementary details:
> > > >
> > > > I have tried to backport timer related features to our production
> > > > kernel.
> > > >
> > > > After completed, i found that advance_expire_delta is frequently greater
> > > > than the fixed value. It's necessary to trun the fixed to dynamically
> > > > values.
> > >
> > > Does this reproduce on an upstream kernel?  If so...
> > >
> > >   1. How much over the 10k cycle limit is the delta?
> > >   2. Any idea what causes the large delta?  E.g. is there something that 
> > > can
> > >  and/or should be fixed elsewhere?
> > >   3. Is it platform/CPU specific?
> >
> > Hi, Sean
> >
> > I have traced the flow on our production kernel and it frequently consumes 
> > more
> > than 10K cycles from sched_out to sched_in.
> > So two scenarios tested on Cascade lake Server(96 pcpu), v5.11 kernel.
> >
> > 1. only cyclictest in guest(88 vcpu and bound with isolated pcpus, w/o mwait
> > exposed, adaptive advance lapic timer is default -1). The ratio of 
> > occurrences:
> >
> > greater_than_10k/total: 29/2060, 1.41%
> >
> > 2. cyclictest in guest(88 vcpu and not bound, w/o mwait exposed, adaptive
> > advance lapic timer is default -1) and stress in host(no isolate). The 
> > ratio of
> > occurrences:
> >
> > greater_than_10k/total: 122381/1017363, 12.03%
>
> Hmm, I'm inclined to say this is working as intended.  If the vCPU isn't 
> affined
> and/or it's getting preempted, then large spikes are expected, and not 
> adjusting
> in reaction to those spikes is desirable.  E.g. adjusting by 20k cycles 
> because
> the timer happened to expire while a vCPU was preempted will cause KVM to busy
> wait for quite a long time if the next timer runs without interference, and 
> then
> KVM will thrash the advancement.
>
> And I don't really see the point in pushing the max adjustment beyond 10k.  
> The
> max _advancement_ is 5000ns, which means that even with a blazing fast 5.0ghz
> system, a max adjustment of 1250 (10k/ 8, the step divisor) should get KVM to
> the 25000 cycle advancement limit relatively quickly.  Since KVM resets to the
> initial 1000ns advancement when it would exceed the 5000ns max, I suspect that
> raising the max adjustment much beyond 10k cycles would quickly push a vCPU to
> the max, cause it to reset, and rinse and repeat.
>
> Note, we definitely don't want to raise the 5000ns max, as waiting with IRQs
> disabled for any longer than that will likely cause system instability.

I see. Thanks for your explanation.

--
Haiwei Li

Re: [PATCH] kvm: lapic: add module parameters for LAPIC_TIMER_ADVANCE_ADJUST_MAX/MIN

2021-03-10 Thread Haiwei Li 
On Wed, Mar 10, 2021 at 7:42 AM Sean Christopherson  wrote:
>
> On Wed, Mar 03, 2021, Haiwei Li wrote:
> > On 21/3/3 10:09, lihaiwei.ker...@gmail.com wrote:
> > > From: Haiwei Li 
> > >
> > > In my test environment, advance_expire_delta is frequently greater than
> > > the fixed LAPIC_TIMER_ADVANCE_ADJUST_MAX. And this will hinder the
> > > adjustment.
> >
> > Supplementary details:
> >
> > I have tried to backport timer related features to our production
> > kernel.
> >
> > After completed, i found that advance_expire_delta is frequently greater
> > than the fixed value. It's necessary to trun the fixed to dynamically
> > values.
>
> Does this reproduce on an upstream kernel?  If so...
>
>   1. How much over the 10k cycle limit is the delta?
>   2. Any idea what causes the large delta?  E.g. is there something that can
>  and/or should be fixed elsewhere?
>   3. Is it platform/CPU specific?

Hi, Sean

I have traced the flow on our production kernel and it frequently consumes more
than 10K cycles from sched_out to sched_in.
So two scenarios tested on Cascade lake Server(96 pcpu), v5.11 kernel.

1. only cyclictest in guest(88 vcpu and bound with isolated pcpus, w/o mwait
exposed, adaptive advance lapic timer is default -1). The ratio of occurrences:

greater_than_10k/total: 29/2060, 1.41%

2. cyclictest in guest(88 vcpu and not bound, w/o mwait exposed, adaptive
advance lapic timer is default -1) and stress in host(no isolate). The ratio of
occurrences:

greater_than_10k/total: 122381/1017363, 12.03%

-- 
Haiwei Li

>
> Ideally, KVM would play nice with "all" environments by default without 
> forcing
> the admin to hand-tune things.

Re: [PATCH] kvm: lapic: add module parameters for LAPIC_TIMER_ADVANCE_ADJUST_MAX/MIN

2021-03-03 Thread Haiwei Li 

On 21/3/3 10:09, lihaiwei.ker...@gmail.com wrote:

From: Haiwei Li 

In my test environment, advance_expire_delta is frequently greater than
the fixed LAPIC_TIMER_ADVANCE_ADJUST_MAX. And this will hinder the
adjustment.


Supplementary details:

I have tried to backport timer related features to our production
kernel.

After completed, i found that advance_expire_delta is frequently greater
than the fixed value. It's necessary to trun the fixed to dynamically
values.

--
Haiwei Li

Re: [PATCH v2] KVM: x86: Add tracepoint for dr_write/dr_read

2020-11-05 Thread Haiwei Li 

Kindly ping. :)

On 20/10/9 11:21, lihaiwei.ker...@gmail.com wrote:

From: Haiwei Li 

When vmexit occurs caused by accessing dr, there is no tracepoint to track
this action. Add tracepoint for this on x86 kvm.

Signed-off-by: Haiwei Li 
---
v1 -> v2:
  * Improve the changelog

  arch/x86/kvm/svm/svm.c |  2 ++
  arch/x86/kvm/trace.h   | 27 +++
  arch/x86/kvm/vmx/vmx.c | 10 --
  arch/x86/kvm/x86.c |  1 +
  4 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 4f401fc6a05d..52c69551aea4 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2423,12 +2423,14 @@ static int dr_interception(struct vcpu_svm *svm)
if (!kvm_require_dr(>vcpu, dr - 16))
return 1;
val = kvm_register_read(>vcpu, reg);
+   trace_kvm_dr_write(dr - 16, val);
kvm_set_dr(>vcpu, dr - 16, val);
} else {
if (!kvm_require_dr(>vcpu, dr))
return 1;
kvm_get_dr(>vcpu, dr, );
kvm_register_write(>vcpu, reg, val);
+   trace_kvm_dr_read(dr, val);
}
  
  	return kvm_skip_emulated_instruction(>vcpu);

diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index aef960f90f26..b3bf54405862 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -405,6 +405,33 @@ TRACE_EVENT(kvm_cr,
  #define trace_kvm_cr_read(cr, val)trace_kvm_cr(0, cr, val)
  #define trace_kvm_cr_write(cr, val)   trace_kvm_cr(1, cr, val)
  
+/*

+ * Tracepoint for guest DR access.
+ */
+TRACE_EVENT(kvm_dr,
+   TP_PROTO(unsigned int rw, unsigned int dr, unsigned long val),
+   TP_ARGS(rw, dr, val),
+
+   TP_STRUCT__entry(
+   __field(unsigned int,   rw  )
+   __field(unsigned int,   dr  )
+   __field(unsigned long,  val )
+   ),
+
+   TP_fast_assign(
+   __entry->rw  = rw;
+   __entry->dr  = dr;
+   __entry->val = val;
+   ),
+
+   TP_printk("dr_%s %x = 0x%lx",
+ __entry->rw ? "write" : "read",
+ __entry->dr, __entry->val)
+);
+
+#define trace_kvm_dr_read(dr, val) trace_kvm_dr(0, dr, val)
+#define trace_kvm_dr_write(dr, val)trace_kvm_dr(1, dr, val)
+
  TRACE_EVENT(kvm_pic_set_irq,
TP_PROTO(__u8 chip, __u8 pin, __u8 elcr, __u8 imr, bool coalesced),
TP_ARGS(chip, pin, elcr, imr, coalesced),
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 4551a7e80ebc..f78fd297d51e 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -5091,10 +5091,16 @@ static int handle_dr(struct kvm_vcpu *vcpu)
  
  		if (kvm_get_dr(vcpu, dr, ))

return 1;
+   trace_kvm_dr_read(dr, val);
kvm_register_write(vcpu, reg, val);
-   } else
-   if (kvm_set_dr(vcpu, dr, kvm_register_readl(vcpu, reg)))
+   } else {
+   unsigned long val;
+
+   val = kvm_register_readl(vcpu, reg);
+   trace_kvm_dr_write(dr, val);
+   if (kvm_set_dr(vcpu, dr, val))
return 1;
+   }
  
  	return kvm_skip_emulated_instruction(vcpu);

  }
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c4015a43cc8a..68cb7b331324 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -11153,6 +11153,7 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_inj_virq);
  EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_page_fault);
  EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_msr);
  EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_cr);
+EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_dr);
  EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_nested_vmrun);
  EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_nested_vmexit);
  EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_nested_vmexit_inject);

Re: [PATCH v2] KVM: x86: Add tracepoint for dr_write/dr_read

2020-10-21 Thread Haiwei Li 

Kindly ping. :)

On 20/10/9 11:21, lihaiwei.ker...@gmail.com wrote:

From: Haiwei Li 

When vmexit occurs caused by accessing dr, there is no tracepoint to track
this action. Add tracepoint for this on x86 kvm.

Signed-off-by: Haiwei Li 
---
v1 -> v2:
  * Improve the changelog

  arch/x86/kvm/svm/svm.c |  2 ++
  arch/x86/kvm/trace.h   | 27 +++
  arch/x86/kvm/vmx/vmx.c | 10 --
  arch/x86/kvm/x86.c |  1 +
  4 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 4f401fc6a05d..52c69551aea4 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2423,12 +2423,14 @@ static int dr_interception(struct vcpu_svm *svm)
if (!kvm_require_dr(>vcpu, dr - 16))
return 1;
val = kvm_register_read(>vcpu, reg);
+   trace_kvm_dr_write(dr - 16, val);
kvm_set_dr(>vcpu, dr - 16, val);
} else {
if (!kvm_require_dr(>vcpu, dr))
return 1;
kvm_get_dr(>vcpu, dr, );
kvm_register_write(>vcpu, reg, val);
+   trace_kvm_dr_read(dr, val);
}
  
  	return kvm_skip_emulated_instruction(>vcpu);

diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index aef960f90f26..b3bf54405862 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -405,6 +405,33 @@ TRACE_EVENT(kvm_cr,
  #define trace_kvm_cr_read(cr, val)trace_kvm_cr(0, cr, val)
  #define trace_kvm_cr_write(cr, val)   trace_kvm_cr(1, cr, val)
  
+/*

+ * Tracepoint for guest DR access.
+ */
+TRACE_EVENT(kvm_dr,
+   TP_PROTO(unsigned int rw, unsigned int dr, unsigned long val),
+   TP_ARGS(rw, dr, val),
+
+   TP_STRUCT__entry(
+   __field(unsigned int,   rw  )
+   __field(unsigned int,   dr  )
+   __field(unsigned long,  val )
+   ),
+
+   TP_fast_assign(
+   __entry->rw  = rw;
+   __entry->dr  = dr;
+   __entry->val = val;
+   ),
+
+   TP_printk("dr_%s %x = 0x%lx",
+ __entry->rw ? "write" : "read",
+ __entry->dr, __entry->val)
+);
+
+#define trace_kvm_dr_read(dr, val) trace_kvm_dr(0, dr, val)
+#define trace_kvm_dr_write(dr, val)trace_kvm_dr(1, dr, val)
+
  TRACE_EVENT(kvm_pic_set_irq,
TP_PROTO(__u8 chip, __u8 pin, __u8 elcr, __u8 imr, bool coalesced),
TP_ARGS(chip, pin, elcr, imr, coalesced),
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 4551a7e80ebc..f78fd297d51e 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -5091,10 +5091,16 @@ static int handle_dr(struct kvm_vcpu *vcpu)
  
  		if (kvm_get_dr(vcpu, dr, ))

return 1;
+   trace_kvm_dr_read(dr, val);
kvm_register_write(vcpu, reg, val);
-   } else
-   if (kvm_set_dr(vcpu, dr, kvm_register_readl(vcpu, reg)))
+   } else {
+   unsigned long val;
+
+   val = kvm_register_readl(vcpu, reg);
+   trace_kvm_dr_write(dr, val);
+   if (kvm_set_dr(vcpu, dr, val))
return 1;
+   }
  
  	return kvm_skip_emulated_instruction(vcpu);

  }
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c4015a43cc8a..68cb7b331324 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -11153,6 +11153,7 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_inj_virq);
  EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_page_fault);
  EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_msr);
  EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_cr);
+EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_dr);
  EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_nested_vmrun);
  EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_nested_vmexit);
  EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_nested_vmexit_inject);

Re: [PATCH v4] KVM: Check the allocation of pv cpu mask

2020-10-19 Thread Haiwei Li 

On 20/10/19 19:23, Vitaly Kuznetsov wrote:

lihaiwei.ker...@gmail.com writes:


From: Haiwei Li 

check the allocation of per-cpu __pv_cpu_mask. Init
'send_IPI_mask_allbutself' only when successful and check the allocation
of __pv_cpu_mask in 'kvm_flush_tlb_others'.

Suggested-by: Vitaly Kuznetsov 
Signed-off-by: Haiwei Li 
---
v1 -> v2:
  * add CONFIG_SMP for kvm_send_ipi_mask_allbutself to prevent build error
v2 -> v3:
  * always check the allocation of __pv_cpu_mask in kvm_flush_tlb_others
v3 -> v4:
  * mov kvm_setup_pv_ipi to kvm_alloc_cpumask and get rid of kvm_apic_init

  arch/x86/kernel/kvm.c | 53 +--
  1 file changed, 36 insertions(+), 17 deletions(-)

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 42c6e0deff9e..be28203cc098 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -547,16 +547,6 @@ static void kvm_send_ipi_mask_allbutself(const struct 
cpumask *mask, int vector)
__send_ipi_mask(local_mask, vector);
  }
  
-/*

- * Set the IPI entry points
- */
-static void kvm_setup_pv_ipi(void)
-{
-   apic->send_IPI_mask = kvm_send_ipi_mask;
-   apic->send_IPI_mask_allbutself = kvm_send_ipi_mask_allbutself;
-   pr_info("setup PV IPIs\n");
-}
-
  static void kvm_smp_send_call_func_ipi(const struct cpumask *mask)
  {
int cpu;
@@ -619,6 +609,11 @@ static void kvm_flush_tlb_others(const struct cpumask 
*cpumask,
struct kvm_steal_time *src;
struct cpumask *flushmask = this_cpu_cpumask_var_ptr(__pv_cpu_mask);
  
+	if (unlikely(!flushmask)) {

+   native_flush_tlb_others(cpumask, info);
+   return;
+   }
+
cpumask_copy(flushmask, cpumask);
/*
 * We have to call flush only on online vCPUs. And
@@ -732,10 +727,6 @@ static uint32_t __init kvm_detect(void)
  
  static void __init kvm_apic_init(void)

  {
-#if defined(CONFIG_SMP)
-   if (pv_ipi_supported())
-   kvm_setup_pv_ipi();
-#endif
  }


Do we still need the now-empty function?


It's not necessary. I will remove it.



  
  static void __init kvm_init_platform(void)

@@ -765,10 +756,18 @@ static __init int activate_jump_labels(void)
  }
  arch_initcall(activate_jump_labels);
  
+static void kvm_free_cpumask(void)

+{
+   unsigned int cpu;
+
+   for_each_possible_cpu(cpu)
+   free_cpumask_var(per_cpu(__pv_cpu_mask, cpu));
+}
+
  static __init int kvm_alloc_cpumask(void)
  {
int cpu;
-   bool alloc = false;
+   bool alloc = false, alloced = true;
  
  	if (!kvm_para_available() || nopv)

return 0;
@@ -783,10 +782,30 @@ static __init int kvm_alloc_cpumask(void)
  
  	if (alloc)

for_each_possible_cpu(cpu) {
-   zalloc_cpumask_var_node(per_cpu_ptr(&__pv_cpu_mask, 
cpu),
-   GFP_KERNEL, cpu_to_node(cpu));
+   if (!zalloc_cpumask_var_node(
+   per_cpu_ptr(&__pv_cpu_mask, cpu),
+   GFP_KERNEL, cpu_to_node(cpu))) {
+   alloced = false;
+   break;
+   }
}
  
+#if defined(CONFIG_SMP)

+   /* Set the IPI entry points */
+   if (pv_ipi_supported()) {


What if we define pv_ipi_supported() in !CONFIG_SMP case as 'false'?

The code we have above:

 if (pv_tlb_flush_supported())
alloc = true;

#if defined(CONFIG_SMP)
 if (pv_ipi_supported())
alloc = true;
#endif

if (alloc)
...

will transform into 'if (pv_tlb_flush_supported() ||
pv_ipi_supported())' and we'll get rid of 'alloc' variable.

Also, we can probably get rid of this new 'alloced' variable and switch
to checking if the cpumask for the last CPU in cpu_possible_mask is not
NULL.


Get it. It's a good point. I will do it. Thanks for your patience and 
kindness.


  

+   apic->send_IPI_mask = kvm_send_ipi_mask;
+   if (alloced)
+   apic->send_IPI_mask_allbutself =
+   kvm_send_ipi_mask_allbutself;
+   pr_info("setup PV IPIs\n");


I'd rather not set 'apic->send_IPI_mask = kvm_send_ipi_mask' in case we
failed to alloc cpumask too. It is weird that in case of an allocation
failure *some* IPIs will use the PV path and some won't. It's going to
be a nightmare to debug.


Agree. And 'pv_ops.mmu.tlb_remove_table = tlb_remove_table' should not 
be set either. What do you think? Thanks.


Haiwei Li

Re: [PATCH] KVM: x86: Add tracepoint for dr_write/dr_read

2020-09-29 Thread Haiwei Li 




On 20/9/29 17:01, Peter Zijlstra wrote:

On Tue, Sep 29, 2020 at 04:55:15PM +0800, lihaiwei.ker...@gmail.com wrote:

From: Haiwei Li 

Add tracepoint trace_kvm_dr_write/trace_kvm_dr_read for x86 kvm.


This is a changelog in the: i++; /* increment i */, style. Totally
inadequate.


I will improve the changelog and resend. Thanks.

Haiwei

Re: [PATCH] KVM: SVM: Add tracepoint for cr_interception

2020-09-23 Thread Haiwei Li 

Kindly ping. :)
On 20/9/4 19:25, Haiwei Li wrote:

From: Haiwei Li 

Add trace_kvm_cr_write and trace_kvm_cr_read for svm.

Signed-off-by: Haiwei Li 
---
  arch/x86/kvm/svm/svm.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 03dd7bac8034..2c6dea48ba62 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2261,6 +2261,7 @@ static int cr_interception(struct vcpu_svm *svm)
  if (cr >= 16) { /* mov to cr */
  cr -= 16;
  val = kvm_register_read(>vcpu, reg);
+    trace_kvm_cr_write(cr, val);
  switch (cr) {
  case 0:
  if (!check_selective_cr0_intercepted(svm, val))
@@ -2306,6 +2307,7 @@ static int cr_interception(struct vcpu_svm *svm)
  return 1;
  }
  kvm_register_write(>vcpu, reg, val);
+    trace_kvm_cr_read(cr, val);
  }
  return kvm_complete_insn_gp(>vcpu, err);
  }
--
2.18.4

Re: [PATCH 1/2] KVM: Fix the build error

2020-09-22 Thread Haiwei Li 

On 20/9/20 21:09, Paolo Bonzini wrote:

On 14/09/20 11:11, lihaiwei.ker...@gmail.com wrote:

From: Haiwei Li 

When CONFIG_SMP is not set, an build error occurs with message "error:
use of undeclared identifier 'kvm_send_ipi_mask_allbutself'"

Fixes: 0f990222108d ("KVM: Check the allocation of pv cpu mask", 2020-09-01)
Reported-by: kernel test robot 
Signed-off-by: Haiwei Li 
---
  arch/x86/kernel/kvm.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 1b51b727b140..7e8be0421720 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -797,7 +797,9 @@ static __init int kvm_alloc_cpumask(void)
}
}
  
+#if defined(CONFIG_SMP)

apic->send_IPI_mask_allbutself = kvm_send_ipi_mask_allbutself;
+#endif
pv_ops.mmu.flush_tlb_others = kvm_flush_tlb_others;
return 0;
  



If CONFIG_SMP is not set you don't need kvm_alloc_cpumask or
pv_ops.mmu.flush_tlb_others at all.  Can you squash these two into the
original patch and re-submit for 5.10?


Hi, Paolo

I'm a little confused. Function kvm_flush_tlb_others doesn't seem to be 
related to CONFIG_SMP.


And my patch like:

---
 arch/x86/kernel/kvm.c | 27 ---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 9663ba31347c..1e5da6db519c 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -553,7 +553,6 @@ static void kvm_send_ipi_mask_allbutself(const 
struct cpumask *mask, int vector)

 static void kvm_setup_pv_ipi(void)
 {
apic->send_IPI_mask = kvm_send_ipi_mask;
-   apic->send_IPI_mask_allbutself = kvm_send_ipi_mask_allbutself;
pr_info("setup PV IPIs\n");
 }

@@ -619,6 +618,11 @@ static void kvm_flush_tlb_others(const struct 
cpumask *cpumask,

struct kvm_steal_time *src;
struct cpumask *flushmask = this_cpu_cpumask_var_ptr(__pv_cpu_mask);

+   if (unlikely(!flushmask)) {
+   native_flush_tlb_others(cpumask, info);
+   return;
+   }
+
cpumask_copy(flushmask, cpumask);
/*
 * We have to call flush only on online vCPUs. And
@@ -765,6 +769,14 @@ static __init int activate_jump_labels(void)
 }
 arch_initcall(activate_jump_labels);

+static void kvm_free_cpumask(void)
+{
+   unsigned int cpu;
+
+   for_each_possible_cpu(cpu)
+   free_cpumask_var(per_cpu(__pv_cpu_mask, cpu));
+}
+
 static __init int kvm_alloc_cpumask(void)
 {
int cpu;
@@ -783,11 +795,20 @@ static __init int kvm_alloc_cpumask(void)

if (alloc)
for_each_possible_cpu(cpu) {
-   zalloc_cpumask_var_node(per_cpu_ptr(&__pv_cpu_mask, 
cpu),
-   GFP_KERNEL, cpu_to_node(cpu));
+   if (!zalloc_cpumask_var_node(
+   per_cpu_ptr(&__pv_cpu_mask, cpu),
+   GFP_KERNEL, cpu_to_node(cpu)))
+   goto zalloc_cpumask_fail;
}

+#if defined(CONFIG_SMP)
+   apic->send_IPI_mask_allbutself = kvm_send_ipi_mask_allbutself;
+#endif
return 0;
+
+zalloc_cpumask_fail:
+   kvm_free_cpumask();
+   return -ENOMEM;
 }
 arch_initcall(kvm_alloc_cpumask);

--
2.18.4

Do you have any suggestion? Thanks.

Haiwei

Re: [PATCH] KVM: SVM: Analyze is_guest_mode() in svm_vcpu_run()

2020-09-22 Thread Haiwei Li 
On Tue, Sep 22, 2020 at 10:56 PM Paolo Bonzini  wrote:
>
> On 22/09/20 16:54, Haiwei Li wrote:
> >> EXIT_FASTPATH_REENTER_GUEST handling up to vcpu_enter_guest)...
> > Hi, Paolo
> >
> > I have sent a patch to do this,
> >
> > https://lore.kernel.org/kvm/20200915113033.61817-1-lihaiwei.ker...@gmail.com/
>
> Cool, thanks.  I think I'll just squash it in Wanpeng's if you don't mind.

It's all ok. I don't mind. Thanks.

Haiwei

Re: [PATCH] KVM: SVM: Analyze is_guest_mode() in svm_vcpu_run()

2020-09-22 Thread Haiwei Li 
On 20/9/22 21:43, Paolo Bonzini wrote:
> On 14/09/20 22:43, Krish Sadhukhan wrote:
>>>
>>
>> Not related to your changes, but should we get rid of the variable
>> 'exit_fastpath' and just do,
>>
>>  return svm_exit_handler_fastpath(vcpu);
>>
>> It seems the variable isn't used anywhere else and svm_vcpu_run()
>> doesn't return from anywhere else either.
>
> Yes (also because vmx will do the same once we can push
> EXIT_FASTPATH_REENTER_GUEST handling up to vcpu_enter_guest)...

Hi, Paolo

I have sent a patch to do this,

https://lore.kernel.org/kvm/20200915113033.61817-1-lihaiwei.ker...@gmail.com/

Thanks.

 Haiwei

Re: [PATCH 1/2] KVM: Fix the build error

2020-09-20 Thread Haiwei Li 




On 20/9/20 21:09, Paolo Bonzini wrote:

On 14/09/20 11:11, lihaiwei.ker...@gmail.com wrote:

From: Haiwei Li 

When CONFIG_SMP is not set, an build error occurs with message "error:
use of undeclared identifier 'kvm_send_ipi_mask_allbutself'"

Fixes: 0f990222108d ("KVM: Check the allocation of pv cpu mask", 2020-09-01)
Reported-by: kernel test robot 
Signed-off-by: Haiwei Li 
---
  arch/x86/kernel/kvm.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 1b51b727b140..7e8be0421720 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -797,7 +797,9 @@ static __init int kvm_alloc_cpumask(void)
}
}
  
+#if defined(CONFIG_SMP)

apic->send_IPI_mask_allbutself = kvm_send_ipi_mask_allbutself;
+#endif
pv_ops.mmu.flush_tlb_others = kvm_flush_tlb_others;
return 0;
  



If CONFIG_SMP is not set you don't need kvm_alloc_cpumask or
pv_ops.mmu.flush_tlb_others at all.  Can you squash these two into the
original patch and re-submit for 5.10?


I will, thanks.

Haiwei Li

Re: [PATCH] Revert "KVM: Check the allocation of pv cpu mask"

2020-09-16 Thread Haiwei Li 
Vitaly Kuznetsov  于2020年9月16日周三 下午7:04写道：
>
> Haiwei Li  writes:
>
> > On 20/9/16 17:03, Vitaly Kuznetsov wrote:
> >> The commit 0f990222108d ("KVM: Check the allocation of pv cpu mask") we
> >> have in 5.9-rc5 has two issue:
> >> 1) Compilation fails for !CONFIG_SMP, see:
> >> https://bugzilla.kernel.org/show_bug.cgi?id=209285
> >>
> >> 2) This commit completely disables PV TLB flush, see
> >> https://lore.kernel.org/kvm/87y2lrnnyf@vitty.brq.redhat.com/
> >>
> >> The allocation problem is likely a theoretical one, if we don't
> >> have memory that early in boot process we're likely doomed anyway.
> >> Let's solve it properly later.
> >
> > Hi, i have sent a patchset to fix this commit.
> >
> > https://lore.kernel.org/kvm/20200914091148.95654-1-lihaiwei.ker...@gmail.com/T/#m6c27184012ee5438e5d91c09b1ba1b6a3ee30ee4
> >
> > What do you think?
>
> Saw it, looks good to me. We are, however, already very, very late in 5.9
> release cycle and the original issue you were addressing (allocation
> failure) is likely a theoretical only I suggest we just revert it before
> 5.9 is released. For 5.9 we can certainly take your PATCH2 merged with
> 0f99022210.
>
> This Paolo's call anyway)

I see.  Thank you.

Haiwei Li

Re: [PATCH] Revert "KVM: Check the allocation of pv cpu mask"

2020-09-16 Thread Haiwei Li 

On 20/9/16 17:03, Vitaly Kuznetsov wrote:

The commit 0f990222108d ("KVM: Check the allocation of pv cpu mask") we
have in 5.9-rc5 has two issue:
1) Compilation fails for !CONFIG_SMP, see:
https://bugzilla.kernel.org/show_bug.cgi?id=209285

2) This commit completely disables PV TLB flush, see
https://lore.kernel.org/kvm/87y2lrnnyf@vitty.brq.redhat.com/

The allocation problem is likely a theoretical one, if we don't
have memory that early in boot process we're likely doomed anyway.
Let's solve it properly later.


Hi, i have sent a patchset to fix this commit.

https://lore.kernel.org/kvm/20200914091148.95654-1-lihaiwei.ker...@gmail.com/T/#m6c27184012ee5438e5d91c09b1ba1b6a3ee30ee4

What do you think?

Haiwei Li

Re: [PATCH] KVM: SVM: Analyze is_guest_mode() in svm_vcpu_run()

2020-09-15 Thread Haiwei Li 




On 20/9/15 04:43, Krish Sadhukhan wrote:


On 9/13/20 11:55 PM, Wanpeng Li wrote:

From: Wanpeng Li 

Analyze is_guest_mode() in svm_vcpu_run() instead of 
svm_exit_handlers_fastpath()

in conformity with VMX version.

Suggested-by: Vitaly Kuznetsov 
Signed-off-by: Wanpeng Li 
---
  arch/x86/kvm/svm/svm.c | 7 +--
  1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 3da5b2f..009035a 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -3393,8 +3393,7 @@ static void svm_cancel_injection(struct kvm_vcpu 
*vcpu)

  static fastpath_t svm_exit_handlers_fastpath(struct kvm_vcpu *vcpu)
  {
-    if (!is_guest_mode(vcpu) &&
-    to_svm(vcpu)->vmcb->control.exit_code == SVM_EXIT_MSR &&
+    if (to_svm(vcpu)->vmcb->control.exit_code == SVM_EXIT_MSR &&
  to_svm(vcpu)->vmcb->control.exit_info_1)
  return handle_fastpath_set_msr_irqoff(vcpu);
@@ -3580,6 +3579,10 @@ static __no_kcsan fastpath_t 
svm_vcpu_run(struct kvm_vcpu *vcpu)

  svm_handle_mce(svm);
  svm_complete_interrupts(svm);
+
+    if (is_guest_mode(vcpu))
+    return EXIT_FASTPATH_NONE;
+
  exit_fastpath = svm_exit_handlers_fastpath(vcpu);
  return exit_fastpath;


Not related to your changes, but should we get rid of the variable 
'exit_fastpath' and just do,


         return svm_exit_handler_fastpath(vcpu);

It seems the variable isn't used anywhere else and svm_vcpu_run() 
doesn't return from anywhere else either.


Also, svm_exit_handlers_fastpath() doesn't have any other caller. Should 
we get rid of it as well ?


I will do this soon, thanks.

Re: [PATCH] KVM: SVM: Add tracepoint for cr_interception

2020-09-04 Thread Haiwei Li 




On 20/9/4 20:01, Vitaly Kuznetsov wrote:

Haiwei Li  writes:


From: Haiwei Li 

Add trace_kvm_cr_write and trace_kvm_cr_read for svm.

Signed-off-by: Haiwei Li 
---
   arch/x86/kvm/svm/svm.c | 2 ++
   1 file changed, 2 insertions(+)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 03dd7bac8034..2c6dea48ba62 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2261,6 +2261,7 @@ static int cr_interception(struct vcpu_svm *svm)


There are two special cases when we go to emulate_on_interception() and
these won't be logged but I don't think this is a must.


if (cr >= 16) { /* mov to cr */
cr -= 16;
val = kvm_register_read(>vcpu, reg);
+   trace_kvm_cr_write(cr, val);
switch (cr) {
case 0:
if (!check_selective_cr0_intercepted(svm, val))
@@ -2306,6 +2307,7 @@ static int cr_interception(struct vcpu_svm *svm)
return 1;
}
kvm_register_write(>vcpu, reg, val);
+   trace_kvm_cr_read(cr, val);


The 'default:' case above does 'return 1;' so we won't get the trace but
I understand you put trace_kvm_cr_read() here so you can log the
returned 'val', #UD should be clearly visible.


}
return kvm_complete_insn_gp(>vcpu, err);
   }
--
2.18.4



Reviewed-by: Vitaly Kuznetsov 


Thanks a lot.

Re: [PATCH v2] KVM: Check the allocation of pv cpu mask

2020-09-04 Thread Haiwei Li 




On 20/9/4 17:53, Vitaly Kuznetsov wrote:

Haiwei Li  writes:


On 20/9/3 18:39, Vitaly Kuznetsov wrote:

Haiwei Li  writes:


From: Haiwei Li 

check the allocation of per-cpu __pv_cpu_mask. Initialize ops only when
successful.

Signed-off-by: Haiwei Li 
---
arch/x86/kernel/kvm.c | 24 
1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 08320b0b2b27..d3c062e551d7 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -555,7 +555,6 @@ static void kvm_send_ipi_mask_allbutself(const
struct cpumask *mask, int vector)
static void kvm_setup_pv_ipi(void)
{
apic->send_IPI_mask = kvm_send_ipi_mask;
-   apic->send_IPI_mask_allbutself = kvm_send_ipi_mask_allbutself;
pr_info("setup PV IPIs\n");
}

@@ -654,7 +653,6 @@ static void __init kvm_guest_init(void)
}

if (pv_tlb_flush_supported()) {
-   pv_ops.mmu.flush_tlb_others = kvm_flush_tlb_others;
pv_ops.mmu.tlb_remove_table = tlb_remove_table;
pr_info("KVM setup pv remote TLB flush\n");
}
@@ -767,6 +765,14 @@ static __init int activate_jump_labels(void)
}
arch_initcall(activate_jump_labels);

+static void kvm_free_pv_cpu_mask(void)
+{
+   unsigned int cpu;
+
+   for_each_possible_cpu(cpu)
+   free_cpumask_var(per_cpu(__pv_cpu_mask, cpu));
+}
+
static __init int kvm_alloc_cpumask(void)
{
int cpu;
@@ -785,11 +791,21 @@ static __init int kvm_alloc_cpumask(void)

if (alloc)
for_each_possible_cpu(cpu) {
-   zalloc_cpumask_var_node(per_cpu_ptr(&__pv_cpu_mask, 
cpu),
-   GFP_KERNEL, cpu_to_node(cpu));
+   if (!zalloc_cpumask_var_node(
+   per_cpu_ptr(&__pv_cpu_mask, cpu),
+   GFP_KERNEL, cpu_to_node(cpu)))
+   goto zalloc_cpumask_fail;
}

+#if defined(CONFIG_SMP)
+   apic->send_IPI_mask_allbutself = kvm_send_ipi_mask_allbutself;
+#endif
+   pv_ops.mmu.flush_tlb_others = kvm_flush_tlb_others;


This is too late I'm afraid. If I'm not mistaken PV patching happens
earlier, so .init.guest_late_init (kvm_guest_init()) is good and
arch_initcall() is bad.


.init.guest_late_init (kvm_guest_init()) is called before
arch_initcall() and kvm_flush_tlb_others && kvm_send_ipi_mask_allbutself
rely on __pv_cpu_mask.  So, i can not put this assign in kvm_guest_init().



Have you checked that with this patch kvm_flush_tlb_others() is still
being called?


yes. I add a printk and i get the log.



This is weird. I do the following on top of your patch:

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index d3c062e551d7..f441209ff0a4 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -620,6 +620,8 @@ static void kvm_flush_tlb_others(const struct cpumask 
*cpumask,
 struct kvm_steal_time *src;
 struct cpumask *flushmask = this_cpu_cpumask_var_ptr(__pv_cpu_mask);
  
+   trace_printk("PV TLB flush %d CPUs\n", cpumask_weight(cpumask));

+
 cpumask_copy(flushmask, cpumask);
 /*
  * We have to call flush only on online vCPUs. And

With your patch I don't see any calls:

# grep -c -v '^#' /sys/kernel/debug/tracing/trace
0

with your patch reverted I see them:

# grep -c -v '^#' /sys/kernel/debug/tracing/trace
4571


I just retested. You are right. I'm sorry.






Actually, there is no need to assign kvm_flush_tlb_others() so late. We
can always check if __pv_cpu_mask was allocated and revert back to the
architectural path if not.

I am sorry i don't really understand. Can you explain in more detail? Thx.



I mean we can always call e.g. kvm_flush_tlb_others(), even if (very
unlikely) the mask wasn't allocated. We just need to check for
that. Something like (completely untested):

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index d3c062e551d7..e3676cdee6a2 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -620,6 +620,11 @@ static void kvm_flush_tlb_others(const struct cpumask 
*cpumask,
 struct kvm_steal_time *src;
 struct cpumask *flushmask = this_cpu_cpumask_var_ptr(__pv_cpu_mask);
  
+   if (unlikely(!flushmask)) {

+   flushmask = cpumask;
+   goto do_native;
+   }
+


I see. I appreciate your patience and kindness.

I will send a new version.


 cpumask_copy(flushmask, cpumask);
 /*
  * We have to call flush only on online vCPUs. And
@@ -635,6 +640,7 @@ static void kvm_flush_tlb_others(const struct cpumask 
*cpumask,
 }
 }
  
+do_native:

 native_flush_tlb_others(flushmask, info);
  }
  




return 0;
+
+zalloc_cpumask_fail:
+   kvm_free_pv_cpu_mask();
+   return -ENOMEM;
}
arch_initcall(kvm_alloc_cpumask);

--
2.18.4

[PATCH] KVM: SVM: Add tracepoint for cr_interception

2020-09-04 Thread Haiwei Li 

From: Haiwei Li 

Add trace_kvm_cr_write and trace_kvm_cr_read for svm.

Signed-off-by: Haiwei Li 
---
 arch/x86/kvm/svm/svm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 03dd7bac8034..2c6dea48ba62 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2261,6 +2261,7 @@ static int cr_interception(struct vcpu_svm *svm)
if (cr >= 16) { /* mov to cr */
cr -= 16;
val = kvm_register_read(>vcpu, reg);
+   trace_kvm_cr_write(cr, val);
switch (cr) {
case 0:
if (!check_selective_cr0_intercepted(svm, val))
@@ -2306,6 +2307,7 @@ static int cr_interception(struct vcpu_svm *svm)
return 1;
}
kvm_register_write(>vcpu, reg, val);
+   trace_kvm_cr_read(cr, val);
}
return kvm_complete_insn_gp(>vcpu, err);
 }
--
2.18.4

Re: [PATCH v2] KVM: Check the allocation of pv cpu mask

2020-09-03 Thread Haiwei Li 




On 20/9/3 18:39, Vitaly Kuznetsov wrote:

Haiwei Li  writes:


From: Haiwei Li 

check the allocation of per-cpu __pv_cpu_mask. Initialize ops only when
successful.

Signed-off-by: Haiwei Li 
---
   arch/x86/kernel/kvm.c | 24 
   1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 08320b0b2b27..d3c062e551d7 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -555,7 +555,6 @@ static void kvm_send_ipi_mask_allbutself(const
struct cpumask *mask, int vector)
   static void kvm_setup_pv_ipi(void)
   {
apic->send_IPI_mask = kvm_send_ipi_mask;
-   apic->send_IPI_mask_allbutself = kvm_send_ipi_mask_allbutself;
pr_info("setup PV IPIs\n");
   }

@@ -654,7 +653,6 @@ static void __init kvm_guest_init(void)
}

if (pv_tlb_flush_supported()) {
-   pv_ops.mmu.flush_tlb_others = kvm_flush_tlb_others;
pv_ops.mmu.tlb_remove_table = tlb_remove_table;
pr_info("KVM setup pv remote TLB flush\n");
}
@@ -767,6 +765,14 @@ static __init int activate_jump_labels(void)
   }
   arch_initcall(activate_jump_labels);

+static void kvm_free_pv_cpu_mask(void)
+{
+   unsigned int cpu;
+
+   for_each_possible_cpu(cpu)
+   free_cpumask_var(per_cpu(__pv_cpu_mask, cpu));
+}
+
   static __init int kvm_alloc_cpumask(void)
   {
int cpu;
@@ -785,11 +791,21 @@ static __init int kvm_alloc_cpumask(void)

if (alloc)
for_each_possible_cpu(cpu) {
-   zalloc_cpumask_var_node(per_cpu_ptr(&__pv_cpu_mask, 
cpu),
-   GFP_KERNEL, cpu_to_node(cpu));
+   if (!zalloc_cpumask_var_node(
+   per_cpu_ptr(&__pv_cpu_mask, cpu),
+   GFP_KERNEL, cpu_to_node(cpu)))
+   goto zalloc_cpumask_fail;
}

+#if defined(CONFIG_SMP)
+   apic->send_IPI_mask_allbutself = kvm_send_ipi_mask_allbutself;
+#endif
+   pv_ops.mmu.flush_tlb_others = kvm_flush_tlb_others;


This is too late I'm afraid. If I'm not mistaken PV patching happens
earlier, so .init.guest_late_init (kvm_guest_init()) is good and
arch_initcall() is bad.


.init.guest_late_init (kvm_guest_init()) is called before 
arch_initcall() and kvm_flush_tlb_others && kvm_send_ipi_mask_allbutself 
rely on __pv_cpu_mask.  So, i can not put this assign in kvm_guest_init().




Have you checked that with this patch kvm_flush_tlb_others() is still
being called?


yes. I add a printk and i get the log.



Actually, there is no need to assign kvm_flush_tlb_others() so late. We
can always check if __pv_cpu_mask was allocated and revert back to the
architectural path if not.

I am sorry i don't really understand. Can you explain in more detail? Thx.




return 0;
+
+zalloc_cpumask_fail:
+   kvm_free_pv_cpu_mask();
+   return -ENOMEM;
   }
   arch_initcall(kvm_alloc_cpumask);

--
2.18.4

[PATCH v2] KVM: Check the allocation of pv cpu mask

2020-09-02 Thread Haiwei Li 

From: Haiwei Li 

check the allocation of per-cpu __pv_cpu_mask. Initialize ops only when
successful.

Signed-off-by: Haiwei Li 
---
 arch/x86/kernel/kvm.c | 24 
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 08320b0b2b27..d3c062e551d7 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -555,7 +555,6 @@ static void kvm_send_ipi_mask_allbutself(const 
struct cpumask *mask, int vector)

 static void kvm_setup_pv_ipi(void)
 {
apic->send_IPI_mask = kvm_send_ipi_mask;
-   apic->send_IPI_mask_allbutself = kvm_send_ipi_mask_allbutself;
pr_info("setup PV IPIs\n");
 }

@@ -654,7 +653,6 @@ static void __init kvm_guest_init(void)
}

if (pv_tlb_flush_supported()) {
-   pv_ops.mmu.flush_tlb_others = kvm_flush_tlb_others;
pv_ops.mmu.tlb_remove_table = tlb_remove_table;
pr_info("KVM setup pv remote TLB flush\n");
}
@@ -767,6 +765,14 @@ static __init int activate_jump_labels(void)
 }
 arch_initcall(activate_jump_labels);

+static void kvm_free_pv_cpu_mask(void)
+{
+   unsigned int cpu;
+
+   for_each_possible_cpu(cpu)
+   free_cpumask_var(per_cpu(__pv_cpu_mask, cpu));
+}
+
 static __init int kvm_alloc_cpumask(void)
 {
int cpu;
@@ -785,11 +791,21 @@ static __init int kvm_alloc_cpumask(void)

if (alloc)
for_each_possible_cpu(cpu) {
-   zalloc_cpumask_var_node(per_cpu_ptr(&__pv_cpu_mask, 
cpu),
-   GFP_KERNEL, cpu_to_node(cpu));
+   if (!zalloc_cpumask_var_node(
+   per_cpu_ptr(&__pv_cpu_mask, cpu),
+   GFP_KERNEL, cpu_to_node(cpu)))
+   goto zalloc_cpumask_fail;
}

+#if defined(CONFIG_SMP)
+   apic->send_IPI_mask_allbutself = kvm_send_ipi_mask_allbutself;
+#endif
+   pv_ops.mmu.flush_tlb_others = kvm_flush_tlb_others;
return 0;
+
+zalloc_cpumask_fail:
+   kvm_free_pv_cpu_mask();
+   return -ENOMEM;
 }
 arch_initcall(kvm_alloc_cpumask);

--
2.18.4

Re: [PATCH] KVM: Check the allocation of pv cpu mask

2020-09-02 Thread Haiwei Li 




On 20/9/2 01:35, kernel test robot wrote:

Hi Haiwei,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on kvm/linux-next]
[also build test ERROR on linus/master v5.9-rc3 next-20200828]
[cannot apply to linux/master vhost/linux-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/Haiwei-Li/KVM-Check-the-allocation-of-pv-cpu-mask/20200901-195412
base:   https://git.kernel.org/pub/scm/virt/kvm/kvm.git linux-next
config: x86_64-randconfig-a011-20200901 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 
c10e63677f5d20f18010f8f68c631ddc97546f7d)
reproduce (this is a W=1 build):
 wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
 chmod +x ~/bin/make.cross
 # install x86_64 cross compiling tool for clang build
 # apt-get install binutils-x86-64-linux-gnu
 # save the attached .config to linux build tree
 COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All errors (new ones prefixed by >>):


arch/x86/kernel/kvm.c:801:35: error: use of undeclared identifier 
'kvm_send_ipi_mask_allbutself'

apic->send_IPI_mask_allbutself = kvm_send_ipi_mask_allbutself;
 ^
1 error generated.


THX, i will fix and resend.



# 
https://github.com/0day-ci/linux/commit/13dd13ab0aefbb5c31bd8681831e6a11ac381509
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review 
Haiwei-Li/KVM-Check-the-allocation-of-pv-cpu-mask/20200901-195412
git checkout 13dd13ab0aefbb5c31bd8681831e6a11ac381509
vim +/kvm_send_ipi_mask_allbutself +801 arch/x86/kernel/kvm.c

791 
792 if (alloc)
793 for_each_possible_cpu(cpu) {
794 if (!zalloc_cpumask_var_node(
795 per_cpu_ptr(&__pv_cpu_mask, cpu),
796 GFP_KERNEL, cpu_to_node(cpu))) {
797 goto zalloc_cpumask_fail;
798 }
799 }
800 
  > 801  apic->send_IPI_mask_allbutself = 
kvm_send_ipi_mask_allbutself;
802 pv_ops.mmu.flush_tlb_others = kvm_flush_tlb_others;
803 return 0;
804 
805 zalloc_cpumask_fail:
806 kvm_free_pv_cpu_mask();
807 return -ENOMEM;
808 }
809 arch_initcall(kvm_alloc_cpumask);
810 

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org

[PATCH] KVM: Check the allocation of pv cpu mask

2020-09-01 Thread Haiwei Li 

From: Haiwei Li 

check the allocation of per-cpu __pv_cpu_mask. Initialize ops only when
successful.

Signed-off-by: Haiwei Li 
---
 arch/x86/kernel/kvm.c | 23 +++
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 08320b0b2b27..a64b894eaac0 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -555,7 +555,6 @@ static void kvm_send_ipi_mask_allbutself(const 
struct cpumask *mask, int vector)

 static void kvm_setup_pv_ipi(void)
 {
apic->send_IPI_mask = kvm_send_ipi_mask;
-   apic->send_IPI_mask_allbutself = kvm_send_ipi_mask_allbutself;
pr_info("setup PV IPIs\n");
 }

@@ -654,7 +653,6 @@ static void __init kvm_guest_init(void)
}

if (pv_tlb_flush_supported()) {
-   pv_ops.mmu.flush_tlb_others = kvm_flush_tlb_others;
pv_ops.mmu.tlb_remove_table = tlb_remove_table;
pr_info("KVM setup pv remote TLB flush\n");
}
@@ -767,6 +765,14 @@ static __init int activate_jump_labels(void)
 }
 arch_initcall(activate_jump_labels);

+static void kvm_free_pv_cpu_mask(void)
+{
+   unsigned int cpu;
+
+   for_each_possible_cpu(cpu)
+   free_cpumask_var(per_cpu(__pv_cpu_mask, cpu));
+}
+
 static __init int kvm_alloc_cpumask(void)
 {
int cpu;
@@ -785,11 +791,20 @@ static __init int kvm_alloc_cpumask(void)

if (alloc)
for_each_possible_cpu(cpu) {
-   zalloc_cpumask_var_node(per_cpu_ptr(&__pv_cpu_mask, 
cpu),
-   GFP_KERNEL, cpu_to_node(cpu));
+   if (!zalloc_cpumask_var_node(
+   per_cpu_ptr(&__pv_cpu_mask, cpu),
+   GFP_KERNEL, cpu_to_node(cpu))) {
+   goto zalloc_cpumask_fail;
+   }
}

+   apic->send_IPI_mask_allbutself = kvm_send_ipi_mask_allbutself;
+   pv_ops.mmu.flush_tlb_others = kvm_flush_tlb_others;
return 0;
+
+zalloc_cpumask_fail:
+   kvm_free_pv_cpu_mask();
+   return -ENOMEM;
 }
 arch_initcall(kvm_alloc_cpumask);

--
2.18.4

[PATCH] perf/x86/svm: Convert 'perf kvm stat report' output lowercase to uppercase

2020-07-29 Thread Haiwei Li 

From: Haiwei Li 

The reason output of 'perf kvm stat report --event=vmexit' is uppercase 
on VMX and lowercase on SVM.


To be consistent with VMX, convert lowercase to uppercase.

Signed-off-by: Haiwei Li 
---
 arch/x86/include/uapi/asm/svm.h   | 184 
+-
 tools/arch/x86/include/uapi/asm/svm.h | 184 
+-

 2 files changed, 184 insertions(+), 184 deletions(-)

diff --git a/arch/x86/include/uapi/asm/svm.h 
b/arch/x86/include/uapi/asm/svm.h

index 2e8a30f..647b6e2 100644
--- a/arch/x86/include/uapi/asm/svm.h
+++ b/arch/x86/include/uapi/asm/svm.h
@@ -83,98 +83,98 @@
 #define SVM_EXIT_ERR   -1

 #define SVM_EXIT_REASONS \
-   { SVM_EXIT_READ_CR0,"read_cr0" }, \
-   { SVM_EXIT_READ_CR2,"read_cr2" }, \
-   { SVM_EXIT_READ_CR3,"read_cr3" }, \
-   { SVM_EXIT_READ_CR4,"read_cr4" }, \
-   { SVM_EXIT_READ_CR8,"read_cr8" }, \
-   { SVM_EXIT_WRITE_CR0,   "write_cr0" }, \
-   { SVM_EXIT_WRITE_CR2,   "write_cr2" }, \
-   { SVM_EXIT_WRITE_CR3,   "write_cr3" }, \
-   { SVM_EXIT_WRITE_CR4,   "write_cr4" }, \
-   { SVM_EXIT_WRITE_CR8,   "write_cr8" }, \
-   { SVM_EXIT_READ_DR0,"read_dr0" }, \
-   { SVM_EXIT_READ_DR1,"read_dr1" }, \
-   { SVM_EXIT_READ_DR2,"read_dr2" }, \
-   { SVM_EXIT_READ_DR3,"read_dr3" }, \
-   { SVM_EXIT_READ_DR4,"read_dr4" }, \
-   { SVM_EXIT_READ_DR5,"read_dr5" }, \
-   { SVM_EXIT_READ_DR6,"read_dr6" }, \
-   { SVM_EXIT_READ_DR7,"read_dr7" }, \
-   { SVM_EXIT_WRITE_DR0,   "write_dr0" }, \
-   { SVM_EXIT_WRITE_DR1,   "write_dr1" }, \
-   { SVM_EXIT_WRITE_DR2,   "write_dr2" }, \
-   { SVM_EXIT_WRITE_DR3,   "write_dr3" }, \
-   { SVM_EXIT_WRITE_DR4,   "write_dr4" }, \
-   { SVM_EXIT_WRITE_DR5,   "write_dr5" }, \
-   { SVM_EXIT_WRITE_DR6,   "write_dr6" }, \
-   { SVM_EXIT_WRITE_DR7,   "write_dr7" }, \
-   { SVM_EXIT_EXCP_BASE + DE_VECTOR,   "DE excp" }, \
-   { SVM_EXIT_EXCP_BASE + DB_VECTOR,   "DB excp" }, \
-   { SVM_EXIT_EXCP_BASE + BP_VECTOR,   "BP excp" }, \
-   { SVM_EXIT_EXCP_BASE + OF_VECTOR,   "OF excp" }, \
-   { SVM_EXIT_EXCP_BASE + BR_VECTOR,   "BR excp" }, \
-   { SVM_EXIT_EXCP_BASE + UD_VECTOR,   "UD excp" }, \
-   { SVM_EXIT_EXCP_BASE + NM_VECTOR,   "NM excp" }, \
-   { SVM_EXIT_EXCP_BASE + DF_VECTOR,   "DF excp" }, \
-   { SVM_EXIT_EXCP_BASE + TS_VECTOR,   "TS excp" }, \
-   { SVM_EXIT_EXCP_BASE + NP_VECTOR,   "NP excp" }, \
-   { SVM_EXIT_EXCP_BASE + SS_VECTOR,   "SS excp" }, \
-   { SVM_EXIT_EXCP_BASE + GP_VECTOR,   "GP excp" }, \
-   { SVM_EXIT_EXCP_BASE + PF_VECTOR,   "PF excp" }, \
-   { SVM_EXIT_EXCP_BASE + MF_VECTOR,   "MF excp" }, \
-   { SVM_EXIT_EXCP_BASE + AC_VECTOR,   "AC excp" }, \
-   { SVM_EXIT_EXCP_BASE + MC_VECTOR,   "MC excp" }, \
-   { SVM_EXIT_EXCP_BASE + XM_VECTOR,   "XF excp" }, \
-   { SVM_EXIT_INTR,"interrupt" }, \
-   { SVM_EXIT_NMI, "nmi" }, \
-   { SVM_EXIT_SMI, "smi" }, \
-   { SVM_EXIT_INIT,"init" }, \
-   { SVM_EXIT_VINTR,   "vintr" }, \
-   { SVM_EXIT_CR0_SEL_WRITE, "cr0_sel_write" }, \
-   { SVM_EXIT_IDTR_READ,   "read_idtr" }, \
-   { SVM_EXIT_GDTR_READ,   "read_gdtr" }, \
-   { SVM_EXIT_LDTR_READ,   "read_ldtr" }, \
-   { SVM_EXIT_TR_READ, "read_rt" }, \
-   { SVM_EXIT_IDTR_WRITE,  "write_idtr" }, \
-   { SVM_EXIT_GDTR_WRITE,  "write_gdtr" }, \
-   { SVM_EXIT_LDTR_WRITE,  "write_ldtr" }, \
-   { SVM_EXIT_TR_WRITE,"write_rt" }, \
-   { SVM_EXIT_RDTSC,   "rdtsc" }, \
-   { SVM_EXIT_RDPMC,   "rdpmc" }, \
-   { SVM_EXIT_PUSHF,   "pushf" }, \
-   { SVM_EXIT_POPF,"popf" }, \
-   { SVM_EXIT_CPUID,   "cpuid" }, \
-   { SVM_EXIT_RSM, "rsm" }, \
-   { SVM_EXIT_IRET,"iret" }, \
-   { SVM_EXIT_SWINT,   "swint" }, \
-   { SVM_EXIT_INVD,"invd" }, \
-   { SVM_EXIT_PAUSE,   "pause" }, \
-   { SVM_EXIT_HLT, "hlt" }, \
-   { SVM_EXIT_INVLPG,  "invlpg" }, \
-   { SVM_EXIT_INVLPGA, "invl

[PATCH] KVM: Using macros instead of magic values

2020-07-21 Thread Haiwei Li 

From: Haiwei Li 

Instead of using magic values, use macros.

Signed-off-by: Haiwei Li 
---
 arch/x86/kvm/lapic.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 47801a4..d5fb2ea 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -2083,7 +2083,8 @@ int kvm_lapic_reg_write(struct kvm_lapic *apic, 
u32 reg, u32 val)


case APIC_SELF_IPI:
if (apic_x2apic_mode(apic)) {
-   kvm_lapic_reg_write(apic, APIC_ICR, 0x4 | (val & 
0xff));
+   kvm_lapic_reg_write(apic, APIC_ICR,
+   APIC_DEST_SELF | (val & 
APIC_VECTOR_MASK));
} else
ret = 1;
break;
--
1.8.3.1

[PATCH] KVM: Fix the indentation to match coding style

2020-05-17 Thread Haiwei Li 

From: Haiwei Li 

There is a bad indentation in next branch. The patch looks like 
fixes nothing though it fixes the indentation.


Before fixing:

if (!handle_fastpath_set_x2apic_icr_irqoff(vcpu, data)) {
kvm_skip_emulated_instruction(vcpu);
ret = EXIT_FASTPATH_EXIT_HANDLED;
   }
break;
case MSR_IA32_TSCDEADLINE:

After fixing:

if (!handle_fastpath_set_x2apic_icr_irqoff(vcpu, data)) {
kvm_skip_emulated_instruction(vcpu);
ret = EXIT_FASTPATH_EXIT_HANDLED;
}
break;
case MSR_IA32_TSCDEADLINE:


Signed-off-by: Haiwei Li 
---
 arch/x86/kvm/x86.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 471fccf..446f747 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1631,7 +1631,7 @@ fastpath_t handle_fastpath_set_msr_irqoff(struct 
kvm_vcpu *vcpu)

if (!handle_fastpath_set_x2apic_icr_irqoff(vcpu, data)) {
kvm_skip_emulated_instruction(vcpu);
ret = EXIT_FASTPATH_EXIT_HANDLED;
-   }
+   }
break;
case MSR_IA32_TSCDEADLINE:
data = kvm_read_edx_eax(vcpu);
--
1.8.3.1