Re: [XEN v5 1/3] xen/arm: Introduce CONFIG_PARTIAL_EMULATION and "partial-emulation" cmd option

[Ayan Kumar Halder <ayankuma@xxxxxxx>]

Hi Jan,

On 20/02/2024 12:33, Jan Beulich wrote:
> On 20.02.2024 13:17, Ayan Kumar Halder wrote:
> > There can be situations when the registers cannot be emulated to their full
> > functionality. This can be due to the complexity involved. In such cases, one
> > can emulate those registers as RAZ/WI for example. We call them as partial
> > emulation.
> >
> > Some registers are non-optional and as such there is nothing preventing an OS
> > from accessing them.
> > Instead of injecting undefined exception (thus crashing a guest), one may want
> > to prefer a partial emulation to let the guest running (in some cases accepting
> > the fact that it might result in unwanted behavior).
> >
> > A suitable example of this (as seen in subsequent patches) is emulation of
> > DBGDTRTX_EL0 (on Arm64) and DBGDTRTXINT(on Arm32). These non-optional
> > registers can be emulated as RAZ/WI and they can be enclosed within
> > CONFIG_PARTIAL_EMULATION.
> >
> > Further, "partial-emulation" command line option allows us to
> > enable/disable partial emulation at run time. While CONFIG_PARTIAL_EMULATION
> > enables support for partial emulation at compile time (i.e. adds code for
> > partial emulation), this option may be enabled or disabled by Yocto or other
> > build systems. However if the build system turns this option on, users
> > can use scripts like Imagebuilder to generate uboot-script which will append
> > "partial-emulation=true" to xen command line to turn on the partial emulation.
> > Thus, it helps to avoid rebuilding xen.
> >
> > By default, "CONFIG_PARTIAL_EMULATION=y" and "partial-emulation=false".
> > This is done so that Xen supports partial emulation. However, customers are
> > fully aware when they enable partial emulation. It's important to note that
> > enabling such support might result in unwanted/non-spec compliant behavior.
> >
> > Added a note in SUPPORT.md to clarify the security support for partial
> > emulation.
> There may have been prior discussion of this, but I view this as too little
> to justify ...
>
> > --- a/SUPPORT.md
> > +++ b/SUPPORT.md
> > @@ -101,6 +101,18 @@ Extension to the GICv3 interrupt controller to support MSI.
> >   
> >       Status: Experimental
> >   
> > +### ARM/Partial Emulation
> > +
> > +Enable partial emulation of registers, otherwise considered unimplemented,
> > +that would normally trigger a fault injection.
> > +
> > +    Status: Supported, with caveats
> > +
> > +Bugs allowing the userspace to attack the guest OS will not be considered
> > +security vulnerabilities.
> > +
> > +Bugs that could compromise Xen will be considered security vulnerabilities.
> ... the odd statement regarding in-guest vulnerabilities that might be
> introduced. I see only two ways of treating this as supported: Either
> you simply refuse emulation when the access is from user space,
I am wondering how do we enforce that.

Let me try to understand this with the current implementation of partial 
emulation for system registers.
1. DBGDTRTX_EL0 :- I understand that EL0 access to this register will 
cause a trap to EL2. The reason being MDCR_EL2.TDA == 1.
In that case, if we refuse emulation then an undef exception is injected 
to the guest (this is the behavior as of today even without this patch).
So, are you saying that the undef exception is to be injected to the 
user space process. This may be possible for Arm64 guests 
(inject_undef64_exception() needs to be changed).
However for Arm32 guests, this may not be possible as the mode changes 
to PSR_MODE_UND.
Let me know if I understood you correctly.

> or you
> support that mode of emulation as much as that of kernel space accesses.
Do you mean we support partial emulation only for traps from EL1 mode ?

- Ayan

> After all "injecting undefined exception (thus crashing a guest)"
> (quoting the description) doesn't seem quite right: If you inject such
> an exception for a user space access, all that ought to happen is the
> user space process to be killed.
>
> Jan