Re: [PATCH v6 1/4] x86/ioreq server: Add HVMOP to map guest ram with p2m_ioreq_server to an ioreq server.

[Yu Zhang <yu.c.zhang@linux.intel.com>]

On 9/9/2016 1:22 PM, Yu Zhang wrote:
>
>
> On 9/2/2016 6:47 PM, Yu Zhang wrote:
>> A new HVMOP - HVMOP_map_mem_type_to_ioreq_server, is added to
>> let one ioreq server claim/disclaim its responsibility for the
>> handling of guest pages with p2m type p2m_ioreq_server. Users
>> of this HVMOP can specify which kind of operation is supposed
>> to be emulated in a parameter named flags. Currently, this HVMOP
>> only support the emulation of write operations. And it can be
>> further extended to support the emulation of read ones if an
>> ioreq server has such requirement in the future.
>>
>> For now, we only support one ioreq server for this p2m type, so
>> once an ioreq server has claimed its ownership, subsequent calls
>> of the HVMOP_map_mem_type_to_ioreq_server will fail. Users can also
>> disclaim the ownership of guest ram pages with p2m_ioreq_server, by
>> triggering this new HVMOP, with ioreq server id set to the current
>> owner's and flags parameter set to 0.
>>
>> Note both HVMOP_map_mem_type_to_ioreq_server and p2m_ioreq_server
>> are only supported for HVMs with HAP enabled.
>>
>> Also note that only after one ioreq server claims its ownership
>> of p2m_ioreq_server, will the p2m type change to p2m_ioreq_server
>> be allowed.
>>
>> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
>> Signed-off-by: Yu Zhang <yu.c.zhang@linux.intel.com>
>> Acked-by: Tim Deegan <tim@xen.org>
>> ---
>> Cc: Paul Durrant <paul.durrant@citrix.com>
>> Cc: Jan Beulich <jbeulich@suse.com>
>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>> Cc: George Dunlap <george.dunlap@eu.citrix.com>
>> Cc: Jun Nakajima <jun.nakajima@intel.com>
>> Cc: Kevin Tian <kevin.tian@intel.com>
>> Cc: Tim Deegan <tim@xen.org>
>>
>> changes in v6:
>>    - Clarify logic in hvmemul_do_io().
>>    - Use recursive lock for ioreq server lock.
>>    - Remove debug print when mapping ioreq server.
>>    - Clarify code in ept_p2m_type_to_flags() for consistency.
>>    - Remove definition of P2M_IOREQ_HANDLE_WRITE_ACCESS.
>>    - Add comments for HVMMEM_ioreq_server to note only changes
>>      to/from HVMMEM_ram_rw are permitted.
>>    - Add domain_pause/unpause() in hvm_map_mem_type_to_ioreq_server()
>>      to avoid the race condition when a vm exit happens on a write-
>>      protected page, just to find the ioreq server has been unmapped
>>      already.
>>    - Introduce a seperate patch to delay the release of p2m
>>      lock to avoid the race condition.
>>    - Introduce a seperate patch to handle the read-modify-write
>>      operations on a write protected page.
>>
>
> Why do we need to do this?  Won't the default case just DTRT if it finds
> that the ioreq server has been unmapped?

Well, patch 4 will either mark the remaining p2m_ioreq_server entries as 
"recal" or
reset to p2m_ram_rw directly. So my understanding is that we do not wish to
see a ept violation due to a p2m_ioreq_server access after the ioreq 
server is unmapped.
Yet without this domain_pause/unpause() pair, VM accesses may trigger an 
ept violation
during the hvmop hypercall(hvm_map_mem_type_to_ioreq_server), just to 
find the ioreq
server is NULL. Then we would have to provide handlers which just do the 
copy to/from
actions for the VM. This seems awkward to me.

IIUC, with domain_pause/unpause(), we can guarantee that the VM will not 
trigger such
ept violation during the unmapping process. After that, accesses to a 
p2m_ioreq_server page
will only trigger ept misconfiguration. :)

Thanks
Yu

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