All of lore.kernel.org
 help / color / mirror / Atom feed
From: Julien Grall <julien.grall@arm.com>
To: Wei Liu <wei.liu2@citrix.com>,
	Stefano Stabellini <sstabellini@kernel.org>
Cc: Bhupinder Thakur <bhupinder.thakur@linaro.org>,
	xen-devel@lists.xenproject.org,
	Ian Jackson <ian.jackson@eu.citrix.com>
Subject: Re: [PATCH 05/10] xen/arm: vpl011: Allocate a new PFN in the toolstack for the virtual console
Date: Wed, 19 Apr 2017 12:01:14 +0100	[thread overview]
Message-ID: <49ce6f5b-e368-2557-dfeb-e4a2f7b7141f@arm.com> (raw)
In-Reply-To: <20170419102811.7y2tlhfkvl5jdfjf@citrix.com>

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@lists.xen.org
https://lists.xen.org/xen-devel

  reply	other threads:[~2017-04-19 11:01 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-03  9:44 [PATCH 00/10] pl011 emulation support in Xen Bhupinder Thakur
2017-04-03  9:44 ` [PATCH 01/10] xen/arm: vpl011: Add pl011 uart emulation " Bhupinder Thakur
2017-04-12 16:32   ` Wei Liu
2017-04-19  0:15   ` Stefano Stabellini
2017-04-19  7:28     ` Bhupinder Thakur
2017-04-19  8:36       ` Julien Grall
2017-04-19 18:40       ` Stefano Stabellini
2017-04-25  7:31         ` Bhupinder Thakur
2017-04-25 17:56           ` Stefano Stabellini
2017-04-26  7:49             ` Bhupinder Thakur
2017-04-26  8:13               ` Julien Grall
2017-04-26 17:03                 ` Stefano Stabellini
2017-04-03  9:44 ` [PATCH 02/10] xen/arm: vpl011: Add new virtual console hvm params " Bhupinder Thakur
2017-04-19  0:22   ` Stefano Stabellini
2017-04-19  8:48     ` Bhupinder Thakur
2017-04-03  9:44 ` [PATCH 03/10] xen/arm: vpl011: Enable pl011 emulation for a guest domain " Bhupinder Thakur
2017-04-19  0:27   ` Stefano Stabellini
2017-04-03  9:44 ` [PATCH 04/10] xen/arm: vpl011: Provide a knob in libxl to enable/disable pl011 emulation Bhupinder Thakur
2017-04-12 16:32   ` Wei Liu
2017-04-13  8:19     ` Bhupinder Thakur
2017-04-13  8:37       ` Wei Liu
2017-04-19  0:29         ` Stefano Stabellini
2017-04-19  9:17           ` Bhupinder Thakur
2017-04-19 10:25             ` Wei Liu
2017-04-19 11:06               ` Julien Grall
2017-04-03  9:44 ` [PATCH 05/10] xen/arm: vpl011: Allocate a new PFN in the toolstack for the virtual console Bhupinder Thakur
2017-04-12 16:33   ` Wei Liu
2017-04-13  8:37     ` Bhupinder Thakur
2017-04-13  8:53       ` Wei Liu
2017-04-19  0:36         ` Stefano Stabellini
2017-04-19 10:28           ` Wei Liu
2017-04-19 11:01             ` Julien Grall [this message]
2017-04-19 13:05               ` Bhupinder Thakur
2017-04-19 13:35                 ` Julien Grall
2017-04-03  9:44 ` [PATCH 06/10] xen/arm: vpl011: Add new parameters to xenstore " Bhupinder Thakur
2017-04-12 16:32   ` Wei Liu
2017-04-25 10:18     ` Bhupinder Thakur
2017-04-25 11:55       ` Wei Liu
2017-04-19 18:58   ` Stefano Stabellini
2017-04-03  9:44 ` [PATCH 07/10] xen/arm: vpl011: Add a new console type to domain structure in xenconsole Bhupinder Thakur
2017-04-12 16:33   ` Wei Liu
2017-04-13  9:49     ` Bhupinder Thakur
2017-04-19 19:09   ` Stefano Stabellini
2017-04-03  9:44 ` [PATCH 08/10] xen/arm: vpl011: Modify the APIs in xenconsole to acces both PV and VCON consoles Bhupinder Thakur
2017-04-12 16:33   ` Wei Liu
2017-04-24 14:52     ` Bhupinder Thakur
2017-04-03  9:44 ` [PATCH 09/10] xen/arm: vpl011: Add new virtual console to xenconsole client Bhupinder Thakur
2017-04-19 18:55   ` Stefano Stabellini
2017-04-03  9:44 ` [PATCH 10/10] xen/arm: vpl011: Add a pl011 uart DT node in the guest device tree Bhupinder Thakur
2017-04-19 18:53   ` Stefano Stabellini
2017-04-20 12:47 ` [PATCH 00/10] pl011 emulation support in Xen Julien Grall
2017-04-26 15:21   ` Bhupinder Thakur
2017-04-26 17:09     ` Stefano Stabellini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=49ce6f5b-e368-2557-dfeb-e4a2f7b7141f@arm.com \
    --to=julien.grall@arm.com \
    --cc=bhupinder.thakur@linaro.org \
    --cc=ian.jackson@eu.citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.