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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

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, <wei.liu2@xxxxxxxxxx> wrote:
> > > > > > > > > Since it is defined in common header file, introduce 
> > > > > > > > > CONFIG_HVM to
> > > > > > > > > Arm to avoid breakage.
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> > > > > > > > > ---
> > > > > > > > >     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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel