Re: [Xen-devel] [PATCH 1/1] xen/arm: Add pl011 uart support in Xen for guest domains

[Julien Grall <julien.grall@xxxxxxx>]

Hi Konrad,

On 02/17/2017 03:27 PM, Konrad Rzeszutek Wilk wrote:
> On Thu, Feb 16, 2017 at 02:34:30PM +0000, Julien Grall wrote:
> > On 15/02/17 15:38, Konrad Rzeszutek Wilk wrote:
> > > On Wed, Feb 15, 2017 at 12:53:34PM +0530, Bhupinder Thakur wrote:
> > We already have some HVM param that are x86 specific (see
> > HVM_PARAM_VIRIDIAN) or interpreted differently whether Xen is compiled for
> > ARM or x86 (see HVM_PARAM_CALLBACK_IRQ).
> >
> > So I would keep the new HVM params ARM specific and enclosed in #ifdef.
> But why couldn't you have pl101 on x86? It is a just a device
> driver?
I assumed that because it is an ARM IP it will not be used on x86. But 
maybe I am wrong here.
> > > > > The second patch would implement this in the generic
> > > > > code (in xen/common/event_channel.c) - perhaps via an
> > > > > secondary function that is NOP on x86 but not so on ARM?
> > > > >
> > > > > Then another patch that fleshes out the emulation code in
> > > > > the hypervisor, then the one in console code, and lastly
> > > > > in libxl to turn this on/off.
> > > > Is it preferable to keep the pl011 emulation feature under its own
> > > > feature CONFIG flag so that it can be compiled off across
> > > > Xen/toolstack/console?
> > > The CONFIG flag are only for hypervisor. I would add it as a
> > > CONFIG
> > What do you mean by the second CONFIG? Is it the one in config/*.mk?
> In the hypervisor (xen/) Kconfig files.
I don't think it hurts to get the pl011 built-in in Xen as it would be 
disabled by default for the guest.
Also, I am not comfortable yet another Kconfig option on ARM until we 
get an embedded .config in Xen. Without that it makes harder to know 
what has been enabled or not by the user.

[...]

> > > > >    Should vpl011.h be in include/xen/public/ ? If so you need
> > > > >    a different license for that file.
> > > > I will try to move this header file in the same folder where vpl011.c is.
> > > Is the ring protocol suppose to be implemented by the Linux kernel? If so
> > > it MUST be in the public/io directory.
> > >
> > > And why does it have to be aring protocol? Why not whatever the pl011 
> > > implements?
> > >
> > > Why not emulate how pl011 works?
> > The ring protocol is between Xen (where the pl011 is emulated) and the
> > console backend. The guest will use a normal pl011 driver and no Xen header
> 'console backend' can be some OS (MiniOS)? So we still need an
> Xen public header I think?
Hmmm. I agree for the ring, but anything related to the emulation of the 
PL011 itself should stay in include/{xen,asm-arm}/
However, the pl011 ring is similar to the console ring. So maybe we 
should re-use the console header rather than re-inventing the wheel.
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel