Revert "xen/domctl: Introduce XEN_DOMCTL_CDF_vpci flag"

Message ID	20211014084718.21733-1-michal.orzel@arm.com (mailing list archive)
State	New, archived
Headers	show Return-Path: <SRS0=FOA3=PC=lists.xenproject.org=xen-devel-bounces@kernel.org> DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org E2208610D2 Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" <xen-devel-bounces@lists.xenproject.org> From: Michal Orzel <michal.orzel@arm.com> To: xen-devel@lists.xenproject.org Cc: Christian Lindig <christian.lindig@citrix.com>, David Scott <dave@recoil.org>, Ian Jackson <iwj@xenproject.org>, Wei Liu <wl@xen.org>, Andrew Cooper <andrew.cooper3@citrix.com>, George Dunlap <george.dunlap@citrix.com>, Jan Beulich <jbeulich@suse.com>, Julien Grall <julien@xen.org>, Stefano Stabellini <sstabellini@kernel.org>, Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>, =?utf-8?q?Roger_Pau_Monn?= =?utf-8?q?=C3=A9?= <roger.pau@citrix.com>, bertrand.marquis@arm.com Subject: [PATCH] Revert "xen/domctl: Introduce XEN_DOMCTL_CDF_vpci flag" Date: Thu, 14 Oct 2021 10:47:18 +0200 Message-Id: <20211014084718.21733-1-michal.orzel@arm.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit
Series	Revert "xen/domctl: Introduce XEN_DOMCTL_CDF_vpci flag" \| expand Revert "xen/domctl: Introduce XEN_DOMCTL_CDF_vpci flag"

Michal Orzel Oct. 14, 2021, 8:47 a.m. UTC

This reverts commit 2075b410ee8087662c880213c3aca196fb7ade22.

During the discussion [1] that took place after
the patch was merged it was agreed that it should
be reverted to avoid introducing a bad interface.

Furthermore, the patch rejected usage of flag
XEN_DOMCTL_CDF_vpci for x86 which is not true
as it should be set for dom0 PVH.

Due to XEN_DOMCTL_CDF_vpmu being introduced after
XEN_DOMCTL_CDF_vpci, modify its bit position
from 8 to 7.

[1] https://marc.info/?t=163354215300039&r=1&w=2

Signed-off-by: Michal Orzel <michal.orzel@arm.com>
---
 tools/ocaml/libs/xc/xenctrl.ml  | 1 -
 tools/ocaml/libs/xc/xenctrl.mli | 1 -
 xen/arch/arm/domain.c           | 3 +--
 xen/arch/x86/domain.c           | 6 ------
 xen/common/domain.c             | 3 +--
 xen/include/public/domctl.h     | 3 +--
 6 files changed, 3 insertions(+), 14 deletions(-)

Jan Beulich Oct. 14, 2021, 8:53 a.m. UTC | #1

On 14.10.2021 10:47, Michal Orzel wrote:
> This reverts commit 2075b410ee8087662c880213c3aca196fb7ade22.
> 
> During the discussion [1] that took place after
> the patch was merged it was agreed that it should
> be reverted to avoid introducing a bad interface.
> 
> Furthermore, the patch rejected usage of flag
> XEN_DOMCTL_CDF_vpci for x86 which is not true
> as it should be set for dom0 PVH.
> 
> Due to XEN_DOMCTL_CDF_vpmu being introduced after
> XEN_DOMCTL_CDF_vpci, modify its bit position
> from 8 to 7.
> 
> [1] https://marc.info/?t=163354215300039&r=1&w=2
> 
> Signed-off-by: Michal Orzel <michal.orzel@arm.com>
> ---
>  tools/ocaml/libs/xc/xenctrl.ml  | 1 -
>  tools/ocaml/libs/xc/xenctrl.mli | 1 -
>  xen/arch/arm/domain.c           | 3 +--
>  xen/arch/x86/domain.c           | 6 ------
>  xen/common/domain.c             | 3 +--
>  xen/include/public/domctl.h     | 3 +--
>  6 files changed, 3 insertions(+), 14 deletions(-)

Applicable parts
Acked-by: Jan Beulich <jbeulich@suse.com>

Jan

Bertrand Marquis Oct. 14, 2021, 8:54 a.m. UTC | #2

Hi Michal,

> On 14 Oct 2021, at 09:47, Michal Orzel <Michal.Orzel@arm.com> wrote:
> 
> This reverts commit 2075b410ee8087662c880213c3aca196fb7ade22.
> 
> During the discussion [1] that took place after
> the patch was merged it was agreed that it should
> be reverted to avoid introducing a bad interface.
> 
> Furthermore, the patch rejected usage of flag
> XEN_DOMCTL_CDF_vpci for x86 which is not true
> as it should be set for dom0 PVH.
> 
> Due to XEN_DOMCTL_CDF_vpmu being introduced after
> XEN_DOMCTL_CDF_vpci, modify its bit position
> from 8 to 7.
> 
> [1] https://marc.info/?t=163354215300039&r=1&w=2
> 
> Signed-off-by: Michal Orzel <michal.orzel@arm.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand

> ---
> tools/ocaml/libs/xc/xenctrl.ml  | 1 -
> tools/ocaml/libs/xc/xenctrl.mli | 1 -
> xen/arch/arm/domain.c           | 3 +--
> xen/arch/x86/domain.c           | 6 ------
> xen/common/domain.c             | 3 +--
> xen/include/public/domctl.h     | 3 +--
> 6 files changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
> index 86758babb3..addcf4cc59 100644
> --- a/tools/ocaml/libs/xc/xenctrl.ml
> +++ b/tools/ocaml/libs/xc/xenctrl.ml
> @@ -69,7 +69,6 @@ type domain_create_flag =
> 	| CDF_XS_DOMAIN
> 	| CDF_IOMMU
> 	| CDF_NESTED_VIRT
> -	| CDF_VPCI
> 	| CDF_VPMU
> 
> type domain_create_iommu_opts =
> diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
> index 0fdb0cc169..0a5ce529e9 100644
> --- a/tools/ocaml/libs/xc/xenctrl.mli
> +++ b/tools/ocaml/libs/xc/xenctrl.mli
> @@ -62,7 +62,6 @@ type domain_create_flag =
>   | CDF_XS_DOMAIN
>   | CDF_IOMMU
>   | CDF_NESTED_VIRT
> -  | CDF_VPCI
>   | CDF_VPMU
> 
> type domain_create_iommu_opts =
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index ad21c9b950..eef0661beb 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -628,8 +628,7 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
> {
>     unsigned int max_vcpus;
>     unsigned int flags_required = (XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap);
> -    unsigned int flags_optional = (XEN_DOMCTL_CDF_iommu | XEN_DOMCTL_CDF_vpci |
> -                                   XEN_DOMCTL_CDF_vpmu);
> +    unsigned int flags_optional = (XEN_DOMCTL_CDF_iommu | XEN_DOMCTL_CDF_vpmu);
> 
>     if ( (config->flags & ~flags_optional) != flags_required )
>     {
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 79c2aa4636..ef1812dc14 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -662,12 +662,6 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
>         return -EINVAL;
>     }
> 
> -    if ( config->flags & XEN_DOMCTL_CDF_vpci )
> -    {
> -        dprintk(XENLOG_INFO, "vPCI cannot be enabled yet\n");
> -        return -EINVAL;
> -    }
> -
>     if ( config->vmtrace_size )
>     {
>         unsigned int size = config->vmtrace_size;
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 8543fb54fd..8b53c49d1e 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -486,8 +486,7 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
>          ~(XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap |
>            XEN_DOMCTL_CDF_s3_integrity | XEN_DOMCTL_CDF_oos_off |
>            XEN_DOMCTL_CDF_xs_domain | XEN_DOMCTL_CDF_iommu |
> -           XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpci |
> -           XEN_DOMCTL_CDF_vpmu) )
> +           XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu) )
>     {
>         dprintk(XENLOG_INFO, "Unknown CDF flags %#x\n", config->flags);
>         return -EINVAL;
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index a53cbd16f4..238384b5ae 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -70,9 +70,8 @@ struct xen_domctl_createdomain {
> #define XEN_DOMCTL_CDF_iommu          (1U<<_XEN_DOMCTL_CDF_iommu)
> #define _XEN_DOMCTL_CDF_nested_virt   6
> #define XEN_DOMCTL_CDF_nested_virt    (1U << _XEN_DOMCTL_CDF_nested_virt)
> -#define XEN_DOMCTL_CDF_vpci           (1U << 7)
> /* Should we expose the vPMU to the guest? */
> -#define XEN_DOMCTL_CDF_vpmu           (1U << 8)
> +#define XEN_DOMCTL_CDF_vpmu           (1U << 7)
> 
> /* Max XEN_DOMCTL_CDF_* constant.  Used for ABI checking. */
> #define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_vpmu
> -- 
> 2.29.0
> 
>

Julien Grall Oct. 14, 2021, 9:33 a.m. UTC | #3

Hi Michal,

On 14/10/2021 09:47, Michal Orzel wrote:
> This reverts commit 2075b410ee8087662c880213c3aca196fb7ade22.
> 
> During the discussion [1] that took place after
> the patch was merged it was agreed that it should
> be reverted to avoid introducing a bad interface.
> 
> Furthermore, the patch rejected usage of flag
> XEN_DOMCTL_CDF_vpci for x86 which is not true
> as it should be set for dom0 PVH.
> 
> Due to XEN_DOMCTL_CDF_vpmu being introduced after
> XEN_DOMCTL_CDF_vpci, modify its bit position
> from 8 to 7.
> 
> [1] https://marc.info/?t=163354215300039&r=1&w=2
> 
> Signed-off-by: Michal Orzel <michal.orzel@arm.com>

Acked-by: Julien Grall <jgrall@amazon.com>

Looking at the thread, we are only missing an ack for...

> ---
>   tools/ocaml/libs/xc/xenctrl.ml  | 1 - >   tools/ocaml/libs/xc/xenctrl.mli | 1 -

the OCAML part. I can commit it once this is done.

Cheers,

Christian Lindig Oct. 14, 2021, 9:42 a.m. UTC | #4

On 14 Oct 2021, at 10:33, Julien Grall <julien@xen.org<mailto:julien@xen.org>> wrote:

Looking at the thread, we are only missing an ack for...

---
 tools/ocaml/libs/xc/xenctrl.ml  | 1 - >   tools/ocaml/libs/xc/xenctrl.mli | 1 -

Acked-by: Christian Lindig <christian.lindig@citrix.com<mailto:christian.lindig@citrix.com>>

Julien Grall Oct. 14, 2021, 10:16 a.m. UTC | #5

On 14/10/2021 10:42, Christian Lindig wrote:
> 
> 
>> On 14 Oct 2021, at 10:33, Julien Grall <julien@xen.org 
>> <mailto:julien@xen.org>> wrote:
>>
>> Looking at the thread, we are only missing an ack for...
>>
>>> ---
>>>  tools/ocaml/libs/xc/xenctrl.ml  | 1 - > 
>>>   tools/ocaml/libs/xc/xenctrl.mli | 1 -
> 
> Acked-by: Christian Lindig <christian.lindig@citrix.com 
> <mailto:christian.lindig@citrix.com>>

Committed. Thank you!

Cheers,

Ian Jackson Oct. 14, 2021, 11:34 a.m. UTC | #6

Michal Orzel writes ("[PATCH] Revert "xen/domctl: Introduce XEN_DOMCTL_CDF_vpci flag""):
> This reverts commit 2075b410ee8087662c880213c3aca196fb7ade22.
> 
> During the discussion [1] that took place after
> the patch was merged it was agreed that it should
> be reverted to avoid introducing a bad interface.
> 
> Furthermore, the patch rejected usage of flag
> XEN_DOMCTL_CDF_vpci for x86 which is not true
> as it should be set for dom0 PVH.
> 
> Due to XEN_DOMCTL_CDF_vpmu being introduced after
> XEN_DOMCTL_CDF_vpci, modify its bit position
> from 8 to 7.
> 
> [1] https://marc.info/?t=163354215300039&r=1&w=2

FTAOD, I don't think this will be necessary, but preemptively,

Release-Acked-by: Ian Jackson <iwj@xenproject.org>

Revert "xen/domctl: Introduce XEN_DOMCTL_CDF_vpci flag"

Commit Message

Comments

Patch