Re: [RFC PATCH] xen/arm: Add emulation of Debug Data Transfer Registers\

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 8 Dec 2023, Bertrand Marquis wrote:
> Hi All,
> 
> Sorry for coming back late on this thread.
> 
> > On 7 Dec 2023, at 22:41, Stefano Stabellini <sstabellini@xxxxxxxxxx> wrote:
> > 
> > On Thu, 7 Dec 2023, Julien Grall wrote:
> >> Hi Stefano,
> >> 
> >> On 05/12/2023 23:21, Stefano Stabellini wrote:
> >>> On Tue, 5 Dec 2023, Julien Grall wrote:
> >>>> I agree that crashing a guest is bad, but is lying to the domain really
> >>>> better? The consequence here is not that bad and hopefully it would be
> >>>> fairly
> >>>> easy to find. But this is not always the case. So I definitely would 
> >>>> place
> >>>> a
> >>>> half-backed emulation more severe than a guest crash.
> >>> 
> >>> 
> >>> I see where Julien is coming from, but I would go with option two:
> >>> "emulate DCC the same way as KVM". That's because I don't think we can
> >>> get away with crashing the guest in all cases. Although the issue came
> >>> up with a Linux guest, it could have been triggered by a proprietary
> >>> operating system that we cannot change, and I think Xen should support
> >>> running unmodified operating systems.
> >>> 
> >>> If we go with a "half-backed emulation" solution, as Julien wrote, then
> >>> it is better to be more similar to other hypervisors, that's why I chose
> >>> option two instead of option three.
> >>> 
> >>> But at the same time I recognize the validity of Julien's words and it
> >>> makes me wonder if we should have a KCONFIG option or command line
> >>> option to switch the Xen behavior. We could use it to gate all the
> >>> "half-backed emulation" we do for compatibility.  Something like:
> >>> 
> >>> config PARTIAL_EMULATION
> >>>     bool "Partial Emulation"
> >>>     ---help---
> >>>           Enables partial, not spec compliant, emulation of certain
> >>> register
> >>>     interfaces (e.g DCC UART) for guest compatibility. If you disable
> >>>     this option, Xen will crash the guest if the guest tries to access
> >>>     interfaces not fully emulated or virtualized.
> >>> 
> >>>     If you enable this option, the guest might misbehave due to non-spec
> >>>     compliant emulation done by Xen.
> >> 
> >> As I wrote to Ayan on Matrix today, I am not in favor of the emulation. 
> >> Yet, I
> >> am not going to oppose (as in Nack it) if the other maintainers agree with 
> >> it.
> > 
> > Thanks for being flexible
> > 
> > 
> >> The KConfig would be nice, the question is whether we want to (security)
> >> support such configuration? E.g. could this potentially introduce a 
> >> security
> >> issue in the guest?
> > 
> > The important question is whether it could introduce a security issue in
> > Xen. If we think it wouldn't increase the attack surface significantly
> > then I would security support it otherwise not.
> > 
> > 
> >> Regarding the  emulation itself, I actually prefer 3 because at least the
> >> Linux drivers will be able to bail out rather than trying to use them.
> > 
> > I don't have a strong opinion between 2 and 3
> 
> Here is my view on it:
> - providing a wrong emulation to please guests is not wrong as it might end
> up hidding something that will be hard to debug so on that point I agree with
> Julien.
> - choosing a solution which might just crash a guest without any other 
> solution
> than recompiling or modifying xen is not something acceptable if we want Xen
> to thrive.
> 
> So i would suggest the following solution:
> - have a Kconfig to surround this code so that "correct" guests can disable 
> it.
> - have a command line option to activate this behavior and turn it off by 
> default.
> One encountering the problem will have to explicitly set a command line 
> parameter
> so cannot do this without knowing.
> - activate the Kconfig option by default and security support it as it is 
> only active if
> a command line parameter is passed.
> 
> The Kconfig parameter should be more generic so that this could apply to a 
> bunch of
> registers we would emulate with RAZ/WI so I am happy with that proposal if we 
> say
> that this must be activated through a command line option passed to Xen at 
> boot.

You are suggesting both a Kconfig and a command line option.

The Kconfig would be useful so that in a strict configuration we can
disable the code even from the build (useful for instance in
configurations for safety certifications.)

The Kconfig option would be enabled by default (which is important for
the out of the box experience otherwise users would have to manually
rebuild Xen to run their guests.)

However, the actual partial emulation is only enabled if a command line
option is passed. The command line option would be off by default. So
the code is there, but wouldn't be used unless the user enables the
command line option explicitly. This way, tools aimed at making the out
of the box experience better (like ImageBuilder) might pass the command
line option by default but production build systems (like Yocto)
wouldn't.

Yes I think this is the best compromise. If it was just for this patch I
would say it is a bit too much but we have seen a few of these cases so
I think this is a general framework that will be useful in multiple
instances. I am fine with this.