Re: [Xen-devel] [PATCH v9 5/7] memory: add check_get_page_from_gfn() as a wrapper...

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Sep 19, 2018 at 03:22:54AM -0600, Jan Beulich wrote:
> >>> On 19.09.18 at 10:54, <Paul.Durrant@xxxxxxxxxx> wrote:
> >>  -----Original Message-----
> >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> >> Sent: 19 September 2018 09:34
> >> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> >> Cc: Julien Grall <julien.grall@xxxxxxx>; Andrew Cooper
> >> <Andrew.Cooper3@xxxxxxxxxx>; George Dunlap <George.Dunlap@xxxxxxxxxx>; Ian
> >> Jackson <Ian.Jackson@xxxxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>; Stefano
> >> Stabellini <sstabellini@xxxxxxxxxx>; xen-devel <xen-
> >> devel@xxxxxxxxxxxxxxxxxxxx>; Konrad Rzeszutek Wilk
> >> <konrad.wilk@xxxxxxxxxx>; Tim (Xen.org) <tim@xxxxxxx>
> >> Subject: RE: [Xen-devel] [PATCH v9 5/7] memory: add
> >> check_get_page_from_gfn() as a wrapper...
> >> 
> >> >>> On 19.09.18 at 09:56, <Paul.Durrant@xxxxxxxxxx> wrote:
> >> >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> >> >> Sent: 19 September 2018 07:03
> >> >>
> >> >> >>> On 18.09.18 at 18:37, <Paul.Durrant@xxxxxxxxxx> wrote:
> >> >> > Moving to p2m-common won't work. The function declaration involves a
> >> >> > p2m_type_t argument and that enum is defined in the arm and x86
> >> specific
> >> >> > headers. I propose therefore to leave this patch as-is.
> >> >>
> >> >> Leaving the duplication in place is just the last resort imo. Does
> >> >> xen/mm.h
> >> >> not work either?
> >> >
> >> > No, it won't. It has to be something *after* the definition of the
> >> > p2m_type_t enum. I could, as Julien suggested, move the inclusion of
> >> > p2m-common after that point, but it would mean the header guards would
> >> cease
> >> > to DTRT of course. I'm not sure whether C will allow me to forward
> >> declare
> >> > the enum (not something I've tried) but I could give that a go. Any
> >> other
> >> > suggestions?
> >> 
> >> I'm afraid I don't understand your concern wrt include guards. Each header
> >> has its own. I'm actually surprised the current inclusion point is at the
> >> top
> >> of the file, rather than after at least the basic type definitions each
> >> arch
> >> has to supply.
> >> 
> > 
> > Ok, if we include p2m-common in the middle of p2m, and then add stuff 
> > dependent on the declaration of p2m_type_t, what happens when a source 
> > module 
> > explicitly includes p2m-common prior to p2m?
> 
> This is not supposed to happen, as this header is supposed to only be
> included from the per-arch p2m.h; we should put an #error there. I
> see there's exactly one violation of this (drivers/vpci/header.c). But
> your series can go in as is; I'll try to remember to clean this up.

Oh, didn't know this, will send a patch to switch header.c to include
p2m.h instead of p2m-common.h.

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel