Re: [Xen-devel] [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@xxxxxxxxxxxxxxx>]

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@xxxxxxxxxx>
> > Signed-off-by: Yu Zhang <yu.c.zhang@xxxxxxxxxxxxxxx>
> > Acked-by: Tim Deegan <tim@xxxxxxx>
> > ---
> > Cc: Paul Durrant <paul.durrant@xxxxxxxxxx>
> > Cc: Jan Beulich <jbeulich@xxxxxxxx>
> > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> > Cc: Jun Nakajima <jun.nakajima@xxxxxxxxx>
> > Cc: Kevin Tian <kevin.tian@xxxxxxxxx>
> > Cc: Tim Deegan <tim@xxxxxxx>
> >
> > 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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel