Re: [Xen-devel] [PATCH 01/18] xen/arm: Introduce a helper to get default HCR_EL2 flags

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 15 Mar 2017, Julien Grall wrote:
> On 15/03/17 07:19, Wei Chen wrote:
> > Hi Stefano,
> 
> Hello,
> 
> > On 2017/3/15 8:24, Stefano Stabellini wrote:
> > > On Mon, 13 Mar 2017, Wei Chen wrote:
> > > > We want to add HCR_EL2 register to Xen context switch. And each copy
> > > > of HCR_EL2 in vcpu structure will be initialized with the same set
> > > > of trap flags as the HCR_EL2 register. We introduce a helper here to
> > > > represent these flags to be reused easily.
> > > > 
> > > > Signed-off-by: Wei Chen <Wei.Chen@xxxxxxx>
> > > > ---
> > > >  xen/arch/arm/traps.c            | 11 ++++++++---
> > > >  xen/include/asm-arm/processor.h |  2 ++
> > > >  2 files changed, 10 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> > > > index 614501f..d343c66 100644
> > > > --- a/xen/arch/arm/traps.c
> > > > +++ b/xen/arch/arm/traps.c
> > > > @@ -115,6 +115,13 @@ static void __init parse_vwfi(const char *s)
> > > >  }
> > > >  custom_param("vwfi", parse_vwfi);
> > > > 
> > > > +register_t get_default_hcr_flags(void)
> > > > +{
> > > > +    return  (HCR_PTW|HCR_BSU_INNER|HCR_AMO|HCR_IMO|HCR_FMO|HCR_VM|
> > > > +             (vwfi != NATIVE ? (HCR_TWI|HCR_TWE) : 0) |
> > > > +             HCR_TSC|HCR_TAC|HCR_SWIO|HCR_TIDCP|HCR_FB);
> > > > +}
> > > 
> > > I haven't finished reading this series yet, but I would make this a
> > > static inline function if possible
> > > 
> > 
> > I had considered to use static inline before. But it must move the
> > 
> > static enum {
> >         TRAP,
> >         NATIVE,
> > } vwfi;
> > 
> > to the header file at the same time. But get_default_hcr_flags would
> > not be used frequently. So I thought it didn't have enough value to
> > change a less relevant code to make this function become static inline.
> 
> Looking at the spec, HCR_EL2 is controlling the behavior of the VM. We only
> need to ensure this to be set before going to EL1/EL0. Note that the reset
> value of some register are architecturally UNKNOWN, but I don't think we care
> here.
> 
> So I would suggest to drop the setting in init_traps and only rely on the one
> in when returning to the guest. And therefore this function could be moved in
> domain.c
> 
> Any opinions?

Fine by me

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel