On 5/3/19 2:57 PM, Jan Beulich wrote:
>>>> On 03.05.19 at 15:43, <ta...@tklengyel.com> wrote:
>> On Fri, May 3, 2019 at 2:27 AM Jan Beulich <jbeul...@suse.com> wrote:
>>>
>>>>>> On 03.05.19 at 00:13, <ta...@tklengyel.com> wrote:
>>>> --- a/xen/arch/x86/mm/mem_sharing.c
>>>> +++ b/xen/arch/x86/mm/mem_sharing.c
>>>> @@ -112,13 +112,48 @@ static inline void page_sharing_dispose(struct
>>>> page_info *page)
>>>>
>>>> #endif /* MEM_SHARING_AUDIT */
>>>>
>>>> -static inline int mem_sharing_page_lock(struct page_info *pg)
>>>> +/*
>>>> + * Private implementations of page_lock/unlock to bypass PV-only
>>>> + * sanity checks not applicable to mem-sharing.
>>>> + */
>>>> +static inline bool _page_lock(struct page_info *page)
>>>> {
>>>> - int rc;
>>>> + unsigned long x, nx;
>>>> +
>>>> + do {
>>>> + while ( (x = page->u.inuse.type_info) & PGT_locked )
>>>> + cpu_relax();
>>>> + nx = x + (1 | PGT_locked);
>>>> + if ( !(x & PGT_validated) ||
>>>> + !(x & PGT_count_mask) ||
>>>> + !(nx & PGT_count_mask) )
>>>> + return false;
>>>
>>> Just for my own understanding: Did you verify that the PGT_validated
>>> check is indeed needed here, or did you copy it "just in case"? In the
>>> latter case a comment may be worthwhile.
>>
>> This is an exact copy of page_lock, sans the asserts that break it
>> from mem_sharing. I didn't investigate which of these flags are
>> necessary for mem_sharing. Frankly, I don't fully understand their
>> meaning and I haven't came across documentation about it yet.
I hope to add some documentation sometime in the next 6 months or so.
I've submitted a talk on the refcounting system to the XenSummit as well.
Short answer: PGT_validated means that the page in question has been
validated as safe to use as the designated type. For PGT_writable, this
is instantaneous (i.e., the type is set to PGT_writable with the
PGT_validated bit set in the same cmpxchg operation).
Other types (such as PGT_l*_table) actually take time to verify, and so
you need to first change the type to the new type (so nobody tries to
use it as yet a different type), then verify that it's OK to use it as a
page table (by recursively checking all the entries), then set the
PGT_validated bit.
> Yes something along these lines is what I'm after. But "these flags"
> really is just PGT_validated.
Here's my take:
The sorts of "needs validation" types are PV-only I believe; if
mem_sharing is only for HVM guests, then the types should only be
PGT_writable, for which PGT_validated should always be set.
*If* we somehow do get to a situation where the type count > 0 but
PGT_validated is clear, something has probably gone terribly wrong; and
it would probably be much safer to just fail the page lock.
But perhaps that implies there should also be an ASSERT(!(x &
PGT_validated))?
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
