> -----Original Message----- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 14 June 2018 16:44 > To: Paul Durrant <paul.durr...@citrix.com> > Cc: Julien Grall <julien.gr...@arm.com>; Andrew Cooper > <andrew.coop...@citrix.com>; Roger Pau Monne <roger....@citrix.com>; > Wei Liu <wei.l...@citrix.com>; George Dunlap <george.dun...@citrix.com>; > Ian Jackson <ian.jack...@citrix.com>; Kevin Tian <kevin.t...@intel.com>; > Stefano Stabellini <sstabell...@kernel.org>; xen-devel <xen- > de...@lists.xenproject.org>; Konrad Rzeszutek Wilk > <konrad.w...@oracle.com>; Tim (Xen.org) <t...@xen.org> > Subject: Re: [PATCH v2 1/2] VT-d: re-phrase logic in > vtd_set_hwdom_mapping() for clarity > > >>> On 12.06.18 at 15:47, <paul.durr...@citrix.com> wrote: > > --- a/xen/drivers/passthrough/vtd/x86/vtd.c > > +++ b/xen/drivers/passthrough/vtd/x86/vtd.c > > @@ -114,26 +114,29 @@ void __hwdom_init > vtd_set_hwdom_mapping(struct domain *d) > > > > BUG_ON(!is_hardware_domain(d)); > > > > - top = max(max_pdx, pfn_to_pdx(0xffffffffUL >> PAGE_SHIFT) + 1); > > + top = max(max_pdx, pfn_to_pdx(GB(4) >> PAGE_SHIFT)); > > At certain boundaries > > pfn_to_pdx(x + 1) != pfn_to_pdx(x) + 1 > > i.e. there was a reason for the way this was phrased before.
Oh, I'd misread the bracketing (and I actually didn't know that either). How about: max_pfn = (GB(4) >> PAGE_SHIFT) - 1; top = max(max_pdx, pfn_to_pdx(max_pfn) + 1); > > > for ( i = 0; i < top; i++ ) > > { > > + unsigned long pfn = pdx_to_pfn(i); > > + bool map; > > int rc = 0; > > > > + if ( pfn >= (GB(4) >> PAGE_SHIFT) && !mfn_valid(_mfn(pfn)) ) > > + continue; > > Why ahead of the comment ... > > > /* > > - * Set up 1:1 mapping for dom0. Default to use only conventional > > RAM > > - * areas and let RMRRs include needed reserved regions. When set, > the > > - * inclusive mapping maps in everything below 4GB except unusable > > - * ranges. > > + * Set up 1:1 mapping for dom0. Default to include only > > + * conventional RAM areas and let RMRRs include needed reserved > > + * regions. When set, the inclusive mapping maps in every pfn up > > + * to 4GB except those that fall in unusable ranges. > > */ > > ... explaining it, when the other parts that get explained follow after it? Because the comment only refers to what happens below 4G, so the prior if statement is not covered (and was not covered by the old version of the comment either). Paul > > Jan > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel