Re: [libvirt] QEMU -M nvdimm=on and hotplug

2017-09-13 Thread Haozhong Zhang
On 09/13/17 17:28 +0200, Michal Privoznik wrote:
> 
> BTW: I ran a migration from no nvdimm qemu to one that had -M nvdimm=on
> and guest migrated happily. So looks like guest ABI is stable (or at
> least stable enough not to crash). But since ACPI table is changed I
> doubt that.

One example that may cause trouble is that
1/ Guest OS got a pointer to an ACPI table A on the source host (w/o nvdimm=on)
2/ After migrating to the destination host (w/ nvdimm=on), the
   location of ACPI table A is occupied by NFIT. If guest OS tries to
   access ACPI table A via the pointer in step 1/, then it will access
   the wrong table.

> 
> Also, I found that hotunplug is not implemented yet?

So far there is no public specification defining NVDIMM hotunplug, so
we didn't implement it.


Haozhong


> 
> error: internal error: unable to execute QEMU command 'device_del':
> nvdimm device hot unplug is not supported yet.
> 
> Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v2 3/3] KVM: VMX: enable guest access to LMCE related MSRs

2016-06-19 Thread Haozhong Zhang
On 06/17/16 14:15, Eduardo Habkost wrote:
> On Fri, Jun 17, 2016 at 09:11:16AM +0800, Haozhong Zhang wrote:
> > On 06/16/16 11:55, Eduardo Habkost wrote:
> > > On Thu, Jun 16, 2016 at 12:04:50PM +0200, Paolo Bonzini wrote:
> > > > On 16/06/2016 08:05, Haozhong Zhang wrote:
> > > > > From: Ashok Raj 
> > > > > 
> > > > > On Intel platforms, this patch adds LMCE to KVM MCE supported
> > > > > capabilities and handles guest access to LMCE related MSRs.
> > > > > 
> > > > > Signed-off-by: Ashok Raj 
> > > > > [Haozhong: macro KVM_MCE_CAP_SUPPORTED => variable 
> > > > > kvm_mce_cap_supported
> > > > >Only enable LMCE on Intel platform
> > > > >  Check MSR_IA32_FEATURE_CONTROL when handling guest
> > > > >access to MSR_IA32_MCG_EXT_CTL]
> > > > > Signed-off-by: Haozhong Zhang 
> > > [...]
> > > > > @@ -6433,6 +6455,8 @@ static __init int hardware_setup(void)
> > > > >  
> > > > >   kvm_set_posted_intr_wakeup_handler(wakeup_handler);
> > > > >  
> > > > > + kvm_mce_cap_supported |= MCG_LMCE_P;
> > > > 
> > > > Ah, so virtual LMCE is available on all processors!  This is
> > > > interesting, but it also makes it more complicated to handle in QEMU; a
> > > > new QEMU generally doesn't require a new kernel.
> > > > 
> > > > Eduardo, any ideas?
> > > 
> > > (CCing libvirt list)
> > > 
> > > As we shouldn't make machine-type changes introduce new host
> > > requirements, it looks like we need to either add a new set of
> > > CPU models (unreasonable), or expect management software to
> > > explicitly enable LMCE after ensuring the host supports it.
> > >
> > > Or we could wait for a reasonable time after the feature is
> > > available in the kernel, and declare that QEMU as a whole
> > > requires a newer kernel. But how much time would be reasonable
> > > for that?
> > > 
> > > Long term, I believe we should think of a better solution. I
> > > don't think it is reasonable to require new libvirt code to be
> > > written for every single low-level feature that requires a newer
> > > kernel or newer host hardware. Maybe new introspection interfaces
> > > that would allow us to drop the "no new requirements on
> > > machine-type changes" rule?
> > >
> > 
> > Because new MSR (MSR_IA32_MCG_EXT_CTL) and new MSR bit
> > (FEATURE_CONTROL_LMCE in MSR_IA32_FEATURE_CONTROL) are introduced by
> > LMCE, QEMU requires new KVM which can handle those changes.
> 
> If I understood correctly, you are describing the second option
> above (declaring that QEMU as a whole requires a newer kernel).
> 
> > 
> > I'm not familiar with libvirt. Does the requirement of new KVM
> > capability bring any troubles to libvirt?
>
> It does, assuming that we still support running QEMU under an
> older kernel where KVM doesn't LMCE. In this case, the pc-2.6
> machine-type will run, but the pc-2.7 machine-type won't.
>
> The requirement of new KVM capabilities based on the machine-type
> is a problem for livirt. libvirt have some host-capabilities APIs
> to allow software to check if the VM can be run on (or migrated
> to) a host, but none of them are based on machine-type.
> 
> This is not necessarily specific to libvirt: people may have
> their own configuration or scripts that use the default "pc"
> alias, and a QEMU upgrade shouldn't break their configuration.
>

Thanks for the explanation!

If we disable LMCE in QEMU by default (even for -cpu host), will it
still be a problem? That is,

- pc-2.7 can continue to run on old kernels unless users explicitly
  require LMCE

- existing libvirt VM configurations can continue to work on pc-2.7
  because LMCE is not specified in those configurations and are
  disabled by default (i.e. no requirement for new kernels)

- existing QEMU configurations/scripts using pc alias can continue to
  work on pc-27 for the same reason above.

Thanks,
Haozhong

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v2 3/3] KVM: VMX: enable guest access to LMCE related MSRs

2016-06-16 Thread Haozhong Zhang
On 06/16/16 11:55, Eduardo Habkost wrote:
> On Thu, Jun 16, 2016 at 12:04:50PM +0200, Paolo Bonzini wrote:
> > On 16/06/2016 08:05, Haozhong Zhang wrote:
> > > From: Ashok Raj 
> > > 
> > > On Intel platforms, this patch adds LMCE to KVM MCE supported
> > > capabilities and handles guest access to LMCE related MSRs.
> > > 
> > > Signed-off-by: Ashok Raj 
> > > [Haozhong: macro KVM_MCE_CAP_SUPPORTED => variable kvm_mce_cap_supported
> > >Only enable LMCE on Intel platform
> > >  Check MSR_IA32_FEATURE_CONTROL when handling guest
> > >access to MSR_IA32_MCG_EXT_CTL]
> > > Signed-off-by: Haozhong Zhang 
> [...]
> > > @@ -6433,6 +6455,8 @@ static __init int hardware_setup(void)
> > >  
> > >   kvm_set_posted_intr_wakeup_handler(wakeup_handler);
> > >  
> > > + kvm_mce_cap_supported |= MCG_LMCE_P;
> > 
> > Ah, so virtual LMCE is available on all processors!  This is
> > interesting, but it also makes it more complicated to handle in QEMU; a
> > new QEMU generally doesn't require a new kernel.
> > 
> > Eduardo, any ideas?
> 
> (CCing libvirt list)
> 
> As we shouldn't make machine-type changes introduce new host
> requirements, it looks like we need to either add a new set of
> CPU models (unreasonable), or expect management software to
> explicitly enable LMCE after ensuring the host supports it.
>
> Or we could wait for a reasonable time after the feature is
> available in the kernel, and declare that QEMU as a whole
> requires a newer kernel. But how much time would be reasonable
> for that?
> 
> Long term, I believe we should think of a better solution. I
> don't think it is reasonable to require new libvirt code to be
> written for every single low-level feature that requires a newer
> kernel or newer host hardware. Maybe new introspection interfaces
> that would allow us to drop the "no new requirements on
> machine-type changes" rule?
>

Because new MSR (MSR_IA32_MCG_EXT_CTL) and new MSR bit
(FEATURE_CONTROL_LMCE in MSR_IA32_FEATURE_CONTROL) are introduced by
LMCE, QEMU requires new KVM which can handle those changes.

I'm not familiar with libvirt. Does the requirement of new KVM
capability bring any troubles to libvirt?

Thanks,
Haozhong

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 3/3] kvm-all: notice KVM of vcpu's TSC rate after migration

2015-10-05 Thread Haozhong Zhang
On Wed, Sep 30, 2015 at 05:36:11PM -0300, Eduardo Habkost wrote:
> On Wed, Sep 30, 2015 at 08:32:26AM +0800, Haozhong Zhang wrote:
> > > [...]
> > > > > Or maybe we shouldn't treat this as VM state, but as configuration, 
> > > > > and
> > > > > let management configure the TSC frequency explicitly if the user 
> > > > > really
> > > > > needs it to stay the same during migration.
> > > > >
> > > > > (CCing libvir-list to see if they have feedback)
> > > > >
> > > > 
> > > > Thanks for CC. I'll consider to add a command line option to control
> > > > the configuration of guest TSC fequency.
> > > 
> > > It already exists, -cpu has a "tsc-freq" option.
> > >
> > 
> > What I'm considering is to add a "-keep-tsc-freq" option to control
> > whether the TSC frequency should be migrated if "tsc-freq" is not
> > presented. Compared to "tsc-freq", "-keep-tsc-freq" can free users
> > from figuring out the guest TSC frequency manually in the migration.
> 
> If you do that, please make it a property of the CPU object, so it will
> can be set as a "-cpu" option.
>

Yes, I'll do so.

- Haozhong

> -- 
> Eduardo
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 3/3] kvm-all: notice KVM of vcpu's TSC rate after migration

2015-09-29 Thread Haozhong Zhang
On Tue, Sep 29, 2015 at 03:02:07PM -0300, Eduardo Habkost wrote:
> On Tue, Sep 29, 2015 at 11:43:34AM +0800, Haozhong Zhang wrote:
> > On Mon, Sep 28, 2015 at 01:37:34PM -0300, Eduardo Habkost wrote:
> > > On Mon, Sep 28, 2015 at 01:38:31PM +0800, Haozhong Zhang wrote:
> [...]
> > > >  static void do_kvm_cpu_synchronize_post_init(void *arg)
> > > >  {
> > > >  CPUState *cpu = arg;
> > > > +CPUX86State *env = &X86_CPU(cpu)->env;
> > > > +int r;
> > > > +
> > > > +/*
> > > > + * XXX: KVM_SET_TSC_KHZ must be done before 
> > > > kvm_arch_put_registers().
> > > 
> > > Could you explain where this requirement comes from?
> > >
> > 
> > kvm_arch_put_registers() below will setup vcpu's MSR_IA32_TSC through
> > KVM ioctl KVM_SET_MSRS. KVM needs to know vcpu's TSC rate so as to
> > correctly scale the TSC value given by QEMU, especially when vcpu's
> > TSC rate is different than the host TSC rate (e.g. it's migrated from
> > another machine w/ different host TSC rate than the current one).
> 
> Thanks. The comment above could contain a short version of this
> explanation, e.g.: "KVM needs KVM_SET_TSC_KHZ to be done before
> KVM_SET_MSRS sets MSR_IA32_TSC (done by kvm_arch_put_registers())".
>

will include this in the comment

> > 
> [...]
> > > Or maybe we shouldn't treat this as VM state, but as configuration, and
> > > let management configure the TSC frequency explicitly if the user really
> > > needs it to stay the same during migration.
> > >
> > > (CCing libvir-list to see if they have feedback)
> > >
> > 
> > Thanks for CC. I'll consider to add a command line option to control
> > the configuration of guest TSC fequency.
> 
> It already exists, -cpu has a "tsc-freq" option.
>

What I'm considering is to add a "-keep-tsc-freq" option to control
whether the TSC frequency should be migrated if "tsc-freq" is not
presented. Compared to "tsc-freq", "-keep-tsc-freq" can free users
from figuring out the guest TSC frequency manually in the migration.

- Haozhong

> -- 
> Eduardo
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 3/3] kvm-all: notice KVM of vcpu's TSC rate after migration

2015-09-28 Thread Haozhong Zhang
On Mon, Sep 28, 2015 at 01:37:34PM -0300, Eduardo Habkost wrote:
> On Mon, Sep 28, 2015 at 01:38:31PM +0800, Haozhong Zhang wrote:
> > When a vcpu is created in KVM, its TSC rate is initially identical to
> > the host TSC rate. If its state is migrated to a vcpu on another
> > machine (target machine) which may uses a different host TSC rate, QEMU
> > on the target machine should notice KVM of the migrated vcpu's TSC
> > rate. In case that KVM on the target machine supports TSC scaling, guest
> > programs running on the migrated vcpu will observe the same TSC rate
> > before and after the migration.
> > 
> > Signed-off-by: Haozhong Zhang 
> > ---
> >  kvm-all.c | 13 +
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/kvm-all.c b/kvm-all.c
> > index 0be4615..e8de038 100644
> > --- a/kvm-all.c
> > +++ b/kvm-all.c
> > @@ -1769,6 +1769,19 @@ void kvm_cpu_synchronize_post_reset(CPUState *cpu)
> >  static void do_kvm_cpu_synchronize_post_init(void *arg)
> >  {
> >  CPUState *cpu = arg;
> > +CPUX86State *env = &X86_CPU(cpu)->env;
> > +int r;
> > +
> > +/*
> > + * XXX: KVM_SET_TSC_KHZ must be done before kvm_arch_put_registers().
> 
> Could you explain where this requirement comes from?
>

kvm_arch_put_registers() below will setup vcpu's MSR_IA32_TSC through
KVM ioctl KVM_SET_MSRS. KVM needs to know vcpu's TSC rate so as to
correctly scale the TSC value given by QEMU, especially when vcpu's
TSC rate is different than the host TSC rate (e.g. it's migrated from
another machine w/ different host TSC rate than the current one).

> > + */
> > +r = kvm_check_extension(cpu->kvm_state, KVM_CAP_TSC_CONTROL);
> > +if (r && env->tsc_khz) {
> > +r = kvm_vcpu_ioctl(cpu, KVM_SET_TSC_KHZ, env->tsc_khz);
> > +if (r < 0) {
> > +fprintf(stderr, "KVM_SET_TSC_KHZ failed\n");
> > +}
> > +}
> 
> This is duplicating the existing KVM_SET_TSC_KHZ call at
> kvm_arch_init_vcpu(). I wonder if there's a way to avoid this
> duplication. Should we set TSC KHz only at
> do_kvm_cpu_synchronize_post_init(), and remove the call from
> kvm_arch_init_vcpu()?
>

I'll check if it's safe to remove the call from kvm_arch_init_vcpu().

> Or maybe we shouldn't treat this as VM state, but as configuration, and
> let management configure the TSC frequency explicitly if the user really
> needs it to stay the same during migration.
>
> (CCing libvir-list to see if they have feedback)
>

Thanks for CC. I'll consider to add a command line option to control
the configuration of guest TSC fequency.

> -- 
> Eduardo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list