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

[Ayan Kumar Halder <ayankuma@xxxxxxx>]

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.
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.

- Ayan

> Cheers,