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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 11 Dec 2023, Ayan Kumar Halder wrote:
> On 11/12/2023 09:33, Julien Grall wrote:
> > Hi,
> 
> Hi Julien/Stefano/Bertrand/Michal,
> 
> It is a great discussion, thanks for your suggestions.
> 
> I think we have an agreement. :-)
> 
> > 
> > On 07/12/2023 21:41, Stefano Stabellini 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.
> > 
> > For this specific emulation, it is unlikely. But I can't make a generic
> > statement here. So we would need to do a case by case basis.
> > 
> > Furthermore, our security statement is also covering a guest userspace
> > attacking a guest OS. We would issue an XSA if this is feasible because of
> > an issue in the hypervisor.
> > 
> > With half-backed emulation, it becomes more difficult to know whether we are
> > not opening a hole and replacing a guest crashes at boot by something worse.
> > 
> > Again unlikely here. But those kind of bugs are no unheard. So this is
> > something to take into account when you want to claim security support for
> > half-backed emulation.
> 
> For this specific emulation, I think we all agree that there is no security
> risk. So we need not add any security support for this.

Rather than "no security risk" I would say "unlikely" as Julien wrote
(one never knows...) Also you wrote "we need not add any security
support" but I think you probably meant the opposite: we could add
security support for it.


> With regards to partial emulation, we all agree that there is no perfect
> solution.
> 
> 
> However, the approach on which we all seem to have consensus :-
> 
> 1. Emulate DCC with TXfull set to 1 (no crash, DCC driver in Linux/Uboot
> returns -ENODEV/-EAGAIN).
> 
> 2. Introduce a Kconfig (say "CONFIG_PARTIAL_EMULATION") option to surround
> this code which will be specific for Arm and enabled by default. This should
> be turned off by a vendor who does not want to provide any form of partial
> emulation.
> 
> 3. Introduce a hypervisor command line option ("partial_emulation" , disabled
> by default) so that this cen be enabled at run time using Imagebuilder/uboot
> scripts.
> 
> The #2 and #3 can be extended in future to cover all forms of partial
> emulation.
> 
> I will send out a patch implementing this approach.

Yes, sounds good