Re: [Xen-devel] [PATCH 05/10] xen/arm: vpl011: Allocate a new PFN in the toolstack for the virtual console

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 19/04/17 11:28, Wei Liu wrote:
> On Tue, Apr 18, 2017 at 05:36:41PM -0700, Stefano Stabellini wrote:
> > On Thu, 13 Apr 2017, Wei Liu wrote:
> > > On Thu, Apr 13, 2017 at 02:07:54PM +0530, Bhupinder Thakur wrote:
> > > > Hi Wei,
> > > >
> > > >
> > > > > >  /* --- pluggable kernel loader ------------------------------------- */
> > > > > > diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
> > > > > > index a7e839e..157381e 100644
> > > > > > --- a/tools/libxc/xc_dom_arm.c
> > > > > > +++ b/tools/libxc/xc_dom_arm.c
> > > > > > @@ -26,10 +26,11 @@
> > > > > >  #include "xg_private.h"
> > > > > >  #include "xc_dom.h"
> > > > > >
> > > > > > -#define NR_MAGIC_PAGES 3
> > > > > > +#define NR_MAGIC_PAGES 4
> > > > > >  #define CONSOLE_PFN_OFFSET 0
> > > > > >  #define XENSTORE_PFN_OFFSET 1
> > > > > >  #define MEMACCESS_PFN_OFFSET 2
> > > > > > +#define VCONSOLE_PFN_OFFSET 3
> > > > > >
> > > > > >  #define LPAE_SHIFT 9
> > > > > >
> > > > > > @@ -85,6 +86,7 @@ static int alloc_magic_pages(struct xc_dom_image *dom)
> > > > > >
> > > > > >      dom->console_pfn = base + CONSOLE_PFN_OFFSET;
> > > > > >      dom->xenstore_pfn = base + XENSTORE_PFN_OFFSET;
> > > > > > +    dom->vconsole_pfn = base + VCONSOLE_PFN_OFFSET;
> > > > > >
> > > > > >      xc_clear_domain_page(dom->xch, dom->guest_domid, dom->console_pfn);
> > > > > >      xc_clear_domain_page(dom->xch, dom->guest_domid, dom->xenstore_pfn);
> > > > > > @@ -95,6 +97,9 @@ static int alloc_magic_pages(struct xc_dom_image *dom)
> > > > > >              dom->xenstore_pfn);
> > > > > >      xc_hvm_param_set(dom->xch, dom->guest_domid, HVM_PARAM_MONITOR_RING_PFN,
> > > > > >              base + MEMACCESS_PFN_OFFSET);
> > > > > > +    xc_hvm_param_set(dom->xch, dom->guest_domid, HVM_PARAM_VCONSOLE_PFN,
> > > > > > +                     base + VCONSOLE_PFN_OFFSET);
> > > > > > +
> > > > > Here is something I don't quite understand (sorry I haven't been
> > > > > following the conversation closely): if pl011 is emulated, why would the
> > > > > guest need to know its PFN?
> > > > This PFN is used by Xen to setup a ring-buffer between xenconsole and itself.
> > > > Xen reads/writes data from/to this ring buffer when it gets a mmio
> > > > read/write request from the guest.
> > > What I was getting at was "does the *guest* need to know the PFN"?  The
> > > hypervisor and xenconsole daemon / client aren't the guest. Does the
> > > guest need to know the exact PFN in order to setup MMIO?
> > >
> > > Ultimately this is going to be decided by ARM maintainers. I'm just
> > > curious about why it is done like this.
> > hvm_params are commonly used to pass parameters from Xen or from libxl
> > to guests. In this case, they are used to pass parameters from the
> > toolstack to Xen.
> >
> > The guest does not need to know the pfn, and in fact it cannot: Xen
> > refuses to return the value of HVM_PARAM_VCONSOLE_PFN to guests (see
> > patch #2).
> Ah, I missed the new restriction in #2.
>
> > Honestly, I don't particularly care about how the pfn is passed from
> > libxc to Xen. hvm_param is an option, or we could introduce a new
> > domctl.
> No opinion from me either. I think using HVM params is fine.
There were some concerns on the first version about using HVM params 
because this is tying the virtual UART to HVM and only allow us to have 
one virtual console.
HVM params are part of the stable ABI, so this would restrict ourselves 
to future extension.
It was suggested to look at the DOMCTL way, I would have expected to 
some investigation and a summary in the cover letter. So why do we keep 
HVM PARAM?
Cheers,

--
Julien Grall

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