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

[Zhongze Liu <blackskygg@gmail.com>]

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