Re: [PATCH v3 1/3] x86/ioreq server(patch for 4.7): Rename p2m_mmio_write_dm to p2m_ioreq_server.

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
> From: Yu, Zhang [mailto:yu.c.zhang@linux.intel.com]
> Sent: 28 April 2016 03:47
> To: Paul Durrant; George Dunlap
> Cc: Kevin Tian; Wei Liu; Jun Nakajima; Andrew Cooper; Tim (Xen.org); xen-
> devel@lists.xen.org; Zhiyuan Lv; Jan Beulich; Keir (Xen.org)
> Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7):
> Rename p2m_mmio_write_dm to p2m_ioreq_server.
> 
> 
> 
> On 4/27/2016 10:42 PM, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: George Dunlap
> >> Sent: 27 April 2016 15:13
> >> To: Paul Durrant
> >> Cc: Yu, Zhang; Jan Beulich; Kevin Tian; Wei Liu; Andrew Cooper; Tim
> >> (Xen.org); xen-devel@lists.xen.org; Zhiyuan Lv; Jun Nakajima; Keir
> (Xen.org)
> >> Subject: Re: [Xen-devel] [PATCH v3 1/3] x86/ioreq server(patch for 4.7):
> >> Rename p2m_mmio_write_dm to p2m_ioreq_server.
> >>
> >> On Mon, Apr 25, 2016 at 6:01 PM, Paul Durrant <Paul.Durrant@citrix.com>
> >> wrote:
> >>> For clarity, do you expect any existing use of
> HVMMEM_mmio_write_dm
> >> to continue to *function*? I agree that things should continue to build,
> but if
> >> they don't need to function then the now redundant p2m type should be
> >> removed IMO and any attempt to set a page to
> HVMMEM_mmio_write_dm
> >> (or the new HVMMEM_unused) name should result in -EINVAL. What is
> your
> >> position on this?
> >>
> >> I sort of feel like we're playing some strange guessing game with the
> >> color of this bike shed, where all 4 of us give a random combination
> >> of constrants and then we have to figure out what the solution is. :-)
> >>
> >> There are two issues: the interface (HVMMEM_*) and the
> >> internals.(p2m_*).
> >>
> >> Jan says that code that calls HVMOP_get_mem_type has to continue to
> >> compile and function.  "Functioning" is easy, as you just don't return
> >> that value and you're done.  Compiling just means having the #ifdef.
> >>
> >> It sounds like we all agree that HVMOP_set_mem_type with the current
> >> HVMMEM_mmio_write_dm value should return -EINVAL.
> >>
> >> Regarding the p2m type which now should be impossible to set -- I
> >> don't think it's critical to remove from the release, since it's just
> >> internal.  I'd normally say just leave it for now to reduce code
> >> churn.
> >>
> >> But mostly I think we just want to get this bike shed painted, so if
> >> anyone thinks we should really remove the p2m type from this release,
> >> then that's fine with me too (assuming it's OK with Wei).
> >>
> >> Does this cover everything?
> >>
> >
> > I think so. Thanks for the clarification.
> >
> > Yu, are you happy to submit a patch with the #ifdef in the header, and that
> removes any ability to set the old type?
> >
> 
> I'm fine with this, and thanks. :)
> So my understanding is that the only difference between the new
> patch and this current one is we do not replace p2m_mmio_write_dm
> with p2m_ioreq_server, hence no need to introduce
> HVMMEM_ioreq_server.
> Is this understanding correct?
> 

I believe so. The main points are that:

a) HVMMEM_mmio_write_dm no longer exists with the new interface version and is replaced by HVMMEM_unused
b) Any attempt to set a page to type HVMMEM_unused results in -EINVAL

> Besides, do you think it acceptable we just replace p2m_mmio_write_dm
> with p2m_ioreq_server in next version, or should we remove this p2m
> type and define p2m_ioreq_server with a different value? :)
> 

p2m_ioreq_server becomes defunct after the aforementioned patch. With that patch applied there will be no way to set that type on a p2m entry so it should be ok to re-use the type.

  Paul

> 
> > I guess leaving the p2m type in place to avoid code churn is reasonable at
> this stage, but anyone looking at the p2m code is probably going to question
> why it's there in 4.7.
> >
> >   Paul
> >
> >>  -George
> 
> B.R.
> Yu

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