Re: [Xen-devel] [PATCH v2 3/3] x86/ioreq server: Add HVMOP to map guest ram with p2m_ioreq_server to an ioreq server

["Yu, Zhang" <yu.c.zhang@xxxxxxxxxxxxxxx>]

On 4/19/2016 6:01 PM, Paul Durrant wrote:
> > -----Original Message-----
> > From: Yu, Zhang [mailto:yu.c.zhang@xxxxxxxxxxxxxxx]
> > Sent: 19 April 2016 10:49
> > To: Paul Durrant; Kevin Tian; Jan Beulich; Nakajima, Jun
> > Cc: Keir (Xen.org); Andrew Cooper; Tim (Xen.org); George Dunlap; xen-
> > devel@xxxxxxxxxxxxx; Lv, Zhiyuan
> > Subject: Re: [Xen-devel] [PATCH v2 3/3] x86/ioreq server: Add HVMOP to
> > map guest ram with p2m_ioreq_server to an ioreq server
> >
> >
> >
> > On 4/19/2016 5:40 PM, Paul Durrant wrote:
> > > > -----Original Message-----
> > > > From: Yu, Zhang [mailto:yu.c.zhang@xxxxxxxxxxxxxxx]
> > > > Sent: 19 April 2016 10:27
> > > > To: Paul Durrant; Kevin Tian; Jan Beulich; Nakajima, Jun
> > > > Cc: Keir (Xen.org); Andrew Cooper; Tim (Xen.org); George Dunlap; xen-
> > > > devel@xxxxxxxxxxxxx; Lv, Zhiyuan
> > > > Subject: Re: [Xen-devel] [PATCH v2 3/3] x86/ioreq server: Add HVMOP to
> > > > map guest ram with p2m_ioreq_server to an ioreq server
> > > >
> > > >
> > > >
> > > > On 4/19/2016 4:46 PM, Paul Durrant wrote:
> > > > > > -----Original Message-----
> > > > > > From: Tian, Kevin [mailto:kevin.tian@xxxxxxxxx]
> > > > > > Sent: 19 April 2016 05:51
> > > > > > To: Yu, Zhang; Jan Beulich; Paul Durrant; Nakajima, Jun
> > > > > > Cc: Andrew Cooper; George Dunlap; Lv, Zhiyuan; xen-
> > > > devel@xxxxxxxxxxxxx;
> > > > > > Keir (Xen.org); Tim (Xen.org)
> > > > > > Subject: RE: [Xen-devel] [PATCH v2 3/3] x86/ioreq server: Add HVMOP
> > to
> > > > > > map guest ram with p2m_ioreq_server to an ioreq server
> > > > > >
> > > > > > > From: Yu, Zhang [mailto:yu.c.zhang@xxxxxxxxxxxxxxx]
> > > > > > > Sent: Thursday, April 14, 2016 5:57 PM
> > > > > > > > > > > > And with all three bits now possibly being clear, aren't we
> > risking
> > > > the
> > > > > > > > > > > > entries to be mis-treated as not-present ones?
> > > > > > > > > > > Hah. You got me. Thanks! :)
> > > > > > > > > > > Now I realized it would be difficult if we wanna to emulate the
> > read
> > > > > > > > > > > operations for HVM. According to Intel mannual, entry->r is to
> > be
> > > > > > > > > > > cleared, so should entry->w if we do not want ept misconfig.
> > And
> > > > > > > > > > > with both read and write permissions being forbidden, entry->x
> > > > can
> > > > > > be
> > > > > > > > > > > set only on processors with EXECUTE_ONLY capability.
> > > > > > > > > > > To avoid any entry to be mis-treated as not-present. We have
> > > > several
> > > > > > > > > > > solutions:
> > > > > > > > > > > a> do not support the read emulation for now - we have no such
> > > > > > usage
> > > > > > > > > > > case;
> > > > > > > > > > > b> add the check of p2m_t against p2m_ioreq_server in
> > > > > > is_epte_present -
> > > > > > > > > > > a bit weird to me.
> > > > > > > > > > > Which one do you prefer? or any other suggestions?
> > > > > > > > > > That question would also need to be asked to others who had
> > > > > > > > > > suggested supporting both. I'd be fine with a, but I also don't view
> > > > > > > > > > b as too awkward.
> > > > > > > > > According to Intel mannual, an entry is regarded as not present, if
> > > > > > > > > bit0:2 is 0. So adding a p2m type check in is_epte_present() means
> > we
> > > > > > > > > will change its semantics, if this is acceptable(with no hurt to
> > > > > > > > > hypervisor). I'd prefer option b>
> > > > > > > > Perhaps time for the VMX maintainers to chime in - such a change is
> > > > > > acceptable
> > > > > > > > only if it doesn't result in changed hardware behavior. I can't think of
> > > > any
> > > > > > such off
> > > > > > > > the top of my head, but this really should be confirmed by the
> > > > > > maintainers before
> > > > > > > > deciding to go such a route.
> > > > > > > Thanks, Jan. :)
> > > > > > > Jun & Kevin, any suggestions?
> > > > > > I'm a bit worried about this change, since it's a fundamental EPT
> > > > > > interface. Can we avoid supporting read emulation now?
> > > > > I'm happy to drop the implementation of read emulation for the
> > moment
> > > > to keep things simple, as long as it is kept in the interface.
> > > > >
> > > > Thanks, Paul.
> > > > So what's the benefit to keep the read in the interface, if we can
> > > > not support its emulation?
> > > Well, if it's in the interface then support can be added in future. If it's not 
> > > in
> > the interface then it needs to be added later and that makes things more
> > awkward compatibility-wise.
> > >     Paul
> > But if we need to support read emulation in the future, we'll have to
> > clear bit0:2 in ept pte, which is the same situation we are facing
> > now. So why do we not support it now?
> >
> > Sorry for seeming so stubborn, I just want to have some more convincing
> > explanations. :)
> That's an argument you need to have with Kevin. If he thinks the risk of making 
> the change now is small then go ahead. All I'm saying is that 4.7 is about to 
> go out of the door so whatever happens now needs to be as low risk and as 
> minimally disruptive as possible.
Got it. Thank you, Paul. :)

Yu

>    Paul
>
> > Yu
> >
> > > > It could be easier to change the is_epte_present definition than
> > > > to remove the read emulation code, but either way would not be
> > > > difficult.
> > > >
> > > > Yu
> > > _______________________________________________
> > > Xen-devel mailing list
> > > Xen-devel@xxxxxxxxxxxxx
> > > http://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel