Re: [Xen-devel] [RFC 10/16] xen/arm: vcpreg: Add wrappers to handle co-proc access trapped by HCR_EL2.TVM

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 5 Nov 2018, Julien Grall wrote:
> Hi Stefano,
> 
> On 11/5/18 7:47 PM, Stefano Stabellini wrote:
> > On Mon, 8 Oct 2018, Julien Grall wrote:
> > > A follow-up patch will require to emulate some accesses to some
> > > co-processors registers trapped by HCR_EL2.TVM. When set, all NS EL1
> > > writes
> > > to the virtual memory control registers will be trapped to the hypervisor.
> > > 
> > > This patch adds the infrastructure to passthrough the access to host
> > > registers. For convenience a bunch of helpers have been added to
> > > generate the different helpers.
> > > 
> > > Note that HCR_EL2.TVM will be set in a follow-up patch dynamically.
> > > 
> > > Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> > > ---
> > >   xen/arch/arm/vcpreg.c        | 144
> > > +++++++++++++++++++++++++++++++++++++++++++
> > >   xen/include/asm-arm/cpregs.h |   1 +
> > >   2 files changed, 145 insertions(+)
> > > 
> > > diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c
> > > index b04d996fd3..49529b97cd 100644
> > > --- a/xen/arch/arm/vcpreg.c
> > > +++ b/xen/arch/arm/vcpreg.c
> > > @@ -24,6 +24,122 @@
> > >   #include <asm/traps.h>
> > >   #include <asm/vtimer.h>
> > >   +/*
> > > + * Macros to help generating helpers for registers trapped when
> > > + * HCR_EL2.TVM is set.
> > > + *
> > > + * Note that it only traps NS write access from EL1.
> > > + *
> > > + *  - TVM_REG() should not be used outside of the macros. It is there to
> > > + *    help defining TVM_REG32() and TVM_REG64()
> > > + *  - TVM_REG32(regname, xreg) and TVM_REG64(regname, xreg) are used to
> > > + *    resp. generate helper accessing 32-bit and 64-bit register.
> > > "regname"
> > > + *    been the Arm32 name and "xreg" the Arm64 name.
> >           ^ is
> > 
> > Please add that we use the Arm64 reg name to call WRITE_SYSREG in the
> > Xen source code even on Arm32 in general
> 
> I am not sure to understand this. It is common use in Xen to use arm64 name
> when code is for both architecture. So why would I need a specific comment
> here?

Yes, that's our convention, but as I was looking through the code, I
couldn't quickly find any places where we wrote the convention down. Is
there? I thought it would be good to start somewhere, this could be a
good place as any, also given that it directly affects this code.


> > > + *  - UPDATE_REG32_COMBINED(lowreg, hireg, xreg) are used to generate a
> > 
> > TVM_REG32_COMBINED
> > 
> > 
> > > + *  pair of registers share the same Arm32 registers. "lowreg" and
> > > + *  "higreg" been resp. the Arm32 name and "xreg" the Arm64 name.
> > > "lowreg"
> > > + *  will use xreg[31:0] and "hireg" will use xreg[63:32].
> > 
> > Please add that xreg is unused in the Arm32 case.
> 
> Why do you think that? xreg is actually used. It will get expanded to whatever
> is the co-processor encoding and caught by reg... in TVM_REG().

It is unused in the TVM_REG32_COMBINED case, which is the comment part I
was replying about. This is the code:

  #define TVM_REG32_COMBINED(lowreg, hireg, xreg)                     \
      /* Use TVM_REG directly to workaround macro expansion. */       \
      TVM_REG(32, vreg_emulate_##lowreg, lowreg)                      \
      TVM_REG(32, vreg_emulate_##hireg, hireg)

xreg is not used?

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