Re: [Xen-devel] [PATCH 05/34] xen: is_hvm_domain should evaluate to 0 when !CONFIG_HVM

2018-08-22 Thread Wei Liu
On Wed, Aug 22, 2018 at 04:11:45PM +0100, Julien Grall wrote:
> Hi Wei,
> 
> On 21/08/18 21:08, Wei Liu wrote:
> > On Tue, Aug 21, 2018 at 11:59:26AM -0700, Stefano Stabellini wrote:
> > > On Tue, 21 Aug 2018, Julien Grall wrote:
> > > > On 08/21/2018 07:49 PM, Wei Liu wrote:
> > > > > On Tue, Aug 21, 2018 at 07:33:56PM +0100, Julien Grall wrote:
> > > > > > Hi Wei,
> > > > > > 
> > > > > > On 08/21/2018 05:31 PM, Wei Liu wrote:
> > > > > > > On Mon, Aug 20, 2018 at 05:51:28AM -0600, Jan Beulich wrote:
> > > > > > > > > > > On 17.08.18 at 17:12,  wrote:
> > > > > > > > > Since it is defined in common header file, introduce 
> > > > > > > > > CONFIG_HVM to
> > > > > > > > > Arm to avoid breakage.
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Wei Liu 
> > > > > > > > > ---
> > > > > > > > > xen/arch/arm/Kconfig| 3 +++
> > > > > > > > > xen/include/xen/sched.h | 6 ++
> > > > > > > > > 2 files changed, 9 insertions(+)
> > > > > > > > > ,
> > > > > > > > > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > > > > > > > > index 586bc62..c0e969e 100644
> > > > > > > > > --- a/xen/arch/arm/Kconfig
> > > > > > > > > +++ b/xen/arch/arm/Kconfig
> > > > > > > > > @@ -52,6 +52,9 @@ config HAS_ITS
> > > > > > > > > prompt "GICv3 ITS MSI controller support" if 
> > > > > > > > > EXPERT = "y"
> > > > > > > > > depends on GICV3 && !NEW_VGIC
> > > > > > > > > +config HVM
> > > > > > > > > +def_bool y
> > > > > > > > 
> > > > > > > > I'm not convinced this is a good idea, but I'll let the ARM
> > > > > > > > maintainers
> > > > > > > > judge.
> > > > > > > 
> > > > > > > Andrew discovered that hvm flag is not set by toolstack so ARM 
> > > > > > > guests
> > > > > > > are PV guests to Xen. I think the addition here can be omitted.
> > > > > > > 
> > > > > > > However I would still like to hear from ARM maintainers what 
> > > > > > > guest type
> > > > > > > should be set for ARM, because sooner or later I will need to 
> > > > > > > change PV
> > > > > > > code as well.
> > > > > > > 
> > > > > > > Grepping for is_{hvm,pv}_* in arch/arm yields no result, but then 
> > > > > > > there
> > > > > > > is common code that we need to take care of.
> > > > > > 
> > > > > > Using PV was more a convenience at the time because was not there. 
> > > > > > The
> > > > > > plan
> > > > > > is to switch to PVH (see RFC [1]).
> > > > > > 
> > > > > > I will try to find some times this week to rework the patch based 
> > > > > > on the
> > > > > > comments.
> > > > > > 
> > > > > > Cheers,
> > > > > > 
> > > > > > [1] 
> > > > > > https://lists.xen.org/archives/html/xen-devel/2018-06/msg01537.html
> > > > > 
> > > > > Yes, switching to PVH in toolstack is ideal.
> > > > > 
> > > > > The problem we discuss here is in the hypervisor. Hypervisor only has
> > > > > HVM and PV. What type should ARM guests be? I think with the move to 
> > > > > use
> > > > > PVH in toolstack, the type in hypervisor should be HVM (as oppose to 
> > > > > PV
> > > > > now)?
> > > > 
> > > > Arm guest are much closer to HVM than PV. So the hypervisor should use 
> > > > HVM
> > > > here.
> > > +1
> > 
> > OK. In that case, what do you guys think about introducing CONFIG_HVM to
> > ARM?
> 
> I am ok with that. Note, you will need my patch series "tools/libxl: Switch
> Arm guest type to PVH" [1] to make it work on Arm.
> 
> Cheers,
> 
> [1]
> https://lists.xenproject.org/archives/html/xen-devel/2018-08/msg01902.html

It is very subtle.  This patch and your series can be applied
independently of each other.

My patch to introduce CONFIG_HVM for ARM doesn't actually switch the
guest type inside hypervisor to HVM. It is more about preserving
relevant code paths in arch agnostic code for ARM. As it turns out this
patch won't have any effect on ARM because up until now ARM guests have
all taken the PV paths.

With your series applied, ARM guests will start taking HVM paths, which
will always happen with or without this patch.  But your patch is
probably prerequisite to make CONFIG_PV work in the future.

Wei.

> 
> -- 
> Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 05/34] xen: is_hvm_domain should evaluate to 0 when !CONFIG_HVM

2018-08-22 Thread Julien Grall

Hi Wei,

On 21/08/18 21:08, Wei Liu wrote:

On Tue, Aug 21, 2018 at 11:59:26AM -0700, Stefano Stabellini wrote:

On Tue, 21 Aug 2018, Julien Grall wrote:

On 08/21/2018 07:49 PM, Wei Liu wrote:

On Tue, Aug 21, 2018 at 07:33:56PM +0100, Julien Grall wrote:

Hi Wei,

On 08/21/2018 05:31 PM, Wei Liu wrote:

On Mon, Aug 20, 2018 at 05:51:28AM -0600, Jan Beulich wrote:

On 17.08.18 at 17:12,  wrote:

Since it is defined in common header file, introduce CONFIG_HVM to
Arm to avoid breakage.

Signed-off-by: Wei Liu 
---
xen/arch/arm/Kconfig| 3 +++
xen/include/xen/sched.h | 6 ++
2 files changed, 9 insertions(+)
,
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 586bc62..c0e969e 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -52,6 +52,9 @@ config HAS_ITS
prompt "GICv3 ITS MSI controller support" if EXPERT = "y"
depends on GICV3 && !NEW_VGIC
+config HVM
+def_bool y


I'm not convinced this is a good idea, but I'll let the ARM
maintainers
judge.


Andrew discovered that hvm flag is not set by toolstack so ARM guests
are PV guests to Xen. I think the addition here can be omitted.

However I would still like to hear from ARM maintainers what guest type
should be set for ARM, because sooner or later I will need to change PV
code as well.

Grepping for is_{hvm,pv}_* in arch/arm yields no result, but then there
is common code that we need to take care of.


Using PV was more a convenience at the time because was not there. The
plan
is to switch to PVH (see RFC [1]).

I will try to find some times this week to rework the patch based on the
comments.

Cheers,

[1] https://lists.xen.org/archives/html/xen-devel/2018-06/msg01537.html


Yes, switching to PVH in toolstack is ideal.

The problem we discuss here is in the hypervisor. Hypervisor only has
HVM and PV. What type should ARM guests be? I think with the move to use
PVH in toolstack, the type in hypervisor should be HVM (as oppose to PV
now)?


Arm guest are much closer to HVM than PV. So the hypervisor should use HVM
here.
  
+1


OK. In that case, what do you guys think about introducing CONFIG_HVM to
ARM?


I am ok with that. Note, you will need my patch series "tools/libxl: 
Switch Arm guest type to PVH" [1] to make it work on Arm.


Cheers,

[1] 
https://lists.xenproject.org/archives/html/xen-devel/2018-08/msg01902.html


--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 05/34] xen: is_hvm_domain should evaluate to 0 when !CONFIG_HVM

2018-08-21 Thread Wei Liu
On Tue, Aug 21, 2018 at 11:59:26AM -0700, Stefano Stabellini wrote:
> On Tue, 21 Aug 2018, Julien Grall wrote:
> > On 08/21/2018 07:49 PM, Wei Liu wrote:
> > > On Tue, Aug 21, 2018 at 07:33:56PM +0100, Julien Grall wrote:
> > > > Hi Wei,
> > > > 
> > > > On 08/21/2018 05:31 PM, Wei Liu wrote:
> > > > > On Mon, Aug 20, 2018 at 05:51:28AM -0600, Jan Beulich wrote:
> > > > > > > > > On 17.08.18 at 17:12,  wrote:
> > > > > > > Since it is defined in common header file, introduce CONFIG_HVM to
> > > > > > > Arm to avoid breakage.
> > > > > > > 
> > > > > > > Signed-off-by: Wei Liu 
> > > > > > > ---
> > > > > > >xen/arch/arm/Kconfig| 3 +++
> > > > > > >xen/include/xen/sched.h | 6 ++
> > > > > > >2 files changed, 9 insertions(+)
> > > > > > > ,
> > > > > > > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > > > > > > index 586bc62..c0e969e 100644
> > > > > > > --- a/xen/arch/arm/Kconfig
> > > > > > > +++ b/xen/arch/arm/Kconfig
> > > > > > > @@ -52,6 +52,9 @@ config HAS_ITS
> > > > > > >prompt "GICv3 ITS MSI controller support" if EXPERT = 
> > > > > > > "y"
> > > > > > >depends on GICV3 && !NEW_VGIC
> > > > > > > +config HVM
> > > > > > > +def_bool y
> > > > > > 
> > > > > > I'm not convinced this is a good idea, but I'll let the ARM
> > > > > > maintainers
> > > > > > judge.
> > > > > 
> > > > > Andrew discovered that hvm flag is not set by toolstack so ARM guests
> > > > > are PV guests to Xen. I think the addition here can be omitted.
> > > > > 
> > > > > However I would still like to hear from ARM maintainers what guest 
> > > > > type
> > > > > should be set for ARM, because sooner or later I will need to change 
> > > > > PV
> > > > > code as well.
> > > > > 
> > > > > Grepping for is_{hvm,pv}_* in arch/arm yields no result, but then 
> > > > > there
> > > > > is common code that we need to take care of.
> > > > 
> > > > Using PV was more a convenience at the time because was not there. The
> > > > plan
> > > > is to switch to PVH (see RFC [1]).
> > > > 
> > > > I will try to find some times this week to rework the patch based on the
> > > > comments.
> > > > 
> > > > Cheers,
> > > > 
> > > > [1] https://lists.xen.org/archives/html/xen-devel/2018-06/msg01537.html
> > > 
> > > Yes, switching to PVH in toolstack is ideal.
> > > 
> > > The problem we discuss here is in the hypervisor. Hypervisor only has
> > > HVM and PV. What type should ARM guests be? I think with the move to use
> > > PVH in toolstack, the type in hypervisor should be HVM (as oppose to PV
> > > now)?
> > 
> > Arm guest are much closer to HVM than PV. So the hypervisor should use HVM
> > here.
>  
> +1

OK. In that case, what do you guys think about introducing CONFIG_HVM to
ARM?

Wei.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 05/34] xen: is_hvm_domain should evaluate to 0 when !CONFIG_HVM

2018-08-21 Thread Stefano Stabellini
On Tue, 21 Aug 2018, Julien Grall wrote:
> On 08/21/2018 07:49 PM, Wei Liu wrote:
> > On Tue, Aug 21, 2018 at 07:33:56PM +0100, Julien Grall wrote:
> > > Hi Wei,
> > > 
> > > On 08/21/2018 05:31 PM, Wei Liu wrote:
> > > > On Mon, Aug 20, 2018 at 05:51:28AM -0600, Jan Beulich wrote:
> > > > > > > > On 17.08.18 at 17:12,  wrote:
> > > > > > Since it is defined in common header file, introduce CONFIG_HVM to
> > > > > > Arm to avoid breakage.
> > > > > > 
> > > > > > Signed-off-by: Wei Liu 
> > > > > > ---
> > > > > >xen/arch/arm/Kconfig| 3 +++
> > > > > >xen/include/xen/sched.h | 6 ++
> > > > > >2 files changed, 9 insertions(+)
> > > > > > ,
> > > > > > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > > > > > index 586bc62..c0e969e 100644
> > > > > > --- a/xen/arch/arm/Kconfig
> > > > > > +++ b/xen/arch/arm/Kconfig
> > > > > > @@ -52,6 +52,9 @@ config HAS_ITS
> > > > > >prompt "GICv3 ITS MSI controller support" if EXPERT = "y"
> > > > > >depends on GICV3 && !NEW_VGIC
> > > > > > +config HVM
> > > > > > +def_bool y
> > > > > 
> > > > > I'm not convinced this is a good idea, but I'll let the ARM
> > > > > maintainers
> > > > > judge.
> > > > 
> > > > Andrew discovered that hvm flag is not set by toolstack so ARM guests
> > > > are PV guests to Xen. I think the addition here can be omitted.
> > > > 
> > > > However I would still like to hear from ARM maintainers what guest type
> > > > should be set for ARM, because sooner or later I will need to change PV
> > > > code as well.
> > > > 
> > > > Grepping for is_{hvm,pv}_* in arch/arm yields no result, but then there
> > > > is common code that we need to take care of.
> > > 
> > > Using PV was more a convenience at the time because was not there. The
> > > plan
> > > is to switch to PVH (see RFC [1]).
> > > 
> > > I will try to find some times this week to rework the patch based on the
> > > comments.
> > > 
> > > Cheers,
> > > 
> > > [1] https://lists.xen.org/archives/html/xen-devel/2018-06/msg01537.html
> > 
> > Yes, switching to PVH in toolstack is ideal.
> > 
> > The problem we discuss here is in the hypervisor. Hypervisor only has
> > HVM and PV. What type should ARM guests be? I think with the move to use
> > PVH in toolstack, the type in hypervisor should be HVM (as oppose to PV
> > now)?
> 
> Arm guest are much closer to HVM than PV. So the hypervisor should use HVM
> here.
 
+1

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 05/34] xen: is_hvm_domain should evaluate to 0 when !CONFIG_HVM

2018-08-21 Thread Julien Grall



On 08/21/2018 07:49 PM, Wei Liu wrote:

On Tue, Aug 21, 2018 at 07:33:56PM +0100, Julien Grall wrote:

Hi Wei,

On 08/21/2018 05:31 PM, Wei Liu wrote:

On Mon, Aug 20, 2018 at 05:51:28AM -0600, Jan Beulich wrote:

On 17.08.18 at 17:12,  wrote:

Since it is defined in common header file, introduce CONFIG_HVM to
Arm to avoid breakage.

Signed-off-by: Wei Liu 
---
   xen/arch/arm/Kconfig| 3 +++
   xen/include/xen/sched.h | 6 ++
   2 files changed, 9 insertions(+)
,
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 586bc62..c0e969e 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -52,6 +52,9 @@ config HAS_ITS
   prompt "GICv3 ITS MSI controller support" if EXPERT = "y"
   depends on GICV3 && !NEW_VGIC
+config HVM
+def_bool y


I'm not convinced this is a good idea, but I'll let the ARM maintainers
judge.


Andrew discovered that hvm flag is not set by toolstack so ARM guests
are PV guests to Xen. I think the addition here can be omitted.

However I would still like to hear from ARM maintainers what guest type
should be set for ARM, because sooner or later I will need to change PV
code as well.

Grepping for is_{hvm,pv}_* in arch/arm yields no result, but then there
is common code that we need to take care of.


Using PV was more a convenience at the time because was not there. The plan
is to switch to PVH (see RFC [1]).

I will try to find some times this week to rework the patch based on the
comments.

Cheers,

[1] https://lists.xen.org/archives/html/xen-devel/2018-06/msg01537.html


Yes, switching to PVH in toolstack is ideal.

The problem we discuss here is in the hypervisor. Hypervisor only has
HVM and PV. What type should ARM guests be? I think with the move to use
PVH in toolstack, the type in hypervisor should be HVM (as oppose to PV
now)?


Arm guest are much closer to HVM than PV. So the hypervisor should use 
HVM here.


Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 05/34] xen: is_hvm_domain should evaluate to 0 when !CONFIG_HVM

2018-08-21 Thread Wei Liu
On Tue, Aug 21, 2018 at 07:33:56PM +0100, Julien Grall wrote:
> Hi Wei,
> 
> On 08/21/2018 05:31 PM, Wei Liu wrote:
> > On Mon, Aug 20, 2018 at 05:51:28AM -0600, Jan Beulich wrote:
> > > > > > On 17.08.18 at 17:12,  wrote:
> > > > Since it is defined in common header file, introduce CONFIG_HVM to
> > > > Arm to avoid breakage.
> > > > 
> > > > Signed-off-by: Wei Liu 
> > > > ---
> > > >   xen/arch/arm/Kconfig| 3 +++
> > > >   xen/include/xen/sched.h | 6 ++
> > > >   2 files changed, 9 insertions(+)
> > > > ,
> > > > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > > > index 586bc62..c0e969e 100644
> > > > --- a/xen/arch/arm/Kconfig
> > > > +++ b/xen/arch/arm/Kconfig
> > > > @@ -52,6 +52,9 @@ config HAS_ITS
> > > >   prompt "GICv3 ITS MSI controller support" if EXPERT = "y"
> > > >   depends on GICV3 && !NEW_VGIC
> > > > +config HVM
> > > > +def_bool y
> > > 
> > > I'm not convinced this is a good idea, but I'll let the ARM maintainers
> > > judge.
> > 
> > Andrew discovered that hvm flag is not set by toolstack so ARM guests
> > are PV guests to Xen. I think the addition here can be omitted.
> > 
> > However I would still like to hear from ARM maintainers what guest type
> > should be set for ARM, because sooner or later I will need to change PV
> > code as well.
> > 
> > Grepping for is_{hvm,pv}_* in arch/arm yields no result, but then there
> > is common code that we need to take care of.
> 
> Using PV was more a convenience at the time because was not there. The plan
> is to switch to PVH (see RFC [1]).
> 
> I will try to find some times this week to rework the patch based on the
> comments.
> 
> Cheers,
> 
> [1] https://lists.xen.org/archives/html/xen-devel/2018-06/msg01537.html

Yes, switching to PVH in toolstack is ideal.

The problem we discuss here is in the hypervisor. Hypervisor only has
HVM and PV. What type should ARM guests be? I think with the move to use
PVH in toolstack, the type in hypervisor should be HVM (as oppose to PV
now)?

Wei.

> 
> -- 
> Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 05/34] xen: is_hvm_domain should evaluate to 0 when !CONFIG_HVM

2018-08-21 Thread Julien Grall

Hi Wei,

On 08/21/2018 05:31 PM, Wei Liu wrote:

On Mon, Aug 20, 2018 at 05:51:28AM -0600, Jan Beulich wrote:

On 17.08.18 at 17:12,  wrote:

Since it is defined in common header file, introduce CONFIG_HVM to
Arm to avoid breakage.

Signed-off-by: Wei Liu 
---
  xen/arch/arm/Kconfig| 3 +++
  xen/include/xen/sched.h | 6 ++
  2 files changed, 9 insertions(+)
,
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 586bc62..c0e969e 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -52,6 +52,9 @@ config HAS_ITS
  prompt "GICv3 ITS MSI controller support" if EXPERT = "y"
  depends on GICV3 && !NEW_VGIC
  
+config HVM

+def_bool y


I'm not convinced this is a good idea, but I'll let the ARM maintainers
judge.


Andrew discovered that hvm flag is not set by toolstack so ARM guests
are PV guests to Xen. I think the addition here can be omitted.

However I would still like to hear from ARM maintainers what guest type
should be set for ARM, because sooner or later I will need to change PV
code as well.

Grepping for is_{hvm,pv}_* in arch/arm yields no result, but then there
is common code that we need to take care of.


Using PV was more a convenience at the time because was not there. The 
plan is to switch to PVH (see RFC [1]).


I will try to find some times this week to rework the patch based on the 
comments.


Cheers,

[1] https://lists.xen.org/archives/html/xen-devel/2018-06/msg01537.html

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 05/34] xen: is_hvm_domain should evaluate to 0 when !CONFIG_HVM

2018-08-21 Thread Wei Liu
On Mon, Aug 20, 2018 at 05:51:28AM -0600, Jan Beulich wrote:
> >>> On 17.08.18 at 17:12,  wrote:
> > Since it is defined in common header file, introduce CONFIG_HVM to
> > Arm to avoid breakage.
> > 
> > Signed-off-by: Wei Liu 
> > ---
> >  xen/arch/arm/Kconfig| 3 +++
> >  xen/include/xen/sched.h | 6 ++
> >  2 files changed, 9 insertions(+)
> > 
> > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > index 586bc62..c0e969e 100644
> > --- a/xen/arch/arm/Kconfig
> > +++ b/xen/arch/arm/Kconfig
> > @@ -52,6 +52,9 @@ config HAS_ITS
> >  prompt "GICv3 ITS MSI controller support" if EXPERT = "y"
> >  depends on GICV3 && !NEW_VGIC
> >  
> > +config HVM
> > +def_bool y
> 
> I'm not convinced this is a good idea, but I'll let the ARM maintainers
> judge.

Andrew discovered that hvm flag is not set by toolstack so ARM guests
are PV guests to Xen. I think the addition here can be omitted.

However I would still like to hear from ARM maintainers what guest type
should be set for ARM, because sooner or later I will need to change PV
code as well.

Grepping for is_{hvm,pv}_* in arch/arm yields no result, but then there
is common code that we need to take care of.

Wei.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 05/34] xen: is_hvm_domain should evaluate to 0 when !CONFIG_HVM

2018-08-20 Thread Jan Beulich
>>> On 17.08.18 at 17:12,  wrote:
> Since it is defined in common header file, introduce CONFIG_HVM to
> Arm to avoid breakage.
> 
> Signed-off-by: Wei Liu 
> ---
>  xen/arch/arm/Kconfig| 3 +++
>  xen/include/xen/sched.h | 6 ++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 586bc62..c0e969e 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -52,6 +52,9 @@ config HAS_ITS
>  prompt "GICv3 ITS MSI controller support" if EXPERT = "y"
>  depends on GICV3 && !NEW_VGIC
>  
> +config HVM
> +def_bool y

I'm not convinced this is a good idea, but I'll let the ARM maintainers
judge.

Jan



___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 05/34] xen: is_hvm_domain should evaluate to 0 when !CONFIG_HVM

2018-08-20 Thread Wei Liu
On Mon, Aug 20, 2018 at 10:23:47AM +0100, Andrew Cooper wrote:
> >
> >> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> >> index 51ceebe..fdd18a7 100644
> >> --- a/xen/include/xen/sched.h
> >> +++ b/xen/include/xen/sched.h
> >> @@ -879,8 +879,17 @@ void watchdog_domain_destroy(struct domain *d);
> >>
> >>  #define is_pv_domain(d) ((d)->guest_type == guest_type_pv)
> >>  #define is_pv_vcpu(v)   (is_pv_domain((v)->domain))
> >> -#define is_hvm_domain(d) ((d)->guest_type == guest_type_hvm)
> >> -#define is_hvm_vcpu(v)   (is_hvm_domain(v->domain))
> >> +
> >> +static inline bool is_hvm_domain(const struct domain *d)
> >> +{
> >> +return IS_ENABLED(CONFIG_HVM) ? d->guest_type == guest_type_hvm : 
> >> false;
> >> +}
> >> +
> >> +static inline bool is_hvm_vcpu(const struct vcpu *v)
> >> +{
> >> +return is_hvm_domain(v->domain);
> >> +}
> >> +
> > This should work too. I'm not too fuss whether is_hvm_* are macros or
> > functions.
> 
> static inlines are superior to macros in a lot of ways.  If in doubt,
> use a static inline (if you can.  we've got some header file tangles
> which occasionally make it very hard to use static inlines).

OK, sure.

Wei.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 05/34] xen: is_hvm_domain should evaluate to 0 when !CONFIG_HVM

2018-08-20 Thread Andrew Cooper
On 20/08/2018 10:06, Wei Liu wrote:
> On Sun, Aug 19, 2018 at 05:48:17PM +0100, Andrew Cooper wrote:
>> On 17/08/2018 16:12, Wei Liu wrote:
>>> Since it is defined in common header file, introduce CONFIG_HVM to
>>> Arm to avoid breakage.
>>>
>>> Signed-off-by: Wei Liu 
>>> ---
>>>  xen/arch/arm/Kconfig| 3 +++
>>>  xen/include/xen/sched.h | 6 ++
>>>  2 files changed, 9 insertions(+)
>>>
>>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
>>> index 586bc62..c0e969e 100644
>>> --- a/xen/arch/arm/Kconfig
>>> +++ b/xen/arch/arm/Kconfig
>>> @@ -52,6 +52,9 @@ config HAS_ITS
>>>  prompt "GICv3 ITS MSI controller support" if EXPERT = "y"
>>>  depends on GICV3 && !NEW_VGIC
>>>  
>>> +config HVM
>>> +def_bool y
>>> +
>>>  config NEW_VGIC
>>> bool
>>> prompt "Use new VGIC implementation"
>>> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
>>> index 51ceebe..8fc3423 100644
>>> --- a/xen/include/xen/sched.h
>>> +++ b/xen/include/xen/sched.h
>>> @@ -879,8 +879,14 @@ void watchdog_domain_destroy(struct domain *d);
>>>  
>>>  #define is_pv_domain(d) ((d)->guest_type == guest_type_pv)
>>>  #define is_pv_vcpu(v)   (is_pv_domain((v)->domain))
>>> +
>>> +#if CONFIG_HVM
>>>  #define is_hvm_domain(d) ((d)->guest_type == guest_type_hvm)
>>> +#else
>>> +#define is_hvm_domain(d) (0)
>>> +#endif
>>>  #define is_hvm_vcpu(v)   (is_hvm_domain(v->domain))
>> The need for the following patch is caused by a bug here, in that you
>> don't evaluate d.
> I know. I didn't classified that as a bug though.

I'm afraid that the x86 maintainership will disagree with you there.

>
>> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
>> index 51ceebe..fdd18a7 100644
>> --- a/xen/include/xen/sched.h
>> +++ b/xen/include/xen/sched.h
>> @@ -879,8 +879,17 @@ void watchdog_domain_destroy(struct domain *d);
>>
>>  #define is_pv_domain(d) ((d)->guest_type == guest_type_pv)
>>  #define is_pv_vcpu(v)   (is_pv_domain((v)->domain))
>> -#define is_hvm_domain(d) ((d)->guest_type == guest_type_hvm)
>> -#define is_hvm_vcpu(v)   (is_hvm_domain(v->domain))
>> +
>> +static inline bool is_hvm_domain(const struct domain *d)
>> +{
>> +return IS_ENABLED(CONFIG_HVM) ? d->guest_type == guest_type_hvm : false;
>> +}
>> +
>> +static inline bool is_hvm_vcpu(const struct vcpu *v)
>> +{
>> +return is_hvm_domain(v->domain);
>> +}
>> +
> This should work too. I'm not too fuss whether is_hvm_* are macros or
> functions.

static inlines are superior to macros in a lot of ways.  If in doubt,
use a static inline (if you can.  we've got some header file tangles
which occasionally make it very hard to use static inlines).

>
>>  #define is_pinned_vcpu(v) ((v)->domain->is_pinned || \
>> cpumask_weight((v)->cpu_hard_affinity) == 1)
>>  #ifdef CONFIG_HAS_PASSTHROUGH
>>
>> seems to compile, and should DTRT including all appropriate parameter
>> evaluation.
> I will test if DCE works properly with static inline functions.

It does (/should).

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 05/34] xen: is_hvm_domain should evaluate to 0 when !CONFIG_HVM

2018-08-20 Thread Wei Liu
On Sun, Aug 19, 2018 at 05:48:17PM +0100, Andrew Cooper wrote:
> On 17/08/2018 16:12, Wei Liu wrote:
> > Since it is defined in common header file, introduce CONFIG_HVM to
> > Arm to avoid breakage.
> >
> > Signed-off-by: Wei Liu 
> > ---
> >  xen/arch/arm/Kconfig| 3 +++
> >  xen/include/xen/sched.h | 6 ++
> >  2 files changed, 9 insertions(+)
> >
> > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > index 586bc62..c0e969e 100644
> > --- a/xen/arch/arm/Kconfig
> > +++ b/xen/arch/arm/Kconfig
> > @@ -52,6 +52,9 @@ config HAS_ITS
> >  prompt "GICv3 ITS MSI controller support" if EXPERT = "y"
> >  depends on GICV3 && !NEW_VGIC
> >  
> > +config HVM
> > +def_bool y
> > +
> >  config NEW_VGIC
> > bool
> > prompt "Use new VGIC implementation"
> > diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> > index 51ceebe..8fc3423 100644
> > --- a/xen/include/xen/sched.h
> > +++ b/xen/include/xen/sched.h
> > @@ -879,8 +879,14 @@ void watchdog_domain_destroy(struct domain *d);
> >  
> >  #define is_pv_domain(d) ((d)->guest_type == guest_type_pv)
> >  #define is_pv_vcpu(v)   (is_pv_domain((v)->domain))
> > +
> > +#if CONFIG_HVM
> >  #define is_hvm_domain(d) ((d)->guest_type == guest_type_hvm)
> > +#else
> > +#define is_hvm_domain(d) (0)
> > +#endif
> >  #define is_hvm_vcpu(v)   (is_hvm_domain(v->domain))
> 
> The need for the following patch is caused by a bug here, in that you
> don't evaluate d.

I know. I didn't classified that as a bug though.

> 
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index 51ceebe..fdd18a7 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -879,8 +879,17 @@ void watchdog_domain_destroy(struct domain *d);
> 
>  #define is_pv_domain(d) ((d)->guest_type == guest_type_pv)
>  #define is_pv_vcpu(v)   (is_pv_domain((v)->domain))
> -#define is_hvm_domain(d) ((d)->guest_type == guest_type_hvm)
> -#define is_hvm_vcpu(v)   (is_hvm_domain(v->domain))
> +
> +static inline bool is_hvm_domain(const struct domain *d)
> +{
> +return IS_ENABLED(CONFIG_HVM) ? d->guest_type == guest_type_hvm : false;
> +}
> +
> +static inline bool is_hvm_vcpu(const struct vcpu *v)
> +{
> +return is_hvm_domain(v->domain);
> +}
> +

This should work too. I'm not too fuss whether is_hvm_* are macros or
functions.

>  #define is_pinned_vcpu(v) ((v)->domain->is_pinned || \
> cpumask_weight((v)->cpu_hard_affinity) == 1)
>  #ifdef CONFIG_HAS_PASSTHROUGH
> 
> seems to compile, and should DTRT including all appropriate parameter
> evaluation.

I will test if DCE works properly with static inline functions.

Wei.

> 
> ~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 05/34] xen: is_hvm_domain should evaluate to 0 when !CONFIG_HVM

2018-08-17 Thread Wei Liu
Since it is defined in common header file, introduce CONFIG_HVM to
Arm to avoid breakage.

Signed-off-by: Wei Liu 
---
 xen/arch/arm/Kconfig| 3 +++
 xen/include/xen/sched.h | 6 ++
 2 files changed, 9 insertions(+)

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 586bc62..c0e969e 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -52,6 +52,9 @@ config HAS_ITS
 prompt "GICv3 ITS MSI controller support" if EXPERT = "y"
 depends on GICV3 && !NEW_VGIC
 
+config HVM
+def_bool y
+
 config NEW_VGIC
bool
prompt "Use new VGIC implementation"
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 51ceebe..8fc3423 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -879,8 +879,14 @@ void watchdog_domain_destroy(struct domain *d);
 
 #define is_pv_domain(d) ((d)->guest_type == guest_type_pv)
 #define is_pv_vcpu(v)   (is_pv_domain((v)->domain))
+
+#if CONFIG_HVM
 #define is_hvm_domain(d) ((d)->guest_type == guest_type_hvm)
+#else
+#define is_hvm_domain(d) (0)
+#endif
 #define is_hvm_vcpu(v)   (is_hvm_domain(v->domain))
+
 #define is_pinned_vcpu(v) ((v)->domain->is_pinned || \
cpumask_weight((v)->cpu_hard_affinity) == 1)
 #ifdef CONFIG_HAS_PASSTHROUGH
-- 
git-series 0.9.1

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel