Re: regression from c/s 22071:c5aed2e049bc (ept: Put locks around ept_get_entry) ?

[George Dunlap <George.Dunlap@eu.citrix.com>]

BTW, let me know if you want the other PoD / p2m / ept patches I
haven't upstreamed yet.  Even if you don't end up backporting them,
they may be handy to have if there are problems later.

 -George

On Tue, Dec 14, 2010 at 2:32 PM, George Dunlap
<George.Dunlap@eu.citrix.com> wrote:
> Try this one.
>
> FYI, the logic is pretty simple: Without this patch, ept_next_level()
> gets a pointer and the logic goes through and reads the actual entry
> piecemeal.  ept_set_entry() gets a pointer and goes through setting
> bits in the actual entry piecemeal as well.  The idea is, have
> ept_next_level() read the entire entry into a local variable, and then
> act on that; and have ept_set_entry() write the new entry into a local
> variable and then write the whole entry once.  So it's mostly changing
> "ept_entry->" to "new_entry."
>
>  -George
>
> On Tue, Dec 14, 2010 at 12:37 PM, Jan Beulich <JBeulich@novell.com> wrote:
>>>>> On 14.12.10 at 11:47, George Dunlap <George.Dunlap@eu.citrix.com> wrote:
>>> Yes, I have to apologize: I have a queue of PoD, p2m, and ept-related
>>> fixes that I haven't pushed to the list because:
>>> * they require non-negligible reworking
>>> * it's been really difficult for me to set up an OSS-based system to test
>>> them
>>>
>>> It actually turns out that doing locking in ept_get_entry() is the
>>> wrong thing to do anyway; it can cause the following deadlock:
>>>
>>> p2m_change_type [grabs p2m lock] -> set_p2m_entry -> ept_set_entry ->
>>> ept_set_middle_level -> p2m_alloc [grabs hap lock]
>>>
>>> write cr4 -> hap_update_paging_modes [grabes hap lock] ->
>>> hap_update_cr3 -> gfn_to_mfn -> ept_get_entry -> [grabs p2m lock]
>>>
>>> Attached is a ported patch that removes locking in ept_get_entry(),
>>> and implements access-once semantics for reading and writing.  This
>>> solves the original problem (a race between reading and writing the
>>> table) without causing deadlocks.  I haven't had a chance to test it
>>> -- can you give it a spin?
>>
>> For really giving this a try I'd have to use it on 4.0, where it
>> doesn't apply at all. Resolving the rejects is non-obvious for me
>> in some cases, as I don't know this code well enough. Hence
>> for the moment we'll just drop the bad backport of your first
>> attempt.
>>
>> Jan
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
>>
>