> -----Original Message----- > From: Jan Beulich [mailto:[email protected]] > Sent: 10 July 2018 15:29 > To: Paul Durrant <[email protected]> > Cc: Julien Grall <[email protected]>; Andrew Cooper > <[email protected]>; George Dunlap > <[email protected]>; Ian Jackson <[email protected]>; Wei Liu > <[email protected]>; Jun Nakajima <[email protected]>; Kevin Tian > <[email protected]>; Stefano Stabellini <[email protected]>; xen- > devel <[email protected]>; Konrad Rzeszutek Wilk > <[email protected]>; Tim (Xen.org) <[email protected]> > Subject: RE: [PATCH v2 03/13] iommu: make use of type-safe BFN and MFN > in exported functions > > >>> On 10.07.18 at 16:10, <[email protected]> wrote: > >> -----Original Message----- > >> From: George Dunlap [mailto:[email protected]] > >> Sent: 10 July 2018 15:01 > >> To: Paul Durrant <[email protected]>; xen- > [email protected] > >> Cc: Jan Beulich <[email protected]>; Andrew Cooper > >> <[email protected]>; George Dunlap > >> <[email protected]>; Ian Jackson <[email protected]>; > Konrad > >> Rzeszutek Wilk <[email protected]>; Stefano Stabellini > >> <[email protected]>; Julien Grall <[email protected]>; Tim > (Xen.org) > >> <[email protected]>; Wei Liu <[email protected]>; Jun Nakajima > >> <[email protected]>; Kevin Tian <[email protected]> > >> Subject: Re: [PATCH v2 03/13] iommu: make use of type-safe BFN and > MFN > >> in exported functions > >> > >> On 07/07/2018 12:05 PM, Paul Durrant wrote: > >> > This patch modifies the declaration of the entry points to the IOMMU > >> > sub-system to use bfn_t and mfn_t in place of unsigned long. A > >> subsequent > >> > patch will similarly modify the methods in the iommu_ops structure. > >> > > >> > Signed-off-by: Paul Durrant <[email protected]> > >> > --- > >> > Cc: Jan Beulich <[email protected]> > >> > Cc: Andrew Cooper <[email protected]> > >> > Cc: George Dunlap <[email protected]> > >> > Cc: Ian Jackson <[email protected]> > >> > Cc: Konrad Rzeszutek Wilk <[email protected]> > >> > Cc: Stefano Stabellini <[email protected]> > >> > Cc: Julien Grall <[email protected]> > >> > Cc: Tim Deegan <[email protected]> > >> > Cc: Wei Liu <[email protected]> > >> > Cc: Jun Nakajima <[email protected]> > >> > Cc: Kevin Tian <[email protected]> > >> > Cc: George Dunlap <[email protected]> > >> > > >> > v2: > >> > - Addressed comments from Jan. > >> > - Use intermediate 'frame' variable to avoid directly encapsulating > >> > mfn or gfn values as bfns. > >> > >> Explain this one to me? At the moment I don't see any value from having > >> the extra variable in the middle. > > > > This was something that Jan wanted. > > Ah, yes, it was in a reply to this patch's v1 that I had suggested a > neutrally named variable in case it can hold values from more than > one space.
Yes, it was: " Along the lines of what I've said earlier about mixing address spaces, this would perhaps not so much need a comment (it's a 1:1 mapping after all), but rather making more obvious that it's a 1:1 mapping. This in particular would mean to me to latch page_to_mfn(page) into a (neutrally named, e.g. "frame") local variable, and use the result in a way that makes obviously especially on the "map" path that this really requests a 1:1 mapping. By implication from the 1:1 mapping it'll then (hopefully) be clear to the reader that which exact name space is used doesn't really matter." > In the particular case of _get_page_type(), where I had > also given the v1 comment, I can't however see that it is now really > obvious that a 1:1 mapping is being established. To me that would > mean passing the same local variable (suitably type wrapped where > needed) twice into iommu_map_page(). It is not clear to me what > use a mfn_to_gmfn() invocation is inside a is_pv_domain() guarded > block, now that we don't permit translated PV domains anymore > (not that I would think that they had worked before the code was > removed). > I can see about trying to clean up the mfn_to_gmfn() and re-phrase things to make it more obvious that the map is 1:1. Paul > Jan > _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
