Xen-Devel Archive on lore.kernel.org
 help / color / Atom feed
WARNING: multiple messages refer to this Message-ID
From: Julien Grall <julien.grall@arm.com>
To: Stefano Stabellini <sstabellini@kernel.org>,
	xen-devel@lists.xenproject.org
Cc: Stefano Stabellini <stefanos@xilinx.com>,
	ian.jackson@eu.citrix.com, wei.liu2@citrix.com,
	Jan Beulich <JBeulich@suse.com>
Subject: Re: [PATCH v2 05/10] libxl/xl: add memory policy option to iomem
Date: Wed, 1 May 2019 10:42:01 +0100
Message-ID: <ef154a23-8b1d-31be-ada5-769c76f73bd6@arm.com> (raw)
In-Reply-To: <1556658172-8824-5-git-send-email-sstabellini@kernel.org>

Hi,

On 30/04/2019 22:02, Stefano Stabellini wrote:
> Add a new memory policy option for the iomem parameter.
> Possible values are:
> - arm_devmem, device nGRE, the default on ARM
> - arm_memory, WB cachable memory
> - x86_uc: uncachable memory, the default on x86
> 
> Store the parameter in a new field in libxl_iomem_range.
> 
> Pass the memory policy option to xc_domain_mem_map_policy.
> 
> Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
> CC: ian.jackson@eu.citrix.com
> CC: wei.liu2@citrix.com
> ---
> Changes in v2:
> - add #define LIBXL_HAVE_MEMORY_POLICY
> - ability to part the memory policy parameter even if gfn is not passed
> - rename cache_policy to memory policy
> - rename MEMORY_POLICY_DEVMEM to MEMORY_POLICY_ARM_DEV_nGRE
> - rename MEMORY_POLICY_MEMORY to MEMORY_POLICY_ARM_MEM_WB
> - rename memory to arm_memory and devmem to arm_devmem
> - expand the non-security support status to non device passthrough iomem
>    configurations
> - rename iomem options
> - add x86 specific iomem option
> ---
>   SUPPORT.md                  |  2 +-
>   docs/man/xl.cfg.5.pod.in    |  7 ++++++-
>   tools/libxl/libxl.h         |  5 +++++
>   tools/libxl/libxl_create.c  | 21 +++++++++++++++++++--
>   tools/libxl/libxl_types.idl |  9 +++++++++
>   tools/xl/xl_parse.c         | 22 +++++++++++++++++++++-
>   6 files changed, 61 insertions(+), 5 deletions(-)
> 
> diff --git a/SUPPORT.md b/SUPPORT.md
> index e4fb15b..f29a299 100644
> --- a/SUPPORT.md
> +++ b/SUPPORT.md
> @@ -649,7 +649,7 @@ to be used in addition to QEMU.
>   
>   	Status: Experimental
>   
> -### ARM/Non-PCI device passthrough
> +### ARM/Non-PCI device passthrough and other iomem configurations

I am not sure why iomem is added here? Also what was the security support before 
hand? Was it supported?

>   
>       Status: Supported, not security supported
>   
> diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
> index c7d70e6..c85857e 100644
> --- a/docs/man/xl.cfg.5.pod.in
> +++ b/docs/man/xl.cfg.5.pod.in
> @@ -1222,7 +1222,7 @@ is given in hexadecimal format and may either be a range, e.g. C<2f8-2ff>
>   It is recommended to only use this option for trusted VMs under
>   administrator's control.
>   
> -=item B<iomem=[ "IOMEM_START,NUM_PAGES[@GFN]", "IOMEM_START,NUM_PAGES[@GFN]", ...]>
> +=item B<iomem=[ "IOMEM_START,NUM_PAGES[@GFN],MEMORY_POLICY", "IOMEM_START,NUM_PAGES[@GFN][,MEMORY_POLICY]", ...]>
>   
>   Allow auto-translated domains to access specific hardware I/O memory pages.
>   
> @@ -1233,6 +1233,11 @@ B<GFN> is not specified, the mapping will be performed using B<IOMEM_START>
>   as a start in the guest's address space, therefore performing a 1:1 mapping
>   by default.
>   All of these values must be given in hexadecimal format.
> +B<MEMORY_POLICY> for ARM platforms:
> +  - "arm_devmem" for Device nGRE, the default on ARM

This does not match the current default. At the moment, it is Device nGnRE.

> +  - "arm_memory" for Outer Shareable Write-Back Cacheable Memory

The two names are quite confusing and will make quite difficult to introduce any 
new one. It also make little sense to use a different naming in xl and libxl. 
This only add an other level of confusion.

Overall, this is not enough for a user to understand the memory policy. As I 
pointed out before, this is not straight forward on Arm as the resulting memory 
attribute will be a combination of stage-2 and stage-1.

We need to explain the implication of using the memory and the consequence of 
misuse it. And particularly as this is not security supported so we don't end up 
to security support in the future something that don't work.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

From: Julien Grall <julien.grall@arm.com>
To: Stefano Stabellini <sstabellini@kernel.org>,
	xen-devel@lists.xenproject.org
Cc: Stefano Stabellini <stefanos@xilinx.com>,
	ian.jackson@eu.citrix.com, wei.liu2@citrix.com,
	Jan Beulich <JBeulich@suse.com>
Subject: Re: [Xen-devel] [PATCH v2 05/10] libxl/xl: add memory policy option to iomem
Date: Wed, 1 May 2019 10:42:01 +0100
Message-ID: <ef154a23-8b1d-31be-ada5-769c76f73bd6@arm.com> (raw)
Message-ID: <20190501094201.sZJONZ_RmaG1fEJpVVSBTBXHweyvba7ohvwQLroxaD4@z> (raw)
In-Reply-To: <1556658172-8824-5-git-send-email-sstabellini@kernel.org>

Hi,

On 30/04/2019 22:02, Stefano Stabellini wrote:
> Add a new memory policy option for the iomem parameter.
> Possible values are:
> - arm_devmem, device nGRE, the default on ARM
> - arm_memory, WB cachable memory
> - x86_uc: uncachable memory, the default on x86
> 
> Store the parameter in a new field in libxl_iomem_range.
> 
> Pass the memory policy option to xc_domain_mem_map_policy.
> 
> Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
> CC: ian.jackson@eu.citrix.com
> CC: wei.liu2@citrix.com
> ---
> Changes in v2:
> - add #define LIBXL_HAVE_MEMORY_POLICY
> - ability to part the memory policy parameter even if gfn is not passed
> - rename cache_policy to memory policy
> - rename MEMORY_POLICY_DEVMEM to MEMORY_POLICY_ARM_DEV_nGRE
> - rename MEMORY_POLICY_MEMORY to MEMORY_POLICY_ARM_MEM_WB
> - rename memory to arm_memory and devmem to arm_devmem
> - expand the non-security support status to non device passthrough iomem
>    configurations
> - rename iomem options
> - add x86 specific iomem option
> ---
>   SUPPORT.md                  |  2 +-
>   docs/man/xl.cfg.5.pod.in    |  7 ++++++-
>   tools/libxl/libxl.h         |  5 +++++
>   tools/libxl/libxl_create.c  | 21 +++++++++++++++++++--
>   tools/libxl/libxl_types.idl |  9 +++++++++
>   tools/xl/xl_parse.c         | 22 +++++++++++++++++++++-
>   6 files changed, 61 insertions(+), 5 deletions(-)
> 
> diff --git a/SUPPORT.md b/SUPPORT.md
> index e4fb15b..f29a299 100644
> --- a/SUPPORT.md
> +++ b/SUPPORT.md
> @@ -649,7 +649,7 @@ to be used in addition to QEMU.
>   
>   	Status: Experimental
>   
> -### ARM/Non-PCI device passthrough
> +### ARM/Non-PCI device passthrough and other iomem configurations

I am not sure why iomem is added here? Also what was the security support before 
hand? Was it supported?

>   
>       Status: Supported, not security supported
>   
> diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
> index c7d70e6..c85857e 100644
> --- a/docs/man/xl.cfg.5.pod.in
> +++ b/docs/man/xl.cfg.5.pod.in
> @@ -1222,7 +1222,7 @@ is given in hexadecimal format and may either be a range, e.g. C<2f8-2ff>
>   It is recommended to only use this option for trusted VMs under
>   administrator's control.
>   
> -=item B<iomem=[ "IOMEM_START,NUM_PAGES[@GFN]", "IOMEM_START,NUM_PAGES[@GFN]", ...]>
> +=item B<iomem=[ "IOMEM_START,NUM_PAGES[@GFN],MEMORY_POLICY", "IOMEM_START,NUM_PAGES[@GFN][,MEMORY_POLICY]", ...]>
>   
>   Allow auto-translated domains to access specific hardware I/O memory pages.
>   
> @@ -1233,6 +1233,11 @@ B<GFN> is not specified, the mapping will be performed using B<IOMEM_START>
>   as a start in the guest's address space, therefore performing a 1:1 mapping
>   by default.
>   All of these values must be given in hexadecimal format.
> +B<MEMORY_POLICY> for ARM platforms:
> +  - "arm_devmem" for Device nGRE, the default on ARM

This does not match the current default. At the moment, it is Device nGnRE.

> +  - "arm_memory" for Outer Shareable Write-Back Cacheable Memory

The two names are quite confusing and will make quite difficult to introduce any 
new one. It also make little sense to use a different naming in xl and libxl. 
This only add an other level of confusion.

Overall, this is not enough for a user to understand the memory policy. As I 
pointed out before, this is not straight forward on Arm as the resulting memory 
attribute will be a combination of stage-2 and stage-1.

We need to explain the implication of using the memory and the consequence of 
misuse it. And particularly as this is not security supported so we don't end up 
to security support in the future something that don't work.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

  parent reply index

Thread overview: 86+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-30 21:02 [PATCH v2 0/10] iomem memory policy Stefano Stabellini
2019-04-30 21:02 ` [Xen-devel] " Stefano Stabellini
2019-04-30 21:02 ` [PATCH v2 01/10] xen: add a p2mt parameter to map_mmio_regions Stefano Stabellini
2019-04-30 21:02   ` [Xen-devel] " Stefano Stabellini
2019-05-02 14:59   ` Jan Beulich
2019-05-02 14:59     ` [Xen-devel] " Jan Beulich
2019-05-02 18:49     ` Stefano Stabellini
2019-05-02 18:49       ` [Xen-devel] " Stefano Stabellini
2019-05-15 13:39   ` Oleksandr
2019-05-15 13:39     ` [Xen-devel] " Oleksandr
2019-04-30 21:02 ` [PATCH v2 02/10] xen: rename un/map_mmio_regions to un/map_regions Stefano Stabellini
2019-04-30 21:02   ` [Xen-devel] " Stefano Stabellini
2019-05-01  9:22   ` Julien Grall
2019-05-01  9:22     ` [Xen-devel] " Julien Grall
2019-06-17 21:24     ` Stefano Stabellini
2019-06-18 11:05       ` Julien Grall
2019-06-18 20:19         ` Stefano Stabellini
2019-05-02 15:03   ` Jan Beulich
2019-05-02 15:03     ` [Xen-devel] " Jan Beulich
2019-05-02 18:55     ` Stefano Stabellini
2019-05-02 18:55       ` [Xen-devel] " Stefano Stabellini
2019-04-30 21:02 ` [PATCH v2 03/10] xen: extend XEN_DOMCTL_memory_mapping to handle memory policy Stefano Stabellini
2019-04-30 21:02   ` [Xen-devel] " Stefano Stabellini
2019-05-02 15:12   ` Jan Beulich
2019-05-02 15:12     ` [Xen-devel] " Jan Beulich
2019-06-17 21:28     ` Stefano Stabellini
2019-06-18  8:59       ` Jan Beulich
2019-06-18 20:32         ` Stefano Stabellini
2019-06-18 23:15           ` Stefano Stabellini
2019-06-19  6:53             ` Jan Beulich
2019-05-07 16:41   ` Julien Grall
2019-05-07 16:41     ` [Xen-devel] " Julien Grall
2019-06-17 22:43     ` Stefano Stabellini
2019-06-18 11:13       ` Julien Grall
2019-05-15 14:40   ` Oleksandr
2019-05-15 14:40     ` [Xen-devel] " Oleksandr
2019-04-30 21:02 ` [PATCH v2 04/10] libxc: introduce xc_domain_mem_map_policy Stefano Stabellini
2019-04-30 21:02   ` [Xen-devel] " Stefano Stabellini
2019-04-30 21:02 ` [PATCH v2 05/10] libxl/xl: add memory policy option to iomem Stefano Stabellini
2019-04-30 21:02   ` [Xen-devel] " Stefano Stabellini
2019-05-01  9:42   ` Julien Grall [this message]
2019-05-01  9:42     ` Julien Grall
2019-06-17 22:32     ` Stefano Stabellini
2019-06-18 11:09       ` Julien Grall
2019-06-18 11:15   ` Julien Grall
2019-06-18 22:07     ` Stefano Stabellini
2019-06-18 22:20       ` Julien Grall
2019-06-18 22:46         ` Stefano Stabellini
2019-04-30 21:02 ` [PATCH v2 06/10] xen/arm: extend device_tree_for_each_node Stefano Stabellini
2019-04-30 21:02   ` [Xen-devel] " Stefano Stabellini
2019-05-07 17:12   ` Julien Grall
2019-05-07 17:12     ` [Xen-devel] " Julien Grall
2019-04-30 21:02 ` [PATCH v2 07/10] xen/arm: make process_memory_node a device_tree_node_func Stefano Stabellini
2019-04-30 21:02   ` [Xen-devel] " Stefano Stabellini
2019-05-01  9:47   ` Julien Grall
2019-05-01  9:47     ` [Xen-devel] " Julien Grall
2019-04-30 21:02 ` [PATCH v2 08/10] xen/arm: keep track of reserved-memory regions Stefano Stabellini
2019-04-30 21:02   ` [Xen-devel] " Stefano Stabellini
2019-05-01 10:03   ` Julien Grall
2019-05-01 10:03     ` [Xen-devel] " Julien Grall
2019-06-21 23:47     ` Stefano Stabellini
2019-05-07 17:21   ` Julien Grall
2019-05-07 17:21     ` [Xen-devel] " Julien Grall
2019-04-30 21:02 ` [PATCH v2 09/10] xen/arm: map reserved-memory regions as normal memory in dom0 Stefano Stabellini
2019-04-30 21:02   ` [Xen-devel] " Stefano Stabellini
2019-05-07 19:52   ` Julien Grall
2019-05-07 19:52     ` [Xen-devel] " Julien Grall
2019-04-30 21:02 ` [PATCH v2 10/10] xen/arm: add reserved-memory regions to the dom0 memory node Stefano Stabellini
2019-04-30 21:02   ` [Xen-devel] " Stefano Stabellini
2019-05-07 20:15   ` Julien Grall
2019-05-07 20:15     ` [Xen-devel] " Julien Grall
2019-05-10 20:51     ` Stefano Stabellini
2019-05-10 20:51       ` [Xen-devel] " Stefano Stabellini
2019-05-10 21:43       ` Julien Grall
2019-05-10 21:43         ` [Xen-devel] " Julien Grall
2019-05-11 12:40         ` Julien Grall
2019-05-11 12:40           ` [Xen-devel] " Julien Grall
2019-05-20 21:26           ` Stefano Stabellini
2019-05-20 21:26             ` [Xen-devel] " Stefano Stabellini
2019-05-20 22:38             ` Julien Grall
2019-05-20 22:38               ` [Xen-devel] " Julien Grall
2019-06-05 16:30               ` Julien Grall
2019-06-21 23:47                 ` Stefano Stabellini
2019-05-16 16:52 ` [PATCH v2 0/10] iomem memory policy Oleksandr
2019-05-16 16:52   ` [Xen-devel] " Oleksandr
2019-06-21 23:48   ` Stefano Stabellini

Reply instructions:

You may reply publically 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=ef154a23-8b1d-31be-ada5-769c76f73bd6@arm.com \
    --to=julien.grall@arm.com \
    --cc=JBeulich@suse.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=stefanos@xilinx.com \
    --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

Xen-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/xen-devel/0 xen-devel/git/0.git
	git clone --mirror https://lore.kernel.org/xen-devel/1 xen-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 xen-devel xen-devel/ https://lore.kernel.org/xen-devel \
		xen-devel@lists.xenproject.org xen-devel@archiver.kernel.org
	public-inbox-index xen-devel


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.xenproject.lists.xen-devel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox