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.gr...@arm.com>
> > > ---
> > > 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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
