[RFC] DOMCTL_memattrs_op : a new DOMCTL to play with stage-2 page attributes

[RFC] DOMCTL_memattrs_op : a new DOMCTL to play with stage-2 page attributes

[Zhongze Liu]

********************************************************************************
   DOMCTL_memattrs_op : a new DOMCTL to play with stage-2 page attributes

                                              Zhongze Liu <blackskygg@gmail.com>

********************************************************************************
  Motivation and Description
  ~~~~~~~~~~~~~~~~~~~~~~~~~~

During the dicussion about the proposal "allow setting up shared memory areas
between VMs from xl config file" (see [1]), it's getting clear that when we
setup shared memory areas for VM communications from xl config file, we would
appreciate the ability to control the permissions and some attributes of the
shared memory pages: in the simplest the cases, regular cacheable RAM with read
write permissions will be enough (For ARM, it would be p2m_ram_rw and MATTR_MEM,
LPAE_SH_INNER). But there are also complicated cases where we might need deeper
control over the permissions, cacheability and shareability of the shared RAM
pages to meet some extra requirements (see [2]). And this could be done via
playing with the stage-2 page tables, on both x86 and ARM.

So there comes to the need for a DOMCTL that can set the permissions and
attributes (currently, only cacheability and shareability is in the plan) of a
given RAM page in the stage-2 page talbes. The only related work can be seen so
far is DOMCTL_pin_mem_cacheattr (see [3]), which is for controlling the
cacheability of pages and is x86 HVM only. There seems to be no arch-neutral
DOMCTL interfaces that can meet our requirements.

That's why we need a new arch-neutral DOMCTL, which is tentatively called
DOMCTL_mem_attrs_op in this proposal and would enable us to control the access
permissions, cacheability and shareability (ARM only) attributes of RAM pages.

********************************************************************************
  Interface Specification
  ~~~~~~~~~~~~~~~~~~~~~~~

A current draft of the interface looks like this:

/*
 * Set access permissions, cacheability and shareability (ARM only) of a
 * continuos range of normal memory (RAM) in the stage-2 page table.
 */
/* XEN_DOMCTL_memattrs_op */

/* set chacheability and shareability */
#define XEN_DOMCTL_MEMATTRS_OP_SET_CACHEATTRS  1
/* set access permissions */
#define XEN_DOMCTL_MEMATTRS_OP_SET_PERMISSIONS 2
/* get chacheability and shareability */
#define XEN_DOMCTL_MEMATTRS_OP_GET_CACHEATTRS  1
/* get access permissions */
#define XEN_DOMCTL_MEMATTRS_OP_GET_PERMISSIONS 2

/* flags for XEN_DOMCTL_MEMATTRS_OP_SET_CACHEATTRS */
/* chacheability flags, the values happen to be the same with those in
 * x86 PAT.  (See [4])
 */
/* uncacheable */
#define XEN_DOMCTL_MEMATTRS_UC         0x00U
/* write combine, x86 only */
#define XEN_DOMCTL_MEMATTRS_CACHE_WC   0x01U
/* write through */
#define XEN_DOMCTL_MEMATTRS_CACHE_WT   0x04U
/* write protect, x86 only */
#define XEN_DOMCTL_MEMATTRS_CACHE_WP   0x05U
/* write back */
#define XEN_DOMCTL_MEMATTRS_CACHE_WB   0x06U
/* strong uncacheable, x86 only*/
#define XEN_DOMCTL_MEMATTRS_SUC        0x07U

/* shareability flags (See [5]), arm only, the value is taken from
 * asm-arm/page.h, but live in the second 8-bit.
 */
#define XEN_DOMCTL_MEMATTRS_SHAREABILITY_SHIFT 8
#define XEN_DOMCTL_MEMATTRS_SH_NON_SHAREABLE (LPAE_SH_NON_SHAREABLE<<8)
#define XEN_DOMCTL_MEMATTRS_SH_UNPREDICTALE  (LPAE_SH_UNPREDICTALE<<8)
#define XEN_DOMCTL_MEMATTRS_SH_OUTER         (LPAE_SH_OUTER<<8)
#define XEN_DOMCTL_MEMATTRS_SH_INNER         (LPAE_SH_INNER<<8)

/* flags for XEN_DOMCTL_MEMATTRS_OP_SET_PERMISSIONS */
#define XEN_DOMCTL_MEMATTRS_ACCESS_N         0x00U
#define XEN_DOMCTL_MEMATTRS_ACCESS_R         (0x01U<<0)
#define XEN_DOMCTL_MEMATTRS_ACCESS_W         (0x01U<<1)
#define XEN_DOMCTL_MEMATTRS_ACCESS_X         (0x01U<<2)
#define XEN_DOMCTL_MEMATTRS_ACCESS_RW        \
(XEN_DOMCTL_MEMATTRS_ACCESS_R|XEN_DOMCTL_MEMATTRS_ACCESS_W)
#define XEN_DOMCTL_MEMATTRS_ACCESS_RX        \
(XEN_DOMCTL_MEMATTRS_ACCESS_R|XEN_DOMCTL_MEMATTRS_ACCESS_X)
#define XEN_DOMCTL_MEMATTRS_ACCESS_WX        \
(XEN_DOMCTL_MEMATTRS_ACCESS_W|XEN_DOMCTL_MEMATTRS_ACCESS_X)
#define XEN_DOMCTL_MEMATTRS_ACCESS_RWX        \
(XEN_DOMCTL_MEMATTRS_ACCESS_RW|XEN_DOMCTL_MEMATTRS_ACCESS_X)

struct xen_domctl_memattrs_op {
  int op;                 /* IN XEN_DOMCTL_MEMATTRS_OP_* */
  xen_pfn_t first_gfn;    /* IN first page in range */
  uint32_t nr_gfns;       /* IN number of pages in range */

  XEN_GUEST_HANDLE(uint32_t) attrs;  /* IN/OUT per-page attrs */

  XEN_GUEST_HANDLE(int) errs;   /* OUT Per gfn error code */
}


  Notes
  ~~~~~
Since neither x86 nor arm support all the cache/share flags above, the
function will return an err if the one or more flags given by the caller are
not supported.


********************************************************************************
  References
  ~~~~~~~~~~
[1] [RFC]Proposal to allow setting up shared memory areas between VMs
from xl config file
    v2: https://lists.xen.org/archives/html/xen-devel/2017-06/msg02256.html
    v1: https://lists.xen.org/archives/html/xen-devel/2017-05/msg01288.html
[2] https://lists.xen.org/archives/html/xen-devel/2017-06/msg02918.html
[3] http://xenbits.xen.org/hg/staging/xen-unstable.hg/file/fe6c71e5586b/xen/include/public/domctl.h#l621
[4] Intel® 64 and IA-32 Architectures Software Developer’s Manual,
Volume 3, 11.3
[5] ARM® Architecture Reference Manual - ARMv8, for ARMv8-A
architecture profile(Issue B.a), B2.7.1

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

Re: [RFC] DOMCTL_memattrs_op : a new DOMCTL to play with stage-2 page attributes

[Stefano Stabellini]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 7271 bytes --]

On Sat, 1 Jul 2017, Zhongze Liu wrote:
> ********************************************************************************
>    DOMCTL_memattrs_op : a new DOMCTL to play with stage-2 page attributes
> 
>                                               Zhongze Liu <blackskygg@gmail.com>
> 
> ********************************************************************************
>   Motivation and Description
>   ~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> During the dicussion about the proposal "allow setting up shared memory areas
> between VMs from xl config file" (see [1]), it's getting clear that when we
> setup shared memory areas for VM communications from xl config file, we would
> appreciate the ability to control the permissions and some attributes of the
> shared memory pages: in the simplest the cases, regular cacheable RAM with read
                                      ^ of

> write permissions will be enough (For ARM, it would be p2m_ram_rw and MATTR_MEM,
> LPAE_SH_INNER). But there are also complicated cases where we might need deeper
> control over the permissions, cacheability and shareability of the shared RAM
> pages to meet some extra requirements (see [2]). And this could be done via
> playing with the stage-2 page tables, on both x86 and ARM.
> 
> So there comes to the need for a DOMCTL that can set the permissions and
> attributes (currently, only cacheability and shareability is in the plan) of a
> given RAM page in the stage-2 page talbes. The only related work can be seen so
                                      ^ tables


> far is DOMCTL_pin_mem_cacheattr (see [3]), which is for controlling the
> cacheability of pages and is x86 HVM only. There seems to be no arch-neutral
> DOMCTL interfaces that can meet our requirements.
> 
> That's why we need a new arch-neutral DOMCTL, which is tentatively called
> DOMCTL_mem_attrs_op in this proposal and would enable us to control the access
> permissions, cacheability and shareability (ARM only) attributes of RAM pages.
> 
> ********************************************************************************
>   Interface Specification
>   ~~~~~~~~~~~~~~~~~~~~~~~
> 
> A current draft of the interface looks like this:
> 
> /*
>  * Set access permissions, cacheability and shareability (ARM only) of a
>  * continuos range of normal memory (RAM) in the stage-2 page table.
>  */
> /* XEN_DOMCTL_memattrs_op */
> 
> /* set chacheability and shareability */
            ^ cacheability


> #define XEN_DOMCTL_MEMATTRS_OP_SET_CACHEATTRS  1
> /* set access permissions */
> #define XEN_DOMCTL_MEMATTRS_OP_SET_PERMISSIONS 2
> /* get chacheability and shareability */
            ^ cacheability

> #define XEN_DOMCTL_MEMATTRS_OP_GET_CACHEATTRS  1
> /* get access permissions */
> #define XEN_DOMCTL_MEMATTRS_OP_GET_PERMISSIONS 2
> 
> /* flags for XEN_DOMCTL_MEMATTRS_OP_SET_CACHEATTRS */
> /* chacheability flags, the values happen to be the same with those in
        ^ cacheability

>  * x86 PAT.  (See [4])
>  */
> /* uncacheable */
> #define XEN_DOMCTL_MEMATTRS_UC         0x00U
> /* write combine, x86 only */
> #define XEN_DOMCTL_MEMATTRS_CACHE_WC   0x01U
> /* write through */
> #define XEN_DOMCTL_MEMATTRS_CACHE_WT   0x04U
> /* write protect, x86 only */
> #define XEN_DOMCTL_MEMATTRS_CACHE_WP   0x05U
> /* write back */
> #define XEN_DOMCTL_MEMATTRS_CACHE_WB   0x06U
> /* strong uncacheable, x86 only*/
> #define XEN_DOMCTL_MEMATTRS_SUC        0x07U

On the ARM side, we are missing BUFFERABLE and WRITEALLOC. I don't know
how they map to these tags, which comes from the x86 world. Maybe we
should just add them separately as ARM only, like:

  /* bufferable, ARM only */
  #define XEN_DOMCTL_MEMATTRS_BUFFERABLE 0x08U
  /* write alloc, ARM only */
  #define XEN_DOMCTL_MEMATTRS_CACHE_WA   0x09U

Theoretically, we could say XEN_DOMCTL_MEMATTRS_UC means "BUFFERABLE" on
ARM and XEN_DOMCTL_MEMATTRS_SUC means "UNCACHED", because that's
actually what they correspond to I think. However using x86 names for
ARM caching attributes is very confusing and error prone. So I would
prefer introducing separate tags for ARM and x86. However, reusing
XEN_DOMCTL_MEMATTRS_UC, XEN_DOMCTL_MEMATTRS_CACHE_WT and
XEN_DOMCTL_MEMATTRS_CACHE_WB as Zhongze did in this proposal would be OK
for me.

Julien, what do you think?


> /* shareability flags (See [5]), arm only, the value is taken from
>  * asm-arm/page.h, but live in the second 8-bit.
>  */
> #define XEN_DOMCTL_MEMATTRS_SHAREABILITY_SHIFT 8
> #define XEN_DOMCTL_MEMATTRS_SH_NON_SHAREABLE (LPAE_SH_NON_SHAREABLE<<8)
> #define XEN_DOMCTL_MEMATTRS_SH_UNPREDICTALE  (LPAE_SH_UNPREDICTALE<<8)

We don't need UNPREDICTALE as a possible value :-)


> #define XEN_DOMCTL_MEMATTRS_SH_OUTER         (LPAE_SH_OUTER<<8)
> #define XEN_DOMCTL_MEMATTRS_SH_INNER         (LPAE_SH_INNER<<8)
> 
> /* flags for XEN_DOMCTL_MEMATTRS_OP_SET_PERMISSIONS */
> #define XEN_DOMCTL_MEMATTRS_ACCESS_N         0x00U
> #define XEN_DOMCTL_MEMATTRS_ACCESS_R         (0x01U<<0)
> #define XEN_DOMCTL_MEMATTRS_ACCESS_W         (0x01U<<1)
> #define XEN_DOMCTL_MEMATTRS_ACCESS_X         (0x01U<<2)
> #define XEN_DOMCTL_MEMATTRS_ACCESS_RW        \
> (XEN_DOMCTL_MEMATTRS_ACCESS_R|XEN_DOMCTL_MEMATTRS_ACCESS_W)
> #define XEN_DOMCTL_MEMATTRS_ACCESS_RX        \
> (XEN_DOMCTL_MEMATTRS_ACCESS_R|XEN_DOMCTL_MEMATTRS_ACCESS_X)
> #define XEN_DOMCTL_MEMATTRS_ACCESS_WX        \
> (XEN_DOMCTL_MEMATTRS_ACCESS_W|XEN_DOMCTL_MEMATTRS_ACCESS_X)
> #define XEN_DOMCTL_MEMATTRS_ACCESS_RWX        \
> (XEN_DOMCTL_MEMATTRS_ACCESS_RW|XEN_DOMCTL_MEMATTRS_ACCESS_X)
> 
> struct xen_domctl_memattrs_op {
>   int op;                 /* IN XEN_DOMCTL_MEMATTRS_OP_* */

uint32_t: we only use explicitly sized integers in hypercalls


>   xen_pfn_t first_gfn;    /* IN first page in range */
>   uint32_t nr_gfns;       /* IN number of pages in range */
> 
>   XEN_GUEST_HANDLE(uint32_t) attrs;  /* IN/OUT per-page attrs */

XEN_GUEST_HANDLE is used for pointers in struct (typically for arrays).
In this case, I don't think we need a pointer, we could just have a
single uint32_t to specify the permissions and attributes for all the
pages in the range.


>   XEN_GUEST_HANDLE(int) errs;   /* OUT Per gfn error code */

I am not sure we need a pointer for the errors either. We could have a
single integer to express the error number and the page that caused it.


> }
> 
> 
>   Notes
>   ~~~~~
> Since neither x86 nor arm support all the cache/share flags above, the
> function will return an err if the one or more flags given by the caller are
> not supported.
> 
> 
> ********************************************************************************
>   References
>   ~~~~~~~~~~
> [1] [RFC]Proposal to allow setting up shared memory areas between VMs
> from xl config file
>     v2: https://lists.xen.org/archives/html/xen-devel/2017-06/msg02256.html
>     v1: https://lists.xen.org/archives/html/xen-devel/2017-05/msg01288.html
> [2] https://lists.xen.org/archives/html/xen-devel/2017-06/msg02918.html
> [3] http://xenbits.xen.org/hg/staging/xen-unstable.hg/file/fe6c71e5586b/xen/include/public/domctl.h#l621
> [4] Intel® 64 and IA-32 Architectures Software Developer’s Manual,
> Volume 3, 11.3
> [5] ARM® Architecture Reference Manual - ARMv8, for ARMv8-A
> architecture profile(Issue B.a), B2.7.1
> 

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

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

Re: [RFC] DOMCTL_memattrs_op : a new DOMCTL to play with stage-2 page attributes

[Zhongze Liu]

It seems that I fed the wrong patch to scripts/get_maintainers.pl.
Sorry for mistakenly Cc'ing George, Konrad, and Tim. Apologies for this sudden
mail in your inbox.

Cheers,

Zhongze Liu

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

Re: [RFC] DOMCTL_memattrs_op : a new DOMCTL to play with stage-2 page attributes

[Zhongze Liu]

Hi Stefano,

Added Julien and removed those who are mistakenly Cc'ed    :-)
will never try to draft emails half asleep again.

2017-07-01 5:48 GMT+08:00 Stefano Stabellini <sstabellini@kernel.org>:
> On Sat, 1 Jul 2017, Zhongze Liu wrote:
>> ********************************************************************************
>>    DOMCTL_memattrs_op : a new DOMCTL to play with stage-2 page attributes
>>
>>                                               Zhongze Liu <blackskygg@gmail.com>
>>
>> ********************************************************************************
>>   Motivation and Description
>>   ~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> During the dicussion about the proposal "allow setting up shared memory areas
>> between VMs from xl config file" (see [1]), it's getting clear that when we
>> setup shared memory areas for VM communications from xl config file, we would
>> appreciate the ability to control the permissions and some attributes of the
>> shared memory pages: in the simplest the cases, regular cacheable RAM with read
>                                       ^ of
>
>> write permissions will be enough (For ARM, it would be p2m_ram_rw and MATTR_MEM,
>> LPAE_SH_INNER). But there are also complicated cases where we might need deeper
>> control over the permissions, cacheability and shareability of the shared RAM
>> pages to meet some extra requirements (see [2]). And this could be done via
>> playing with the stage-2 page tables, on both x86 and ARM.
>>
>> So there comes to the need for a DOMCTL that can set the permissions and
>> attributes (currently, only cacheability and shareability is in the plan) of a
>> given RAM page in the stage-2 page talbes. The only related work can be seen so
>                                       ^ tables
>

Sorry for all the typos in this proposal. I'll fix them later.

>
>> far is DOMCTL_pin_mem_cacheattr (see [3]), which is for controlling the
>> cacheability of pages and is x86 HVM only. There seems to be no arch-neutral
>> DOMCTL interfaces that can meet our requirements.
>>
>> That's why we need a new arch-neutral DOMCTL, which is tentatively called
>> DOMCTL_mem_attrs_op in this proposal and would enable us to control the access
>> permissions, cacheability and shareability (ARM only) attributes of RAM pages.
>>
>> ********************************************************************************
>>   Interface Specification
>>   ~~~~~~~~~~~~~~~~~~~~~~~
>>
>> A current draft of the interface looks like this:
>>
>> /*
>>  * Set access permissions, cacheability and shareability (ARM only) of a
>>  * continuos range of normal memory (RAM) in the stage-2 page table.
>>  */
>> /* XEN_DOMCTL_memattrs_op */
>>
>> /* set chacheability and shareability */
>             ^ cacheability
>
>
>> #define XEN_DOMCTL_MEMATTRS_OP_SET_CACHEATTRS  1
>> /* set access permissions */
>> #define XEN_DOMCTL_MEMATTRS_OP_SET_PERMISSIONS 2
>> /* get chacheability and shareability */
>             ^ cacheability
>
>> #define XEN_DOMCTL_MEMATTRS_OP_GET_CACHEATTRS  1
>> /* get access permissions */
>> #define XEN_DOMCTL_MEMATTRS_OP_GET_PERMISSIONS 2
>>
>> /* flags for XEN_DOMCTL_MEMATTRS_OP_SET_CACHEATTRS */
>> /* chacheability flags, the values happen to be the same with those in
>         ^ cacheability
>
>>  * x86 PAT.  (See [4])
>>  */
>> /* uncacheable */
>> #define XEN_DOMCTL_MEMATTRS_UC         0x00U
>> /* write combine, x86 only */
>> #define XEN_DOMCTL_MEMATTRS_CACHE_WC   0x01U
>> /* write through */
>> #define XEN_DOMCTL_MEMATTRS_CACHE_WT   0x04U
>> /* write protect, x86 only */
>> #define XEN_DOMCTL_MEMATTRS_CACHE_WP   0x05U
>> /* write back */
>> #define XEN_DOMCTL_MEMATTRS_CACHE_WB   0x06U
>> /* strong uncacheable, x86 only*/
>> #define XEN_DOMCTL_MEMATTRS_SUC        0x07U
>
> On the ARM side, we are missing BUFFERABLE and WRITEALLOC. I don't know
> how they map to these tags, which comes from the x86 world. Maybe we
> should just add them separately as ARM only, like:
>
>   /* bufferable, ARM only */
>   #define XEN_DOMCTL_MEMATTRS_BUFFERABLE 0x08U
>   /* write alloc, ARM only */
>   #define XEN_DOMCTL_MEMATTRS_CACHE_WA   0x09U
>
> Theoretically, we could say XEN_DOMCTL_MEMATTRS_UC means "BUFFERABLE" on
> ARM and XEN_DOMCTL_MEMATTRS_SUC means "UNCACHED", because that's
> actually what they correspond to I think. However using x86 names for
> ARM caching attributes is very confusing and error prone. So I would
> prefer introducing separate tags for ARM and x86. However, reusing
> XEN_DOMCTL_MEMATTRS_UC, XEN_DOMCTL_MEMATTRS_CACHE_WT and
> XEN_DOMCTL_MEMATTRS_CACHE_WB as Zhongze did in this proposal would be OK
> for me.
>
> Julien, what do you think?
>

sorry for missing the 'write-allocate' flag for ARM. I agree with you
in adding some
ARM-only flags, coz using x86 terminologies does look confusing. But
let's hear what other
maintainers say.

>
>> /* shareability flags (See [5]), arm only, the value is taken from
>>  * asm-arm/page.h, but live in the second 8-bit.
>>  */
>> #define XEN_DOMCTL_MEMATTRS_SHAREABILITY_SHIFT 8
>> #define XEN_DOMCTL_MEMATTRS_SH_NON_SHAREABLE (LPAE_SH_NON_SHAREABLE<<8)
>> #define XEN_DOMCTL_MEMATTRS_SH_UNPREDICTALE  (LPAE_SH_UNPREDICTALE<<8)
>
> We don't need UNPREDICTALE as a possible value :-)
>

Will remove this.

>
>> #define XEN_DOMCTL_MEMATTRS_SH_OUTER         (LPAE_SH_OUTER<<8)
>> #define XEN_DOMCTL_MEMATTRS_SH_INNER         (LPAE_SH_INNER<<8)
>>
>> /* flags for XEN_DOMCTL_MEMATTRS_OP_SET_PERMISSIONS */
>> #define XEN_DOMCTL_MEMATTRS_ACCESS_N         0x00U
>> #define XEN_DOMCTL_MEMATTRS_ACCESS_R         (0x01U<<0)
>> #define XEN_DOMCTL_MEMATTRS_ACCESS_W         (0x01U<<1)
>> #define XEN_DOMCTL_MEMATTRS_ACCESS_X         (0x01U<<2)
>> #define XEN_DOMCTL_MEMATTRS_ACCESS_RW        \
>> (XEN_DOMCTL_MEMATTRS_ACCESS_R|XEN_DOMCTL_MEMATTRS_ACCESS_W)
>> #define XEN_DOMCTL_MEMATTRS_ACCESS_RX        \
>> (XEN_DOMCTL_MEMATTRS_ACCESS_R|XEN_DOMCTL_MEMATTRS_ACCESS_X)
>> #define XEN_DOMCTL_MEMATTRS_ACCESS_WX        \
>> (XEN_DOMCTL_MEMATTRS_ACCESS_W|XEN_DOMCTL_MEMATTRS_ACCESS_X)
>> #define XEN_DOMCTL_MEMATTRS_ACCESS_RWX        \
>> (XEN_DOMCTL_MEMATTRS_ACCESS_RW|XEN_DOMCTL_MEMATTRS_ACCESS_X)
>>
>> struct xen_domctl_memattrs_op {
>>   int op;                 /* IN XEN_DOMCTL_MEMATTRS_OP_* */
>
> uint32_t: we only use explicitly sized integers in hypercalls
>

Ok, I'll make it an uint32_t.

>
>>   xen_pfn_t first_gfn;    /* IN first page in range */
>>   uint32_t nr_gfns;       /* IN number of pages in range */
>>
>>   XEN_GUEST_HANDLE(uint32_t) attrs;  /* IN/OUT per-page attrs */
>
> XEN_GUEST_HANDLE is used for pointers in struct (typically for arrays).
> In this case, I don't think we need a pointer, we could just have a
> single uint32_t to specify the permissions and attributes for all the
> pages in the range.
>

I'm not sure about this.
I think using an array here and below will make the hypercall more flexible --
similar to XENMEM_add_to_physmap_batch.
But according to our needs, using one attr parameter for the whole range
actually makes the whole thing more handy.

>
>>   XEN_GUEST_HANDLE(int) errs;   /* OUT Per gfn error code */
>
> I am not sure we need a pointer for the errors either. We could have a
> single integer to express the error number and the page that caused it.
>
>
>> }
>>
>>
>>   Notes
>>   ~~~~~
>> Since neither x86 nor arm support all the cache/share flags above, the
>> function will return an err if the one or more flags given by the caller are
>> not supported.
>>
>>
>> ********************************************************************************
>>   References
>>   ~~~~~~~~~~
>> [1] [RFC]Proposal to allow setting up shared memory areas between VMs
>> from xl config file
>>     v2: https://lists.xen.org/archives/html/xen-devel/2017-06/msg02256.html
>>     v1: https://lists.xen.org/archives/html/xen-devel/2017-05/msg01288.html
>> [2] https://lists.xen.org/archives/html/xen-devel/2017-06/msg02918.html
>> [3] http://xenbits.xen.org/hg/staging/xen-unstable.hg/file/fe6c71e5586b/xen/include/public/domctl.h#l621
>> [4] Intel® 64 and IA-32 Architectures Software Developer’s Manual,
>> Volume 3, 11.3
>> [5] ARM® Architecture Reference Manual - ARMv8, for ARMv8-A
>> architecture profile(Issue B.a), B2.7.1
>>

Cheers,

Zhongze Liu.

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

Re: [RFC] DOMCTL_memattrs_op : a new DOMCTL to play with stage-2 page attributes

[Zhongze Liu]

2017-07-01 17:16 GMT+08:00 Zhongze Liu <blackskygg@gmail.com>:
> Hi Stefano,
>
> Added Julien and removed those who are mistakenly Cc'ed    :-)
> will never try to draft emails half asleep again.
>
> 2017-07-01 5:48 GMT+08:00 Stefano Stabellini <sstabellini@kernel.org>:
>> On Sat, 1 Jul 2017, Zhongze Liu wrote:
>>> ********************************************************************************
>>>    DOMCTL_memattrs_op : a new DOMCTL to play with stage-2 page attributes
>>>
>>>                                               Zhongze Liu <blackskygg@gmail.com>
>>>
>>> ********************************************************************************
>>>   Motivation and Description
>>>   ~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>
>>> During the dicussion about the proposal "allow setting up shared memory areas
>>> between VMs from xl config file" (see [1]), it's getting clear that when we
>>> setup shared memory areas for VM communications from xl config file, we would
>>> appreciate the ability to control the permissions and some attributes of the
>>> shared memory pages: in the simplest the cases, regular cacheable RAM with read
>>                                       ^ of
>>
>>> write permissions will be enough (For ARM, it would be p2m_ram_rw and MATTR_MEM,
>>> LPAE_SH_INNER). But there are also complicated cases where we might need deeper
>>> control over the permissions, cacheability and shareability of the shared RAM
>>> pages to meet some extra requirements (see [2]). And this could be done via
>>> playing with the stage-2 page tables, on both x86 and ARM.
>>>
>>> So there comes to the need for a DOMCTL that can set the permissions and
>>> attributes (currently, only cacheability and shareability is in the plan) of a
>>> given RAM page in the stage-2 page talbes. The only related work can be seen so
>>                                       ^ tables
>>
>
> Sorry for all the typos in this proposal. I'll fix them later.
>
>>
>>> far is DOMCTL_pin_mem_cacheattr (see [3]), which is for controlling the
>>> cacheability of pages and is x86 HVM only. There seems to be no arch-neutral
>>> DOMCTL interfaces that can meet our requirements.
>>>
>>> That's why we need a new arch-neutral DOMCTL, which is tentatively called
>>> DOMCTL_mem_attrs_op in this proposal and would enable us to control the access
>>> permissions, cacheability and shareability (ARM only) attributes of RAM pages.
>>>
>>> ********************************************************************************
>>>   Interface Specification
>>>   ~~~~~~~~~~~~~~~~~~~~~~~
>>>
>>> A current draft of the interface looks like this:
>>>
>>> /*
>>>  * Set access permissions, cacheability and shareability (ARM only) of a
>>>  * continuos range of normal memory (RAM) in the stage-2 page table.
>>>  */
>>> /* XEN_DOMCTL_memattrs_op */
>>>
>>> /* set chacheability and shareability */
>>             ^ cacheability
>>
>>
>>> #define XEN_DOMCTL_MEMATTRS_OP_SET_CACHEATTRS  1
>>> /* set access permissions */
>>> #define XEN_DOMCTL_MEMATTRS_OP_SET_PERMISSIONS 2
>>> /* get chacheability and shareability */
>>             ^ cacheability
>>
>>> #define XEN_DOMCTL_MEMATTRS_OP_GET_CACHEATTRS  1
>>> /* get access permissions */
>>> #define XEN_DOMCTL_MEMATTRS_OP_GET_PERMISSIONS 2
>>>
>>> /* flags for XEN_DOMCTL_MEMATTRS_OP_SET_CACHEATTRS */
>>> /* chacheability flags, the values happen to be the same with those in
>>         ^ cacheability
>>
>>>  * x86 PAT.  (See [4])
>>>  */
>>> /* uncacheable */
>>> #define XEN_DOMCTL_MEMATTRS_UC         0x00U
>>> /* write combine, x86 only */
>>> #define XEN_DOMCTL_MEMATTRS_CACHE_WC   0x01U
>>> /* write through */
>>> #define XEN_DOMCTL_MEMATTRS_CACHE_WT   0x04U
>>> /* write protect, x86 only */
>>> #define XEN_DOMCTL_MEMATTRS_CACHE_WP   0x05U
>>> /* write back */
>>> #define XEN_DOMCTL_MEMATTRS_CACHE_WB   0x06U
>>> /* strong uncacheable, x86 only*/
>>> #define XEN_DOMCTL_MEMATTRS_SUC        0x07U
>>
>> On the ARM side, we are missing BUFFERABLE and WRITEALLOC. I don't know
>> how they map to these tags, which comes from the x86 world. Maybe we
>> should just add them separately as ARM only, like:
>>
>>   /* bufferable, ARM only */
>>   #define XEN_DOMCTL_MEMATTRS_BUFFERABLE 0x08U
>>   /* write alloc, ARM only */
>>   #define XEN_DOMCTL_MEMATTRS_CACHE_WA   0x09U
>>
>> Theoretically, we could say XEN_DOMCTL_MEMATTRS_UC means "BUFFERABLE" on
>> ARM and XEN_DOMCTL_MEMATTRS_SUC means "UNCACHED", because that's
>> actually what they correspond to I think. However using x86 names for
>> ARM caching attributes is very confusing and error prone. So I would
>> prefer introducing separate tags for ARM and x86. However, reusing
>> XEN_DOMCTL_MEMATTRS_UC, XEN_DOMCTL_MEMATTRS_CACHE_WT and
>> XEN_DOMCTL_MEMATTRS_CACHE_WB as Zhongze did in this proposal would be OK
>> for me.
>>
>> Julien, what do you think?
>>
>
> sorry for missing the 'write-allocate' flag for ARM. I agree with you
> in adding some
> ARM-only flags, coz using x86 terminologies does look confusing. But
> let's hear what other
> maintainers say.
>
>>
>>> /* shareability flags (See [5]), arm only, the value is taken from
>>>  * asm-arm/page.h, but live in the second 8-bit.
>>>  */
>>> #define XEN_DOMCTL_MEMATTRS_SHAREABILITY_SHIFT 8
>>> #define XEN_DOMCTL_MEMATTRS_SH_NON_SHAREABLE (LPAE_SH_NON_SHAREABLE<<8)
>>> #define XEN_DOMCTL_MEMATTRS_SH_UNPREDICTALE  (LPAE_SH_UNPREDICTALE<<8)
>>
>> We don't need UNPREDICTALE as a possible value :-)
>>
>
> Will remove this.
>
>>
>>> #define XEN_DOMCTL_MEMATTRS_SH_OUTER         (LPAE_SH_OUTER<<8)
>>> #define XEN_DOMCTL_MEMATTRS_SH_INNER         (LPAE_SH_INNER<<8)
>>>
>>> /* flags for XEN_DOMCTL_MEMATTRS_OP_SET_PERMISSIONS */
>>> #define XEN_DOMCTL_MEMATTRS_ACCESS_N         0x00U
>>> #define XEN_DOMCTL_MEMATTRS_ACCESS_R         (0x01U<<0)
>>> #define XEN_DOMCTL_MEMATTRS_ACCESS_W         (0x01U<<1)
>>> #define XEN_DOMCTL_MEMATTRS_ACCESS_X         (0x01U<<2)
>>> #define XEN_DOMCTL_MEMATTRS_ACCESS_RW        \
>>> (XEN_DOMCTL_MEMATTRS_ACCESS_R|XEN_DOMCTL_MEMATTRS_ACCESS_W)
>>> #define XEN_DOMCTL_MEMATTRS_ACCESS_RX        \
>>> (XEN_DOMCTL_MEMATTRS_ACCESS_R|XEN_DOMCTL_MEMATTRS_ACCESS_X)
>>> #define XEN_DOMCTL_MEMATTRS_ACCESS_WX        \
>>> (XEN_DOMCTL_MEMATTRS_ACCESS_W|XEN_DOMCTL_MEMATTRS_ACCESS_X)
>>> #define XEN_DOMCTL_MEMATTRS_ACCESS_RWX        \
>>> (XEN_DOMCTL_MEMATTRS_ACCESS_RW|XEN_DOMCTL_MEMATTRS_ACCESS_X)
>>>
>>> struct xen_domctl_memattrs_op {
>>>   int op;                 /* IN XEN_DOMCTL_MEMATTRS_OP_* */
>>
>> uint32_t: we only use explicitly sized integers in hypercalls
>>
>
> Ok, I'll make it an uint32_t.
>
>>
>>>   xen_pfn_t first_gfn;    /* IN first page in range */
>>>   uint32_t nr_gfns;       /* IN number of pages in range */
>>>
>>>   XEN_GUEST_HANDLE(uint32_t) attrs;  /* IN/OUT per-page attrs */
>>
>> XEN_GUEST_HANDLE is used for pointers in struct (typically for arrays).
>> In this case, I don't think we need a pointer, we could just have a
>> single uint32_t to specify the permissions and attributes for all the
>> pages in the range.
>>
>
> I'm not sure about this.
> I think using an array here and below will make the hypercall more flexible --
> similar to XENMEM_add_to_physmap_batch.
> But according to our needs, using one attr parameter for the whole range
> actually makes the whole thing more handy.

But given that the hypercall also supports the *_GET_* operations,
using an array here seems to be more reasonable.


Cheers,

Zhongze Liu

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

Re: [RFC] DOMCTL_memattrs_op : a new DOMCTL to play with stage-2 page attributes

[Julien Grall]

Hi,

On 01/07/17 10:16, Zhongze Liu wrote:
>> On the ARM side, we are missing BUFFERABLE and WRITEALLOC. I don't know
>> how they map to these tags, which comes from the x86 world. Maybe we
>> should just add them separately as ARM only, like:
>>
>>   /* bufferable, ARM only */
>>   #define XEN_DOMCTL_MEMATTRS_BUFFERABLE 0x08U
>>   /* write alloc, ARM only */
>>   #define XEN_DOMCTL_MEMATTRS_CACHE_WA   0x09U
>>
>> Theoretically, we could say XEN_DOMCTL_MEMATTRS_UC means "BUFFERABLE" on
>> ARM and XEN_DOMCTL_MEMATTRS_SUC means "UNCACHED", because that's
>> actually what they correspond to I think. However using x86 names for
>> ARM caching attributes is very confusing and error prone. So I would
>> prefer introducing separate tags for ARM and x86. However, reusing
>> XEN_DOMCTL_MEMATTRS_UC, XEN_DOMCTL_MEMATTRS_CACHE_WT and
>> XEN_DOMCTL_MEMATTRS_CACHE_WB as Zhongze did in this proposal would be OK
>> for me.

When I read bufferable it is unclear if you speak about normal memory or 
device. I am looking at renaming the memory attribute with prefixing 
them with the type memory.

For instance BUFFERABLE would be renamed to NORMAL_NC...

>>
>> Julien, what do you think?

I will only speak about ARM as my knowledge is very limited on x86.

For ARM, the resulting memory attribute is a combination of stage-1 and 
stage-2 (see Table D4-43 in ARM DDI 0487B.a). It adds further 
restriction to the memory attributes defined by the Guest in its 
page-tables.

This means that even the memory attribute used in stage-2 is normal 
cacheable, a guest is free to make it non-cacheable via stage-1 page 
table. This is not really clear in the description of the DOMCTL what is 
the real purpose. Is it restricting possibility of the guest?

Now, looking at the description, this domctl will be called after we 
mapped the RAM in the guest memory. So you will switch from write-back 
cacheable to another memory attribute. I think this will require cache 
maintainance to remove potential stall cache line.

Furthermore, you don't have any restriction on when this domctl will be 
called. It would be possible to call it when the guest is running or 
called on a range with memory attribute already changed. This will 
require some thoughts on how to do the cache maintenance.

Finally, Xen ARM64 will always have the whole RAM memory mapped in Xen 
with write-allocate memory attribute. This may result a memory attribute 
mismatch if the region is accessed by Xen (see B2.8).

This may take sometimes to get the implementation of the DOMCTL right. 
So I would rather focus to be able to share page between guest and an 
future-proof toolstack interface.

If you still have time at the end of the GSOC, you can look at using 
different memory attributes

Cheers,

-- 
Julien Grall

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

Re: [RFC] DOMCTL_memattrs_op : a new DOMCTL to play with stage-2 page attributes

[Jan Beulich]

>>> On 30.06.17 at 22:15, <blackskygg@gmail.com> wrote:
> /*
>  * Set access permissions, cacheability and shareability (ARM only) of a
>  * continuos range of normal memory (RAM) in the stage-2 page table.
>  */
> /* XEN_DOMCTL_memattrs_op */
> 
> /* set chacheability and shareability */
> #define XEN_DOMCTL_MEMATTRS_OP_SET_CACHEATTRS  1
> /* set access permissions */
> #define XEN_DOMCTL_MEMATTRS_OP_SET_PERMISSIONS 2
> /* get chacheability and shareability */
> #define XEN_DOMCTL_MEMATTRS_OP_GET_CACHEATTRS  1
> /* get access permissions */
> #define XEN_DOMCTL_MEMATTRS_OP_GET_PERMISSIONS 2
> 
> /* flags for XEN_DOMCTL_MEMATTRS_OP_SET_CACHEATTRS */
> /* chacheability flags, the values happen to be the same with those in
>  * x86 PAT.  (See [4])
>  */
> /* uncacheable */
> #define XEN_DOMCTL_MEMATTRS_UC         0x00U
> /* write combine, x86 only */
> #define XEN_DOMCTL_MEMATTRS_CACHE_WC   0x01U
> /* write through */
> #define XEN_DOMCTL_MEMATTRS_CACHE_WT   0x04U
> /* write protect, x86 only */
> #define XEN_DOMCTL_MEMATTRS_CACHE_WP   0x05U
> /* write back */
> #define XEN_DOMCTL_MEMATTRS_CACHE_WB   0x06U
> /* strong uncacheable, x86 only*/
> #define XEN_DOMCTL_MEMATTRS_SUC        0x07U

I think if we really want to go this route, this part should mean
removal of DOMCTL_pin_mem_cacheattr then. However, ...

> /* shareability flags (See [5]), arm only, the value is taken from
>  * asm-arm/page.h, but live in the second 8-bit.
>  */
> #define XEN_DOMCTL_MEMATTRS_SHAREABILITY_SHIFT 8
> #define XEN_DOMCTL_MEMATTRS_SH_NON_SHAREABLE (LPAE_SH_NON_SHAREABLE<<8)
> #define XEN_DOMCTL_MEMATTRS_SH_UNPREDICTALE  (LPAE_SH_UNPREDICTALE<<8)
> #define XEN_DOMCTL_MEMATTRS_SH_OUTER         (LPAE_SH_OUTER<<8)
> #define XEN_DOMCTL_MEMATTRS_SH_INNER         (LPAE_SH_INNER<<8)
> 
> /* flags for XEN_DOMCTL_MEMATTRS_OP_SET_PERMISSIONS */
> #define XEN_DOMCTL_MEMATTRS_ACCESS_N         0x00U
> #define XEN_DOMCTL_MEMATTRS_ACCESS_R         (0x01U<<0)
> #define XEN_DOMCTL_MEMATTRS_ACCESS_W         (0x01U<<1)
> #define XEN_DOMCTL_MEMATTRS_ACCESS_X         (0x01U<<2)
> #define XEN_DOMCTL_MEMATTRS_ACCESS_RW        \
> (XEN_DOMCTL_MEMATTRS_ACCESS_R|XEN_DOMCTL_MEMATTRS_ACCESS_W)
> #define XEN_DOMCTL_MEMATTRS_ACCESS_RX        \
> (XEN_DOMCTL_MEMATTRS_ACCESS_R|XEN_DOMCTL_MEMATTRS_ACCESS_X)
> #define XEN_DOMCTL_MEMATTRS_ACCESS_WX        \
> (XEN_DOMCTL_MEMATTRS_ACCESS_W|XEN_DOMCTL_MEMATTRS_ACCESS_X)
> #define XEN_DOMCTL_MEMATTRS_ACCESS_RWX        \
> (XEN_DOMCTL_MEMATTRS_ACCESS_RW|XEN_DOMCTL_MEMATTRS_ACCESS_X)

... with this basically duplicating
XENMEM_access_op_{set,get}_access I now wonder whether
we don't already have all you need (apart from an ARM variant of
DOMCTL_pin_mem_cacheattr).

This being an RFC, I'll skip pointing out various cosmetic issues with
your proposal.

Jan


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

Re: [RFC] DOMCTL_memattrs_op : a new DOMCTL to play with stage-2 page attributes

[Zhongze Liu]

Hi Julien,

2017-07-03 19:16 GMT+08:00 Julien Grall <julien.grall@arm.com>:
> Hi,
>
> On 01/07/17 10:16, Zhongze Liu wrote:
>>>
>>> On the ARM side, we are missing BUFFERABLE and WRITEALLOC. I don't know
>>> how they map to these tags, which comes from the x86 world. Maybe we
>>> should just add them separately as ARM only, like:
>>>
>>>   /* bufferable, ARM only */
>>>   #define XEN_DOMCTL_MEMATTRS_BUFFERABLE 0x08U
>>>   /* write alloc, ARM only */
>>>   #define XEN_DOMCTL_MEMATTRS_CACHE_WA   0x09U
>>>
>>> Theoretically, we could say XEN_DOMCTL_MEMATTRS_UC means "BUFFERABLE" on
>>> ARM and XEN_DOMCTL_MEMATTRS_SUC means "UNCACHED", because that's
>>> actually what they correspond to I think. However using x86 names for
>>> ARM caching attributes is very confusing and error prone. So I would
>>> prefer introducing separate tags for ARM and x86. However, reusing
>>> XEN_DOMCTL_MEMATTRS_UC, XEN_DOMCTL_MEMATTRS_CACHE_WT and
>>> XEN_DOMCTL_MEMATTRS_CACHE_WB as Zhongze did in this proposal would be OK
>>> for me.
>
>
> When I read bufferable it is unclear if you speak about normal memory or
> device. I am looking at renaming the memory attribute with prefixing them
> with the type memory.
>
> For instance BUFFERABLE would be renamed to NORMAL_NC...
>
>>>
>>> Julien, what do you think?
>
>
> I will only speak about ARM as my knowledge is very limited on x86.
>
> For ARM, the resulting memory attribute is a combination of stage-1 and
> stage-2 (see Table D4-43 in ARM DDI 0487B.a). It adds further restriction to
> the memory attributes defined by the Guest in its page-tables.
>
> This means that even the memory attribute used in stage-2 is normal
> cacheable, a guest is free to make it non-cacheable via stage-1 page table.
> This is not really clear in the description of the DOMCTL what is the real
> purpose. Is it restricting possibility of the guest?

Yes. this only deals with the stage-2 table entries, and thus only serves as a
restriction on what the DomU's can do. And the DomU's can do whatever they
want to their stage-1 table entries, as long as they don't try to
break the restrictions.

>
> Now, looking at the description, this domctl will be called after we mapped
> the RAM in the guest memory. So you will switch from write-back cacheable to
> another memory attribute. I think this will require cache maintainance to
> remove potential stall cache line.
>
> Furthermore, you don't have any restriction on when this domctl will be
> called. It would be possible to call it when the guest is running or called
> on a range with memory attribute already changed. This will require some
> thoughts on how to do the cache maintenance.
>
> Finally, Xen ARM64 will always have the whole RAM memory mapped in Xen with
> write-allocate memory attribute. This may result a memory attribute mismatch
> if the region is accessed by Xen (see B2.8).

Actually, I was considering whether the shared areas should be set up during
domain construction, I think this will make better sense.

@Stebellini: what do you think?

>
> This may take sometimes to get the implementation of the DOMCTL right. So I
> would rather focus to be able to share page between guest and an
> future-proof toolstack interface.
>
> If you still have time at the end of the GSOC, you can look at using
> different memory attributes

Agree. That's what Stabellini has also suggested me to do. From now on, I'll be
focusing more on the other parts of this project, while waiting for
more feedback
on how to do this attribute stuff right.


Cheers,

Zhongze Liu.

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

Re: [RFC] DOMCTL_memattrs_op : a new DOMCTL to play with stage-2 page attributes

[Zhongze Liu]

Hi Jan,

2017-07-03 22:25 GMT+08:00 Jan Beulich <JBeulich@suse.com>:
>>>> On 30.06.17 at 22:15, <blackskygg@gmail.com> wrote:
>> /*
>>  * Set access permissions, cacheability and shareability (ARM only) of a
>>  * continuos range of normal memory (RAM) in the stage-2 page table.
>>  */
>> /* XEN_DOMCTL_memattrs_op */
>>
>> /* set chacheability and shareability */
>> #define XEN_DOMCTL_MEMATTRS_OP_SET_CACHEATTRS  1
>> /* set access permissions */
>> #define XEN_DOMCTL_MEMATTRS_OP_SET_PERMISSIONS 2
>> /* get chacheability and shareability */
>> #define XEN_DOMCTL_MEMATTRS_OP_GET_CACHEATTRS  1
>> /* get access permissions */
>> #define XEN_DOMCTL_MEMATTRS_OP_GET_PERMISSIONS 2
>>
>> /* flags for XEN_DOMCTL_MEMATTRS_OP_SET_CACHEATTRS */
>> /* chacheability flags, the values happen to be the same with those in
>>  * x86 PAT.  (See [4])
>>  */
>> /* uncacheable */
>> #define XEN_DOMCTL_MEMATTRS_UC         0x00U
>> /* write combine, x86 only */
>> #define XEN_DOMCTL_MEMATTRS_CACHE_WC   0x01U
>> /* write through */
>> #define XEN_DOMCTL_MEMATTRS_CACHE_WT   0x04U
>> /* write protect, x86 only */
>> #define XEN_DOMCTL_MEMATTRS_CACHE_WP   0x05U
>> /* write back */
>> #define XEN_DOMCTL_MEMATTRS_CACHE_WB   0x06U
>> /* strong uncacheable, x86 only*/
>> #define XEN_DOMCTL_MEMATTRS_SUC        0x07U
>
> I think if we really want to go this route, this part should mean
> removal of DOMCTL_pin_mem_cacheattr then. However, ...
>
>> /* shareability flags (See [5]), arm only, the value is taken from
>>  * asm-arm/page.h, but live in the second 8-bit.
>>  */
>> #define XEN_DOMCTL_MEMATTRS_SHAREABILITY_SHIFT 8
>> #define XEN_DOMCTL_MEMATTRS_SH_NON_SHAREABLE (LPAE_SH_NON_SHAREABLE<<8)
>> #define XEN_DOMCTL_MEMATTRS_SH_UNPREDICTALE  (LPAE_SH_UNPREDICTALE<<8)
>> #define XEN_DOMCTL_MEMATTRS_SH_OUTER         (LPAE_SH_OUTER<<8)
>> #define XEN_DOMCTL_MEMATTRS_SH_INNER         (LPAE_SH_INNER<<8)
>>
>> /* flags for XEN_DOMCTL_MEMATTRS_OP_SET_PERMISSIONS */
>> #define XEN_DOMCTL_MEMATTRS_ACCESS_N         0x00U
>> #define XEN_DOMCTL_MEMATTRS_ACCESS_R         (0x01U<<0)
>> #define XEN_DOMCTL_MEMATTRS_ACCESS_W         (0x01U<<1)
>> #define XEN_DOMCTL_MEMATTRS_ACCESS_X         (0x01U<<2)
>> #define XEN_DOMCTL_MEMATTRS_ACCESS_RW        \
>> (XEN_DOMCTL_MEMATTRS_ACCESS_R|XEN_DOMCTL_MEMATTRS_ACCESS_W)
>> #define XEN_DOMCTL_MEMATTRS_ACCESS_RX        \
>> (XEN_DOMCTL_MEMATTRS_ACCESS_R|XEN_DOMCTL_MEMATTRS_ACCESS_X)
>> #define XEN_DOMCTL_MEMATTRS_ACCESS_WX        \
>> (XEN_DOMCTL_MEMATTRS_ACCESS_W|XEN_DOMCTL_MEMATTRS_ACCESS_X)
>> #define XEN_DOMCTL_MEMATTRS_ACCESS_RWX        \
>> (XEN_DOMCTL_MEMATTRS_ACCESS_RW|XEN_DOMCTL_MEMATTRS_ACCESS_X)
>
> ... with this basically duplicating
> XENMEM_access_op_{set,get}_access I now wonder whether
> we don't already have all you need (apart from an ARM variant of
> DOMCTL_pin_mem_cacheattr).

In fact, there isn't much description on the usage of this
interface, so I turned to the implementation in
xen/common/mem_access.c, where I see this
interface invoking  p2m_set_mem_acess, which further invokes
set_mem_acess and finally
p2m->set_entry(), so I guess this might be the right interface to use.
To confirm the guess, I turned to Stabellini for help, and he told me
that XENMEM_access_op
is "for getting very detail info on what the guest is accessing", and
might not be suitable
for this scenario, so I just gave up using it, and that's why I have this RFC.
I'll re-confirm this with Stabellini.

>
> This being an RFC, I'll skip pointing out various cosmetic issues with
> your proposal.
>
> Jan
>

Thank you.


Cheers,

Zhongze Liu

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

Re: [RFC] DOMCTL_memattrs_op : a new DOMCTL to play with stage-2 page attributes

[Stefano Stabellini]

On Mon, 3 Jul 2017, Zhongze Liu wrote:
> Hi Julien,
> 
> 2017-07-03 19:16 GMT+08:00 Julien Grall <julien.grall@arm.com>:
> > Hi,
> >
> > On 01/07/17 10:16, Zhongze Liu wrote:
> >>>
> >>> On the ARM side, we are missing BUFFERABLE and WRITEALLOC. I don't know
> >>> how they map to these tags, which comes from the x86 world. Maybe we
> >>> should just add them separately as ARM only, like:
> >>>
> >>>   /* bufferable, ARM only */
> >>>   #define XEN_DOMCTL_MEMATTRS_BUFFERABLE 0x08U
> >>>   /* write alloc, ARM only */
> >>>   #define XEN_DOMCTL_MEMATTRS_CACHE_WA   0x09U
> >>>
> >>> Theoretically, we could say XEN_DOMCTL_MEMATTRS_UC means "BUFFERABLE" on
> >>> ARM and XEN_DOMCTL_MEMATTRS_SUC means "UNCACHED", because that's
> >>> actually what they correspond to I think. However using x86 names for
> >>> ARM caching attributes is very confusing and error prone. So I would
> >>> prefer introducing separate tags for ARM and x86. However, reusing
> >>> XEN_DOMCTL_MEMATTRS_UC, XEN_DOMCTL_MEMATTRS_CACHE_WT and
> >>> XEN_DOMCTL_MEMATTRS_CACHE_WB as Zhongze did in this proposal would be OK
> >>> for me.
> >
> >
> > When I read bufferable it is unclear if you speak about normal memory or
> > device. I am looking at renaming the memory attribute with prefixing them
> > with the type memory.
> >
> > For instance BUFFERABLE would be renamed to NORMAL_NC...
> >
> >>>
> >>> Julien, what do you think?
> >
> >
> > I will only speak about ARM as my knowledge is very limited on x86.
> >
> > For ARM, the resulting memory attribute is a combination of stage-1 and
> > stage-2 (see Table D4-43 in ARM DDI 0487B.a). It adds further restriction to
> > the memory attributes defined by the Guest in its page-tables.
> >
> > This means that even the memory attribute used in stage-2 is normal
> > cacheable, a guest is free to make it non-cacheable via stage-1 page table.
> > This is not really clear in the description of the DOMCTL what is the real
> > purpose. Is it restricting possibility of the guest?
> 
> Yes. this only deals with the stage-2 table entries, and thus only serves as a
> restriction on what the DomU's can do. And the DomU's can do whatever they
> want to their stage-1 table entries, as long as they don't try to
> break the restrictions.
> 
> >
> > Now, looking at the description, this domctl will be called after we mapped
> > the RAM in the guest memory. So you will switch from write-back cacheable to
> > another memory attribute. I think this will require cache maintainance to
> > remove potential stall cache line.
> >
> > Furthermore, you don't have any restriction on when this domctl will be
> > called. It would be possible to call it when the guest is running or called
> > on a range with memory attribute already changed. This will require some
> > thoughts on how to do the cache maintenance.
> >
> > Finally, Xen ARM64 will always have the whole RAM memory mapped in Xen with
> > write-allocate memory attribute. This may result a memory attribute mismatch
> > if the region is accessed by Xen (see B2.8).
> 
> Actually, I was considering whether the shared areas should be set up during
> domain construction, I think this will make better sense.
> 
> @Stebellini: what do you think?

Yes, it makes sense to set them up during domain construction.

However what Julien was also pointing out is that if the user requests
memory attributes different from the one used by Xen (because Xen will
also have the same pages already mapped in its own pagetables
regardless), then it will be an issue for Xen to access them: it could
result in a memory attribute mismatch.

We would need to make sure somehow that those pages are not accessed by
Xen for as long as they are shared between guests with different memory
attributes in the guest stage2 pagetables.

This is another reason for not implementing different memory attributes
right now :-)

I would add a note about this unsolved problem in future versions of
this document.


> >
> > This may take sometimes to get the implementation of the DOMCTL right. So I
> > would rather focus to be able to share page between guest and an
> > future-proof toolstack interface.
> >
> > If you still have time at the end of the GSOC, you can look at using
> > different memory attributes
> 
> Agree. That's what Stabellini has also suggested me to do. From now on, I'll be
> focusing more on the other parts of this project, while waiting for
> more feedback
> on how to do this attribute stuff right.

Yes, good idea, thank you. This is delicate and will likely take some
time, without actually affecting the rest of the code much.

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

Re: [RFC] DOMCTL_memattrs_op : a new DOMCTL to play with stage-2 page attributes

[Stefano Stabellini]

On Mon, 3 Jul 2017, Zhongze Liu wrote:
> Hi Jan,
> 
> 2017-07-03 22:25 GMT+08:00 Jan Beulich <JBeulich@suse.com>:
> >>>> On 30.06.17 at 22:15, <blackskygg@gmail.com> wrote:
> >> /*
> >>  * Set access permissions, cacheability and shareability (ARM only) of a
> >>  * continuos range of normal memory (RAM) in the stage-2 page table.
> >>  */
> >> /* XEN_DOMCTL_memattrs_op */
> >>
> >> /* set chacheability and shareability */
> >> #define XEN_DOMCTL_MEMATTRS_OP_SET_CACHEATTRS  1
> >> /* set access permissions */
> >> #define XEN_DOMCTL_MEMATTRS_OP_SET_PERMISSIONS 2
> >> /* get chacheability and shareability */
> >> #define XEN_DOMCTL_MEMATTRS_OP_GET_CACHEATTRS  1
> >> /* get access permissions */
> >> #define XEN_DOMCTL_MEMATTRS_OP_GET_PERMISSIONS 2
> >>
> >> /* flags for XEN_DOMCTL_MEMATTRS_OP_SET_CACHEATTRS */
> >> /* chacheability flags, the values happen to be the same with those in
> >>  * x86 PAT.  (See [4])
> >>  */
> >> /* uncacheable */
> >> #define XEN_DOMCTL_MEMATTRS_UC         0x00U
> >> /* write combine, x86 only */
> >> #define XEN_DOMCTL_MEMATTRS_CACHE_WC   0x01U
> >> /* write through */
> >> #define XEN_DOMCTL_MEMATTRS_CACHE_WT   0x04U
> >> /* write protect, x86 only */
> >> #define XEN_DOMCTL_MEMATTRS_CACHE_WP   0x05U
> >> /* write back */
> >> #define XEN_DOMCTL_MEMATTRS_CACHE_WB   0x06U
> >> /* strong uncacheable, x86 only*/
> >> #define XEN_DOMCTL_MEMATTRS_SUC        0x07U
> >
> > I think if we really want to go this route, this part should mean
> > removal of DOMCTL_pin_mem_cacheattr then. However, ...
> >
> >> /* shareability flags (See [5]), arm only, the value is taken from
> >>  * asm-arm/page.h, but live in the second 8-bit.
> >>  */
> >> #define XEN_DOMCTL_MEMATTRS_SHAREABILITY_SHIFT 8
> >> #define XEN_DOMCTL_MEMATTRS_SH_NON_SHAREABLE (LPAE_SH_NON_SHAREABLE<<8)
> >> #define XEN_DOMCTL_MEMATTRS_SH_UNPREDICTALE  (LPAE_SH_UNPREDICTALE<<8)
> >> #define XEN_DOMCTL_MEMATTRS_SH_OUTER         (LPAE_SH_OUTER<<8)
> >> #define XEN_DOMCTL_MEMATTRS_SH_INNER         (LPAE_SH_INNER<<8)
> >>
> >> /* flags for XEN_DOMCTL_MEMATTRS_OP_SET_PERMISSIONS */
> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_N         0x00U
> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_R         (0x01U<<0)
> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_W         (0x01U<<1)
> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_X         (0x01U<<2)
> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_RW        \
> >> (XEN_DOMCTL_MEMATTRS_ACCESS_R|XEN_DOMCTL_MEMATTRS_ACCESS_W)
> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_RX        \
> >> (XEN_DOMCTL_MEMATTRS_ACCESS_R|XEN_DOMCTL_MEMATTRS_ACCESS_X)
> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_WX        \
> >> (XEN_DOMCTL_MEMATTRS_ACCESS_W|XEN_DOMCTL_MEMATTRS_ACCESS_X)
> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_RWX        \
> >> (XEN_DOMCTL_MEMATTRS_ACCESS_RW|XEN_DOMCTL_MEMATTRS_ACCESS_X)
> >
> > ... with this basically duplicating
> > XENMEM_access_op_{set,get}_access I now wonder whether
> > we don't already have all you need (apart from an ARM variant of
> > DOMCTL_pin_mem_cacheattr).
> 
> In fact, there isn't much description on the usage of this
> interface, so I turned to the implementation in
> xen/common/mem_access.c, where I see this
> interface invoking  p2m_set_mem_acess, which further invokes
> set_mem_acess and finally
> p2m->set_entry(), so I guess this might be the right interface to use.
> To confirm the guess, I turned to Stabellini for help, and he told me
> that XENMEM_access_op
> is "for getting very detail info on what the guest is accessing", and
> might not be suitable
> for this scenario, so I just gave up using it, and that's why I have this RFC.
> I'll re-confirm this with Stabellini.

I thought that those two hypercalls were meant to be used for mem_access
and vm_event operations, as in xen/arch/arm/mem_access.c and
xen/arch/x86/mm/mem_access.c. The only caller is
tools/tests/xen-access/xen-access.c. They are enabled separatly as part
of the mem_access interface: their build is conditional to
CONFIG_HAS_MEM_ACCESS. Unless we want to move them from XENMEM_access_*
to DOMCTL_* operations, I don't think they could be used?

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

Re: [RFC] DOMCTL_memattrs_op : a new DOMCTL to play with stage-2 page attributes

[Jan Beulich]

>>> On 03.07.17 at 19:58, <sstabellini@kernel.org> wrote:
> On Mon, 3 Jul 2017, Zhongze Liu wrote:
>> 2017-07-03 22:25 GMT+08:00 Jan Beulich <JBeulich@suse.com>:
>> >>>> On 30.06.17 at 22:15, <blackskygg@gmail.com> wrote:
>> >> /* flags for XEN_DOMCTL_MEMATTRS_OP_SET_PERMISSIONS */
>> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_N         0x00U
>> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_R         (0x01U<<0)
>> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_W         (0x01U<<1)
>> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_X         (0x01U<<2)
>> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_RW        \
>> >> (XEN_DOMCTL_MEMATTRS_ACCESS_R|XEN_DOMCTL_MEMATTRS_ACCESS_W)
>> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_RX        \
>> >> (XEN_DOMCTL_MEMATTRS_ACCESS_R|XEN_DOMCTL_MEMATTRS_ACCESS_X)
>> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_WX        \
>> >> (XEN_DOMCTL_MEMATTRS_ACCESS_W|XEN_DOMCTL_MEMATTRS_ACCESS_X)
>> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_RWX        \
>> >> (XEN_DOMCTL_MEMATTRS_ACCESS_RW|XEN_DOMCTL_MEMATTRS_ACCESS_X)
>> >
>> > ... with this basically duplicating
>> > XENMEM_access_op_{set,get}_access I now wonder whether
>> > we don't already have all you need (apart from an ARM variant of
>> > DOMCTL_pin_mem_cacheattr).
>> 
>> In fact, there isn't much description on the usage of this
>> interface, so I turned to the implementation in
>> xen/common/mem_access.c, where I see this
>> interface invoking  p2m_set_mem_acess, which further invokes
>> set_mem_acess and finally
>> p2m->set_entry(), so I guess this might be the right interface to use.
>> To confirm the guess, I turned to Stabellini for help, and he told me
>> that XENMEM_access_op
>> is "for getting very detail info on what the guest is accessing", and
>> might not be suitable
>> for this scenario, so I just gave up using it, and that's why I have this RFC.
>> I'll re-confirm this with Stabellini.
> 
> I thought that those two hypercalls were meant to be used for mem_access
> and vm_event operations, as in xen/arch/arm/mem_access.c and
> xen/arch/x86/mm/mem_access.c. The only caller is
> tools/tests/xen-access/xen-access.c. They are enabled separatly as part
> of the mem_access interface: their build is conditional to
> CONFIG_HAS_MEM_ACCESS. Unless we want to move them from XENMEM_access_*
> to DOMCTL_* operations, I don't think they could be used?

For one, we could remove the CONFIG_HAS_MEM_ACCESS around
them if a broader use is planned. And in general we should try to
avoid having two ways of doing the same thing, unless backwards
compatibility makes this a requirement. Hence if a new, better way
is to be introduced, the old one should at once go away. Finally, I'm
still unconvinced a new DOMCTL_* is better here than a (tool stack
only) XENMEM_*, but I agree the boundary between when to use
what is at best fuzzy.

Jan


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

Re: [RFC] DOMCTL_memattrs_op : a new DOMCTL to play with stage-2 page attributes

[Stefano Stabellini]

On Tue, 4 Jul 2017, Jan Beulich wrote:
> >>> On 03.07.17 at 19:58, <sstabellini@kernel.org> wrote:
> > On Mon, 3 Jul 2017, Zhongze Liu wrote:
> >> 2017-07-03 22:25 GMT+08:00 Jan Beulich <JBeulich@suse.com>:
> >> >>>> On 30.06.17 at 22:15, <blackskygg@gmail.com> wrote:
> >> >> /* flags for XEN_DOMCTL_MEMATTRS_OP_SET_PERMISSIONS */
> >> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_N         0x00U
> >> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_R         (0x01U<<0)
> >> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_W         (0x01U<<1)
> >> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_X         (0x01U<<2)
> >> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_RW        \
> >> >> (XEN_DOMCTL_MEMATTRS_ACCESS_R|XEN_DOMCTL_MEMATTRS_ACCESS_W)
> >> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_RX        \
> >> >> (XEN_DOMCTL_MEMATTRS_ACCESS_R|XEN_DOMCTL_MEMATTRS_ACCESS_X)
> >> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_WX        \
> >> >> (XEN_DOMCTL_MEMATTRS_ACCESS_W|XEN_DOMCTL_MEMATTRS_ACCESS_X)
> >> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_RWX        \
> >> >> (XEN_DOMCTL_MEMATTRS_ACCESS_RW|XEN_DOMCTL_MEMATTRS_ACCESS_X)
> >> >
> >> > ... with this basically duplicating
> >> > XENMEM_access_op_{set,get}_access I now wonder whether
> >> > we don't already have all you need (apart from an ARM variant of
> >> > DOMCTL_pin_mem_cacheattr).
> >> 
> >> In fact, there isn't much description on the usage of this
> >> interface, so I turned to the implementation in
> >> xen/common/mem_access.c, where I see this
> >> interface invoking  p2m_set_mem_acess, which further invokes
> >> set_mem_acess and finally
> >> p2m->set_entry(), so I guess this might be the right interface to use.
> >> To confirm the guess, I turned to Stabellini for help, and he told me
> >> that XENMEM_access_op
> >> is "for getting very detail info on what the guest is accessing", and
> >> might not be suitable
> >> for this scenario, so I just gave up using it, and that's why I have this RFC.
> >> I'll re-confirm this with Stabellini.
> > 
> > I thought that those two hypercalls were meant to be used for mem_access
> > and vm_event operations, as in xen/arch/arm/mem_access.c and
> > xen/arch/x86/mm/mem_access.c. The only caller is
> > tools/tests/xen-access/xen-access.c. They are enabled separatly as part
> > of the mem_access interface: their build is conditional to
> > CONFIG_HAS_MEM_ACCESS. Unless we want to move them from XENMEM_access_*
> > to DOMCTL_* operations, I don't think they could be used?
> 
> For one, we could remove the CONFIG_HAS_MEM_ACCESS around
> them if a broader use is planned. And in general we should try to
> avoid having two ways of doing the same thing, unless backwards
> compatibility makes this a requirement. Hence if a new, better way
> is to be introduced, the old one should at once go away. Finally, I'm
> still unconvinced a new DOMCTL_* is better here than a (tool stack
> only) XENMEM_*, but I agree the boundary between when to use
> what is at best fuzzy.

Do we maintain ABI compatibility for XENMEM_* hypercalls? I think we do,
don't we? Also, XENMEM_* hypercalls are usually available to both
guests and toolstack, right?

We don't want two ways of doing the same thing, but at the same time
XENMEM_ hypercalls are very different from DOMCTLs, which don't come
with any ABI compatibility guarantees and are only available to the
toolstack. And these two specific XENMEM hypercalls even depend on
CONFIG_HAS_MEM_ACCESS.

I am not completely sure about what the best way forward would be. I am
OK with anything that is clear and maintainable. I would probably still
go with updating DOMCTL_pin_mem_cacheattr into something that can handle
both ARM and permissions, but I am also OK with making changes to
XENMEM_access_op_{set,get}_access so that they become an option for this
use case.

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

Re: [RFC] DOMCTL_memattrs_op : a new DOMCTL to play with stage-2 page attributes

[Julien Grall]

Hi,

On 05/07/2017 19:35, Stefano Stabellini wrote:
> On Tue, 4 Jul 2017, Jan Beulich wrote:
>>>>> On 03.07.17 at 19:58, <sstabellini@kernel.org> wrote:
>>> On Mon, 3 Jul 2017, Zhongze Liu wrote:
>>>> 2017-07-03 22:25 GMT+08:00 Jan Beulich <JBeulich@suse.com>:
>>>>>>>> On 30.06.17 at 22:15, <blackskygg@gmail.com> wrote:
>>>>>> /* flags for XEN_DOMCTL_MEMATTRS_OP_SET_PERMISSIONS */
>>>>>> #define XEN_DOMCTL_MEMATTRS_ACCESS_N         0x00U
>>>>>> #define XEN_DOMCTL_MEMATTRS_ACCESS_R         (0x01U<<0)
>>>>>> #define XEN_DOMCTL_MEMATTRS_ACCESS_W         (0x01U<<1)
>>>>>> #define XEN_DOMCTL_MEMATTRS_ACCESS_X         (0x01U<<2)
>>>>>> #define XEN_DOMCTL_MEMATTRS_ACCESS_RW        \
>>>>>> (XEN_DOMCTL_MEMATTRS_ACCESS_R|XEN_DOMCTL_MEMATTRS_ACCESS_W)
>>>>>> #define XEN_DOMCTL_MEMATTRS_ACCESS_RX        \
>>>>>> (XEN_DOMCTL_MEMATTRS_ACCESS_R|XEN_DOMCTL_MEMATTRS_ACCESS_X)
>>>>>> #define XEN_DOMCTL_MEMATTRS_ACCESS_WX        \
>>>>>> (XEN_DOMCTL_MEMATTRS_ACCESS_W|XEN_DOMCTL_MEMATTRS_ACCESS_X)
>>>>>> #define XEN_DOMCTL_MEMATTRS_ACCESS_RWX        \
>>>>>> (XEN_DOMCTL_MEMATTRS_ACCESS_RW|XEN_DOMCTL_MEMATTRS_ACCESS_X)
>>>>>
>>>>> ... with this basically duplicating
>>>>> XENMEM_access_op_{set,get}_access I now wonder whether
>>>>> we don't already have all you need (apart from an ARM variant of
>>>>> DOMCTL_pin_mem_cacheattr).
>>>>
>>>> In fact, there isn't much description on the usage of this
>>>> interface, so I turned to the implementation in
>>>> xen/common/mem_access.c, where I see this
>>>> interface invoking  p2m_set_mem_acess, which further invokes
>>>> set_mem_acess and finally
>>>> p2m->set_entry(), so I guess this might be the right interface to use.
>>>> To confirm the guess, I turned to Stabellini for help, and he told me
>>>> that XENMEM_access_op
>>>> is "for getting very detail info on what the guest is accessing", and
>>>> might not be suitable
>>>> for this scenario, so I just gave up using it, and that's why I have this RFC.
>>>> I'll re-confirm this with Stabellini.
>>>
>>> I thought that those two hypercalls were meant to be used for mem_access
>>> and vm_event operations, as in xen/arch/arm/mem_access.c and
>>> xen/arch/x86/mm/mem_access.c. The only caller is
>>> tools/tests/xen-access/xen-access.c. They are enabled separatly as part
>>> of the mem_access interface: their build is conditional to
>>> CONFIG_HAS_MEM_ACCESS. Unless we want to move them from XENMEM_access_*
>>> to DOMCTL_* operations, I don't think they could be used?
>>
>> For one, we could remove the CONFIG_HAS_MEM_ACCESS around
>> them if a broader use is planned. And in general we should try to
>> avoid having two ways of doing the same thing, unless backwards
>> compatibility makes this a requirement. Hence if a new, better way
>> is to be introduced, the old one should at once go away. Finally, I'm
>> still unconvinced a new DOMCTL_* is better here than a (tool stack
>> only) XENMEM_*, but I agree the boundary between when to use
>> what is at best fuzzy.
>
> Do we maintain ABI compatibility for XENMEM_* hypercalls? I think we do,
> don't we? Also, XENMEM_* hypercalls are usually available to both
> guests and toolstack, right?
>
> We don't want two ways of doing the same thing, but at the same time
> XENMEM_ hypercalls are very different from DOMCTLs, which don't come
> with any ABI compatibility guarantees and are only available to the
> toolstack. And these two specific XENMEM hypercalls even depend on
> CONFIG_HAS_MEM_ACCESS.
>
> I am not completely sure about what the best way forward would be. I am
> OK with anything that is clear and maintainable. I would probably still
> go with updating DOMCTL_pin_mem_cacheattr into something that can handle
> both ARM and permissions, but I am also OK with making changes to
> XENMEM_access_op_{set,get}_access so that they become an option for this
> use case.

I am struggling to understand how you could make memaccess_op_*_access 
supporting 2 distinct use cases. They are currently used to instrospect 
memory by restricting the permission. All the faults will be forwarded 
to a monitor.

Here you suggest to extend them to restrict permission. But we want to 
be able to support introspection on that share page (I don't see why 
not) and we don't want to have to set-up a VM-event ring just for 
restrict the page.

Moreover, you would have to store the access permission for the 
time-being... whilst here you just modify the permission of the page for 
good.

Am I missing something here?

Cheers,

-- 
Julien Grall

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

Re: [RFC] DOMCTL_memattrs_op : a new DOMCTL to play with stage-2 page attributes

[Jan Beulich]

>>> On 05.07.17 at 20:35, <sstabellini@kernel.org> wrote:
> On Tue, 4 Jul 2017, Jan Beulich wrote:
>> >>> On 03.07.17 at 19:58, <sstabellini@kernel.org> wrote:
>> > On Mon, 3 Jul 2017, Zhongze Liu wrote:
>> >> 2017-07-03 22:25 GMT+08:00 Jan Beulich <JBeulich@suse.com>:
>> >> >>>> On 30.06.17 at 22:15, <blackskygg@gmail.com> wrote:
>> >> >> /* flags for XEN_DOMCTL_MEMATTRS_OP_SET_PERMISSIONS */
>> >> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_N         0x00U
>> >> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_R         (0x01U<<0)
>> >> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_W         (0x01U<<1)
>> >> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_X         (0x01U<<2)
>> >> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_RW        \
>> >> >> (XEN_DOMCTL_MEMATTRS_ACCESS_R|XEN_DOMCTL_MEMATTRS_ACCESS_W)
>> >> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_RX        \
>> >> >> (XEN_DOMCTL_MEMATTRS_ACCESS_R|XEN_DOMCTL_MEMATTRS_ACCESS_X)
>> >> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_WX        \
>> >> >> (XEN_DOMCTL_MEMATTRS_ACCESS_W|XEN_DOMCTL_MEMATTRS_ACCESS_X)
>> >> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_RWX        \
>> >> >> (XEN_DOMCTL_MEMATTRS_ACCESS_RW|XEN_DOMCTL_MEMATTRS_ACCESS_X)
>> >> >
>> >> > ... with this basically duplicating
>> >> > XENMEM_access_op_{set,get}_access I now wonder whether
>> >> > we don't already have all you need (apart from an ARM variant of
>> >> > DOMCTL_pin_mem_cacheattr).
>> >> 
>> >> In fact, there isn't much description on the usage of this
>> >> interface, so I turned to the implementation in
>> >> xen/common/mem_access.c, where I see this
>> >> interface invoking  p2m_set_mem_acess, which further invokes
>> >> set_mem_acess and finally
>> >> p2m->set_entry(), so I guess this might be the right interface to use.
>> >> To confirm the guess, I turned to Stabellini for help, and he told me
>> >> that XENMEM_access_op
>> >> is "for getting very detail info on what the guest is accessing", and
>> >> might not be suitable
>> >> for this scenario, so I just gave up using it, and that's why I have this 
> RFC.
>> >> I'll re-confirm this with Stabellini.
>> > 
>> > I thought that those two hypercalls were meant to be used for mem_access
>> > and vm_event operations, as in xen/arch/arm/mem_access.c and
>> > xen/arch/x86/mm/mem_access.c. The only caller is
>> > tools/tests/xen-access/xen-access.c. They are enabled separatly as part
>> > of the mem_access interface: their build is conditional to
>> > CONFIG_HAS_MEM_ACCESS. Unless we want to move them from XENMEM_access_*
>> > to DOMCTL_* operations, I don't think they could be used?
>> 
>> For one, we could remove the CONFIG_HAS_MEM_ACCESS around
>> them if a broader use is planned. And in general we should try to
>> avoid having two ways of doing the same thing, unless backwards
>> compatibility makes this a requirement. Hence if a new, better way
>> is to be introduced, the old one should at once go away. Finally, I'm
>> still unconvinced a new DOMCTL_* is better here than a (tool stack
>> only) XENMEM_*, but I agree the boundary between when to use
>> what is at best fuzzy.
> 
> Do we maintain ABI compatibility for XENMEM_* hypercalls? I think we do,
> don't we? Also, XENMEM_* hypercalls are usually available to both
> guests and toolstack, right?

There are ones of both kinds. Which one belongs to which group
depends on whether they sit in a __XEN__/__XEN_TOOLS__ framed
section.

Jan


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

Re: [RFC] DOMCTL_memattrs_op : a new DOMCTL to play with stage-2 page attributes

[Jan Beulich]

>>> On 05.07.17 at 22:39, <julien.grall@arm.com> wrote:
> On 05/07/2017 19:35, Stefano Stabellini wrote:
>> On Tue, 4 Jul 2017, Jan Beulich wrote:
>>> For one, we could remove the CONFIG_HAS_MEM_ACCESS around
>>> them if a broader use is planned. And in general we should try to
>>> avoid having two ways of doing the same thing, unless backwards
>>> compatibility makes this a requirement. Hence if a new, better way
>>> is to be introduced, the old one should at once go away. Finally, I'm
>>> still unconvinced a new DOMCTL_* is better here than a (tool stack
>>> only) XENMEM_*, but I agree the boundary between when to use
>>> what is at best fuzzy.
>>
>> Do we maintain ABI compatibility for XENMEM_* hypercalls? I think we do,
>> don't we? Also, XENMEM_* hypercalls are usually available to both
>> guests and toolstack, right?
>>
>> We don't want two ways of doing the same thing, but at the same time
>> XENMEM_ hypercalls are very different from DOMCTLs, which don't come
>> with any ABI compatibility guarantees and are only available to the
>> toolstack. And these two specific XENMEM hypercalls even depend on
>> CONFIG_HAS_MEM_ACCESS.
>>
>> I am not completely sure about what the best way forward would be. I am
>> OK with anything that is clear and maintainable. I would probably still
>> go with updating DOMCTL_pin_mem_cacheattr into something that can handle
>> both ARM and permissions, but I am also OK with making changes to
>> XENMEM_access_op_{set,get}_access so that they become an option for this
>> use case.
> 
> I am struggling to understand how you could make memaccess_op_*_access 
> supporting 2 distinct use cases. They are currently used to instrospect 
> memory by restricting the permission. All the faults will be forwarded 
> to a monitor.

There's nothing memaccess-specific in the handler of the operation.
Where faults go merely depends on whether a monitor has been
registered afaict. Hence ...

> Here you suggest to extend them to restrict permission. But we want to 
> be able to support introspection on that share page (I don't see why 
> not) and we don't want to have to set-up a VM-event ring just for 
> restrict the page.
> 
> Moreover, you would have to store the access permission for the 
> time-being... whilst here you just modify the permission of the page for 
> good.

... no matter which way we allow setting the permissions for the
purpose here, we'll have to deal with what you describe, except
that as per above the question of setting up a ring looks orthogonal
to the apparent conflicts here.

Considering the intended purpose here (as far as I recall it), was it
already taken into consideration to request suitable attributes right
at the time the page gets installed into the physmap? Iirc there's no
need to actually "play" with the attributes at random times.

Jan


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

Re: [RFC] DOMCTL_memattrs_op : a new DOMCTL to play with stage-2 page attributes

[Stefano Stabellini]

On Thu, 6 Jul 2017, Jan Beulich wrote:
> >>> On 05.07.17 at 22:39, <julien.grall@arm.com> wrote:
> > On 05/07/2017 19:35, Stefano Stabellini wrote:
> >> On Tue, 4 Jul 2017, Jan Beulich wrote:
> >>> For one, we could remove the CONFIG_HAS_MEM_ACCESS around
> >>> them if a broader use is planned. And in general we should try to
> >>> avoid having two ways of doing the same thing, unless backwards
> >>> compatibility makes this a requirement. Hence if a new, better way
> >>> is to be introduced, the old one should at once go away. Finally, I'm
> >>> still unconvinced a new DOMCTL_* is better here than a (tool stack
> >>> only) XENMEM_*, but I agree the boundary between when to use
> >>> what is at best fuzzy.
> >>
> >> Do we maintain ABI compatibility for XENMEM_* hypercalls? I think we do,
> >> don't we? Also, XENMEM_* hypercalls are usually available to both
> >> guests and toolstack, right?
> >>
> >> We don't want two ways of doing the same thing, but at the same time
> >> XENMEM_ hypercalls are very different from DOMCTLs, which don't come
> >> with any ABI compatibility guarantees and are only available to the
> >> toolstack. And these two specific XENMEM hypercalls even depend on
> >> CONFIG_HAS_MEM_ACCESS.
> >>
> >> I am not completely sure about what the best way forward would be. I am
> >> OK with anything that is clear and maintainable. I would probably still
> >> go with updating DOMCTL_pin_mem_cacheattr into something that can handle
> >> both ARM and permissions, but I am also OK with making changes to
> >> XENMEM_access_op_{set,get}_access so that they become an option for this
> >> use case.
> > 
> > I am struggling to understand how you could make memaccess_op_*_access 
> > supporting 2 distinct use cases. They are currently used to instrospect 
> > memory by restricting the permission. All the faults will be forwarded 
> > to a monitor.
> 
> There's nothing memaccess-specific in the handler of the operation.
> Where faults go merely depends on whether a monitor has been
> registered afaict. Hence ...
> 
> > Here you suggest to extend them to restrict permission. But we want to 
> > be able to support introspection on that share page (I don't see why 
> > not) and we don't want to have to set-up a VM-event ring just for 
> > restrict the page.
> > 
> > Moreover, you would have to store the access permission for the 
> > time-being... whilst here you just modify the permission of the page for 
> > good.
> 
> ... no matter which way we allow setting the permissions for the
> purpose here, we'll have to deal with what you describe, except
> that as per above the question of setting up a ring looks orthogonal
> to the apparent conflicts here.
> 
> Considering the intended purpose here (as far as I recall it), was it
> already taken into consideration to request suitable attributes right
> at the time the page gets installed into the physmap? Iirc there's no
> need to actually "play" with the attributes at random times.

This operation would be done before the guest starts.


Let's give a look at the list the changes that would be required to make
these hypercalls suitable for this task:

1) remove the dependency on CONFIG_HAS_MEM_ACCESS
2) remove the p2m_mem_access_sanity_check check for these two hypercalls
3) remove the (!d->vm_event->monitor.ring_page) check for these two hypercalls
4) prevent p2m->mem_access_enabled from being set for these two hypercalls

Am I missing anything? After we do this, would they still be useful for
their original mem_access related purpose?

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

Re: [RFC] DOMCTL_memattrs_op : a new DOMCTL to play with stage-2 page attributes

[Julien Grall]

Hi,

On 07/06/2017 06:42 PM, Stefano Stabellini wrote:
> On Thu, 6 Jul 2017, Jan Beulich wrote:
>>>>> On 05.07.17 at 22:39, <julien.grall@arm.com> wrote:
>>> On 05/07/2017 19:35, Stefano Stabellini wrote:
>>>> On Tue, 4 Jul 2017, Jan Beulich wrote:
>>>>> For one, we could remove the CONFIG_HAS_MEM_ACCESS around
>>>>> them if a broader use is planned. And in general we should try to
>>>>> avoid having two ways of doing the same thing, unless backwards
>>>>> compatibility makes this a requirement. Hence if a new, better way
>>>>> is to be introduced, the old one should at once go away. Finally, I'm
>>>>> still unconvinced a new DOMCTL_* is better here than a (tool stack
>>>>> only) XENMEM_*, but I agree the boundary between when to use
>>>>> what is at best fuzzy.
>>>>
>>>> Do we maintain ABI compatibility for XENMEM_* hypercalls? I think we do,
>>>> don't we? Also, XENMEM_* hypercalls are usually available to both
>>>> guests and toolstack, right?
>>>>
>>>> We don't want two ways of doing the same thing, but at the same time
>>>> XENMEM_ hypercalls are very different from DOMCTLs, which don't come
>>>> with any ABI compatibility guarantees and are only available to the
>>>> toolstack. And these two specific XENMEM hypercalls even depend on
>>>> CONFIG_HAS_MEM_ACCESS.
>>>>
>>>> I am not completely sure about what the best way forward would be. I am
>>>> OK with anything that is clear and maintainable. I would probably still
>>>> go with updating DOMCTL_pin_mem_cacheattr into something that can handle
>>>> both ARM and permissions, but I am also OK with making changes to
>>>> XENMEM_access_op_{set,get}_access so that they become an option for this
>>>> use case.
>>>
>>> I am struggling to understand how you could make memaccess_op_*_access
>>> supporting 2 distinct use cases. They are currently used to instrospect
>>> memory by restricting the permission. All the faults will be forwarded
>>> to a monitor.
>>
>> There's nothing memaccess-specific in the handler of the operation.
>> Where faults go merely depends on whether a monitor has been
>> registered afaict. Hence ...
>>
>>> Here you suggest to extend them to restrict permission. But we want to
>>> be able to support introspection on that share page (I don't see why
>>> not) and we don't want to have to set-up a VM-event ring just for
>>> restrict the page.
>>>
>>> Moreover, you would have to store the access permission for the
>>> time-being... whilst here you just modify the permission of the page for
>>> good.
>>
>> ... no matter which way we allow setting the permissions for the
>> purpose here, we'll have to deal with what you describe, except
>> that as per above the question of setting up a ring looks orthogonal
>> to the apparent conflicts here.
>>
>> Considering the intended purpose here (as far as I recall it), was it
>> already taken into consideration to request suitable attributes right
>> at the time the page gets installed into the physmap? Iirc there's no
>> need to actually "play" with the attributes at random times.
> 
> This operation would be done before the guest starts.
> 
> 
> Let's give a look at the list the changes that would be required to make
> these hypercalls suitable for this task:
> 
> 1) remove the dependency on CONFIG_HAS_MEM_ACCESS
> 2) remove the p2m_mem_access_sanity_check check for these two hypercalls
> 3) remove the (!d->vm_event->monitor.ring_page) check for these two hypercalls
> 4) prevent p2m->mem_access_enabled from being set for these two hypercalls
> 
> Am I missing anything? After we do this, would they still be useful for
> their original mem_access related purpose?

But how would you handle mem_access on those regions in that case? This 
looks completely incompatible.

The memaccess code has to store the previous permission in order to look 
for the fault. Here you want to modify for good.

Furthermore, memaccess is only here to modify permission. It does not 
handle cacheability... So it looks to me you are trying to re-purposing 
an hypercall that will not fit all our needs in the future.

I think the way forward is to introduce an hypercall which populate/map 
memory with a given set of attributes and permissions.

This would simplify quite a lot the logic (one hypercall instead of 
multiple one) and avoid to worry about attributes changed multiple time 
even before the guest is booting.

Cheers,

-- 
Julien Grall

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

Re: [RFC] DOMCTL_memattrs_op : a new DOMCTL to play with stage-2 page attributes

[Jan Beulich]

>>> On 06.07.17 at 20:07, <julien.grall@arm.com> wrote:
> On 07/06/2017 06:42 PM, Stefano Stabellini wrote:
>> On Thu, 6 Jul 2017, Jan Beulich wrote:
>>> Considering the intended purpose here (as far as I recall it), was it
>>> already taken into consideration to request suitable attributes right
>>> at the time the page gets installed into the physmap? Iirc there's no
>>> need to actually "play" with the attributes at random times.
>> 
>> This operation would be done before the guest starts.
>> 
>> 
>> Let's give a look at the list the changes that would be required to make
>> these hypercalls suitable for this task:
>> 
>> 1) remove the dependency on CONFIG_HAS_MEM_ACCESS
>> 2) remove the p2m_mem_access_sanity_check check for these two hypercalls
>> 3) remove the (!d->vm_event->monitor.ring_page) check for these two hypercalls
>> 4) prevent p2m->mem_access_enabled from being set for these two hypercalls
>> 
>> Am I missing anything? After we do this, would they still be useful for
>> their original mem_access related purpose?
> 
> But how would you handle mem_access on those regions in that case? This 
> looks completely incompatible.
> 
> The memaccess code has to store the previous permission in order to look 
> for the fault. Here you want to modify for good.
> 
> Furthermore, memaccess is only here to modify permission. It does not 
> handle cacheability... So it looks to me you are trying to re-purposing 
> an hypercall that will not fit all our needs in the future.
> 
> I think the way forward is to introduce an hypercall which populate/map 
> memory with a given set of attributes and permissions.
> 
> This would simplify quite a lot the logic (one hypercall instead of 
> multiple one) and avoid to worry about attributes changed multiple time 
> even before the guest is booting.

Right - that's what I was suggesting with the last paragraph of my
previous reply; I have to admit that I have trouble seeing how
Stefano's response relates to that.

Jan


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

Re: [RFC] DOMCTL_memattrs_op : a new DOMCTL to play with stage-2 page attributes

[George Dunlap]

On 07/07/2017 08:27 AM, Jan Beulich wrote:
>>>> On 06.07.17 at 20:07, <julien.grall@arm.com> wrote:
>> On 07/06/2017 06:42 PM, Stefano Stabellini wrote:
>>> On Thu, 6 Jul 2017, Jan Beulich wrote:
>>>> Considering the intended purpose here (as far as I recall it), was it
>>>> already taken into consideration to request suitable attributes right
>>>> at the time the page gets installed into the physmap? Iirc there's no
>>>> need to actually "play" with the attributes at random times.
>>>
>>> This operation would be done before the guest starts.
>>>
>>>
>>> Let's give a look at the list the changes that would be required to make
>>> these hypercalls suitable for this task:
>>>
>>> 1) remove the dependency on CONFIG_HAS_MEM_ACCESS
>>> 2) remove the p2m_mem_access_sanity_check check for these two hypercalls
>>> 3) remove the (!d->vm_event->monitor.ring_page) check for these two hypercalls
>>> 4) prevent p2m->mem_access_enabled from being set for these two hypercalls
>>>
>>> Am I missing anything? After we do this, would they still be useful for
>>> their original mem_access related purpose?
>>
>> But how would you handle mem_access on those regions in that case? This 
>> looks completely incompatible.
>>
>> The memaccess code has to store the previous permission in order to look 
>> for the fault. Here you want to modify for good.
>>
>> Furthermore, memaccess is only here to modify permission. It does not 
>> handle cacheability... So it looks to me you are trying to re-purposing 
>> an hypercall that will not fit all our needs in the future.
>>
>> I think the way forward is to introduce an hypercall which populate/map 
>> memory with a given set of attributes and permissions.
>>
>> This would simplify quite a lot the logic (one hypercall instead of 
>> multiple one) and avoid to worry about attributes changed multiple time 
>> even before the guest is booting.
> 
> Right - that's what I was suggesting with the last paragraph of my
> previous reply; I have to admit that I have trouble seeing how
> Stefano's response relates to that.

I had a hard time interpreting that paragraph.  Did you mean:

"Have you considered trying to populate the p2m table with the correct
permissions when first populating it, rather than populating it with
plain rw ram and then changing it afterwards?"

 -George

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

Re: [RFC] DOMCTL_memattrs_op : a new DOMCTL to play with stage-2 page attributes

[Jan Beulich]

>>> On 07.07.17 at 10:56, <george.dunlap@citrix.com> wrote:
> On 07/07/2017 08:27 AM, Jan Beulich wrote:
>>>>> On 06.07.17 at 20:07, <julien.grall@arm.com> wrote:
>>> On 07/06/2017 06:42 PM, Stefano Stabellini wrote:
>>>> On Thu, 6 Jul 2017, Jan Beulich wrote:
>>>>> Considering the intended purpose here (as far as I recall it), was it
>>>>> already taken into consideration to request suitable attributes right
>>>>> at the time the page gets installed into the physmap? Iirc there's no
>>>>> need to actually "play" with the attributes at random times.
>>>>
>>>> This operation would be done before the guest starts.
>>>>
>>>>
>>>> Let's give a look at the list the changes that would be required to make
>>>> these hypercalls suitable for this task:
>>>>
>>>> 1) remove the dependency on CONFIG_HAS_MEM_ACCESS
>>>> 2) remove the p2m_mem_access_sanity_check check for these two hypercalls
>>>> 3) remove the (!d->vm_event->monitor.ring_page) check for these two hypercalls
>>>> 4) prevent p2m->mem_access_enabled from being set for these two hypercalls
>>>>
>>>> Am I missing anything? After we do this, would they still be useful for
>>>> their original mem_access related purpose?
>>>
>>> But how would you handle mem_access on those regions in that case? This 
>>> looks completely incompatible.
>>>
>>> The memaccess code has to store the previous permission in order to look 
>>> for the fault. Here you want to modify for good.
>>>
>>> Furthermore, memaccess is only here to modify permission. It does not 
>>> handle cacheability... So it looks to me you are trying to re-purposing 
>>> an hypercall that will not fit all our needs in the future.
>>>
>>> I think the way forward is to introduce an hypercall which populate/map 
>>> memory with a given set of attributes and permissions.
>>>
>>> This would simplify quite a lot the logic (one hypercall instead of 
>>> multiple one) and avoid to worry about attributes changed multiple time 
>>> even before the guest is booting.
>> 
>> Right - that's what I was suggesting with the last paragraph of my
>> previous reply; I have to admit that I have trouble seeing how
>> Stefano's response relates to that.
> 
> I had a hard time interpreting that paragraph.  Did you mean:
> 
> "Have you considered trying to populate the p2m table with the correct
> permissions when first populating it, rather than populating it with
> plain rw ram and then changing it afterwards?"

Yes. I'm sorry for having badly expressed myself.

Jan


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