| Message ID | 1610066796-17266-2-git-send-email-igor.druzhinin@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] viridian: remove implicit limit of 64 VPs per partition | expand |
> -----Original Message----- > From: Igor Druzhinin <igor.druzhinin@citrix.com> > Sent: 08 January 2021 00:47 > To: xen-devel@lists.xenproject.org > Cc: paul@xen.org; wl@xen.org; iwj@xenproject.org; anthony.perard@citrix.com; > andrew.cooper3@citrix.com; george.dunlap@citrix.com; jbeulich@suse.com; julien@xen.org; > sstabellini@kernel.org; roger.pau@citrix.com; Igor Druzhinin <igor.druzhinin@citrix.com> > Subject: [PATCH 2/2] viridian: allow vCPU hotplug for Windows VMs > > If Viridian extensions are enabled, Windows wouldn't currently allow > a hotplugged vCPU to be brought up dynamically. We need to expose a special > bit to let the guest know we allow it. It appears we can just start exposing > it without worrying too much about compatibility - see relevant QEMU > discussion here: > > https://patchwork.kernel.org/project/qemu-devel/patch/1455364815-19586-1-git-send-email- > den@openvz.org/ I don't think that discussion really confirmed it was safe... just that empirically it appeared to be so. I think we should err on the side of caution and have this behind a feature flag (but I'm happy for it to default to on). Paul > > Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com> > --- > xen/arch/x86/hvm/viridian/viridian.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c > index ae1ea86..76e8291 100644 > --- a/xen/arch/x86/hvm/viridian/viridian.c > +++ b/xen/arch/x86/hvm/viridian/viridian.c > @@ -76,6 +76,7 @@ typedef union _HV_CRASH_CTL_REG_CONTENTS > } HV_CRASH_CTL_REG_CONTENTS; > > /* Viridian CPUID leaf 3, Hypervisor Feature Indication */ > +#define CPUID3D_CPU_DYNAMIC_PARTITIONING (1 << 3) > #define CPUID3D_CRASH_MSRS (1 << 10) > #define CPUID3D_SINT_POLLING (1 << 17) > > @@ -179,8 +180,11 @@ void cpuid_viridian_leaves(const struct vcpu *v, uint32_t leaf, > res->a = u.lo; > res->b = u.hi; > > + /* Expose ability to bring up VPs dynamically - allows vCPU hotplug */ > + res->d = CPUID3D_CPU_DYNAMIC_PARTITIONING; > + > if ( viridian_feature_mask(d) & HVMPV_crash_ctl ) > - res->d = CPUID3D_CRASH_MSRS; > + res->d |= CPUID3D_CRASH_MSRS; > if ( viridian_feature_mask(d) & HVMPV_synic ) > res->d |= CPUID3D_SINT_POLLING; > > -- > 2.7.4
On 08/01/2021 08:38, Paul Durrant wrote: >> -----Original Message----- >> From: Igor Druzhinin <igor.druzhinin@citrix.com> >> Sent: 08 January 2021 00:47 >> To: xen-devel@lists.xenproject.org >> Cc: paul@xen.org; wl@xen.org; iwj@xenproject.org; anthony.perard@citrix.com; >> andrew.cooper3@citrix.com; george.dunlap@citrix.com; jbeulich@suse.com; julien@xen.org; >> sstabellini@kernel.org; roger.pau@citrix.com; Igor Druzhinin <igor.druzhinin@citrix.com> >> Subject: [PATCH 2/2] viridian: allow vCPU hotplug for Windows VMs >> >> If Viridian extensions are enabled, Windows wouldn't currently allow >> a hotplugged vCPU to be brought up dynamically. We need to expose a special >> bit to let the guest know we allow it. It appears we can just start exposing >> it without worrying too much about compatibility - see relevant QEMU >> discussion here: >> >> https://patchwork.kernel.org/project/qemu-devel/patch/1455364815-19586-1-git-send-email- >> den@openvz.org/ > > I don't think that discussion really confirmed it was safe... just that empirically it appeared to be so. I think we should err on > the side of caution and have this behind a feature flag (but I'm happy for it to default to on). QEMU was having this code since 2016 and nobody complained is good enough for me - but if you insist we need an option - ok, I will add one. Igor
> -----Original Message----- > From: Igor Druzhinin <igor.druzhinin@citrix.com> > Sent: 08 January 2021 11:30 > To: paul@xen.org; xen-devel@lists.xenproject.org > Cc: wl@xen.org; iwj@xenproject.org; anthony.perard@citrix.com; andrew.cooper3@citrix.com; > george.dunlap@citrix.com; jbeulich@suse.com; julien@xen.org; sstabellini@kernel.org; > roger.pau@citrix.com > Subject: Re: [PATCH 2/2] viridian: allow vCPU hotplug for Windows VMs > > On 08/01/2021 08:38, Paul Durrant wrote: > >> -----Original Message----- > >> From: Igor Druzhinin <igor.druzhinin@citrix.com> > >> Sent: 08 January 2021 00:47 > >> To: xen-devel@lists.xenproject.org > >> Cc: paul@xen.org; wl@xen.org; iwj@xenproject.org; anthony.perard@citrix.com; > >> andrew.cooper3@citrix.com; george.dunlap@citrix.com; jbeulich@suse.com; julien@xen.org; > >> sstabellini@kernel.org; roger.pau@citrix.com; Igor Druzhinin <igor.druzhinin@citrix.com> > >> Subject: [PATCH 2/2] viridian: allow vCPU hotplug for Windows VMs > >> > >> If Viridian extensions are enabled, Windows wouldn't currently allow > >> a hotplugged vCPU to be brought up dynamically. We need to expose a special > >> bit to let the guest know we allow it. It appears we can just start exposing > >> it without worrying too much about compatibility - see relevant QEMU > >> discussion here: > >> > >> https://patchwork.kernel.org/project/qemu-devel/patch/1455364815-19586-1-git-send-email- > >> den@openvz.org/ > > > > I don't think that discussion really confirmed it was safe... just that empirically it appeared to > be so. I think we should err on > > the side of caution and have this behind a feature flag (but I'm happy for it to default to on). > > QEMU was having this code since 2016 and nobody complained is good > enough for me - but if you insist we need an option - ok, I will add one. > Given that it has not yet been in a release, perhaps you could just guard this and the implementation of leaf 0x40000005 using HVMPV_ex_processor_masks? Paul > Igor
On 08/01/2021 11:33, Paul Durrant wrote: >> -----Original Message----- >> From: Igor Druzhinin <igor.druzhinin@citrix.com> >> Sent: 08 January 2021 11:30 >> To: paul@xen.org; xen-devel@lists.xenproject.org >> Cc: wl@xen.org; iwj@xenproject.org; anthony.perard@citrix.com; andrew.cooper3@citrix.com; >> george.dunlap@citrix.com; jbeulich@suse.com; julien@xen.org; sstabellini@kernel.org; >> roger.pau@citrix.com >> Subject: Re: [PATCH 2/2] viridian: allow vCPU hotplug for Windows VMs >> >> On 08/01/2021 08:38, Paul Durrant wrote: >>>> -----Original Message----- >>>> From: Igor Druzhinin <igor.druzhinin@citrix.com> >>>> Sent: 08 January 2021 00:47 >>>> To: xen-devel@lists.xenproject.org >>>> Cc: paul@xen.org; wl@xen.org; iwj@xenproject.org; anthony.perard@citrix.com; >>>> andrew.cooper3@citrix.com; george.dunlap@citrix.com; jbeulich@suse.com; julien@xen.org; >>>> sstabellini@kernel.org; roger.pau@citrix.com; Igor Druzhinin <igor.druzhinin@citrix.com> >>>> Subject: [PATCH 2/2] viridian: allow vCPU hotplug for Windows VMs >>>> >>>> If Viridian extensions are enabled, Windows wouldn't currently allow >>>> a hotplugged vCPU to be brought up dynamically. We need to expose a special >>>> bit to let the guest know we allow it. It appears we can just start exposing >>>> it without worrying too much about compatibility - see relevant QEMU >>>> discussion here: >>>> >>>> https://patchwork.kernel.org/project/qemu-devel/patch/1455364815-19586-1-git-send-email- >>>> den@openvz.org/ >>> >>> I don't think that discussion really confirmed it was safe... just that empirically it appeared to >> be so. I think we should err on >>> the side of caution and have this behind a feature flag (but I'm happy for it to default to on). >> >> QEMU was having this code since 2016 and nobody complained is good >> enough for me - but if you insist we need an option - ok, I will add one. >> > > Given that it has not yet been in a release, perhaps you could just guard this and the implementation of leaf 0x40000005 using HVMPV_ex_processor_masks? That looks sloppy and confusing to me - let's have a separate option instead. Igor
> -----Original Message----- > From: Igor Druzhinin <igor.druzhinin@citrix.com> > Sent: 08 January 2021 11:36 > To: paul@xen.org; xen-devel@lists.xenproject.org > Cc: wl@xen.org; iwj@xenproject.org; anthony.perard@citrix.com; andrew.cooper3@citrix.com; > george.dunlap@citrix.com; jbeulich@suse.com; julien@xen.org; sstabellini@kernel.org; > roger.pau@citrix.com > Subject: Re: [PATCH 2/2] viridian: allow vCPU hotplug for Windows VMs > > On 08/01/2021 11:33, Paul Durrant wrote: > >> -----Original Message----- > >> From: Igor Druzhinin <igor.druzhinin@citrix.com> > >> Sent: 08 January 2021 11:30 > >> To: paul@xen.org; xen-devel@lists.xenproject.org > >> Cc: wl@xen.org; iwj@xenproject.org; anthony.perard@citrix.com; andrew.cooper3@citrix.com; > >> george.dunlap@citrix.com; jbeulich@suse.com; julien@xen.org; sstabellini@kernel.org; > >> roger.pau@citrix.com > >> Subject: Re: [PATCH 2/2] viridian: allow vCPU hotplug for Windows VMs > >> > >> On 08/01/2021 08:38, Paul Durrant wrote: > >>>> -----Original Message----- > >>>> From: Igor Druzhinin <igor.druzhinin@citrix.com> > >>>> Sent: 08 January 2021 00:47 > >>>> To: xen-devel@lists.xenproject.org > >>>> Cc: paul@xen.org; wl@xen.org; iwj@xenproject.org; anthony.perard@citrix.com; > >>>> andrew.cooper3@citrix.com; george.dunlap@citrix.com; jbeulich@suse.com; julien@xen.org; > >>>> sstabellini@kernel.org; roger.pau@citrix.com; Igor Druzhinin <igor.druzhinin@citrix.com> > >>>> Subject: [PATCH 2/2] viridian: allow vCPU hotplug for Windows VMs > >>>> > >>>> If Viridian extensions are enabled, Windows wouldn't currently allow > >>>> a hotplugged vCPU to be brought up dynamically. We need to expose a special > >>>> bit to let the guest know we allow it. It appears we can just start exposing > >>>> it without worrying too much about compatibility - see relevant QEMU > >>>> discussion here: > >>>> > >>>> https://patchwork.kernel.org/project/qemu-devel/patch/1455364815-19586-1-git-send-email- > >>>> den@openvz.org/ > >>> > >>> I don't think that discussion really confirmed it was safe... just that empirically it appeared to > >> be so. I think we should err on > >>> the side of caution and have this behind a feature flag (but I'm happy for it to default to on). > >> > >> QEMU was having this code since 2016 and nobody complained is good > >> enough for me - but if you insist we need an option - ok, I will add one. > >> > > > > Given that it has not yet been in a release, perhaps you could just guard this and the > implementation of leaf 0x40000005 using HVMPV_ex_processor_masks? > > That looks sloppy and confusing to me - let's have a separate option instead. > Yes, for this I guess it is really a separate thing. Using HVMPV_ex_processor_masks to control the presence of leaf 0x40000005 seems reasonable (since it's all about being able to use >64 vcpus). Perhaps a new HVMPV_cpu_hotplug for this one? Paul > Igor
On 08/01/2021 11:40, Paul Durrant wrote: >> -----Original Message----- >> From: Igor Druzhinin <igor.druzhinin@citrix.com> >> Sent: 08 January 2021 11:36 >> To: paul@xen.org; xen-devel@lists.xenproject.org >> Cc: wl@xen.org; iwj@xenproject.org; anthony.perard@citrix.com; andrew.cooper3@citrix.com; >> george.dunlap@citrix.com; jbeulich@suse.com; julien@xen.org; sstabellini@kernel.org; >> roger.pau@citrix.com >> Subject: Re: [PATCH 2/2] viridian: allow vCPU hotplug for Windows VMs >> >> On 08/01/2021 11:33, Paul Durrant wrote: >>>> -----Original Message----- >>>> From: Igor Druzhinin <igor.druzhinin@citrix.com> >>>> Sent: 08 January 2021 11:30 >>>> To: paul@xen.org; xen-devel@lists.xenproject.org >>>> Cc: wl@xen.org; iwj@xenproject.org; anthony.perard@citrix.com; andrew.cooper3@citrix.com; >>>> george.dunlap@citrix.com; jbeulich@suse.com; julien@xen.org; sstabellini@kernel.org; >>>> roger.pau@citrix.com >>>> Subject: Re: [PATCH 2/2] viridian: allow vCPU hotplug for Windows VMs >>>> >>>> On 08/01/2021 08:38, Paul Durrant wrote: >>>>>> -----Original Message----- >>>>>> From: Igor Druzhinin <igor.druzhinin@citrix.com> >>>>>> Sent: 08 January 2021 00:47 >>>>>> To: xen-devel@lists.xenproject.org >>>>>> Cc: paul@xen.org; wl@xen.org; iwj@xenproject.org; anthony.perard@citrix.com; >>>>>> andrew.cooper3@citrix.com; george.dunlap@citrix.com; jbeulich@suse.com; julien@xen.org; >>>>>> sstabellini@kernel.org; roger.pau@citrix.com; Igor Druzhinin <igor.druzhinin@citrix.com> >>>>>> Subject: [PATCH 2/2] viridian: allow vCPU hotplug for Windows VMs >>>>>> >>>>>> If Viridian extensions are enabled, Windows wouldn't currently allow >>>>>> a hotplugged vCPU to be brought up dynamically. We need to expose a special >>>>>> bit to let the guest know we allow it. It appears we can just start exposing >>>>>> it without worrying too much about compatibility - see relevant QEMU >>>>>> discussion here: >>>>>> >>>>>> https://patchwork.kernel.org/project/qemu-devel/patch/1455364815-19586-1-git-send-email- >>>>>> den@openvz.org/ >>>>> >>>>> I don't think that discussion really confirmed it was safe... just that empirically it appeared to >>>> be so. I think we should err on >>>>> the side of caution and have this behind a feature flag (but I'm happy for it to default to on). >>>> >>>> QEMU was having this code since 2016 and nobody complained is good >>>> enough for me - but if you insist we need an option - ok, I will add one. >>>> >>> >>> Given that it has not yet been in a release, perhaps you could just guard this and the >> implementation of leaf 0x40000005 using HVMPV_ex_processor_masks? >> >> That looks sloppy and confusing to me - let's have a separate option instead. >> > > Yes, for this I guess it is really a separate thing. Using HVMPV_ex_processor_masks to control the presence of leaf 0x40000005 seems reasonable (since it's all about being able to use >64 vcpus). Perhaps a new HVMPV_cpu_hotplug for this one? Agree. Igor
diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c index ae1ea86..76e8291 100644 --- a/xen/arch/x86/hvm/viridian/viridian.c +++ b/xen/arch/x86/hvm/viridian/viridian.c @@ -76,6 +76,7 @@ typedef union _HV_CRASH_CTL_REG_CONTENTS } HV_CRASH_CTL_REG_CONTENTS; /* Viridian CPUID leaf 3, Hypervisor Feature Indication */ +#define CPUID3D_CPU_DYNAMIC_PARTITIONING (1 << 3) #define CPUID3D_CRASH_MSRS (1 << 10) #define CPUID3D_SINT_POLLING (1 << 17) @@ -179,8 +180,11 @@ void cpuid_viridian_leaves(const struct vcpu *v, uint32_t leaf, res->a = u.lo; res->b = u.hi; + /* Expose ability to bring up VPs dynamically - allows vCPU hotplug */ + res->d = CPUID3D_CPU_DYNAMIC_PARTITIONING; + if ( viridian_feature_mask(d) & HVMPV_crash_ctl ) - res->d = CPUID3D_CRASH_MSRS; + res->d |= CPUID3D_CRASH_MSRS; if ( viridian_feature_mask(d) & HVMPV_synic ) res->d |= CPUID3D_SINT_POLLING;
If Viridian extensions are enabled, Windows wouldn't currently allow a hotplugged vCPU to be brought up dynamically. We need to expose a special bit to let the guest know we allow it. It appears we can just start exposing it without worrying too much about compatibility - see relevant QEMU discussion here: https://patchwork.kernel.org/project/qemu-devel/patch/1455364815-19586-1-git-send-email-den@openvz.org/ Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com> --- xen/arch/x86/hvm/viridian/viridian.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)