Re: [Xen-devel] [PATCH 02/11] xen/arm: vpl011: Add new hvm params in Xen for ring buffer/event setup

[Julien Grall <julien.grall@xxxxxxx>]

Hi Jan,

On 06/03/17 08:06, Jan Beulich wrote:
> > > > On 05.03.17 at 13:35, <julien.grall@xxxxxxx> wrote:
> > On 02/21/2017 11:25 AM, Bhupinder Thakur wrote:
> > > --- a/xen/include/public/hvm/params.h
> > > +++ b/xen/include/public/hvm/params.h
> > > @@ -203,10 +203,17 @@
> > >   */
> > >  #define HVM_PARAM_ACPI_IOPORTS_LOCATION 19
> > >
> > > -/* Deprecated */
> > > +#if defined(__arm__) || defined(__aarch64__)
> > > +#define HVM_PARAM_VPL011_CONSOLE_PFN    20
> > > +#define HVM_PARAM_VPL011_CONSOLE_EVTCHN 21
> > > +#define HVM_PARAM_VPL011_VIRQ           22
> > > +#else
> > >  #define HVM_PARAM_MEMORY_EVENT_CR0          20
> > >  #define HVM_PARAM_MEMORY_EVENT_CR3          21
> > >  #define HVM_PARAM_MEMORY_EVENT_CR4          22
> > Those parameters are still deprecated but you drop the comment stating that.
> >
> > > +#endif
> > > +
> > Those params are x86 specific so should have never been set on ARM. But
> > I am not sure if it is fine to re-purpose deprecated number.
> >
> > I have CCed "The REST" maintainers to have their input here.
> I think re-purposing something that was never (meant to be) used is
> fine in a case like this. However, the question is moot with your
> suggestion to not use params here in the first place.
I suggested to drop HVM_PARAM_VPL011_VIRQ because we can hardcode the 
guest interrupt number for the UART as we already do for the MMIO 
region. The 2 other HVM_PARAM looks sensible to me.
I thought a bit more about those params. I think the name should be 
generic and not tie to pl011 because we may want to emulate different 
UART for the guest in the future.
Also, by re-using deprecated encoding it means that it will not be 
possible to use those parameters on x86 if you ever decide to emulate 
UART in Xen. I am not sure whether if you are happy with that?
Cheers,

--
Julien Grall

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